linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
@ 2025-12-04 22:31 Jarkko Sakkinen
  2025-12-04 23:20 ` Jarkko Sakkinen
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-12-04 22:31 UTC (permalink / raw)
  To: linux-integrity
  Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, James Bottomley,
	Mimi Zohar, David Howells, Paul Moore, James Morris,
	Serge E. Hallyn, open list, open list:KEYS-TRUSTED,
	open list:SECURITY SUBSYSTEM

tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and calls to
tpm_buf_append_name() cause the exact same TPM2_ReadPublic command to be
sent to the chip, causing unnecessary traffic.

1. Export tpm2_read_public in order to make it callable from 'trusted_tpm2'.
2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted() in order to
   halve the name resolutions required:
2a. Move tpm2_read_public() calls into trusted_tpm2.
2b. Pass TPM name to tpm_buf_append_name().
2c. Rework tpm_buf_append_name() to use the pre-resolved name.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm2-cmd.c               |   3 +-
 drivers/char/tpm/tpm2-sessions.c          |  95 +++++------------
 include/linux/tpm.h                       |  10 +-
 security/keys/trusted-keys/trusted_tpm2.c | 124 ++++++++++++++--------
 4 files changed, 118 insertions(+), 114 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 3a77be7ebf4a..1f561ad3bdcf 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -202,7 +202,8 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	}
 
 	if (!disable_pcr_integrity) {
-		rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+		rc = tpm_buf_append_name(chip, &buf, pcr_idx, (u8 *)&pcr_idx,
+					 sizeof(u32));
 		if (rc) {
 			tpm_buf_destroy(&buf);
 			return rc;
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 4149379665c4..e33be09446ff 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -136,8 +136,8 @@ struct tpm2_auth {
 	 * handle, but they are part of the session by name, which
 	 * we must compute and remember
 	 */
-	u32 name_h[AUTH_MAX_NAMES];
 	u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
+	u16 name_size_tbl[AUTH_MAX_NAMES];
 };
 
 #ifdef CONFIG_TCG_TPM2_HMAC
@@ -163,7 +163,17 @@ static int name_size(const u8 *name)
 	}
 }
 
-static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
+/**
+ * tpm2_read_public: Resolve TPM name for a handle
+ * @chip:		TPM chip to use.
+ * @handle:		TPM handle.
+ * @name:		A buffer for returning the name blob. Must have a
+ *			capacity of 'SHA512_DIGET_SIZE + 2' bytes at minimum
+ *
+ * Returns size of TPM handle name of success.
+ * Returns tpm_transmit_cmd error codes when TPM2_ReadPublic fails.
+ */
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
 {
 	u32 mso = tpm2_handle_mso(handle);
 	off_t offset = TPM_HEADER_SIZE;
@@ -219,14 +229,16 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
 	memcpy(name, &buf.data[offset], rc);
 	return name_size_alg;
 }
+EXPORT_SYMBOL_GPL(tpm2_read_public);
 #endif /* CONFIG_TCG_TPM2_HMAC */
 
 /**
- * tpm_buf_append_name() - add a handle area to the buffer
- * @chip: the TPM chip structure
- * @buf: The buffer to be appended
- * @handle: The handle to be appended
- * @name: The name of the handle (may be NULL)
+ * tpm_buf_append_name() - Append a handle and store TPM name
+ * @chip:		TPM chip to use.
+ * @buf:		TPM buffer containing the TPM command in-transit.
+ * @handle:		TPM handle to be appended.
+ * @name:		TPM name of the handle
+ * @name_size:		Size of the TPM name.
  *
  * In order to compute session HMACs, we need to know the names of the
  * objects pointed to by the handles.  For most objects, this is simply
@@ -243,15 +255,14 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
  * will be caused by an incorrect programming model and indicated by a
  * kernel message.
  *
- * Ends the authorization session on failure.
+ * Returns zero on success.
+ * Returns -EIO when the authorization area state is malformed.
  */
 int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
-			u32 handle, u8 *name)
+			u32 handle, u8 *name, u16 name_size)
 {
 #ifdef CONFIG_TCG_TPM2_HMAC
-	enum tpm2_mso_type mso = tpm2_handle_mso(handle);
 	struct tpm2_auth *auth;
-	u16 name_size_alg;
 	int slot;
 	int ret;
 #endif
@@ -276,36 +287,15 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 	}
 	tpm_buf_append_u32(buf, handle);
 	auth->session += 4;
-
-	if (mso == TPM2_MSO_PERSISTENT ||
-	    mso == TPM2_MSO_VOLATILE ||
-	    mso == TPM2_MSO_NVRAM) {
-		if (!name) {
-			ret = tpm2_read_public(chip, handle, auth->name[slot]);
-			if (ret < 0)
-				goto err;
-
-			name_size_alg = ret;
-		}
-	} else {
-		if (name) {
-			dev_err(&chip->dev, "handle 0x%08x does not use a name\n",
-				handle);
-			ret = -EIO;
-			goto err;
-		}
-	}
-
-	auth->name_h[slot] = handle;
-	if (name)
-		memcpy(auth->name[slot], name, name_size_alg);
+	memcpy(auth->name[slot], name, name_size);
+	auth->name_size_tbl[slot] = name_size;
 #endif
 	return 0;
 
 #ifdef CONFIG_TCG_TPM2_HMAC
 err:
 	tpm2_end_auth_session(chip);
-	return tpm_ret_to_err(ret);
+	return ret;
 #endif
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -613,22 +603,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
 	attrs = chip->cc_attrs_tbl[i];
 
 	handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+	offset_s += handles * sizeof(u32);
 
-	/*
-	 * just check the names, it's easy to make mistakes.  This
-	 * would happen if someone added a handle via
-	 * tpm_buf_append_u32() instead of tpm_buf_append_name()
-	 */
-	for (i = 0; i < handles; i++) {
-		u32 handle = tpm_buf_read_u32(buf, &offset_s);
-
-		if (auth->name_h[i] != handle) {
-			dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
-			ret = -EIO;
-			goto err;
-		}
-	}
-	/* point offset_s to the start of the sessions */
 	val = tpm_buf_read_u32(buf, &offset_s);
 	/* point offset_p to the start of the parameters */
 	offset_p = offset_s + val;
@@ -689,23 +665,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
 	/* ordinal is already BE */
 	sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
 	/* add the handle names */
-	for (i = 0; i < handles; i++) {
-		enum tpm2_mso_type mso = tpm2_handle_mso(auth->name_h[i]);
-
-		if (mso == TPM2_MSO_PERSISTENT ||
-		    mso == TPM2_MSO_VOLATILE ||
-		    mso == TPM2_MSO_NVRAM) {
-			ret = name_size(auth->name[i]);
-			if (ret < 0)
-				goto err;
-
-			sha256_update(&sctx, auth->name[i], ret);
-		} else {
-			__be32 h = cpu_to_be32(auth->name_h[i]);
-
-			sha256_update(&sctx, (u8 *)&h, 4);
-		}
-	}
+	for (i = 0; i < handles; i++)
+		sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
 	if (offset_s != tpm_buf_length(buf))
 		sha256_update(&sctx, &buf->data[offset_s],
 			      tpm_buf_length(buf) - offset_s);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 202da079d500..319ba75dd79a 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -530,7 +530,7 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
 }
 
 int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
-			u32 handle, u8 *name);
+			u32 handle, u8 *name, u16 name_size);
 void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 				 u8 attributes, u8 *passphrase,
 				 int passphraselen);
@@ -544,6 +544,7 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
 int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
 				int rc);
 void tpm2_end_auth_session(struct tpm_chip *chip);
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name);
 #else
 #include <linux/unaligned.h>
 
@@ -567,6 +568,13 @@ static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
 {
 	return rc;
 }
+
+static inline int tpm2_read_public(struct tpm_chip *chip, u32 handle,
+				   void *name)
+{
+	memcpy(name, &handle, sizeof(u32));
+	return sizeof(u32);
+}
 #endif	/* CONFIG_TCG_TPM2_HMAC */
 
 #endif
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a7ea4a1c3bed..88bafbcc011a 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -233,8 +233,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options)
 {
+	u8 parent_name[2 + SHA512_DIGEST_SIZE];
 	off_t offset = TPM_HEADER_SIZE;
 	struct tpm_buf buf, sized;
+	u16 parent_name_size;
 	int blob_len = 0;
 	int hash;
 	u32 flags;
@@ -251,6 +253,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (rc)
 		return rc;
 
+	rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+	if (rc < 0)
+		goto out_put;
+
+	parent_name_size = rc;
+
 	rc = tpm2_start_auth_session(chip);
 	if (rc)
 		goto out_put;
@@ -268,7 +276,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out_put;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+				 parent_name_size);
 	if (rc)
 		goto out;
 
@@ -355,21 +364,25 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 }
 
 /**
- * tpm2_load_cmd() - execute a TPM2_Load command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: returned blob handle
+ * tpm2_load_cmd() - Execute TPM2_Load
+ * @chip:		TPM chip to use.
+ * @payload:		Key data in clear text.
+ * @options:		Trusted key options.
+ * @parent_name:	A cryptographic name, i.e. a TPMT_HA blob, of the
+ *			parent key.
+ * @blob:		The decoded payload for the key.
+ * @blob_handle:	On success, will contain handle to the loaded keyedhash
+ *			blob.
  *
- * Return: 0 on success.
- *        -E2BIG on wrong payload size.
- *        -EPERM on tpm error status.
- *        < 0 error from tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
  */
 static int tpm2_load_cmd(struct tpm_chip *chip,
 			 struct trusted_key_payload *payload,
 			 struct trusted_key_options *options,
+			 u8 *parent_name,
+			 u16 parent_name_size,
+			 const u8 *blob,
 			 u32 *blob_handle)
 {
 	u8 *blob_ref __free(kfree) = NULL;
@@ -377,27 +390,13 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	unsigned int private_len;
 	unsigned int public_len;
 	unsigned int blob_len;
-	u8 *blob, *pub;
+	const u8 *pub;
 	int rc;
 	u32 attrs;
 
-	rc = tpm2_key_decode(payload, options, &blob);
-	if (rc) {
-		/* old form */
-		blob = payload->blob;
-		payload->old_format = 1;
-	} else {
-		/* Bind for cleanup: */
-		blob_ref = blob;
-	}
-
-	/* new format carries keyhandle but old format doesn't */
-	if (!options->keyhandle)
-		return -EINVAL;
-
 	/* must be big enough for at least the two be16 size counts */
 	if (payload->blob_len < 4)
-		return -EINVAL;
+		return -E2BIG;
 
 	private_len = get_unaligned_be16(blob);
 
@@ -433,7 +432,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		return rc;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+				 parent_name_size);
 	if (rc)
 		goto out;
 
@@ -465,20 +465,23 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 }
 
 /**
- * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ * tpm2_unseal_cmd() - Execute TPM2_Unload
  *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: blob handle
+ * @chip:		TPM chip to use
+ * @payload:		Key data in clear text.
+ * @options:		Trusted key options.
+ * @parent_name:	A cryptographic name, i.e. a TPMT_HA blob, of the
+ *			parent key.
+ * @blob_handle:	Handle to the loaded keyedhash blob.
  *
- * Return: 0 on success
- *         -EPERM on tpm error status
- *         < 0 error from tpm_send
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
  */
 static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
+			   u8 *parent_name,
+			   u16 parent_name_size,
 			   u32 blob_handle)
 {
 	struct tpm_header *head;
@@ -498,7 +501,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 		return rc;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+				 parent_name_size);
 	if (rc)
 		goto out;
 
@@ -573,30 +577,60 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 }
 
 /**
- * tpm2_unseal_trusted() - unseal the payload of a trusted key
+ * tpm2_unseal_trusted() - Unseal a trusted key
+ * @chip:	TPM chip to use.
+ * @payload:	Key data in clear text.
+ * @options:	Trusted key options.
  *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: Same as with tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Return -EINVAL when parent's key handle has not been set.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load or TPM2_Unseal
+ * fails.
  */
 int tpm2_unseal_trusted(struct tpm_chip *chip,
 			struct trusted_key_payload *payload,
 			struct trusted_key_options *options)
 {
+	u8 *blob_ref __free(kfree) = NULL;
+	u8 parent_name[2 + SHA512_DIGEST_SIZE];
+	u16 parent_name_size;
 	u32 blob_handle;
+	u8 *blob;
 	int rc;
 
+	/*
+	 * Try to decode the provided blob as an ASN.1 blob. Assume that the
+	 * blob is in the legacy format if decoding does not end successfully.
+	 */
+	rc = tpm2_key_decode(payload, options, &blob);
+	if (rc) {
+		blob = payload->blob;
+		payload->old_format = 1;
+	} else {
+		blob_ref = blob;
+	}
+
+	if (!options->keyhandle)
+		return -EINVAL;
+
 	rc = tpm_try_get_ops(chip);
 	if (rc)
 		return rc;
 
-	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+	rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+	if (rc < 0)
+		goto out;
+
+	parent_name_size = rc;
+
+	rc = tpm2_load_cmd(chip, payload, options, parent_name,
+			   parent_name_size, blob, &blob_handle);
 	if (rc)
 		goto out;
 
-	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+	rc = tpm2_unseal_cmd(chip, payload, options, parent_name,
+			     parent_name_size, blob_handle);
+
 	tpm2_flush_context(chip, blob_handle);
 
 out:
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
  2025-12-04 22:31 [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls Jarkko Sakkinen
@ 2025-12-04 23:20 ` Jarkko Sakkinen
  2025-12-05  0:49   ` Jarkko Sakkinen
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-12-04 23:20 UTC (permalink / raw)
  To: linux-integrity
  Cc: Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
	David Howells, Paul Moore, James Morris, Serge E. Hallyn,
	open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM

On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and calls to
> tpm_buf_append_name() cause the exact same TPM2_ReadPublic command to be
> sent to the chip, causing unnecessary traffic.
> 
> 1. Export tpm2_read_public in order to make it callable from 'trusted_tpm2'.
> 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted() in order to
>    halve the name resolutions required:
> 2a. Move tpm2_read_public() calls into trusted_tpm2.
> 2b. Pass TPM name to tpm_buf_append_name().
> 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

If ASN.1 blob would contain also name of the parent then zero
tpm2_read_public() calls would be required i.e., the main bottleneck
here inherits from the limitations of the file format itself.

BR, Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
  2025-12-04 23:20 ` Jarkko Sakkinen
@ 2025-12-05  0:49   ` Jarkko Sakkinen
  2025-12-07  6:33     ` James Bottomley
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-12-05  0:49 UTC (permalink / raw)
  To: linux-integrity
  Cc: Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
	David Howells, Paul Moore, James Morris, Serge E. Hallyn,
	open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and calls to
> > tpm_buf_append_name() cause the exact same TPM2_ReadPublic command to be
> > sent to the chip, causing unnecessary traffic.
> > 
> > 1. Export tpm2_read_public in order to make it callable from 'trusted_tpm2'.
> > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted() in order to
> >    halve the name resolutions required:
> > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > 2b. Pass TPM name to tpm_buf_append_name().
> > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> If ASN.1 blob would contain also name of the parent then zero
> tpm2_read_public() calls would be required i.e., the main bottleneck
> here inherits from the limitations of the file format itself.

Along the lines of attached patch.

BR, Jarkko

[-- Attachment #2: 0001-KEYS-trusted-Extend-TPMKey-ASN.1-definition-with-par.patch --]
[-- Type: text/plain, Size: 7032 bytes --]

From c5fc7e38aae838dd1190b33545a8b1a1696e9ce8 Mon Sep 17 00:00:00 2001
From: Jarkko Sakkinen <jarkko@kernel.org>
Date: Fri, 5 Dec 2025 01:53:33 +0200
Subject: [PATCH] KEYS: trusted: Extend TPMKey ASN.1 definition with
 'parentName'

Extend TPMKey ASN.1 definition [1] with an optional 'parentName' attribute
containing TPMT_HA blob for the parent. Encode this attribute for the
generated TPM keys, which allows skipping TPM2_ReadPublic when unsealing
the key.

[1] https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.txt

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/trusted-keys/tpm2key.asn1   | 17 ++++-
 security/keys/trusted-keys/trusted_tpm2.c | 80 +++++++++++++++--------
 2 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
index f57f869ad600..080f0e399982 100644
--- a/security/keys/trusted-keys/tpm2key.asn1
+++ b/security/keys/trusted-keys/tpm2key.asn1
@@ -1,11 +1,26 @@
 ---
 --- ASN.1 for TPM 2.0 keys
 ---
+TPMPolicy ::= SEQUENCE {
+	commandCode	[0] EXPLICIT INTEGER,
+	commandPolicy	[1] EXPLICIT OCTET STRING
+}
+
+TPMAuthPolicy ::= SEQUENCE {
+	name		[0] EXPLICIT UTF8String OPTIONAL,
+	policy		[1] EXPLICIT SEQUENCE OF TPMPolicy
+}
 
 TPMKey ::= SEQUENCE {
 	type		OBJECT IDENTIFIER ({tpm2_key_type}),
 	emptyAuth	[0] EXPLICIT BOOLEAN OPTIONAL,
+	policy		[1] EXPLICIT SEQUENCE OF TPMPolicy OPTIONAL,
+	secret		[2] EXPLICIT OCTET STRING OPTIONAL,
+	authPolicy	[3] EXPLICIT SEQUENCE OF TPMAuthPolicy OPTIONAL,
+	description	[4] EXPLICIT UTF8String OPTIONAL,
+	rsaParent	[5] EXPLICIT BOOLEAN OPTIONAL,
+	parentName	[6] EXPLICIT OCTET STRING ({tpm2_key_parent_name}),
 	parent		INTEGER ({tpm2_key_parent}),
 	pubkey		OCTET STRING ({tpm2_key_pub}),
 	privkey		OCTET STRING ({tpm2_key_priv})
-	}
+}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 88bafbcc011a..85fd34457431 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -20,16 +20,26 @@
 
 static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
 
+enum tpm_key_tag {
+	TPM_KEY_TAG_EMPTY_AUTH	= 0,
+	TPM_KEY_TAG_POLICY	= 1,
+	TPM_KEY_TAG_SECRET	= 2,
+	TPM_KEY_TAG_AUTH_POLICY	= 3,
+	TPM_KEY_TAG_DESCRIPTION = 4,
+	TPM_KEY_TAG_RSA_PARENT	= 5,
+	TPM_KEY_TAG_PARENT_NAME	= 6,
+};
+
 static int tpm2_key_encode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 *src, u32 len)
+			   u8 *src, u32 len, u8 *parent_name,
+			   u16 parent_name_size)
 {
 	const int SCRATCH_SIZE = PAGE_SIZE;
-	u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
-	u8 *work = scratch, *work1;
-	u8 *end_work = scratch + SCRATCH_SIZE;
-	u8 *priv, *pub;
+	u8 *end_work, *end_name;
 	u16 priv_len, pub_len;
+	u8 *work, *work1;
+	u8 *priv, *pub;
 	int ret;
 
 	priv_len = get_unaligned_be16(src) + 2;
@@ -40,9 +50,13 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	pub_len = get_unaligned_be16(src) + 2;
 	pub = src;
 
+	u8 *scratch __free(kfree) = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
 	if (!scratch)
 		return -ENOMEM;
 
+	work = scratch;
+	end_work = scratch + SCRATCH_SIZE;
+
 	work = asn1_encode_oid(work, end_work, tpm2key_oid,
 			       asn1_oid_len(tpm2key_oid));
 
@@ -50,13 +64,22 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 		unsigned char bool[3], *w = bool;
 		/* tag 0 is emptyAuth */
 		w = asn1_encode_boolean(w, w + sizeof(bool), true);
-		if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) {
-			ret = PTR_ERR(w);
-			goto err;
-		}
-		work = asn1_encode_tag(work, end_work, 0, bool, w - bool);
+		if (WARN(IS_ERR(w), "BUG: Boolean failed to encode"))
+			return PTR_ERR(w);
+		work = asn1_encode_tag(work, end_work, TPM_KEY_TAG_EMPTY_AUTH,
+				       bool, w - bool);
 	}
 
+	u8 *name_encoded __free(kfree) = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
+	if (!name_encoded)
+		return -ENOMEM;
+
+	end_name = asn1_encode_octet_string(name_encoded,
+					    name_encoded + SCRATCH_SIZE,
+					    parent_name, parent_name_size);
+	work = asn1_encode_tag(work, end_work, TPM_KEY_TAG_PARENT_NAME,
+			       name_encoded, end_name - name_encoded);
+
 	/*
 	 * Assume both octet strings will encode to a 2 byte definite length
 	 *
@@ -65,8 +88,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	 */
 	if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE,
 		 "BUG: scratch buffer is too small")) {
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	work = asn1_encode_integer(work, end_work, options->keyhandle);
@@ -79,15 +101,10 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	if (IS_ERR(work1)) {
 		ret = PTR_ERR(work1);
 		pr_err("BUG: ASN.1 encoder failed with %d\n", ret);
-		goto err;
+		return ret;
 	}
 
-	kfree(scratch);
 	return work1 - payload->blob;
-
-err:
-	kfree(scratch);
-	return ret;
 }
 
 struct tpm2_key_context {
@@ -96,11 +113,13 @@ struct tpm2_key_context {
 	u32 pub_len;
 	const u8 *priv;
 	u32 priv_len;
+	const u8 *name;
+	u32 name_len;
 };
 
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u8 **buf)
+			   u8 **buf, u8 *parent_name, u16 *parent_name_size)
 {
 	int ret;
 	struct tpm2_key_context ctx;
@@ -127,6 +146,8 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 	blob += ctx.priv_len;
 
 	memcpy(blob, ctx.pub, ctx.pub_len);
+	memcpy(parent_name, ctx.name, ctx.name_len);
+	*parent_name_size = ctx.name_len;
 
 	return 0;
 }
@@ -190,6 +211,16 @@ int tpm2_key_priv(void *context, size_t hdrlen,
 	return 0;
 }
 
+int tpm2_key_parent_name(void *context, size_t hdrlen, unsigned char tag,
+			 const void *value, size_t vlen)
+{
+	struct tpm2_key_context *ctx = context;
+
+	ctx->name = value;
+	ctx->name_len = vlen;
+
+	return 0;
+}
 /**
  * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
  *
@@ -347,7 +378,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+	blob_len = tpm2_key_encode(payload, options, &buf.data[offset],
+				   blob_len, parent_name, parent_name_size);
 	if (blob_len < 0)
 		rc = blob_len;
 
@@ -602,7 +634,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	 * Try to decode the provided blob as an ASN.1 blob. Assume that the
 	 * blob is in the legacy format if decoding does not end successfully.
 	 */
-	rc = tpm2_key_decode(payload, options, &blob);
+	rc = tpm2_key_decode(payload, options, &blob, &parent_name[0], &parent_name_size);
 	if (rc) {
 		blob = payload->blob;
 		payload->old_format = 1;
@@ -617,12 +649,6 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	if (rc)
 		return rc;
 
-	rc = tpm2_read_public(chip, options->keyhandle, parent_name);
-	if (rc < 0)
-		goto out;
-
-	parent_name_size = rc;
-
 	rc = tpm2_load_cmd(chip, payload, options, parent_name,
 			   parent_name_size, blob, &blob_handle);
 	if (rc)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
  2025-12-05  0:49   ` Jarkko Sakkinen
@ 2025-12-07  6:33     ` James Bottomley
  2025-12-07  9:19       ` Jarkko Sakkinen
  0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2025-12-07  6:33 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: Peter Huewe, Jason Gunthorpe, Mimi Zohar, David Howells,
	Paul Moore, James Morris, Serge E. Hallyn, open list,
	open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM

On Fri, 2025-12-05 at 02:49 +0200, Jarkko Sakkinen wrote:
> On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> > On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and
> > > calls to
> > > tpm_buf_append_name() cause the exact same TPM2_ReadPublic
> > > command to be
> > > sent to the chip, causing unnecessary traffic.
> > > 
> > > 1. Export tpm2_read_public in order to make it callable from
> > > 'trusted_tpm2'.
> > > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted()
> > > in order to
> > >    halve the name resolutions required:
> > > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > > 2b. Pass TPM name to tpm_buf_append_name().
> > > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > If ASN.1 blob would contain also name of the parent then zero
> > tpm2_read_public() calls would be required i.e., the main
> > bottleneck here inherits from the limitations of the file format
> > itself.
> 
> Along the lines of attached patch.

Well firstly [6] is already being taken by the creation data proposal,
so this would introduce an incompatibility between the kernel and the
spec, but secondly, if you want something like this in the spec before
it goes to the IETF you really need to propose it now.

The problem with this particular addition is that it would be Linux
Kernel specific.  All the current TSSs already do a cached read public
under the covers when they add the session wrappings so the user facing
API they expose has nowhere to insert (or easily extract) a name field
and thus TSS based implementations would have no incentive to either
output or consume this field.  That's not to say the standard can't
have additions for crazy or niche use cases (that's what the rsaParent
flag is: a one off to support a niche SUSE use case) but it would be
hard to persuade user implementations to do this so the kernel would
have to interoperate with the case where it didn't exist anyway.

The standard use today is with permanent handles for parents, where the
parent is created on the fly, so the name is actually returned from
TPM2_CreatePrimary for this use case.  It should be a simple matter to
bring the kernel implementation up to doing this as well.

Regards,

James
  


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
  2025-12-07  6:33     ` James Bottomley
@ 2025-12-07  9:19       ` Jarkko Sakkinen
  2025-12-07  9:28         ` Jarkko Sakkinen
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-12-07  9:19 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Mimi Zohar,
	David Howells, Paul Moore, James Morris, Serge E. Hallyn,
	open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM

On Sun, Dec 07, 2025 at 03:33:17PM +0900, James Bottomley wrote:
> On Fri, 2025-12-05 at 02:49 +0200, Jarkko Sakkinen wrote:
> > On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> > > On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > > > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and
> > > > calls to
> > > > tpm_buf_append_name() cause the exact same TPM2_ReadPublic
> > > > command to be
> > > > sent to the chip, causing unnecessary traffic.
> > > > 
> > > > 1. Export tpm2_read_public in order to make it callable from
> > > > 'trusted_tpm2'.
> > > > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted()
> > > > in order to
> > > >    halve the name resolutions required:
> > > > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > > > 2b. Pass TPM name to tpm_buf_append_name().
> > > > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > > > 
> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > 
> > > If ASN.1 blob would contain also name of the parent then zero
> > > tpm2_read_public() calls would be required i.e., the main
> > > bottleneck here inherits from the limitations of the file format
> > > itself.
> > 
> > Along the lines of attached patch.
> 
> Well firstly [6] is already being taken by the creation data proposal,
> so this would introduce an incompatibility between the kernel and the
> spec, but secondly, if you want something like this in the spec before
> it goes to the IETF you really need to propose it now.

What is the mailing list for the working group, or is this still
unclear as of today?

> 
> The problem with this particular addition is that it would be Linux
> Kernel specific.  All the current TSSs already do a cached read public
> under the covers when they add the session wrappings so the user facing
> API they expose has nowhere to insert (or easily extract) a name field
> and thus TSS based implementations would have no incentive to either
> output or consume this field.  That's not to say the standard can't
> have additions for crazy or niche use cases (that's what the rsaParent
> flag is: a one off to support a niche SUSE use case) but it would be
> hard to persuade user implementations to do this so the kernel would
> have to interoperate with the case where it didn't exist anyway.

It is just matter of conditionally calling tpm2_read_public(). Not
a big deal.

> 
> The standard use today is with permanent handles for parents, where the
> parent is created on the fly, so the name is actually returned from
> TPM2_CreatePrimary for this use case.  It should be a simple matter to
> bring the kernel implementation up to doing this as well.

Intercepting TPM2_CreatePrimary does not provide full coverage, as
it does not address persistent keys, which kernel interface does
support.

> 
> Regards,
> 
> James
>   
> 

BR, Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
  2025-12-07  9:19       ` Jarkko Sakkinen
@ 2025-12-07  9:28         ` Jarkko Sakkinen
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-12-07  9:28 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Mimi Zohar,
	David Howells, Paul Moore, James Morris, Serge E. Hallyn,
	open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM

On Sun, Dec 07, 2025 at 11:19:10AM +0200, Jarkko Sakkinen wrote:
> On Sun, Dec 07, 2025 at 03:33:17PM +0900, James Bottomley wrote:
> > On Fri, 2025-12-05 at 02:49 +0200, Jarkko Sakkinen wrote:
> > > On Fri, Dec 05, 2025 at 01:20:30AM +0200, Jarkko Sakkinen wrote:
> > > > On Fri, Dec 05, 2025 at 12:31:27AM +0200, Jarkko Sakkinen wrote:
> > > > > tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and
> > > > > calls to
> > > > > tpm_buf_append_name() cause the exact same TPM2_ReadPublic
> > > > > command to be
> > > > > sent to the chip, causing unnecessary traffic.
> > > > > 
> > > > > 1. Export tpm2_read_public in order to make it callable from
> > > > > 'trusted_tpm2'.
> > > > > 2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted()
> > > > > in order to
> > > > >    halve the name resolutions required:
> > > > > 2a. Move tpm2_read_public() calls into trusted_tpm2.
> > > > > 2b. Pass TPM name to tpm_buf_append_name().
> > > > > 2c. Rework tpm_buf_append_name() to use the pre-resolved name.
> > > > > 
> > > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > 
> > > > If ASN.1 blob would contain also name of the parent then zero
> > > > tpm2_read_public() calls would be required i.e., the main
> > > > bottleneck here inherits from the limitations of the file format
> > > > itself.
> > > 
> > > Along the lines of attached patch.
> > 
> > Well firstly [6] is already being taken by the creation data proposal,
> > so this would introduce an incompatibility between the kernel and the
> > spec, but secondly, if you want something like this in the spec before
> > it goes to the IETF you really need to propose it now.
> 
> What is the mailing list for the working group, or is this still
> unclear as of today?
> 
> > 
> > The problem with this particular addition is that it would be Linux
> > Kernel specific.  All the current TSSs already do a cached read public
> > under the covers when they add the session wrappings so the user facing
> > API they expose has nowhere to insert (or easily extract) a name field
> > and thus TSS based implementations would have no incentive to either
> > output or consume this field.  That's not to say the standard can't
> > have additions for crazy or niche use cases (that's what the rsaParent
> > flag is: a one off to support a niche SUSE use case) but it would be
> > hard to persuade user implementations to do this so the kernel would
> > have to interoperate with the case where it didn't exist anyway.
> 
> It is just matter of conditionally calling tpm2_read_public(). Not
> a big deal.
> 
> > 
> > The standard use today is with permanent handles for parents, where the
> > parent is created on the fly, so the name is actually returned from
> > TPM2_CreatePrimary for this use case.  It should be a simple matter to
> > bring the kernel implementation up to doing this as well.
> 
> Intercepting TPM2_CreatePrimary does not provide full coverage, as
> it does not address persistent keys, which kernel interface does
> support.

It also weaker solution in the sense that it lacks support for secondary
keys as parents.

Name digest is universal as it scales primary keys, secondary keys both
persistent and transient.

I'm not really sure why we should use TSS implementations as reference
for anything in this space. We always want to support "superset' because
it actually creates innovation and distruption to the pre-existing
science.

BR, Jarkko



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-12-07  9:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04 22:31 [PATCH] KEYS: trusted: Re-orchestrate tpm2_read_public() calls Jarkko Sakkinen
2025-12-04 23:20 ` Jarkko Sakkinen
2025-12-05  0:49   ` Jarkko Sakkinen
2025-12-07  6:33     ` James Bottomley
2025-12-07  9:19       ` Jarkko Sakkinen
2025-12-07  9:28         ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).