linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] KEYS: trusted_tpm1: HMAC fix and cleanup
@ 2025-08-09 17:19 Eric Biggers
  2025-08-09 17:19 ` [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time Eric Biggers
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Eric Biggers @ 2025-08-09 17:19 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, keyrings
  Cc: David Howells, linux-integrity, linux-crypto, linux-kernel,
	Eric Biggers

Patch 1 fixes the HMAC-SHA1 comparison in trusted_tpm1 to be
constant-time.

Patch 2 simplifies the SHA-1 and HMAC-SHA1 computation in trusted_tpm1
by using library APIs instead of crypto_shash.  Note that this depends
on the SHA-1 and HMAC-SHA1 library APIs that were merged for v6.17-rc1.

Patch 3 is a trusted_tpm1 cleanup that moves private functionality out
of a public header.

Changed in v2:
    - Added the requested note to the commit message of patch 1.
    - Added a Reviewed-by tag

Eric Biggers (3):
  KEYS: trusted_tpm1: Compare HMAC values in constant time
  KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash
  KEYS: trusted_tpm1: Move private functionality out of public header

 include/keys/trusted_tpm.h                |  79 ------
 security/keys/trusted-keys/Kconfig        |   5 +-
 security/keys/trusted-keys/trusted_tpm1.c | 284 ++++++++--------------
 3 files changed, 100 insertions(+), 268 deletions(-)


base-commit: 561c80369df0733ba0574882a1635287b20f9de2
-- 
2.50.1


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

* [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time
  2025-08-09 17:19 [PATCH v2 0/3] KEYS: trusted_tpm1: HMAC fix and cleanup Eric Biggers
@ 2025-08-09 17:19 ` Eric Biggers
  2025-08-12 16:23   ` Jarkko Sakkinen
  2025-08-09 17:19 ` [PATCH v2 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash Eric Biggers
  2025-08-09 17:19 ` [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header Eric Biggers
  2 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2025-08-09 17:19 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, keyrings
  Cc: David Howells, linux-integrity, linux-crypto, linux-kernel,
	Eric Biggers, stable

To prevent timing attacks, HMAC value comparison needs to be constant
time.  Replace the memcmp() with the correct function, crypto_memneq().

[For the Fixes commit I used the commit that introduced the memcmp().
It predates the introduction of crypto_memneq(), but it was still a bug
at the time even though a helper function didn't exist yet.]

Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 security/keys/trusted-keys/trusted_tpm1.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 89c9798d18007..e73f2c6c817a0 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -5,10 +5,11 @@
  *
  * See Documentation/security/keys/trusted-encrypted.rst
  */
 
 #include <crypto/hash_info.h>
+#include <crypto/utils.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/parser.h>
 #include <linux/string.h>
 #include <linux/err.h>
@@ -239,11 +240,11 @@ int TSS_checkhmac1(unsigned char *buffer,
 			  TPM_NONCE_SIZE, enonce, TPM_NONCE_SIZE, ononce,
 			  1, continueflag, 0, 0);
 	if (ret < 0)
 		goto out;
 
-	if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE))
+	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
 		ret = -EINVAL;
 out:
 	kfree_sensitive(sdesc);
 	return ret;
 }
@@ -332,20 +333,20 @@ static int TSS_checkhmac2(unsigned char *buffer,
 	ret = TSS_rawhmac(testhmac1, key1, keylen1, SHA1_DIGEST_SIZE,
 			  paramdigest, TPM_NONCE_SIZE, enonce1,
 			  TPM_NONCE_SIZE, ononce, 1, continueflag1, 0, 0);
 	if (ret < 0)
 		goto out;
-	if (memcmp(testhmac1, authdata1, SHA1_DIGEST_SIZE)) {
+	if (crypto_memneq(testhmac1, authdata1, SHA1_DIGEST_SIZE)) {
 		ret = -EINVAL;
 		goto out;
 	}
 	ret = TSS_rawhmac(testhmac2, key2, keylen2, SHA1_DIGEST_SIZE,
 			  paramdigest, TPM_NONCE_SIZE, enonce2,
 			  TPM_NONCE_SIZE, ononce, 1, continueflag2, 0, 0);
 	if (ret < 0)
 		goto out;
-	if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE))
+	if (crypto_memneq(testhmac2, authdata2, SHA1_DIGEST_SIZE))
 		ret = -EINVAL;
 out:
 	kfree_sensitive(sdesc);
 	return ret;
 }
-- 
2.50.1


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

* [PATCH v2 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash
  2025-08-09 17:19 [PATCH v2 0/3] KEYS: trusted_tpm1: HMAC fix and cleanup Eric Biggers
  2025-08-09 17:19 ` [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time Eric Biggers
@ 2025-08-09 17:19 ` Eric Biggers
  2025-08-12 16:24   ` Jarkko Sakkinen
  2025-08-09 17:19 ` [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header Eric Biggers
  2 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2025-08-09 17:19 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, keyrings
  Cc: David Howells, linux-integrity, linux-crypto, linux-kernel,
	Eric Biggers

Use the SHA-1 and HMAC-SHA1 library functions instead of crypto_shash.
This is simpler and faster.

Replace the selection of CRYPTO, CRYPTO_HMAC, and CRYPTO_SHA1 with
CRYPTO_LIB_SHA1 and CRYPTO_LIB_UTILS.  The latter is needed for
crypto_memneq() which was previously being pulled in via CRYPTO.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 security/keys/trusted-keys/Kconfig        |   5 +-
 security/keys/trusted-keys/trusted_tpm1.c | 221 ++++------------------
 2 files changed, 36 insertions(+), 190 deletions(-)

diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
index 1fb8aa0019953..204a68c1429df 100644
--- a/security/keys/trusted-keys/Kconfig
+++ b/security/keys/trusted-keys/Kconfig
@@ -3,14 +3,13 @@ config HAVE_TRUSTED_KEYS
 
 config TRUSTED_KEYS_TPM
 	bool "TPM-based trusted keys"
 	depends on TCG_TPM >= TRUSTED_KEYS
 	default y
-	select CRYPTO
-	select CRYPTO_HMAC
-	select CRYPTO_SHA1
 	select CRYPTO_HASH_INFO
+	select CRYPTO_LIB_SHA1
+	select CRYPTO_LIB_UTILS
 	select ASN1_ENCODER
 	select OID_REGISTRY
 	select ASN1
 	select HAVE_TRUSTED_KEYS
 	help
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index e73f2c6c817a0..126437459a74d 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -5,90 +5,37 @@
  *
  * See Documentation/security/keys/trusted-encrypted.rst
  */
 
 #include <crypto/hash_info.h>
+#include <crypto/sha1.h>
 #include <crypto/utils.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/parser.h>
 #include <linux/string.h>
 #include <linux/err.h>
 #include <keys/trusted-type.h>
 #include <linux/key-type.h>
-#include <linux/crypto.h>
-#include <crypto/hash.h>
-#include <crypto/sha1.h>
 #include <linux/tpm.h>
 #include <linux/tpm_command.h>
 
 #include <keys/trusted_tpm.h>
 
-static const char hmac_alg[] = "hmac(sha1)";
-static const char hash_alg[] = "sha1";
 static struct tpm_chip *chip;
 static struct tpm_digest *digests;
 
-struct sdesc {
-	struct shash_desc shash;
-	char ctx[];
-};
-
-static struct crypto_shash *hashalg;
-static struct crypto_shash *hmacalg;
-
-static struct sdesc *init_sdesc(struct crypto_shash *alg)
-{
-	struct sdesc *sdesc;
-	int size;
-
-	size = sizeof(struct shash_desc) + crypto_shash_descsize(alg);
-	sdesc = kmalloc(size, GFP_KERNEL);
-	if (!sdesc)
-		return ERR_PTR(-ENOMEM);
-	sdesc->shash.tfm = alg;
-	return sdesc;
-}
-
-static int TSS_sha1(const unsigned char *data, unsigned int datalen,
-		    unsigned char *digest)
-{
-	struct sdesc *sdesc;
-	int ret;
-
-	sdesc = init_sdesc(hashalg);
-	if (IS_ERR(sdesc)) {
-		pr_info("can't alloc %s\n", hash_alg);
-		return PTR_ERR(sdesc);
-	}
-
-	ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest);
-	kfree_sensitive(sdesc);
-	return ret;
-}
-
 static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 		       unsigned int keylen, ...)
 {
-	struct sdesc *sdesc;
+	struct hmac_sha1_ctx hmac_ctx;
 	va_list argp;
 	unsigned int dlen;
 	unsigned char *data;
-	int ret;
-
-	sdesc = init_sdesc(hmacalg);
-	if (IS_ERR(sdesc)) {
-		pr_info("can't alloc %s\n", hmac_alg);
-		return PTR_ERR(sdesc);
-	}
+	int ret = 0;
 
-	ret = crypto_shash_setkey(hmacalg, key, keylen);
-	if (ret < 0)
-		goto out;
-	ret = crypto_shash_init(&sdesc->shash);
-	if (ret < 0)
-		goto out;
+	hmac_sha1_init_usingrawkey(&hmac_ctx, key, keylen);
 
 	va_start(argp, keylen);
 	for (;;) {
 		dlen = va_arg(argp, unsigned int);
 		if (dlen == 0)
@@ -96,19 +43,15 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 		data = va_arg(argp, unsigned char *);
 		if (data == NULL) {
 			ret = -EINVAL;
 			break;
 		}
-		ret = crypto_shash_update(&sdesc->shash, data, dlen);
-		if (ret < 0)
-			break;
+		hmac_sha1_update(&hmac_ctx, data, dlen);
 	}
 	va_end(argp);
 	if (!ret)
-		ret = crypto_shash_final(&sdesc->shash, digest);
-out:
-	kfree_sensitive(sdesc);
+		hmac_sha1_final(&hmac_ctx, digest);
 	return ret;
 }
 
 /*
  * calculate authorization info fields to send to TPM
@@ -116,53 +59,41 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 int TSS_authhmac(unsigned char *digest, const unsigned char *key,
 			unsigned int keylen, unsigned char *h1,
 			unsigned char *h2, unsigned int h3, ...)
 {
 	unsigned char paramdigest[SHA1_DIGEST_SIZE];
-	struct sdesc *sdesc;
+	struct sha1_ctx sha_ctx;
 	unsigned int dlen;
 	unsigned char *data;
 	unsigned char c;
-	int ret;
+	int ret = 0;
 	va_list argp;
 
 	if (!chip)
 		return -ENODEV;
 
-	sdesc = init_sdesc(hashalg);
-	if (IS_ERR(sdesc)) {
-		pr_info("can't alloc %s\n", hash_alg);
-		return PTR_ERR(sdesc);
-	}
-
 	c = !!h3;
-	ret = crypto_shash_init(&sdesc->shash);
-	if (ret < 0)
-		goto out;
+	sha1_init(&sha_ctx);
 	va_start(argp, h3);
 	for (;;) {
 		dlen = va_arg(argp, unsigned int);
 		if (dlen == 0)
 			break;
 		data = va_arg(argp, unsigned char *);
 		if (!data) {
 			ret = -EINVAL;
 			break;
 		}
-		ret = crypto_shash_update(&sdesc->shash, data, dlen);
-		if (ret < 0)
-			break;
+		sha1_update(&sha_ctx, data, dlen);
 	}
 	va_end(argp);
 	if (!ret)
-		ret = crypto_shash_final(&sdesc->shash, paramdigest);
+		sha1_final(&sha_ctx, paramdigest);
 	if (!ret)
 		ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
 				  paramdigest, TPM_NONCE_SIZE, h1,
 				  TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
-out:
-	kfree_sensitive(sdesc);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(TSS_authhmac);
 
 /*
@@ -181,11 +112,11 @@ int TSS_checkhmac1(unsigned char *buffer,
 	unsigned char *enonce;
 	unsigned char *continueflag;
 	unsigned char *authdata;
 	unsigned char testhmac[SHA1_DIGEST_SIZE];
 	unsigned char paramdigest[SHA1_DIGEST_SIZE];
-	struct sdesc *sdesc;
+	struct sha1_ctx sha_ctx;
 	unsigned int dlen;
 	unsigned int dpos;
 	va_list argp;
 	int ret;
 
@@ -202,53 +133,33 @@ int TSS_checkhmac1(unsigned char *buffer,
 		return -EINVAL;
 	authdata = buffer + bufsize - SHA1_DIGEST_SIZE;
 	continueflag = authdata - 1;
 	enonce = continueflag - TPM_NONCE_SIZE;
 
-	sdesc = init_sdesc(hashalg);
-	if (IS_ERR(sdesc)) {
-		pr_info("can't alloc %s\n", hash_alg);
-		return PTR_ERR(sdesc);
-	}
-	ret = crypto_shash_init(&sdesc->shash);
-	if (ret < 0)
-		goto out;
-	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&result,
-				  sizeof result);
-	if (ret < 0)
-		goto out;
-	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&ordinal,
-				  sizeof ordinal);
-	if (ret < 0)
-		goto out;
+	sha1_init(&sha_ctx);
+	sha1_update(&sha_ctx, (const u8 *)&result, sizeof(result));
+	sha1_update(&sha_ctx, (const u8 *)&ordinal, sizeof(ordinal));
 	va_start(argp, keylen);
 	for (;;) {
 		dlen = va_arg(argp, unsigned int);
 		if (dlen == 0)
 			break;
 		dpos = va_arg(argp, unsigned int);
-		ret = crypto_shash_update(&sdesc->shash, buffer + dpos, dlen);
-		if (ret < 0)
-			break;
+		sha1_update(&sha_ctx, buffer + dpos, dlen);
 	}
 	va_end(argp);
-	if (!ret)
-		ret = crypto_shash_final(&sdesc->shash, paramdigest);
-	if (ret < 0)
-		goto out;
+	sha1_final(&sha_ctx, paramdigest);
 
 	ret = TSS_rawhmac(testhmac, key, keylen, SHA1_DIGEST_SIZE, paramdigest,
 			  TPM_NONCE_SIZE, enonce, TPM_NONCE_SIZE, ononce,
 			  1, continueflag, 0, 0);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
-		ret = -EINVAL;
-out:
-	kfree_sensitive(sdesc);
-	return ret;
+		return -EINVAL;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(TSS_checkhmac1);
 
 /*
  * verify the AUTH2_COMMAND (unseal) result from TPM
@@ -272,11 +183,11 @@ static int TSS_checkhmac2(unsigned char *buffer,
 	unsigned char *continueflag2;
 	unsigned char *authdata2;
 	unsigned char testhmac1[SHA1_DIGEST_SIZE];
 	unsigned char testhmac2[SHA1_DIGEST_SIZE];
 	unsigned char paramdigest[SHA1_DIGEST_SIZE];
-	struct sdesc *sdesc;
+	struct sha1_ctx sha_ctx;
 	unsigned int dlen;
 	unsigned int dpos;
 	va_list argp;
 	int ret;
 
@@ -295,62 +206,40 @@ static int TSS_checkhmac2(unsigned char *buffer,
 	continueflag1 = authdata1 - 1;
 	continueflag2 = authdata2 - 1;
 	enonce1 = continueflag1 - TPM_NONCE_SIZE;
 	enonce2 = continueflag2 - TPM_NONCE_SIZE;
 
-	sdesc = init_sdesc(hashalg);
-	if (IS_ERR(sdesc)) {
-		pr_info("can't alloc %s\n", hash_alg);
-		return PTR_ERR(sdesc);
-	}
-	ret = crypto_shash_init(&sdesc->shash);
-	if (ret < 0)
-		goto out;
-	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&result,
-				  sizeof result);
-	if (ret < 0)
-		goto out;
-	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&ordinal,
-				  sizeof ordinal);
-	if (ret < 0)
-		goto out;
+	sha1_init(&sha_ctx);
+	sha1_update(&sha_ctx, (const u8 *)&result, sizeof(result));
+	sha1_update(&sha_ctx, (const u8 *)&ordinal, sizeof(ordinal));
 
 	va_start(argp, keylen2);
 	for (;;) {
 		dlen = va_arg(argp, unsigned int);
 		if (dlen == 0)
 			break;
 		dpos = va_arg(argp, unsigned int);
-		ret = crypto_shash_update(&sdesc->shash, buffer + dpos, dlen);
-		if (ret < 0)
-			break;
+		sha1_update(&sha_ctx, buffer + dpos, dlen);
 	}
 	va_end(argp);
-	if (!ret)
-		ret = crypto_shash_final(&sdesc->shash, paramdigest);
-	if (ret < 0)
-		goto out;
+	sha1_final(&sha_ctx, paramdigest);
 
 	ret = TSS_rawhmac(testhmac1, key1, keylen1, SHA1_DIGEST_SIZE,
 			  paramdigest, TPM_NONCE_SIZE, enonce1,
 			  TPM_NONCE_SIZE, ononce, 1, continueflag1, 0, 0);
 	if (ret < 0)
-		goto out;
-	if (crypto_memneq(testhmac1, authdata1, SHA1_DIGEST_SIZE)) {
-		ret = -EINVAL;
-		goto out;
-	}
+		return ret;
+	if (crypto_memneq(testhmac1, authdata1, SHA1_DIGEST_SIZE))
+		return -EINVAL;
 	ret = TSS_rawhmac(testhmac2, key2, keylen2, SHA1_DIGEST_SIZE,
 			  paramdigest, TPM_NONCE_SIZE, enonce2,
 			  TPM_NONCE_SIZE, ononce, 1, continueflag2, 0, 0);
 	if (ret < 0)
-		goto out;
+		return ret;
 	if (crypto_memneq(testhmac2, authdata2, SHA1_DIGEST_SIZE))
-		ret = -EINVAL;
-out:
-	kfree_sensitive(sdesc);
-	return ret;
+		return -EINVAL;
+	return 0;
 }
 
 /*
  * For key specific tpm requests, we will generate and send our
  * own TPM command packets using the drivers send function.
@@ -497,13 +386,11 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
 	dump_sess(&sess);
 
 	/* calculate encrypted authorization value */
 	memcpy(td->xorwork, sess.secret, SHA1_DIGEST_SIZE);
 	memcpy(td->xorwork + SHA1_DIGEST_SIZE, sess.enonce, SHA1_DIGEST_SIZE);
-	ret = TSS_sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash);
-	if (ret < 0)
-		goto out;
+	sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash);
 
 	ret = tpm_get_random(chip, td->nonceodd, TPM_NONCE_SIZE);
 	if (ret < 0)
 		goto out;
 
@@ -988,44 +875,10 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
 {
 	return tpm_get_random(chip, key, key_len);
 }
 
-static void trusted_shash_release(void)
-{
-	if (hashalg)
-		crypto_free_shash(hashalg);
-	if (hmacalg)
-		crypto_free_shash(hmacalg);
-}
-
-static int __init trusted_shash_alloc(void)
-{
-	int ret;
-
-	hmacalg = crypto_alloc_shash(hmac_alg, 0, 0);
-	if (IS_ERR(hmacalg)) {
-		pr_info("could not allocate crypto %s\n",
-			hmac_alg);
-		return PTR_ERR(hmacalg);
-	}
-
-	hashalg = crypto_alloc_shash(hash_alg, 0, 0);
-	if (IS_ERR(hashalg)) {
-		pr_info("could not allocate crypto %s\n",
-			hash_alg);
-		ret = PTR_ERR(hashalg);
-		goto hashalg_fail;
-	}
-
-	return 0;
-
-hashalg_fail:
-	crypto_free_shash(hmacalg);
-	return ret;
-}
-
 static int __init init_digests(void)
 {
 	int i;
 
 	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
@@ -1048,19 +901,14 @@ static int __init trusted_tpm_init(void)
 		return -ENODEV;
 
 	ret = init_digests();
 	if (ret < 0)
 		goto err_put;
-	ret = trusted_shash_alloc();
-	if (ret < 0)
-		goto err_free;
 	ret = register_key_type(&key_type_trusted);
 	if (ret < 0)
-		goto err_release;
+		goto err_free;
 	return 0;
-err_release:
-	trusted_shash_release();
 err_free:
 	kfree(digests);
 err_put:
 	put_device(&chip->dev);
 	return ret;
@@ -1069,11 +917,10 @@ static int __init trusted_tpm_init(void)
 static void trusted_tpm_exit(void)
 {
 	if (chip) {
 		put_device(&chip->dev);
 		kfree(digests);
-		trusted_shash_release();
 		unregister_key_type(&key_type_trusted);
 	}
 }
 
 struct trusted_key_ops trusted_key_tpm_ops = {
-- 
2.50.1


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

* [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header
  2025-08-09 17:19 [PATCH v2 0/3] KEYS: trusted_tpm1: HMAC fix and cleanup Eric Biggers
  2025-08-09 17:19 ` [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time Eric Biggers
  2025-08-09 17:19 ` [PATCH v2 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash Eric Biggers
@ 2025-08-09 17:19 ` Eric Biggers
  2025-08-12 16:24   ` Jarkko Sakkinen
  2 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2025-08-09 17:19 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, keyrings
  Cc: David Howells, linux-integrity, linux-crypto, linux-kernel,
	Eric Biggers

Move functionality used only by trusted_tpm1.c out of the public header
<keys/trusted_tpm.h>.  Specifically, change the exported functions into
static functions, since they are not used outside trusted_tpm1.c, and
move various other definitions and inline functions to trusted_tpm1.c.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/keys/trusted_tpm.h                | 79 ----------------------
 security/keys/trusted-keys/trusted_tpm1.c | 80 ++++++++++++++++++++---
 2 files changed, 72 insertions(+), 87 deletions(-)

diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index a088b33fd0e3b..0fadc6a4f1663 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -3,94 +3,15 @@
 #define __TRUSTED_TPM_H
 
 #include <keys/trusted-type.h>
 #include <linux/tpm_command.h>
 
-/* implementation specific TPM constants */
-#define TPM_SIZE_OFFSET			2
-#define TPM_RETURN_OFFSET		6
-#define TPM_DATA_OFFSET			10
-
-#define LOAD32(buffer, offset)	(ntohl(*(uint32_t *)&buffer[offset]))
-#define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
-#define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
-
 extern struct trusted_key_ops trusted_key_tpm_ops;
 
-struct osapsess {
-	uint32_t handle;
-	unsigned char secret[SHA1_DIGEST_SIZE];
-	unsigned char enonce[TPM_NONCE_SIZE];
-};
-
-/* discrete values, but have to store in uint16_t for TPM use */
-enum {
-	SEAL_keytype = 1,
-	SRK_keytype = 4
-};
-
-int TSS_authhmac(unsigned char *digest, const unsigned char *key,
-			unsigned int keylen, unsigned char *h1,
-			unsigned char *h2, unsigned int h3, ...);
-int TSS_checkhmac1(unsigned char *buffer,
-			  const uint32_t command,
-			  const unsigned char *ononce,
-			  const unsigned char *key,
-			  unsigned int keylen, ...);
-
-int trusted_tpm_send(unsigned char *cmd, size_t buflen);
-int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
-
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options);
 int tpm2_unseal_trusted(struct tpm_chip *chip,
 			struct trusted_key_payload *payload,
 			struct trusted_key_options *options);
 
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
-static inline void dump_options(struct trusted_key_options *o)
-{
-	pr_info("sealing key type %d\n", o->keytype);
-	pr_info("sealing key handle %0X\n", o->keyhandle);
-	pr_info("pcrlock %d\n", o->pcrlock);
-	pr_info("pcrinfo %d\n", o->pcrinfo_len);
-	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
-		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
-		       16, 1, &s->handle, 4, 0);
-	pr_info("secret:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
-		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-	pr_info("trusted-key: enonce:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
-		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
-	int len;
-
-	pr_info("\ntpm buffer\n");
-	len = LOAD32(buf, TPM_SIZE_OFFSET);
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
-}
-#endif
 #endif
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 126437459a74d..636acb66a4f69 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -22,10 +22,78 @@
 #include <keys/trusted_tpm.h>
 
 static struct tpm_chip *chip;
 static struct tpm_digest *digests;
 
+/* implementation specific TPM constants */
+#define TPM_SIZE_OFFSET			2
+#define TPM_RETURN_OFFSET		6
+#define TPM_DATA_OFFSET			10
+
+#define LOAD32(buffer, offset)	(ntohl(*(uint32_t *)&buffer[offset]))
+#define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
+#define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
+
+struct osapsess {
+	uint32_t handle;
+	unsigned char secret[SHA1_DIGEST_SIZE];
+	unsigned char enonce[TPM_NONCE_SIZE];
+};
+
+/* discrete values, but have to store in uint16_t for TPM use */
+enum {
+	SEAL_keytype = 1,
+	SRK_keytype = 4
+};
+
+#define TPM_DEBUG 0
+
+#if TPM_DEBUG
+static inline void dump_options(struct trusted_key_options *o)
+{
+	pr_info("sealing key type %d\n", o->keytype);
+	pr_info("sealing key handle %0X\n", o->keyhandle);
+	pr_info("pcrlock %d\n", o->pcrlock);
+	pr_info("pcrinfo %d\n", o->pcrinfo_len);
+	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
+		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
+}
+
+static inline void dump_sess(struct osapsess *s)
+{
+	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
+		       16, 1, &s->handle, 4, 0);
+	pr_info("secret:\n");
+	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
+	pr_info("trusted-key: enonce:\n");
+	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
+}
+
+static inline void dump_tpm_buf(unsigned char *buf)
+{
+	int len;
+
+	pr_info("\ntpm buffer\n");
+	len = LOAD32(buf, TPM_SIZE_OFFSET);
+	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
+}
+#else
+static inline void dump_options(struct trusted_key_options *o)
+{
+}
+
+static inline void dump_sess(struct osapsess *s)
+{
+}
+
+static inline void dump_tpm_buf(unsigned char *buf)
+{
+}
+#endif
+
 static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 		       unsigned int keylen, ...)
 {
 	struct hmac_sha1_ctx hmac_ctx;
 	va_list argp;
@@ -54,11 +122,11 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 }
 
 /*
  * calculate authorization info fields to send to TPM
  */
-int TSS_authhmac(unsigned char *digest, const unsigned char *key,
+static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
 			unsigned int keylen, unsigned char *h1,
 			unsigned char *h2, unsigned int h3, ...)
 {
 	unsigned char paramdigest[SHA1_DIGEST_SIZE];
 	struct sha1_ctx sha_ctx;
@@ -92,16 +160,15 @@ int TSS_authhmac(unsigned char *digest, const unsigned char *key,
 		ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
 				  paramdigest, TPM_NONCE_SIZE, h1,
 				  TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(TSS_authhmac);
 
 /*
  * verify the AUTH1_COMMAND (Seal) result from TPM
  */
-int TSS_checkhmac1(unsigned char *buffer,
+static int TSS_checkhmac1(unsigned char *buffer,
 			  const uint32_t command,
 			  const unsigned char *ononce,
 			  const unsigned char *key,
 			  unsigned int keylen, ...)
 {
@@ -157,11 +224,10 @@ int TSS_checkhmac1(unsigned char *buffer,
 
 	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
 		return -EINVAL;
 	return 0;
 }
-EXPORT_SYMBOL_GPL(TSS_checkhmac1);
 
 /*
  * verify the AUTH2_COMMAND (unseal) result from TPM
  */
 static int TSS_checkhmac2(unsigned char *buffer,
@@ -242,11 +308,11 @@ static int TSS_checkhmac2(unsigned char *buffer,
 
 /*
  * For key specific tpm requests, we will generate and send our
  * own TPM command packets using the drivers send function.
  */
-int trusted_tpm_send(unsigned char *cmd, size_t buflen)
+static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
 {
 	struct tpm_buf buf;
 	int rc;
 
 	if (!chip)
@@ -268,11 +334,10 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
 		rc = -EPERM;
 
 	tpm_put_ops(chip);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(trusted_tpm_send);
 
 /*
  * Lock a trusted key, by extending a selected PCR.
  *
  * Prevents a trusted key that is sealed to PCRs from being accessed.
@@ -322,11 +387,11 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
 }
 
 /*
  * Create an object independent authorisation protocol (oiap) session
  */
-int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
+static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
 {
 	int ret;
 
 	if (!chip)
 		return -ENODEV;
@@ -339,11 +404,10 @@ int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
 	*handle = LOAD32(tb->data, TPM_DATA_OFFSET);
 	memcpy(nonce, &tb->data[TPM_DATA_OFFSET + sizeof(uint32_t)],
 	       TPM_NONCE_SIZE);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(oiap);
 
 struct tpm_digests {
 	unsigned char encauth[SHA1_DIGEST_SIZE];
 	unsigned char pubauth[SHA1_DIGEST_SIZE];
 	unsigned char xorwork[SHA1_DIGEST_SIZE * 2];
-- 
2.50.1


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

* Re: [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time
  2025-08-09 17:19 ` [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time Eric Biggers
@ 2025-08-12 16:23   ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-08-12 16:23 UTC (permalink / raw)
  To: Eric Biggers
  Cc: James Bottomley, Mimi Zohar, keyrings, David Howells,
	linux-integrity, linux-crypto, linux-kernel, stable

On Sat, Aug 09, 2025 at 10:19:39AM -0700, Eric Biggers wrote:
> To prevent timing attacks, HMAC value comparison needs to be constant
> time.  Replace the memcmp() with the correct function, crypto_memneq().
> 
> [For the Fixes commit I used the commit that introduced the memcmp().
> It predates the introduction of crypto_memneq(), but it was still a bug
> at the time even though a helper function didn't exist yet.]
> 
> Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  security/keys/trusted-keys/trusted_tpm1.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 89c9798d18007..e73f2c6c817a0 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -5,10 +5,11 @@
>   *
>   * See Documentation/security/keys/trusted-encrypted.rst
>   */
>  
>  #include <crypto/hash_info.h>
> +#include <crypto/utils.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
>  #include <linux/parser.h>
>  #include <linux/string.h>
>  #include <linux/err.h>
> @@ -239,11 +240,11 @@ int TSS_checkhmac1(unsigned char *buffer,
>  			  TPM_NONCE_SIZE, enonce, TPM_NONCE_SIZE, ononce,
>  			  1, continueflag, 0, 0);
>  	if (ret < 0)
>  		goto out;
>  
> -	if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE))
> +	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
>  		ret = -EINVAL;
>  out:
>  	kfree_sensitive(sdesc);
>  	return ret;
>  }
> @@ -332,20 +333,20 @@ static int TSS_checkhmac2(unsigned char *buffer,
>  	ret = TSS_rawhmac(testhmac1, key1, keylen1, SHA1_DIGEST_SIZE,
>  			  paramdigest, TPM_NONCE_SIZE, enonce1,
>  			  TPM_NONCE_SIZE, ononce, 1, continueflag1, 0, 0);
>  	if (ret < 0)
>  		goto out;
> -	if (memcmp(testhmac1, authdata1, SHA1_DIGEST_SIZE)) {
> +	if (crypto_memneq(testhmac1, authdata1, SHA1_DIGEST_SIZE)) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
>  	ret = TSS_rawhmac(testhmac2, key2, keylen2, SHA1_DIGEST_SIZE,
>  			  paramdigest, TPM_NONCE_SIZE, enonce2,
>  			  TPM_NONCE_SIZE, ononce, 1, continueflag2, 0, 0);
>  	if (ret < 0)
>  		goto out;
> -	if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE))
> +	if (crypto_memneq(testhmac2, authdata2, SHA1_DIGEST_SIZE))
>  		ret = -EINVAL;
>  out:
>  	kfree_sensitive(sdesc);
>  	return ret;
>  }
> -- 
> 2.50.1
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

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

* Re: [PATCH v2 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash
  2025-08-09 17:19 ` [PATCH v2 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash Eric Biggers
@ 2025-08-12 16:24   ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-08-12 16:24 UTC (permalink / raw)
  To: Eric Biggers
  Cc: James Bottomley, Mimi Zohar, keyrings, David Howells,
	linux-integrity, linux-crypto, linux-kernel

On Sat, Aug 09, 2025 at 10:19:40AM -0700, Eric Biggers wrote:
> Use the SHA-1 and HMAC-SHA1 library functions instead of crypto_shash.
> This is simpler and faster.
> 
> Replace the selection of CRYPTO, CRYPTO_HMAC, and CRYPTO_SHA1 with
> CRYPTO_LIB_SHA1 and CRYPTO_LIB_UTILS.  The latter is needed for
> crypto_memneq() which was previously being pulled in via CRYPTO.
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  security/keys/trusted-keys/Kconfig        |   5 +-
>  security/keys/trusted-keys/trusted_tpm1.c | 221 ++++------------------
>  2 files changed, 36 insertions(+), 190 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
> index 1fb8aa0019953..204a68c1429df 100644
> --- a/security/keys/trusted-keys/Kconfig
> +++ b/security/keys/trusted-keys/Kconfig
> @@ -3,14 +3,13 @@ config HAVE_TRUSTED_KEYS
>  
>  config TRUSTED_KEYS_TPM
>  	bool "TPM-based trusted keys"
>  	depends on TCG_TPM >= TRUSTED_KEYS
>  	default y
> -	select CRYPTO
> -	select CRYPTO_HMAC
> -	select CRYPTO_SHA1
>  	select CRYPTO_HASH_INFO
> +	select CRYPTO_LIB_SHA1
> +	select CRYPTO_LIB_UTILS
>  	select ASN1_ENCODER
>  	select OID_REGISTRY
>  	select ASN1
>  	select HAVE_TRUSTED_KEYS
>  	help
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index e73f2c6c817a0..126437459a74d 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -5,90 +5,37 @@
>   *
>   * See Documentation/security/keys/trusted-encrypted.rst
>   */
>  
>  #include <crypto/hash_info.h>
> +#include <crypto/sha1.h>
>  #include <crypto/utils.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
>  #include <linux/parser.h>
>  #include <linux/string.h>
>  #include <linux/err.h>
>  #include <keys/trusted-type.h>
>  #include <linux/key-type.h>
> -#include <linux/crypto.h>
> -#include <crypto/hash.h>
> -#include <crypto/sha1.h>
>  #include <linux/tpm.h>
>  #include <linux/tpm_command.h>
>  
>  #include <keys/trusted_tpm.h>
>  
> -static const char hmac_alg[] = "hmac(sha1)";
> -static const char hash_alg[] = "sha1";
>  static struct tpm_chip *chip;
>  static struct tpm_digest *digests;
>  
> -struct sdesc {
> -	struct shash_desc shash;
> -	char ctx[];
> -};
> -
> -static struct crypto_shash *hashalg;
> -static struct crypto_shash *hmacalg;
> -
> -static struct sdesc *init_sdesc(struct crypto_shash *alg)
> -{
> -	struct sdesc *sdesc;
> -	int size;
> -
> -	size = sizeof(struct shash_desc) + crypto_shash_descsize(alg);
> -	sdesc = kmalloc(size, GFP_KERNEL);
> -	if (!sdesc)
> -		return ERR_PTR(-ENOMEM);
> -	sdesc->shash.tfm = alg;
> -	return sdesc;
> -}
> -
> -static int TSS_sha1(const unsigned char *data, unsigned int datalen,
> -		    unsigned char *digest)
> -{
> -	struct sdesc *sdesc;
> -	int ret;
> -
> -	sdesc = init_sdesc(hashalg);
> -	if (IS_ERR(sdesc)) {
> -		pr_info("can't alloc %s\n", hash_alg);
> -		return PTR_ERR(sdesc);
> -	}
> -
> -	ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest);
> -	kfree_sensitive(sdesc);
> -	return ret;
> -}
> -
>  static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>  		       unsigned int keylen, ...)
>  {
> -	struct sdesc *sdesc;
> +	struct hmac_sha1_ctx hmac_ctx;
>  	va_list argp;
>  	unsigned int dlen;
>  	unsigned char *data;
> -	int ret;
> -
> -	sdesc = init_sdesc(hmacalg);
> -	if (IS_ERR(sdesc)) {
> -		pr_info("can't alloc %s\n", hmac_alg);
> -		return PTR_ERR(sdesc);
> -	}
> +	int ret = 0;
>  
> -	ret = crypto_shash_setkey(hmacalg, key, keylen);
> -	if (ret < 0)
> -		goto out;
> -	ret = crypto_shash_init(&sdesc->shash);
> -	if (ret < 0)
> -		goto out;
> +	hmac_sha1_init_usingrawkey(&hmac_ctx, key, keylen);
>  
>  	va_start(argp, keylen);
>  	for (;;) {
>  		dlen = va_arg(argp, unsigned int);
>  		if (dlen == 0)
> @@ -96,19 +43,15 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>  		data = va_arg(argp, unsigned char *);
>  		if (data == NULL) {
>  			ret = -EINVAL;
>  			break;
>  		}
> -		ret = crypto_shash_update(&sdesc->shash, data, dlen);
> -		if (ret < 0)
> -			break;
> +		hmac_sha1_update(&hmac_ctx, data, dlen);
>  	}
>  	va_end(argp);
>  	if (!ret)
> -		ret = crypto_shash_final(&sdesc->shash, digest);
> -out:
> -	kfree_sensitive(sdesc);
> +		hmac_sha1_final(&hmac_ctx, digest);
>  	return ret;
>  }
>  
>  /*
>   * calculate authorization info fields to send to TPM
> @@ -116,53 +59,41 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>  int TSS_authhmac(unsigned char *digest, const unsigned char *key,
>  			unsigned int keylen, unsigned char *h1,
>  			unsigned char *h2, unsigned int h3, ...)
>  {
>  	unsigned char paramdigest[SHA1_DIGEST_SIZE];
> -	struct sdesc *sdesc;
> +	struct sha1_ctx sha_ctx;
>  	unsigned int dlen;
>  	unsigned char *data;
>  	unsigned char c;
> -	int ret;
> +	int ret = 0;
>  	va_list argp;
>  
>  	if (!chip)
>  		return -ENODEV;
>  
> -	sdesc = init_sdesc(hashalg);
> -	if (IS_ERR(sdesc)) {
> -		pr_info("can't alloc %s\n", hash_alg);
> -		return PTR_ERR(sdesc);
> -	}
> -
>  	c = !!h3;
> -	ret = crypto_shash_init(&sdesc->shash);
> -	if (ret < 0)
> -		goto out;
> +	sha1_init(&sha_ctx);
>  	va_start(argp, h3);
>  	for (;;) {
>  		dlen = va_arg(argp, unsigned int);
>  		if (dlen == 0)
>  			break;
>  		data = va_arg(argp, unsigned char *);
>  		if (!data) {
>  			ret = -EINVAL;
>  			break;
>  		}
> -		ret = crypto_shash_update(&sdesc->shash, data, dlen);
> -		if (ret < 0)
> -			break;
> +		sha1_update(&sha_ctx, data, dlen);
>  	}
>  	va_end(argp);
>  	if (!ret)
> -		ret = crypto_shash_final(&sdesc->shash, paramdigest);
> +		sha1_final(&sha_ctx, paramdigest);
>  	if (!ret)
>  		ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
>  				  paramdigest, TPM_NONCE_SIZE, h1,
>  				  TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
> -out:
> -	kfree_sensitive(sdesc);
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(TSS_authhmac);
>  
>  /*
> @@ -181,11 +112,11 @@ int TSS_checkhmac1(unsigned char *buffer,
>  	unsigned char *enonce;
>  	unsigned char *continueflag;
>  	unsigned char *authdata;
>  	unsigned char testhmac[SHA1_DIGEST_SIZE];
>  	unsigned char paramdigest[SHA1_DIGEST_SIZE];
> -	struct sdesc *sdesc;
> +	struct sha1_ctx sha_ctx;
>  	unsigned int dlen;
>  	unsigned int dpos;
>  	va_list argp;
>  	int ret;
>  
> @@ -202,53 +133,33 @@ int TSS_checkhmac1(unsigned char *buffer,
>  		return -EINVAL;
>  	authdata = buffer + bufsize - SHA1_DIGEST_SIZE;
>  	continueflag = authdata - 1;
>  	enonce = continueflag - TPM_NONCE_SIZE;
>  
> -	sdesc = init_sdesc(hashalg);
> -	if (IS_ERR(sdesc)) {
> -		pr_info("can't alloc %s\n", hash_alg);
> -		return PTR_ERR(sdesc);
> -	}
> -	ret = crypto_shash_init(&sdesc->shash);
> -	if (ret < 0)
> -		goto out;
> -	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&result,
> -				  sizeof result);
> -	if (ret < 0)
> -		goto out;
> -	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&ordinal,
> -				  sizeof ordinal);
> -	if (ret < 0)
> -		goto out;
> +	sha1_init(&sha_ctx);
> +	sha1_update(&sha_ctx, (const u8 *)&result, sizeof(result));
> +	sha1_update(&sha_ctx, (const u8 *)&ordinal, sizeof(ordinal));
>  	va_start(argp, keylen);
>  	for (;;) {
>  		dlen = va_arg(argp, unsigned int);
>  		if (dlen == 0)
>  			break;
>  		dpos = va_arg(argp, unsigned int);
> -		ret = crypto_shash_update(&sdesc->shash, buffer + dpos, dlen);
> -		if (ret < 0)
> -			break;
> +		sha1_update(&sha_ctx, buffer + dpos, dlen);
>  	}
>  	va_end(argp);
> -	if (!ret)
> -		ret = crypto_shash_final(&sdesc->shash, paramdigest);
> -	if (ret < 0)
> -		goto out;
> +	sha1_final(&sha_ctx, paramdigest);
>  
>  	ret = TSS_rawhmac(testhmac, key, keylen, SHA1_DIGEST_SIZE, paramdigest,
>  			  TPM_NONCE_SIZE, enonce, TPM_NONCE_SIZE, ononce,
>  			  1, continueflag, 0, 0);
>  	if (ret < 0)
> -		goto out;
> +		return ret;
>  
>  	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
> -		ret = -EINVAL;
> -out:
> -	kfree_sensitive(sdesc);
> -	return ret;
> +		return -EINVAL;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(TSS_checkhmac1);
>  
>  /*
>   * verify the AUTH2_COMMAND (unseal) result from TPM
> @@ -272,11 +183,11 @@ static int TSS_checkhmac2(unsigned char *buffer,
>  	unsigned char *continueflag2;
>  	unsigned char *authdata2;
>  	unsigned char testhmac1[SHA1_DIGEST_SIZE];
>  	unsigned char testhmac2[SHA1_DIGEST_SIZE];
>  	unsigned char paramdigest[SHA1_DIGEST_SIZE];
> -	struct sdesc *sdesc;
> +	struct sha1_ctx sha_ctx;
>  	unsigned int dlen;
>  	unsigned int dpos;
>  	va_list argp;
>  	int ret;
>  
> @@ -295,62 +206,40 @@ static int TSS_checkhmac2(unsigned char *buffer,
>  	continueflag1 = authdata1 - 1;
>  	continueflag2 = authdata2 - 1;
>  	enonce1 = continueflag1 - TPM_NONCE_SIZE;
>  	enonce2 = continueflag2 - TPM_NONCE_SIZE;
>  
> -	sdesc = init_sdesc(hashalg);
> -	if (IS_ERR(sdesc)) {
> -		pr_info("can't alloc %s\n", hash_alg);
> -		return PTR_ERR(sdesc);
> -	}
> -	ret = crypto_shash_init(&sdesc->shash);
> -	if (ret < 0)
> -		goto out;
> -	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&result,
> -				  sizeof result);
> -	if (ret < 0)
> -		goto out;
> -	ret = crypto_shash_update(&sdesc->shash, (const u8 *)&ordinal,
> -				  sizeof ordinal);
> -	if (ret < 0)
> -		goto out;
> +	sha1_init(&sha_ctx);
> +	sha1_update(&sha_ctx, (const u8 *)&result, sizeof(result));
> +	sha1_update(&sha_ctx, (const u8 *)&ordinal, sizeof(ordinal));
>  
>  	va_start(argp, keylen2);
>  	for (;;) {
>  		dlen = va_arg(argp, unsigned int);
>  		if (dlen == 0)
>  			break;
>  		dpos = va_arg(argp, unsigned int);
> -		ret = crypto_shash_update(&sdesc->shash, buffer + dpos, dlen);
> -		if (ret < 0)
> -			break;
> +		sha1_update(&sha_ctx, buffer + dpos, dlen);
>  	}
>  	va_end(argp);
> -	if (!ret)
> -		ret = crypto_shash_final(&sdesc->shash, paramdigest);
> -	if (ret < 0)
> -		goto out;
> +	sha1_final(&sha_ctx, paramdigest);
>  
>  	ret = TSS_rawhmac(testhmac1, key1, keylen1, SHA1_DIGEST_SIZE,
>  			  paramdigest, TPM_NONCE_SIZE, enonce1,
>  			  TPM_NONCE_SIZE, ononce, 1, continueflag1, 0, 0);
>  	if (ret < 0)
> -		goto out;
> -	if (crypto_memneq(testhmac1, authdata1, SHA1_DIGEST_SIZE)) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> +		return ret;
> +	if (crypto_memneq(testhmac1, authdata1, SHA1_DIGEST_SIZE))
> +		return -EINVAL;
>  	ret = TSS_rawhmac(testhmac2, key2, keylen2, SHA1_DIGEST_SIZE,
>  			  paramdigest, TPM_NONCE_SIZE, enonce2,
>  			  TPM_NONCE_SIZE, ononce, 1, continueflag2, 0, 0);
>  	if (ret < 0)
> -		goto out;
> +		return ret;
>  	if (crypto_memneq(testhmac2, authdata2, SHA1_DIGEST_SIZE))
> -		ret = -EINVAL;
> -out:
> -	kfree_sensitive(sdesc);
> -	return ret;
> +		return -EINVAL;
> +	return 0;
>  }
>  
>  /*
>   * For key specific tpm requests, we will generate and send our
>   * own TPM command packets using the drivers send function.
> @@ -497,13 +386,11 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
>  	dump_sess(&sess);
>  
>  	/* calculate encrypted authorization value */
>  	memcpy(td->xorwork, sess.secret, SHA1_DIGEST_SIZE);
>  	memcpy(td->xorwork + SHA1_DIGEST_SIZE, sess.enonce, SHA1_DIGEST_SIZE);
> -	ret = TSS_sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash);
> -	if (ret < 0)
> -		goto out;
> +	sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash);
>  
>  	ret = tpm_get_random(chip, td->nonceodd, TPM_NONCE_SIZE);
>  	if (ret < 0)
>  		goto out;
>  
> @@ -988,44 +875,10 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>  static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
>  {
>  	return tpm_get_random(chip, key, key_len);
>  }
>  
> -static void trusted_shash_release(void)
> -{
> -	if (hashalg)
> -		crypto_free_shash(hashalg);
> -	if (hmacalg)
> -		crypto_free_shash(hmacalg);
> -}
> -
> -static int __init trusted_shash_alloc(void)
> -{
> -	int ret;
> -
> -	hmacalg = crypto_alloc_shash(hmac_alg, 0, 0);
> -	if (IS_ERR(hmacalg)) {
> -		pr_info("could not allocate crypto %s\n",
> -			hmac_alg);
> -		return PTR_ERR(hmacalg);
> -	}
> -
> -	hashalg = crypto_alloc_shash(hash_alg, 0, 0);
> -	if (IS_ERR(hashalg)) {
> -		pr_info("could not allocate crypto %s\n",
> -			hash_alg);
> -		ret = PTR_ERR(hashalg);
> -		goto hashalg_fail;
> -	}
> -
> -	return 0;
> -
> -hashalg_fail:
> -	crypto_free_shash(hmacalg);
> -	return ret;
> -}
> -
>  static int __init init_digests(void)
>  {
>  	int i;
>  
>  	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
> @@ -1048,19 +901,14 @@ static int __init trusted_tpm_init(void)
>  		return -ENODEV;
>  
>  	ret = init_digests();
>  	if (ret < 0)
>  		goto err_put;
> -	ret = trusted_shash_alloc();
> -	if (ret < 0)
> -		goto err_free;
>  	ret = register_key_type(&key_type_trusted);
>  	if (ret < 0)
> -		goto err_release;
> +		goto err_free;
>  	return 0;
> -err_release:
> -	trusted_shash_release();
>  err_free:
>  	kfree(digests);
>  err_put:
>  	put_device(&chip->dev);
>  	return ret;
> @@ -1069,11 +917,10 @@ static int __init trusted_tpm_init(void)
>  static void trusted_tpm_exit(void)
>  {
>  	if (chip) {
>  		put_device(&chip->dev);
>  		kfree(digests);
> -		trusted_shash_release();
>  		unregister_key_type(&key_type_trusted);
>  	}
>  }
>  
>  struct trusted_key_ops trusted_key_tpm_ops = {
> -- 
> 2.50.1
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

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

* Re: [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header
  2025-08-09 17:19 ` [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header Eric Biggers
@ 2025-08-12 16:24   ` Jarkko Sakkinen
  2025-08-12 16:29     ` Jarkko Sakkinen
  0 siblings, 1 reply; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-08-12 16:24 UTC (permalink / raw)
  To: Eric Biggers
  Cc: James Bottomley, Mimi Zohar, keyrings, David Howells,
	linux-integrity, linux-crypto, linux-kernel

On Sat, Aug 09, 2025 at 10:19:41AM -0700, Eric Biggers wrote:
> Move functionality used only by trusted_tpm1.c out of the public header
> <keys/trusted_tpm.h>.  Specifically, change the exported functions into
> static functions, since they are not used outside trusted_tpm1.c, and
> move various other definitions and inline functions to trusted_tpm1.c.
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  include/keys/trusted_tpm.h                | 79 ----------------------
>  security/keys/trusted-keys/trusted_tpm1.c | 80 ++++++++++++++++++++---
>  2 files changed, 72 insertions(+), 87 deletions(-)
> 
> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> index a088b33fd0e3b..0fadc6a4f1663 100644
> --- a/include/keys/trusted_tpm.h
> +++ b/include/keys/trusted_tpm.h
> @@ -3,94 +3,15 @@
>  #define __TRUSTED_TPM_H
>  
>  #include <keys/trusted-type.h>
>  #include <linux/tpm_command.h>
>  
> -/* implementation specific TPM constants */
> -#define TPM_SIZE_OFFSET			2
> -#define TPM_RETURN_OFFSET		6
> -#define TPM_DATA_OFFSET			10
> -
> -#define LOAD32(buffer, offset)	(ntohl(*(uint32_t *)&buffer[offset]))
> -#define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
> -#define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
> -
>  extern struct trusted_key_ops trusted_key_tpm_ops;
>  
> -struct osapsess {
> -	uint32_t handle;
> -	unsigned char secret[SHA1_DIGEST_SIZE];
> -	unsigned char enonce[TPM_NONCE_SIZE];
> -};
> -
> -/* discrete values, but have to store in uint16_t for TPM use */
> -enum {
> -	SEAL_keytype = 1,
> -	SRK_keytype = 4
> -};
> -
> -int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> -			unsigned int keylen, unsigned char *h1,
> -			unsigned char *h2, unsigned int h3, ...);
> -int TSS_checkhmac1(unsigned char *buffer,
> -			  const uint32_t command,
> -			  const unsigned char *ononce,
> -			  const unsigned char *key,
> -			  unsigned int keylen, ...);
> -
> -int trusted_tpm_send(unsigned char *cmd, size_t buflen);
> -int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
> -
>  int tpm2_seal_trusted(struct tpm_chip *chip,
>  		      struct trusted_key_payload *payload,
>  		      struct trusted_key_options *options);
>  int tpm2_unseal_trusted(struct tpm_chip *chip,
>  			struct trusted_key_payload *payload,
>  			struct trusted_key_options *options);
>  
> -#define TPM_DEBUG 0
> -
> -#if TPM_DEBUG
> -static inline void dump_options(struct trusted_key_options *o)
> -{
> -	pr_info("sealing key type %d\n", o->keytype);
> -	pr_info("sealing key handle %0X\n", o->keyhandle);
> -	pr_info("pcrlock %d\n", o->pcrlock);
> -	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> -	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> -		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> -}
> -
> -static inline void dump_sess(struct osapsess *s)
> -{
> -	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> -		       16, 1, &s->handle, 4, 0);
> -	pr_info("secret:\n");
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> -		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> -	pr_info("trusted-key: enonce:\n");
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> -		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> -}
> -
> -static inline void dump_tpm_buf(unsigned char *buf)
> -{
> -	int len;
> -
> -	pr_info("\ntpm buffer\n");
> -	len = LOAD32(buf, TPM_SIZE_OFFSET);
> -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> -}
> -#else
> -static inline void dump_options(struct trusted_key_options *o)
> -{
> -}
> -
> -static inline void dump_sess(struct osapsess *s)
> -{
> -}
> -
> -static inline void dump_tpm_buf(unsigned char *buf)
> -{
> -}
> -#endif
>  #endif
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 126437459a74d..636acb66a4f69 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -22,10 +22,78 @@
>  #include <keys/trusted_tpm.h>
>  
>  static struct tpm_chip *chip;
>  static struct tpm_digest *digests;
>  
> +/* implementation specific TPM constants */
> +#define TPM_SIZE_OFFSET			2
> +#define TPM_RETURN_OFFSET		6
> +#define TPM_DATA_OFFSET			10
> +
> +#define LOAD32(buffer, offset)	(ntohl(*(uint32_t *)&buffer[offset]))
> +#define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
> +#define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
> +
> +struct osapsess {
> +	uint32_t handle;
> +	unsigned char secret[SHA1_DIGEST_SIZE];
> +	unsigned char enonce[TPM_NONCE_SIZE];
> +};
> +
> +/* discrete values, but have to store in uint16_t for TPM use */
> +enum {
> +	SEAL_keytype = 1,
> +	SRK_keytype = 4
> +};
> +
> +#define TPM_DEBUG 0
> +
> +#if TPM_DEBUG
> +static inline void dump_options(struct trusted_key_options *o)
> +{
> +	pr_info("sealing key type %d\n", o->keytype);
> +	pr_info("sealing key handle %0X\n", o->keyhandle);
> +	pr_info("pcrlock %d\n", o->pcrlock);
> +	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> +	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> +		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> +}
> +
> +static inline void dump_sess(struct osapsess *s)
> +{
> +	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> +		       16, 1, &s->handle, 4, 0);
> +	pr_info("secret:\n");
> +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> +		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> +	pr_info("trusted-key: enonce:\n");
> +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> +		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> +}
> +
> +static inline void dump_tpm_buf(unsigned char *buf)
> +{
> +	int len;
> +
> +	pr_info("\ntpm buffer\n");
> +	len = LOAD32(buf, TPM_SIZE_OFFSET);
> +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> +}
> +#else
> +static inline void dump_options(struct trusted_key_options *o)
> +{
> +}
> +
> +static inline void dump_sess(struct osapsess *s)
> +{
> +}
> +
> +static inline void dump_tpm_buf(unsigned char *buf)
> +{
> +}
> +#endif
> +
>  static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>  		       unsigned int keylen, ...)
>  {
>  	struct hmac_sha1_ctx hmac_ctx;
>  	va_list argp;
> @@ -54,11 +122,11 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
>  }
>  
>  /*
>   * calculate authorization info fields to send to TPM
>   */
> -int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> +static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
>  			unsigned int keylen, unsigned char *h1,
>  			unsigned char *h2, unsigned int h3, ...)
>  {
>  	unsigned char paramdigest[SHA1_DIGEST_SIZE];
>  	struct sha1_ctx sha_ctx;
> @@ -92,16 +160,15 @@ int TSS_authhmac(unsigned char *digest, const unsigned char *key,
>  		ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
>  				  paramdigest, TPM_NONCE_SIZE, h1,
>  				  TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(TSS_authhmac);
>  
>  /*
>   * verify the AUTH1_COMMAND (Seal) result from TPM
>   */
> -int TSS_checkhmac1(unsigned char *buffer,
> +static int TSS_checkhmac1(unsigned char *buffer,
>  			  const uint32_t command,
>  			  const unsigned char *ononce,
>  			  const unsigned char *key,
>  			  unsigned int keylen, ...)
>  {
> @@ -157,11 +224,10 @@ int TSS_checkhmac1(unsigned char *buffer,
>  
>  	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
>  		return -EINVAL;
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(TSS_checkhmac1);
>  
>  /*
>   * verify the AUTH2_COMMAND (unseal) result from TPM
>   */
>  static int TSS_checkhmac2(unsigned char *buffer,
> @@ -242,11 +308,11 @@ static int TSS_checkhmac2(unsigned char *buffer,
>  
>  /*
>   * For key specific tpm requests, we will generate and send our
>   * own TPM command packets using the drivers send function.
>   */
> -int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> +static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
>  {
>  	struct tpm_buf buf;
>  	int rc;
>  
>  	if (!chip)
> @@ -268,11 +334,10 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
>  		rc = -EPERM;
>  
>  	tpm_put_ops(chip);
>  	return rc;
>  }
> -EXPORT_SYMBOL_GPL(trusted_tpm_send);
>  
>  /*
>   * Lock a trusted key, by extending a selected PCR.
>   *
>   * Prevents a trusted key that is sealed to PCRs from being accessed.
> @@ -322,11 +387,11 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
>  }
>  
>  /*
>   * Create an object independent authorisation protocol (oiap) session
>   */
> -int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> +static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
>  {
>  	int ret;
>  
>  	if (!chip)
>  		return -ENODEV;
> @@ -339,11 +404,10 @@ int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
>  	*handle = LOAD32(tb->data, TPM_DATA_OFFSET);
>  	memcpy(nonce, &tb->data[TPM_DATA_OFFSET + sizeof(uint32_t)],
>  	       TPM_NONCE_SIZE);
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(oiap);
>  
>  struct tpm_digests {
>  	unsigned char encauth[SHA1_DIGEST_SIZE];
>  	unsigned char pubauth[SHA1_DIGEST_SIZE];
>  	unsigned char xorwork[SHA1_DIGEST_SIZE * 2];
> -- 
> 2.50.1
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

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

* Re: [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header
  2025-08-12 16:24   ` Jarkko Sakkinen
@ 2025-08-12 16:29     ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2025-08-12 16:29 UTC (permalink / raw)
  To: Eric Biggers
  Cc: James Bottomley, Mimi Zohar, keyrings, David Howells,
	linux-integrity, linux-crypto, linux-kernel

On Tue, Aug 12, 2025 at 07:24:48PM +0300, Jarkko Sakkinen wrote:
> On Sat, Aug 09, 2025 at 10:19:41AM -0700, Eric Biggers wrote:
> > Move functionality used only by trusted_tpm1.c out of the public header
> > <keys/trusted_tpm.h>.  Specifically, change the exported functions into
> > static functions, since they are not used outside trusted_tpm1.c, and
> > move various other definitions and inline functions to trusted_tpm1.c.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >  include/keys/trusted_tpm.h                | 79 ----------------------
> >  security/keys/trusted-keys/trusted_tpm1.c | 80 ++++++++++++++++++++---
> >  2 files changed, 72 insertions(+), 87 deletions(-)
> > 
> > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> > index a088b33fd0e3b..0fadc6a4f1663 100644
> > --- a/include/keys/trusted_tpm.h
> > +++ b/include/keys/trusted_tpm.h
> > @@ -3,94 +3,15 @@
> >  #define __TRUSTED_TPM_H
> >  
> >  #include <keys/trusted-type.h>
> >  #include <linux/tpm_command.h>
> >  
> > -/* implementation specific TPM constants */
> > -#define TPM_SIZE_OFFSET			2
> > -#define TPM_RETURN_OFFSET		6
> > -#define TPM_DATA_OFFSET			10
> > -
> > -#define LOAD32(buffer, offset)	(ntohl(*(uint32_t *)&buffer[offset]))
> > -#define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
> > -#define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
> > -
> >  extern struct trusted_key_ops trusted_key_tpm_ops;
> >  
> > -struct osapsess {
> > -	uint32_t handle;
> > -	unsigned char secret[SHA1_DIGEST_SIZE];
> > -	unsigned char enonce[TPM_NONCE_SIZE];
> > -};
> > -
> > -/* discrete values, but have to store in uint16_t for TPM use */
> > -enum {
> > -	SEAL_keytype = 1,
> > -	SRK_keytype = 4
> > -};
> > -
> > -int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> > -			unsigned int keylen, unsigned char *h1,
> > -			unsigned char *h2, unsigned int h3, ...);
> > -int TSS_checkhmac1(unsigned char *buffer,
> > -			  const uint32_t command,
> > -			  const unsigned char *ononce,
> > -			  const unsigned char *key,
> > -			  unsigned int keylen, ...);
> > -
> > -int trusted_tpm_send(unsigned char *cmd, size_t buflen);
> > -int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
> > -
> >  int tpm2_seal_trusted(struct tpm_chip *chip,
> >  		      struct trusted_key_payload *payload,
> >  		      struct trusted_key_options *options);
> >  int tpm2_unseal_trusted(struct tpm_chip *chip,
> >  			struct trusted_key_payload *payload,
> >  			struct trusted_key_options *options);
> >  
> > -#define TPM_DEBUG 0
> > -
> > -#if TPM_DEBUG
> > -static inline void dump_options(struct trusted_key_options *o)
> > -{
> > -	pr_info("sealing key type %d\n", o->keytype);
> > -	pr_info("sealing key handle %0X\n", o->keyhandle);
> > -	pr_info("pcrlock %d\n", o->pcrlock);
> > -	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> > -	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> > -		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> > -}
> > -
> > -static inline void dump_sess(struct osapsess *s)
> > -{
> > -	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> > -		       16, 1, &s->handle, 4, 0);
> > -	pr_info("secret:\n");
> > -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > -		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> > -	pr_info("trusted-key: enonce:\n");
> > -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > -		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> > -}
> > -
> > -static inline void dump_tpm_buf(unsigned char *buf)
> > -{
> > -	int len;
> > -
> > -	pr_info("\ntpm buffer\n");
> > -	len = LOAD32(buf, TPM_SIZE_OFFSET);
> > -	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> > -}
> > -#else
> > -static inline void dump_options(struct trusted_key_options *o)
> > -{
> > -}
> > -
> > -static inline void dump_sess(struct osapsess *s)
> > -{
> > -}
> > -
> > -static inline void dump_tpm_buf(unsigned char *buf)
> > -{
> > -}
> > -#endif
> >  #endif
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 126437459a74d..636acb66a4f69 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -22,10 +22,78 @@
> >  #include <keys/trusted_tpm.h>
> >  
> >  static struct tpm_chip *chip;
> >  static struct tpm_digest *digests;
> >  
> > +/* implementation specific TPM constants */
> > +#define TPM_SIZE_OFFSET			2
> > +#define TPM_RETURN_OFFSET		6
> > +#define TPM_DATA_OFFSET			10
> > +
> > +#define LOAD32(buffer, offset)	(ntohl(*(uint32_t *)&buffer[offset]))
> > +#define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
> > +#define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
> > +
> > +struct osapsess {
> > +	uint32_t handle;
> > +	unsigned char secret[SHA1_DIGEST_SIZE];
> > +	unsigned char enonce[TPM_NONCE_SIZE];
> > +};
> > +
> > +/* discrete values, but have to store in uint16_t for TPM use */
> > +enum {
> > +	SEAL_keytype = 1,
> > +	SRK_keytype = 4
> > +};
> > +
> > +#define TPM_DEBUG 0
> > +
> > +#if TPM_DEBUG
> > +static inline void dump_options(struct trusted_key_options *o)
> > +{
> > +	pr_info("sealing key type %d\n", o->keytype);
> > +	pr_info("sealing key handle %0X\n", o->keyhandle);
> > +	pr_info("pcrlock %d\n", o->pcrlock);
> > +	pr_info("pcrinfo %d\n", o->pcrinfo_len);
> > +	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> > +		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> > +}
> > +
> > +static inline void dump_sess(struct osapsess *s)
> > +{
> > +	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> > +		       16, 1, &s->handle, 4, 0);
> > +	pr_info("secret:\n");
> > +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > +		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> > +	pr_info("trusted-key: enonce:\n");
> > +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > +		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> > +}
> > +
> > +static inline void dump_tpm_buf(unsigned char *buf)
> > +{
> > +	int len;
> > +
> > +	pr_info("\ntpm buffer\n");
> > +	len = LOAD32(buf, TPM_SIZE_OFFSET);
> > +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> > +}
> > +#else
> > +static inline void dump_options(struct trusted_key_options *o)
> > +{
> > +}
> > +
> > +static inline void dump_sess(struct osapsess *s)
> > +{
> > +}
> > +
> > +static inline void dump_tpm_buf(unsigned char *buf)
> > +{
> > +}
> > +#endif
> > +
> >  static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> >  		       unsigned int keylen, ...)
> >  {
> >  	struct hmac_sha1_ctx hmac_ctx;
> >  	va_list argp;
> > @@ -54,11 +122,11 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> >  }
> >  
> >  /*
> >   * calculate authorization info fields to send to TPM
> >   */
> > -int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> > +static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> >  			unsigned int keylen, unsigned char *h1,
> >  			unsigned char *h2, unsigned int h3, ...)
> >  {
> >  	unsigned char paramdigest[SHA1_DIGEST_SIZE];
> >  	struct sha1_ctx sha_ctx;
> > @@ -92,16 +160,15 @@ int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> >  		ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
> >  				  paramdigest, TPM_NONCE_SIZE, h1,
> >  				  TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
> >  	return ret;
> >  }
> > -EXPORT_SYMBOL_GPL(TSS_authhmac);
> >  
> >  /*
> >   * verify the AUTH1_COMMAND (Seal) result from TPM
> >   */
> > -int TSS_checkhmac1(unsigned char *buffer,
> > +static int TSS_checkhmac1(unsigned char *buffer,
> >  			  const uint32_t command,
> >  			  const unsigned char *ononce,
> >  			  const unsigned char *key,
> >  			  unsigned int keylen, ...)
> >  {
> > @@ -157,11 +224,10 @@ int TSS_checkhmac1(unsigned char *buffer,
> >  
> >  	if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
> >  		return -EINVAL;
> >  	return 0;
> >  }
> > -EXPORT_SYMBOL_GPL(TSS_checkhmac1);
> >  
> >  /*
> >   * verify the AUTH2_COMMAND (unseal) result from TPM
> >   */
> >  static int TSS_checkhmac2(unsigned char *buffer,
> > @@ -242,11 +308,11 @@ static int TSS_checkhmac2(unsigned char *buffer,
> >  
> >  /*
> >   * For key specific tpm requests, we will generate and send our
> >   * own TPM command packets using the drivers send function.
> >   */
> > -int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > +static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> >  {
> >  	struct tpm_buf buf;
> >  	int rc;
> >  
> >  	if (!chip)
> > @@ -268,11 +334,10 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> >  		rc = -EPERM;
> >  
> >  	tpm_put_ops(chip);
> >  	return rc;
> >  }
> > -EXPORT_SYMBOL_GPL(trusted_tpm_send);
> >  
> >  /*
> >   * Lock a trusted key, by extending a selected PCR.
> >   *
> >   * Prevents a trusted key that is sealed to PCRs from being accessed.
> > @@ -322,11 +387,11 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> >  }
> >  
> >  /*
> >   * Create an object independent authorisation protocol (oiap) session
> >   */
> > -int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> > +static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> >  {
> >  	int ret;
> >  
> >  	if (!chip)
> >  		return -ENODEV;
> > @@ -339,11 +404,10 @@ int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> >  	*handle = LOAD32(tb->data, TPM_DATA_OFFSET);
> >  	memcpy(nonce, &tb->data[TPM_DATA_OFFSET + sizeof(uint32_t)],
> >  	       TPM_NONCE_SIZE);
> >  	return 0;
> >  }
> > -EXPORT_SYMBOL_GPL(oiap);
> >  
> >  struct tpm_digests {
> >  	unsigned char encauth[SHA1_DIGEST_SIZE];
> >  	unsigned char pubauth[SHA1_DIGEST_SIZE];
> >  	unsigned char xorwork[SHA1_DIGEST_SIZE * 2];
> > -- 
> > 2.50.1
> > 
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

also applied (all of three)

thank you!

BR, Jarkko

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

end of thread, other threads:[~2025-08-12 16:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-09 17:19 [PATCH v2 0/3] KEYS: trusted_tpm1: HMAC fix and cleanup Eric Biggers
2025-08-09 17:19 ` [PATCH v2 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time Eric Biggers
2025-08-12 16:23   ` Jarkko Sakkinen
2025-08-09 17:19 ` [PATCH v2 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash Eric Biggers
2025-08-12 16:24   ` Jarkko Sakkinen
2025-08-09 17:19 ` [PATCH v2 3/3] KEYS: trusted_tpm1: Move private functionality out of public header Eric Biggers
2025-08-12 16:24   ` Jarkko Sakkinen
2025-08-12 16:29     ` 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).