From: David Howells <dhowells@redhat.com>

Implement a PGP data parser for the crypto key type to use when
instantiating a key.

This parser attempts to parse the instantiation data as a PGP packet
sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
algorithm key or subkey from it.

If it finds such a key, it will set up a public_key subtype payload with
appropriate handler routines (RSA) and attach it to the key.

Thanks to Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> for pointing
out some errors.

Signed-off-by: David Howells <dhowells@redhat.com>
Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 crypto/asymmetric_keys/Kconfig          |  11 +
 crypto/asymmetric_keys/Makefile         |   4 +
 crypto/asymmetric_keys/pgp_parser.h     |  18 +
 crypto/asymmetric_keys/pgp_public_key.c | 416 ++++++++++++++++++++++++
 4 files changed, 449 insertions(+)
 create mode 100644 crypto/asymmetric_keys/pgp_parser.h
 create mode 100644 crypto/asymmetric_keys/pgp_public_key.c

diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index ebe9dc88d975..ebde5ef5d65f 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -92,4 +92,15 @@ config PGP_LIBRARY
 	  This option enables a library that provides a number of simple
 	  utility functions for parsing PGP (RFC 4880) packet-based messages.
 
+config PGP_KEY_PARSER
+	tristate "PGP key parser"
+	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select PGP_LIBRARY
+	select MD5 # V3 fingerprint generation
+	select SHA1 # V4 fingerprint generation
+	help
+	  This option provides support for parsing PGP (RFC 4880) format blobs
+	  for key data and provides the ability to instantiate a crypto key
+	  from a public key packet found inside the blob.
+
 endif # ASYMMETRIC_KEY_TYPE
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index f7e5ee59857f..36a27cf2daff 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -93,3 +93,7 @@ $(obj)/tpm.asn1.o: $(obj)/tpm.asn1.c $(obj)/tpm.asn1.h
 # PGP handling
 #
 obj-$(CONFIG_PGP_LIBRARY) += pgp_library.o
+
+obj-$(CONFIG_PGP_KEY_PARSER) += pgp_key_parser.o
+pgp_key_parser-y := \
+	pgp_public_key.o
diff --git a/crypto/asymmetric_keys/pgp_parser.h b/crypto/asymmetric_keys/pgp_parser.h
new file mode 100644
index 000000000000..1a560ce32415
--- /dev/null
+++ b/crypto/asymmetric_keys/pgp_parser.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* PGP crypto data parser internal definitions
+ *
+ * Copyright (C) 2011 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#include "pgplib.h"
+
+#define kenter(FMT, ...) \
+	pr_devel("==> %s("FMT")\n", __func__, ##__VA_ARGS__)
+#define kleave(FMT, ...) \
+	pr_devel("<== %s()"FMT"\n", __func__, ##__VA_ARGS__)
+
+/*
+ * pgp_public_key.c
+ */
+extern const char *pgp_to_public_key_algo[PGP_PUBKEY__LAST];
diff --git a/crypto/asymmetric_keys/pgp_public_key.c b/crypto/asymmetric_keys/pgp_public_key.c
new file mode 100644
index 000000000000..0529c8ce2d43
--- /dev/null
+++ b/crypto/asymmetric_keys/pgp_public_key.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Instantiate a public key crypto key from PGP format data [RFC 4880]
+ *
+ * Copyright (C) 2011 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#define pr_fmt(fmt) "PGP: "fmt
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/mpi.h>
+#include <keys/asymmetric-subtype.h>
+#include <keys/asymmetric-parser.h>
+#include <crypto/hash.h>
+#include <crypto/public_key.h>
+
+#include "pgp_parser.h"
+
+#define MAX_MPI 5
+#define KEYCTL_SUPPORTS_ENCDEC \
+	(KEYCTL_SUPPORTS_ENCRYPT | KEYCTL_SUPPORTS_DECRYPT)
+#define KEYCTL_SUPPORTS_SIGVER (KEYCTL_SUPPORTS_SIGN | KEYCTL_SUPPORTS_VERIFY)
+
+MODULE_LICENSE("GPL");
+
+const char *pgp_to_public_key_algo[PGP_PUBKEY__LAST] = {
+	[PGP_PUBKEY_RSA_ENC_OR_SIG]	= "rsa",
+	[PGP_PUBKEY_RSA_ENC_ONLY]	= "rsa",
+	[PGP_PUBKEY_RSA_SIG_ONLY]	= "rsa",
+	[PGP_PUBKEY_ELGAMAL]		= NULL,
+	[PGP_PUBKEY_DSA]		= NULL,
+};
+
+static const int pgp_key_algo_p_num_mpi[PGP_PUBKEY__LAST] = {
+	[PGP_PUBKEY_RSA_ENC_OR_SIG]	= 2,
+	[PGP_PUBKEY_RSA_ENC_ONLY]	= 2,
+	[PGP_PUBKEY_RSA_SIG_ONLY]	= 2,
+	[PGP_PUBKEY_ELGAMAL]		= 3,
+	[PGP_PUBKEY_DSA]		= 4,
+};
+
+static const u8 pgp_public_key_capabilities[PGP_PUBKEY__LAST] = {
+	[PGP_PUBKEY_RSA_ENC_OR_SIG]	= KEYCTL_SUPPORTS_ENCDEC |
+					  KEYCTL_SUPPORTS_SIGVER,
+	[PGP_PUBKEY_RSA_ENC_ONLY]	= KEYCTL_SUPPORTS_ENCDEC,
+	[PGP_PUBKEY_RSA_SIG_ONLY]	= KEYCTL_SUPPORTS_SIGVER,
+	[PGP_PUBKEY_ELGAMAL]		= 0,
+	[PGP_PUBKEY_DSA]		= 0,
+};
+
+struct pgp_key_data_parse_context {
+	struct pgp_parse_context pgp;
+	u8 key[1024];
+	size_t keylen;
+	u8 keyid_buf[1024];
+	size_t keyid_buf_len;
+	char user_id[512];
+	size_t user_id_len;
+	const char *algo;
+	u8 raw_fingerprint[HASH_MAX_DIGESTSIZE];
+	size_t raw_fingerprint_len;
+	unsigned int version;
+};
+
+static inline void write_keyid_buf_char(struct pgp_key_data_parse_context *ctx,
+					uint8_t ch)
+{
+	memcpy(&ctx->keyid_buf[ctx->keyid_buf_len++], &ch, 1);
+}
+
+/*
+ * Build buffer to calculate the public key ID (RFC4880 12.2)
+ */
+static int pgp_build_pkey_keyid_buf(struct pgp_key_data_parse_context *ctx,
+				    struct pgp_parse_pubkey *pgp)
+{
+	unsigned int nb[MAX_MPI];
+	unsigned int nn[MAX_MPI];
+	unsigned int n;
+	size_t keylen = ctx->keylen;
+	u8 *key_ptr = ctx->key;
+	u8 *pp[MAX_MPI];
+	u32 a32;
+	int npkey = pgp_key_algo_p_num_mpi[pgp->pubkey_algo];
+	int i, ret;
+
+	kenter("");
+
+	n = (pgp->version < PGP_KEY_VERSION_4) ? 8 : 6;
+	for (i = 0; i < npkey; i++) {
+		ret = mpi_key_length(key_ptr, keylen, nb + i, nn + i);
+		if (ret < 0) {
+			kleave(" = %d", ret);
+			return ret;
+		}
+
+		if (keylen < 2 + nn[i])
+			break;
+
+		pp[i] = key_ptr + 2;
+		key_ptr += 2 + nn[i];
+		keylen -= 2 + nn[i];
+		n += 2 + nn[i];
+	}
+
+	if (keylen != 0) {
+		pr_debug("excess %zu\n", keylen);
+		kleave(" = -EBADMSG");
+		return -EBADMSG;
+	}
+
+	write_keyid_buf_char(ctx, 0x99);	/* ctb */
+	write_keyid_buf_char(ctx, n >> 8);	/* 16-bit header length */
+	write_keyid_buf_char(ctx, n);
+
+	write_keyid_buf_char(ctx, pgp->version);
+
+	a32 = pgp->creation_time;
+	write_keyid_buf_char(ctx, a32 >> 24);
+	write_keyid_buf_char(ctx, a32 >> 16);
+	write_keyid_buf_char(ctx, a32 >> 8);
+	write_keyid_buf_char(ctx, a32 >> 0);
+
+	if (pgp->version < PGP_KEY_VERSION_4) {
+		u16 a16;
+
+		if (pgp->expires_at)
+			a16 = (pgp->expires_at - pgp->creation_time) / 86400UL;
+		else
+			a16 = 0;
+		write_keyid_buf_char(ctx, a16 >> 8);
+		write_keyid_buf_char(ctx, a16 >> 0);
+	}
+
+	write_keyid_buf_char(ctx, pgp->pubkey_algo);
+
+	for (i = 0; i < npkey; i++) {
+		write_keyid_buf_char(ctx, nb[i] >> 8);
+		write_keyid_buf_char(ctx, nb[i]);
+		memcpy(&ctx->keyid_buf[ctx->keyid_buf_len], pp[i], nn[i]);
+		ctx->keyid_buf_len += nn[i];
+	}
+
+	kleave(" = 0");
+	return 0;
+}
+
+/*
+ * Extract a public key or public subkey from the PGP stream.
+ */
+static int pgp_process_public_key(struct pgp_parse_context *context,
+				  enum pgp_packet_tag type,
+				  u8 headerlen,
+				  const u8 *data,
+				  size_t datalen)
+{
+	struct pgp_key_data_parse_context *ctx =
+		container_of(context, struct pgp_key_data_parse_context, pgp);
+	struct pgp_parse_pubkey pgp;
+	u8 capabilities;
+	int ret;
+
+	kenter(",%u,%u,,%zu", type, headerlen, datalen);
+
+	if (type == PGP_PKT_USER_ID) {
+		if (!ctx->user_id_len) {
+			if (ctx->user_id_len > sizeof(ctx->user_id)) {
+				kleave(" = -E2BIG");
+				return -E2BIG;
+			}
+
+			memcpy(ctx->user_id, data, datalen);
+			ctx->user_id_len = datalen;
+		}
+		kleave(" = 0 [user ID]");
+		return 0;
+	}
+
+	if (ctx->keyid_buf_len) {
+		kleave(" = -EBADMSG");
+		return -EBADMSG;
+	}
+
+	ret = pgp_parse_public_key(&data, &datalen, &pgp);
+	if (ret < 0) {
+		kleave(" = %d", ret);
+		return ret;
+	}
+
+	ctx->version = pgp.version;
+
+	if (pgp.pubkey_algo < PGP_PUBKEY__LAST)
+		ctx->algo = pgp_to_public_key_algo[pgp.pubkey_algo];
+
+	if (!ctx->algo) {
+		pr_debug("Unsupported public key algorithm %u\n",
+			 pgp.pubkey_algo);
+		kleave(" = -ENOPKG");
+		return -ENOPKG;
+	}
+
+	/*
+	 * It's the public half of a key, so that only gives us encrypt and
+	 * verify capabilities.
+	 */
+	capabilities = pgp_public_key_capabilities[pgp.pubkey_algo] &
+		       (KEYCTL_SUPPORTS_ENCRYPT | KEYCTL_SUPPORTS_VERIFY);
+	/*
+	 * Capabilities are not stored anymore in the public key, store only
+	 * those that allow signature verification.
+	 */
+	if (!(capabilities & KEYCTL_SUPPORTS_VERIFY)) {
+		pr_debug("Public key cannot be used for verification\n");
+		kleave(" = -ENOPKG");
+		return -ENOPKG;
+	}
+
+	if (datalen > sizeof(ctx->key)) {
+		kleave(" = -E2BIG");
+		return -E2BIG;
+	}
+
+	memcpy(ctx->key, data, datalen);
+	ctx->keylen = datalen;
+
+	ret = pgp_build_pkey_keyid_buf(ctx, &pgp);
+
+	kleave(" = %d", ret);
+	return ret;
+}
+
+/*
+ * Calculate the public key ID fingerprint
+ */
+static int pgp_generate_fingerprint(struct pgp_key_data_parse_context *ctx)
+{
+	struct crypto_shash *tfm;
+	struct shash_desc *digest;
+	char fingerprint[HASH_MAX_DIGESTSIZE * 2 + 1] = { 0 };
+	size_t offset;
+	int ret;
+
+	ret = -ENOMEM;
+	tfm = crypto_alloc_shash(ctx->version < PGP_KEY_VERSION_4 ?
+				 "md5" : "sha1", 0, 0);
+	if (!tfm)
+		goto cleanup;
+
+	digest = kmalloc(sizeof(*digest) + crypto_shash_descsize(tfm),
+			 GFP_KERNEL);
+	if (!digest)
+		goto cleanup_tfm;
+
+	digest->tfm = tfm;
+	crypto_shash_set_flags(digest->tfm, CRYPTO_TFM_REQ_MAY_SLEEP);
+	ret = crypto_shash_init(digest);
+	if (ret < 0)
+		goto cleanup_hash;
+
+	crypto_shash_update(digest, ctx->keyid_buf, ctx->keyid_buf_len);
+
+	ctx->raw_fingerprint_len = crypto_shash_digestsize(tfm);
+
+	ret = crypto_shash_final(digest, ctx->raw_fingerprint);
+	if (ret < 0)
+		goto cleanup_hash;
+
+	offset = ctx->raw_fingerprint_len - 8;
+	pr_debug("offset %lu/%lu\n", offset, ctx->raw_fingerprint_len);
+
+	bin2hex(fingerprint, ctx->raw_fingerprint, ctx->raw_fingerprint_len);
+	pr_debug("fingerprint %s\n", fingerprint);
+
+	ret = 0;
+cleanup_hash:
+	kfree(digest);
+cleanup_tfm:
+	crypto_free_shash(tfm);
+cleanup:
+	return ret;
+}
+
+static struct asymmetric_key_ids *pgp_key_generate_id(
+					struct pgp_key_data_parse_context *ctx)
+{
+	struct asymmetric_key_ids *kids;
+	struct asymmetric_key_id *kid;
+
+	kids = kzalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
+	if (!kids)
+		return kids;
+
+	kid = asymmetric_key_generate_id(ctx->raw_fingerprint,
+					 ctx->raw_fingerprint_len, NULL, 0);
+	if (IS_ERR(kid))
+		goto error;
+
+	kids->id[0] = kid;
+	kids->id[1] = kmemdup(kid, sizeof(*kid) + ctx->raw_fingerprint_len,
+			      GFP_KERNEL);
+	if (!kids->id[1])
+		goto error;
+
+	return kids;
+error:
+	kfree(kids->id[0]);
+	kfree(kids);
+
+	return NULL;
+}
+
+/*
+ * Attempt to parse the instantiation data blob for a key as a PGP packet
+ * message holding a key.
+ */
+static int pgp_key_parse(struct key_preparsed_payload *prep)
+{
+	struct pgp_key_data_parse_context *ctx;
+	struct public_key *pub = NULL;
+	int ret;
+
+	kenter("");
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx) {
+		kleave(" = -ENOMEM");
+		return -ENOMEM;
+	}
+
+	ctx->pgp.types_of_interest = (1 << PGP_PKT_PUBLIC_KEY) |
+				     (1 << PGP_PKT_USER_ID);
+	ctx->pgp.process_packet = pgp_process_public_key;
+
+	ret = pgp_parse_packets(prep->data, prep->datalen, &ctx->pgp);
+	if (ret < 0)
+		goto error;
+
+	ret = pgp_generate_fingerprint(ctx);
+	if (ret < 0)
+		goto error;
+
+	pub = kzalloc(sizeof(struct public_key), GFP_KERNEL);
+	if (!pub) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	pub->key = kmemdup(ctx->key, ctx->keylen, GFP_KERNEL);
+	if (!pub->key) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	pub->keylen = ctx->keylen;
+	pub->id_type = "PGP";
+	pub->pkey_algo = ctx->algo;
+
+	if (ctx->user_id && ctx->user_id_len > 0) {
+		/*
+		 * Propose a description for the key (user ID without the
+		 * comment).
+		 */
+		size_t ulen = ctx->user_id_len;
+
+		if (ulen > 255 - 9)
+			ulen = 255 - 9;
+		prep->description = kmalloc(ulen + 1 + 8 + 1, GFP_KERNEL);
+		ret = -ENOMEM;
+		if (!prep->description)
+			goto error;
+		memcpy(prep->description, ctx->user_id, ulen);
+		prep->description[ulen] = ' ';
+		bin2hex(prep->description + ulen + 1,
+			ctx->raw_fingerprint + ctx->raw_fingerprint_len - 4, 4);
+		prep->description[ulen + 9] = '\0';
+		pr_debug("desc '%s'\n", prep->description);
+	}
+
+	/* We're pinning the module by being linked against it */
+	__module_get(public_key_subtype.owner);
+	prep->payload.data[asym_subtype] = &public_key_subtype;
+	prep->payload.data[asym_key_ids] = pgp_key_generate_id(ctx);
+	prep->payload.data[asym_crypto] = pub;
+	prep->quotalen = 100;
+	kfree(ctx);
+	return 0;
+
+error:
+	public_key_free(pub);
+	kfree(ctx);
+	kleave(" = %d", ret);
+	return ret;
+}
+
+static struct asymmetric_key_parser pgp_key_parser = {
+	.owner		= THIS_MODULE,
+	.name		= "pgp",
+	.parse		= pgp_key_parse,
+};
+
+/*
+ * Module stuff
+ */
+static int __init pgp_key_init(void)
+{
+	return register_asymmetric_key_parser(&pgp_key_parser);
+}
+
+static void __exit pgp_key_exit(void)
+{
+	unregister_asymmetric_key_parser(&pgp_key_parser);
+}
+
+module_init(pgp_key_init);
+module_exit(pgp_key_exit);
-- 
2.34.1

On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
> From: David Howells <dhowells@redhat.com>
> 
> Implement a PGP data parser for the crypto key type to use when
> instantiating a key.
> 
> This parser attempts to parse the instantiation data as a PGP packet
> sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
> algorithm key or subkey from it.

I don't understand why we want to do this in-kernel instead of in
userspace and then pass in the actual key.

On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
> On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
> > From: David Howells <dhowells@redhat.com>
> > 
> > Implement a PGP data parser for the crypto key type to use when
> > instantiating a key.
> > 
> > This parser attempts to parse the instantiation data as a PGP packet
> > sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
> > algorithm key or subkey from it.
> 
> I don't understand why we want to do this in-kernel instead of in
> userspace and then pass in the actual key.

Sigh, this is a long discussion.

PGP keys would be used as a system-wide trust anchor to verify RPM
package headers, which already contain file digests that can be used as
reference values for kernel-enforced integrity appraisal.

With the assumptions that:

- In a locked-down system the kernel has more privileges than root
- The kernel cannot offload this task to an user space process due to
  insufficient isolation

the only available option is to do it in the kernel (that is what I got
as suggestion).

Roberto

On Fri, Feb 16, 2024 at 05:53:01PM +0100, Roberto Sassu wrote:
> On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
> > On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
> > > From: David Howells <dhowells@redhat.com>
> > > 
> > > Implement a PGP data parser for the crypto key type to use when
> > > instantiating a key.
> > > 
> > > This parser attempts to parse the instantiation data as a PGP packet
> > > sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
> > > algorithm key or subkey from it.
> > 
> > I don't understand why we want to do this in-kernel instead of in
> > userspace and then pass in the actual key.
> 
> Sigh, this is a long discussion.

Well, yes.  When you don't lay out why this is of value, it turns into a
long discussion.  This isn't fun for me either.

> PGP keys would be used as a system-wide trust anchor to verify RPM
> package headers, which already contain file digests that can be used as
> reference values for kernel-enforced integrity appraisal.

The one example we have of usage comes in patch 7 of this series and is:

gpg --dearmor < <PGP key> | keyctl padd asymmetric "" @u

And you're already using two userspace programs there.  Why not a third?

gpg --dearmor < <PGP key> | ./scripts/parse-pgp-packets | keyctl padd asymmetric "" @u

> With the assumptions that:
> 
> - In a locked-down system the kernel has more privileges than root
> - The kernel cannot offload this task to an user space process due to
>   insufficient isolation
> 
> the only available option is to do it in the kernel (that is what I got
> as suggestion).

This sounds like there's some other way of getting the key into the
kernel which doesn't rely on userspace.  Or are you assuming that nobody
bothered to trojan 'cat'?

On 2/16/2024 7:44 PM, Matthew Wilcox wrote:
> On Fri, Feb 16, 2024 at 05:53:01PM +0100, Roberto Sassu wrote:
>> On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
>>> On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
>>>> From: David Howells <dhowells@redhat.com>
>>>>
>>>> Implement a PGP data parser for the crypto key type to use when
>>>> instantiating a key.
>>>>
>>>> This parser attempts to parse the instantiation data as a PGP packet
>>>> sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
>>>> algorithm key or subkey from it.
>>>
>>> I don't understand why we want to do this in-kernel instead of in
>>> userspace and then pass in the actual key.
>>
>> Sigh, this is a long discussion.
> 
> Well, yes.  When you don't lay out why this is of value, it turns into a
> long discussion.  This isn't fun for me either.
> 
>> PGP keys would be used as a system-wide trust anchor to verify RPM
>> package headers, which already contain file digests that can be used as
>> reference values for kernel-enforced integrity appraisal.
> 
> The one example we have of usage comes in patch 7 of this series and is:
> 
> gpg --dearmor < <PGP key> | keyctl padd asymmetric "" @u
> 
> And you're already using two userspace programs there.  Why not a third?

I think this is very easy to answer. Why not extracting the public key 
from an x509 certificate in user space, sending it to the kernel, and 
using it for kernel module verification?

> gpg --dearmor < <PGP key> | ./scripts/parse-pgp-packets | keyctl padd asymmetric "" @u
> 
>> With the assumptions that:
>>
>> - In a locked-down system the kernel has more privileges than root
>> - The kernel cannot offload this task to an user space process due to
>>    insufficient isolation
>>
>> the only available option is to do it in the kernel (that is what I got
>> as suggestion).
> 
> This sounds like there's some other way of getting the key into the
> kernel which doesn't rely on userspace.  Or are you assuming that nobody
> bothered to trojan 'cat'?

Apologies for not providing the full information at once. I'm worried 
that would be too long, and pieces can be lost in the way. If it is not 
a problem, I'm going to clarify on request.

Ok, so, I'm not going to use cat to upload the PGP keys. These will be 
embedded in the kernel image, when the Linux distribution vendors build 
their kernel.

This works for both secure boot and trusted boot, since the kernel image 
can be measured/verified by the boot loader.

Another source for keys is the MOK database, since users might want the 
ability to verify their own software, which does not come from the Linux 
distribution.

I briefly anticipated the full picture, but I will tell it more explicitly.

The kernel, with the embedded PGP keys, will be able to verify the 
signature of the RPM package headers.

A component that I recently developed, the digest_cache LSM, has the 
ability to extract file digests from RPM headers and provide a simple 
interface for IMA, IPE, BPF LSM and any other component to query the 
calculated digest of files being accessed, and allow/deny access to them 
depending on whether the query is successful or not.

I already anticipate the question, if you have the problem parsing PGP 
keys, why don't you have the problem parsing RPM package headers?

I started finding a solution before this became available, and the only 
alternative I found was to formally verify my code. So, I took Frama-C, 
wrote the assertions, and verified that not only the code is 
functionally correct for correct sequences of bytes, but that there is 
no illegal memory access for any arbitrary sequence (unfortunately, I 
can prove for a small buffer size).

So, I'm probably going to do the same for the PGP parser, if this does 
not fly. But, we were very optimistic that this could be a valid 
alternative!

Roberto

On Fri, 2024-02-16 at 20:54 +0100, Roberto Sassu wrote:
> On 2/16/2024 7:44 PM, Matthew Wilcox wrote:
> > On Fri, Feb 16, 2024 at 05:53:01PM +0100, Roberto Sassu wrote:
> > > On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
> > > > On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
> > > > > From: David Howells <dhowells@redhat.com>
> > > > > 
> > > > > Implement a PGP data parser for the crypto key type to use when
> > > > > instantiating a key.
> > > > > 
> > > > > This parser attempts to parse the instantiation data as a PGP packet
> > > > > sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
> > > > > algorithm key or subkey from it.
> > > > 
> > > > I don't understand why we want to do this in-kernel instead of in
> > > > userspace and then pass in the actual key.
> > > 
> > > Sigh, this is a long discussion.
> > 
> > Well, yes.  When you don't lay out why this is of value, it turns into a
> > long discussion.  This isn't fun for me either.
> > 
> > > PGP keys would be used as a system-wide trust anchor to verify RPM
> > > package headers, which already contain file digests that can be used as
> > > reference values for kernel-enforced integrity appraisal.
> > 
> > The one example we have of usage comes in patch 7 of this series and is:
> > 
> > gpg --dearmor < <PGP key> | keyctl padd asymmetric "" @u
> > 
> > And you're already using two userspace programs there.  Why not a third?
> 
> I think this is very easy to answer. Why not extracting the public key 
> from an x509 certificate in user space, sending it to the kernel, and 
> using it for kernel module verification?
> 
> > gpg --dearmor < <PGP key> | ./scripts/parse-pgp-packets | keyctl padd asymmetric "" @u
> > 
> > > With the assumptions that:
> > > 
> > > - In a locked-down system the kernel has more privileges than root
> > > - The kernel cannot offload this task to an user space process due to
> > >    insufficient isolation
> > > 
> > > the only available option is to do it in the kernel (that is what I got
> > > as suggestion).
> > 
> > This sounds like there's some other way of getting the key into the
> > kernel which doesn't rely on userspace.  Or are you assuming that nobody
> > bothered to trojan 'cat'?
> 
> Apologies for not providing the full information at once. I'm worried 
> that would be too long, and pieces can be lost in the way. If it is not 
> a problem, I'm going to clarify on request.
> 
> Ok, so, I'm not going to use cat to upload the PGP keys. These will be 
> embedded in the kernel image, when the Linux distribution vendors build 
> their kernel.
> 
> This works for both secure boot and trusted boot, since the kernel image 
> can be measured/verified by the boot loader.
> 
> Another source for keys is the MOK database, since users might want the 
> ability to verify their own software, which does not come from the Linux 
> distribution.
> 
> I briefly anticipated the full picture, but I will tell it more explicitly.
> 
> The kernel, with the embedded PGP keys, will be able to verify the 
> signature of the RPM package headers.
> 
> A component that I recently developed, the digest_cache LSM, has the 
> ability to extract file digests from RPM headers and provide a simple 
> interface for IMA, IPE, BPF LSM and any other component to query the 
> calculated digest of files being accessed, and allow/deny access to them 
> depending on whether the query is successful or not.

Not the proper thread, but since we talked about it...

I finally put together the PGP key and signature parser, the
digest_cache LSM and the IMA integration patch sets, and built three
openSUSE Tumbleweed packages (kernel, digest-cache-tools, dracut),
which basically enable the integrity features that the kernel (IMA)
supports:

- IMA measurement list with RPM headers and (eventually) unknown files 
  that are not from Tumbleweed;

- Ability to obtain a deterministic TPM PCR value, suitable for sealing
  of TPM keys

- Out of the box integrity enforcement on executable code, based on the
  provenance from openSUSE Tumbleweed; nothing else is required other
  than those three packages

An introduction and a guide with configuration steps can be found at:

https://github.com/linux-integrity/digest-cache-tools

I would also appreciate your comments.

Thanks

Roberto

> I already anticipate the question, if you have the problem parsing PGP 
> keys, why don't you have the problem parsing RPM package headers?
> 
> I started finding a solution before this became available, and the only 
> alternative I found was to formally verify my code. So, I took Frama-C, 
> wrote the assertions, and verified that not only the code is 
> functionally correct for correct sequences of bytes, but that there is 
> no illegal memory access for any arbitrary sequence (unfortunately, I 
> can prove for a small buffer size).
> 
> So, I'm probably going to do the same for the PGP parser, if this does 
> not fly. But, we were very optimistic that this could be a valid 
> alternative!
> 
> Roberto

On February 16, 2024 8:53:01 AM PST, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
>> On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
>> > From: David Howells <dhowells@redhat.com>
>> > 
>> > Implement a PGP data parser for the crypto key type to use when
>> > instantiating a key.
>> > 
>> > This parser attempts to parse the instantiation data as a PGP packet
>> > sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
>> > algorithm key or subkey from it.
>> 
>> I don't understand why we want to do this in-kernel instead of in
>> userspace and then pass in the actual key.
>
>Sigh, this is a long discussion.
>
>PGP keys would be used as a system-wide trust anchor to verify RPM
>package headers, which already contain file digests that can be used as
>reference values for kernel-enforced integrity appraisal.
>
>With the assumptions that:
>
>- In a locked-down system the kernel has more privileges than root
>- The kernel cannot offload this task to an user space process due to
>  insufficient isolation
>
>the only available option is to do it in the kernel (that is what I got
>as suggestion).
>
>Roberto
>
>

Ok, at least one of those assumptions is false, and *definitely* this approach seems to be a solution in search of a problem.

On 2/16/2024 6:08 PM, H. Peter Anvin wrote:
> On February 16, 2024 8:53:01 AM PST, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>> On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
>>> On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
>>>> From: David Howells <dhowells@redhat.com>
>>>>
>>>> Implement a PGP data parser for the crypto key type to use when
>>>> instantiating a key.
>>>>
>>>> This parser attempts to parse the instantiation data as a PGP packet
>>>> sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
>>>> algorithm key or subkey from it.
>>>
>>> I don't understand why we want to do this in-kernel instead of in
>>> userspace and then pass in the actual key.
>>
>> Sigh, this is a long discussion.
>>
>> PGP keys would be used as a system-wide trust anchor to verify RPM
>> package headers, which already contain file digests that can be used as
>> reference values for kernel-enforced integrity appraisal.
>>
>> With the assumptions that:
>>
>> - In a locked-down system the kernel has more privileges than root
>> - The kernel cannot offload this task to an user space process due to
>>  insufficient isolation
>>
>> the only available option is to do it in the kernel (that is what I got
>> as suggestion).
>>
>> Roberto
>>
>>
> 
> Ok, at least one of those assumptions is false, and *definitely* this approach seems to be a solution in search of a problem.

As a matter of fact, there is some truth to this observation.

The frustrating story of Roberto's PGP parser sparked the idea, but it
would clearly be overkill to add all this code just for this one parser.
I started looking around if there are other potential uses of a sandbox
mode, which might justify the effort. I quickly found out that it is
difficult to find a self-contained part of the kernel.

Now I believe that these dependencies among different parts of the
kernel present an issue, both to kernel security and to maintainability
of the source code. Even if sandbox mode as such is rejected (hopefully
with an explanation of the reasons), I believe that it is good to split
the kernel into smaller parts and reduce their interdependencies. In
this sense, sandbox mode is a way to express and enforce the remaining
dependencies.

Petr T

On February 20, 2024 2:55:12 AM PST, Petr Tesarik <petr.tesarik1@huawei-partners.com> wrote:
>On 2/16/2024 6:08 PM, H. Peter Anvin wrote:
>> On February 16, 2024 8:53:01 AM PST, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>>> On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
>>>> On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
>>>>> From: David Howells <dhowells@redhat.com>
>>>>>
>>>>> Implement a PGP data parser for the crypto key type to use when
>>>>> instantiating a key.
>>>>>
>>>>> This parser attempts to parse the instantiation data as a PGP packet
>>>>> sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
>>>>> algorithm key or subkey from it.
>>>>
>>>> I don't understand why we want to do this in-kernel instead of in
>>>> userspace and then pass in the actual key.
>>>
>>> Sigh, this is a long discussion.
>>>
>>> PGP keys would be used as a system-wide trust anchor to verify RPM
>>> package headers, which already contain file digests that can be used as
>>> reference values for kernel-enforced integrity appraisal.
>>>
>>> With the assumptions that:
>>>
>>> - In a locked-down system the kernel has more privileges than root
>>> - The kernel cannot offload this task to an user space process due to
>>>  insufficient isolation
>>>
>>> the only available option is to do it in the kernel (that is what I got
>>> as suggestion).
>>>
>>> Roberto
>>>
>>>
>> 
>> Ok, at least one of those assumptions is false, and *definitely* this approach seems to be a solution in search of a problem.
>
>As a matter of fact, there is some truth to this observation.
>
>The frustrating story of Roberto's PGP parser sparked the idea, but it
>would clearly be overkill to add all this code just for this one parser.
>I started looking around if there are other potential uses of a sandbox
>mode, which might justify the effort. I quickly found out that it is
>difficult to find a self-contained part of the kernel.
>
>Now I believe that these dependencies among different parts of the
>kernel present an issue, both to kernel security and to maintainability
>of the source code. Even if sandbox mode as such is rejected (hopefully
>with an explanation of the reasons), I believe that it is good to split
>the kernel into smaller parts and reduce their interdependencies. In
>this sense, sandbox mode is a way to express and enforce the remaining
>dependencies.
>
>Petr T

Congratulations. You just reinvented the microkernel.

On Wed, 21 Feb 2024 06:02:30 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> On February 20, 2024 2:55:12 AM PST, Petr Tesarik <petr.tesarik1@huawei-partners.com> wrote:
> >On 2/16/2024 6:08 PM, H. Peter Anvin wrote:  
> >> On February 16, 2024 8:53:01 AM PST, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:  
> >>> On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:  
> >>>> On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:  
> >>>>> From: David Howells <dhowells@redhat.com>
> >>>>>
> >>>>> Implement a PGP data parser for the crypto key type to use when
> >>>>> instantiating a key.
> >>>>>
> >>>>> This parser attempts to parse the instantiation data as a PGP packet
> >>>>> sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
> >>>>> algorithm key or subkey from it.  
> >>>>
> >>>> I don't understand why we want to do this in-kernel instead of in
> >>>> userspace and then pass in the actual key.  
> >>>
> >>> Sigh, this is a long discussion.
> >>>
> >>> PGP keys would be used as a system-wide trust anchor to verify RPM
> >>> package headers, which already contain file digests that can be used as
> >>> reference values for kernel-enforced integrity appraisal.
> >>>
> >>> With the assumptions that:
> >>>
> >>> - In a locked-down system the kernel has more privileges than root
> >>> - The kernel cannot offload this task to an user space process due to
> >>>  insufficient isolation
> >>>
> >>> the only available option is to do it in the kernel (that is what I got
> >>> as suggestion).
> >>>
> >>> Roberto
> >>>
> >>>  
> >> 
> >> Ok, at least one of those assumptions is false, and *definitely* this approach seems to be a solution in search of a problem.  
> >
> >As a matter of fact, there is some truth to this observation.
> >
> >The frustrating story of Roberto's PGP parser sparked the idea, but it
> >would clearly be overkill to add all this code just for this one parser.
> >I started looking around if there are other potential uses of a sandbox
> >mode, which might justify the effort. I quickly found out that it is
> >difficult to find a self-contained part of the kernel.
> >
> >Now I believe that these dependencies among different parts of the
> >kernel present an issue, both to kernel security and to maintainability
> >of the source code. Even if sandbox mode as such is rejected (hopefully
> >with an explanation of the reasons), I believe that it is good to split
> >the kernel into smaller parts and reduce their interdependencies. In
> >this sense, sandbox mode is a way to express and enforce the remaining
> >dependencies.
> >
> >Petr T  
> 
> Congratulations. You just reinvented the microkernel.

Oh, I have never claimed that the idea is completely new. There is a
lot of prior research in this field; the most advanced project is
probably Lightweight Execution Domains (LXDs), presented at USENIX ATC
in 2019 [1]. This one even adds a full-blown microkernel...

However, these projects have not gone anywhere, for some reason. I
tried to understand the reason and it seems to me that it is not the
underlying concept. I believe the main issue is that it would require
extremely intrusive changes in the overall design of the kernel. For
example LXDs run their microkernel on a dedicated CPU core and it uses
IDL to generate the glue code which passes data between Linux and this
newly introduced microkernel...

Our development process is more suited to incremental changes. This is
reflected by SandBox Mode. It allows to start small, keep the existing
overall design, and chip off only a few selected parts.

In short, SBM does borrow some ideas from microkernels but it does not
turn Linux into a microkernel. OTOH it enhances your freedom of
choice. If you change your mind and decide to make a Linux microkernel
after all, SBM will be able to help you during the transition.

Petr T

[1] https://www.usenix.org/system/files/atc19-narayanan.pdf

On Fri, 2024-02-16 at 09:08 -0800, H. Peter Anvin wrote:
> On February 16, 2024 8:53:01 AM PST, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > On Fri, 2024-02-16 at 16:44 +0000, Matthew Wilcox wrote:
> > > On Fri, Feb 16, 2024 at 04:24:33PM +0100, Petr Tesarik wrote:
> > > > From: David Howells <dhowells@redhat.com>
> > > > 
> > > > Implement a PGP data parser for the crypto key type to use when
> > > > instantiating a key.
> > > > 
> > > > This parser attempts to parse the instantiation data as a PGP packet
> > > > sequence (RFC 4880) and if it parses okay, attempts to extract a public-key
> > > > algorithm key or subkey from it.
> > > 
> > > I don't understand why we want to do this in-kernel instead of in
> > > userspace and then pass in the actual key.
> > 
> > Sigh, this is a long discussion.
> > 
> > PGP keys would be used as a system-wide trust anchor to verify RPM
> > package headers, which already contain file digests that can be used as
> > reference values for kernel-enforced integrity appraisal.
> > 
> > With the assumptions that:
> > 
> > - In a locked-down system the kernel has more privileges than root
> > - The kernel cannot offload this task to an user space process due to
> >  insufficient isolation
> > 
> > the only available option is to do it in the kernel (that is what I got
> > as suggestion).
> > 
> > Roberto
> > 
> > 
> 
> Ok, at least one of those assumptions is false, and *definitely* this approach seems to be a solution in search of a problem.

I'm looking for a solution to this for a long time. Could you please
explain?

Thanks

Roberto