[PATCH v2 0/2] tpm: Clean up HMAC validation and computation

[PATCH v2 0/2] tpm: Clean up HMAC validation and computation

[Eric Biggers]

Patch 1 updates the tpm driver to compare HMAC values in constant time.

Patch 2 simplifies the HMAC computation in the tpm driver by using the
library API instead of an open-coded HMAC implementation.  Note that
this depends on the HMAC library API that was merged for v6.17-rc1.

Changed in v2:
  - Updated commit message of patch 1 to no longer characterize it as a
    fix.  Explained why the side channel seems to have been benign.

Eric Biggers (2):
  tpm: Compare HMAC values in constant time
  tpm: Use HMAC-SHA256 library instead of open-coded HMAC

 drivers/char/tpm/Kconfig         |   1 +
 drivers/char/tpm/tpm2-sessions.c | 104 +++++++++----------------------
 2 files changed, 31 insertions(+), 74 deletions(-)


base-commit: d6084bb815c453de27af8071a23163a711586a6c
-- 
2.50.1

[PATCH v2 1/2] tpm: Compare HMAC values in constant time

[Eric Biggers]

In tpm_buf_check_hmac_response(), compare the HMAC values in constant
time using crypto_memneq() instead of in variable time using memcmp().

This is worthwhile to follow best practices and to be consistent with
MAC comparisons elsewhere in the kernel.  However, in this driver the
side channel seems to have been benign: the HMAC input data is
guaranteed to always be unique, which makes the usual MAC forgery via
timing side channel not possible.  Specifically, the HMAC input data in
tpm_buf_check_hmac_response() includes the "our_nonce" field, which was
generated by the kernel earlier, remains under the control of the
kernel, and is unique for each call to tpm_buf_check_hmac_response().

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 drivers/char/tpm/Kconfig         | 1 +
 drivers/char/tpm/tpm2-sessions.c | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index dddd702b2454a..f9d8a4e966867 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -31,10 +31,11 @@ config TCG_TPM2_HMAC
 	bool "Use HMAC and encrypted transactions on the TPM bus"
 	default X86_64
 	select CRYPTO_ECDH
 	select CRYPTO_LIB_AESCFB
 	select CRYPTO_LIB_SHA256
+	select CRYPTO_LIB_UTILS
 	help
 	  Setting this causes us to deploy a scheme which uses request
 	  and response HMACs in addition to encryption for
 	  communicating with the TPM to prevent or detect bus snooping
 	  and interposer attacks (see tpm-security.rst).  Saying Y
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index bdb119453dfbe..5fbd62ee50903 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -69,10 +69,11 @@
 #include <linux/unaligned.h>
 #include <crypto/kpp.h>
 #include <crypto/ecdh.h>
 #include <crypto/hash.h>
 #include <crypto/hmac.h>
+#include <crypto/utils.h>
 
 /* maximum number of names the TPM must remember for authorization */
 #define AUTH_MAX_NAMES	3
 
 #define AES_KEY_BYTES	AES_KEYSIZE_128
@@ -827,16 +828,15 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
 	sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce));
 	sha256_update(&sctx, &auth->attrs, 1);
 	/* we're done with the rphash, so put our idea of the hmac there */
 	tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
 			+ auth->passphrase_len, rphash);
-	if (memcmp(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE) == 0) {
-		rc = 0;
-	} else {
+	if (crypto_memneq(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE)) {
 		dev_err(&chip->dev, "TPM: HMAC check failed\n");
 		goto out;
 	}
+	rc = 0;
 
 	/* now do response decryption */
 	if (auth->attrs & TPM2_SA_ENCRYPT) {
 		/* need key and IV */
 		tpm2_KDFa(auth->session_key, SHA256_DIGEST_SIZE
-- 
2.50.1

[PATCH v2 2/2] tpm: Use HMAC-SHA256 library instead of open-coded HMAC

[Eric Biggers]

Now that there are easy-to-use HMAC-SHA256 library functions, use these
in tpm2-sessions.c instead of open-coding the HMAC algorithm.

Note that the new implementation correctly handles keys longer than 64
bytes (SHA256_BLOCK_SIZE), whereas the old implementation handled such
keys incorrectly.  But it doesn't appear that such keys were being used.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 drivers/char/tpm/tpm2-sessions.c | 98 +++++++++-----------------------
 1 file changed, 27 insertions(+), 71 deletions(-)

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 5fbd62ee50903..6d03c224e6b21 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -67,12 +67,11 @@
 #include <linux/random.h>
 #include <linux/scatterlist.h>
 #include <linux/unaligned.h>
 #include <crypto/kpp.h>
 #include <crypto/ecdh.h>
-#include <crypto/hash.h>
-#include <crypto/hmac.h>
+#include <crypto/sha2.h>
 #include <crypto/utils.h>
 
 /* maximum number of names the TPM must remember for authorization */
 #define AUTH_MAX_NAMES	3
 
@@ -383,55 +382,10 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_hmac_session);
 #ifdef CONFIG_TCG_TPM2_HMAC
 
 static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
 			       u32 *handle, u8 *name);
 
-/*
- * It turns out the crypto hmac(sha256) is hard for us to consume
- * because it assumes a fixed key and the TPM seems to change the key
- * on every operation, so we weld the hmac init and final functions in
- * here to give it the same usage characteristics as a regular hash
- */
-static void tpm2_hmac_init(struct sha256_ctx *sctx, u8 *key, u32 key_len)
-{
-	u8 pad[SHA256_BLOCK_SIZE];
-	int i;
-
-	sha256_init(sctx);
-	for (i = 0; i < sizeof(pad); i++) {
-		if (i < key_len)
-			pad[i] = key[i];
-		else
-			pad[i] = 0;
-		pad[i] ^= HMAC_IPAD_VALUE;
-	}
-	sha256_update(sctx, pad, sizeof(pad));
-}
-
-static void tpm2_hmac_final(struct sha256_ctx *sctx, u8 *key, u32 key_len,
-			    u8 *out)
-{
-	u8 pad[SHA256_BLOCK_SIZE];
-	int i;
-
-	for (i = 0; i < sizeof(pad); i++) {
-		if (i < key_len)
-			pad[i] = key[i];
-		else
-			pad[i] = 0;
-		pad[i] ^= HMAC_OPAD_VALUE;
-	}
-
-	/* collect the final hash;  use out as temporary storage */
-	sha256_final(sctx, out);
-
-	sha256_init(sctx);
-	sha256_update(sctx, pad, sizeof(pad));
-	sha256_update(sctx, out, SHA256_DIGEST_SIZE);
-	sha256_final(sctx, out);
-}
-
 /*
  * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE but
  * otherwise standard tpm2_KDFa.  Note output is in bytes not bits.
  */
 static void tpm2_KDFa(u8 *key, u32 key_len, const char *label, u8 *u,
@@ -439,20 +393,20 @@ static void tpm2_KDFa(u8 *key, u32 key_len, const char *label, u8 *u,
 {
 	u32 counter = 1;
 	const __be32 bits = cpu_to_be32(bytes * 8);
 
 	while (bytes > 0) {
-		struct sha256_ctx sctx;
+		struct hmac_sha256_ctx hctx;
 		__be32 c = cpu_to_be32(counter);
 
-		tpm2_hmac_init(&sctx, key, key_len);
-		sha256_update(&sctx, (u8 *)&c, sizeof(c));
-		sha256_update(&sctx, label, strlen(label)+1);
-		sha256_update(&sctx, u, SHA256_DIGEST_SIZE);
-		sha256_update(&sctx, v, SHA256_DIGEST_SIZE);
-		sha256_update(&sctx, (u8 *)&bits, sizeof(bits));
-		tpm2_hmac_final(&sctx, key, key_len, out);
+		hmac_sha256_init_usingrawkey(&hctx, key, key_len);
+		hmac_sha256_update(&hctx, (u8 *)&c, sizeof(c));
+		hmac_sha256_update(&hctx, label, strlen(label) + 1);
+		hmac_sha256_update(&hctx, u, SHA256_DIGEST_SIZE);
+		hmac_sha256_update(&hctx, v, SHA256_DIGEST_SIZE);
+		hmac_sha256_update(&hctx, (u8 *)&bits, sizeof(bits));
+		hmac_sha256_final(&hctx, out);
 
 		bytes -= SHA256_DIGEST_SIZE;
 		counter++;
 		out += SHA256_DIGEST_SIZE;
 	}
@@ -592,10 +546,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
 	off_t offset_s = TPM_HEADER_SIZE, offset_p;
 	u8 *hmac = NULL;
 	u32 attrs;
 	u8 cphash[SHA256_DIGEST_SIZE];
 	struct sha256_ctx sctx;
+	struct hmac_sha256_ctx hctx;
 
 	if (!auth)
 		return;
 
 	/* save the command code in BE format */
@@ -703,18 +658,18 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
 		sha256_update(&sctx, &buf->data[offset_s],
 			      tpm_buf_length(buf) - offset_s);
 	sha256_final(&sctx, cphash);
 
 	/* now calculate the hmac */
-	tpm2_hmac_init(&sctx, auth->session_key, sizeof(auth->session_key)
-		       + auth->passphrase_len);
-	sha256_update(&sctx, cphash, sizeof(cphash));
-	sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce));
-	sha256_update(&sctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
-	sha256_update(&sctx, &auth->attrs, 1);
-	tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
-			+ auth->passphrase_len, hmac);
+	hmac_sha256_init_usingrawkey(&hctx, auth->session_key,
+				     sizeof(auth->session_key) +
+					     auth->passphrase_len);
+	hmac_sha256_update(&hctx, cphash, sizeof(cphash));
+	hmac_sha256_update(&hctx, auth->our_nonce, sizeof(auth->our_nonce));
+	hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
+	hmac_sha256_update(&hctx, &auth->attrs, 1);
+	hmac_sha256_final(&hctx, hmac);
 }
 EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
 
 /**
  * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
@@ -750,10 +705,11 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
 	struct tpm2_auth *auth = chip->auth;
 	off_t offset_s, offset_p;
 	u8 rphash[SHA256_DIGEST_SIZE];
 	u32 attrs, cc;
 	struct sha256_ctx sctx;
+	struct hmac_sha256_ctx hctx;
 	u16 tag = be16_to_cpu(head->tag);
 	int parm_len, len, i, handles;
 
 	if (!auth)
 		return rc;
@@ -819,19 +775,19 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
 	sha256_update(&sctx, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
 	sha256_update(&sctx, &buf->data[offset_p], parm_len);
 	sha256_final(&sctx, rphash);
 
 	/* now calculate the hmac */
-	tpm2_hmac_init(&sctx, auth->session_key, sizeof(auth->session_key)
-		       + auth->passphrase_len);
-	sha256_update(&sctx, rphash, sizeof(rphash));
-	sha256_update(&sctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
-	sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce));
-	sha256_update(&sctx, &auth->attrs, 1);
+	hmac_sha256_init_usingrawkey(&hctx, auth->session_key,
+				     sizeof(auth->session_key) +
+					     auth->passphrase_len);
+	hmac_sha256_update(&hctx, rphash, sizeof(rphash));
+	hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
+	hmac_sha256_update(&hctx, auth->our_nonce, sizeof(auth->our_nonce));
+	hmac_sha256_update(&hctx, &auth->attrs, 1);
 	/* we're done with the rphash, so put our idea of the hmac there */
-	tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
-			+ auth->passphrase_len, rphash);
+	hmac_sha256_final(&hctx, rphash);
 	if (crypto_memneq(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE)) {
 		dev_err(&chip->dev, "TPM: HMAC check failed\n");
 		goto out;
 	}
 	rc = 0;
-- 
2.50.1

Re: [PATCH v2 1/2] tpm: Compare HMAC values in constant time

[Jarkko Sakkinen]

On Fri, Aug 01, 2025 at 02:24:21PM -0700, Eric Biggers wrote:
> In tpm_buf_check_hmac_response(), compare the HMAC values in constant
> time using crypto_memneq() instead of in variable time using memcmp().
> 
> This is worthwhile to follow best practices and to be consistent with
> MAC comparisons elsewhere in the kernel.  However, in this driver the
> side channel seems to have been benign: the HMAC input data is
> guaranteed to always be unique, which makes the usual MAC forgery via
> timing side channel not possible.  Specifically, the HMAC input data in
> tpm_buf_check_hmac_response() includes the "our_nonce" field, which was
> generated by the kernel earlier, remains under the control of the
> kernel, and is unique for each call to tpm_buf_check_hmac_response().
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  drivers/char/tpm/Kconfig         | 1 +
>  drivers/char/tpm/tpm2-sessions.c | 6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index dddd702b2454a..f9d8a4e966867 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -31,10 +31,11 @@ config TCG_TPM2_HMAC
>  	bool "Use HMAC and encrypted transactions on the TPM bus"
>  	default X86_64
>  	select CRYPTO_ECDH
>  	select CRYPTO_LIB_AESCFB
>  	select CRYPTO_LIB_SHA256
> +	select CRYPTO_LIB_UTILS
>  	help
>  	  Setting this causes us to deploy a scheme which uses request
>  	  and response HMACs in addition to encryption for
>  	  communicating with the TPM to prevent or detect bus snooping
>  	  and interposer attacks (see tpm-security.rst).  Saying Y
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index bdb119453dfbe..5fbd62ee50903 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -69,10 +69,11 @@
>  #include <linux/unaligned.h>
>  #include <crypto/kpp.h>
>  #include <crypto/ecdh.h>
>  #include <crypto/hash.h>
>  #include <crypto/hmac.h>
> +#include <crypto/utils.h>
>  
>  /* maximum number of names the TPM must remember for authorization */
>  #define AUTH_MAX_NAMES	3
>  
>  #define AES_KEY_BYTES	AES_KEYSIZE_128
> @@ -827,16 +828,15 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
>  	sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce));
>  	sha256_update(&sctx, &auth->attrs, 1);
>  	/* we're done with the rphash, so put our idea of the hmac there */
>  	tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
>  			+ auth->passphrase_len, rphash);
> -	if (memcmp(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE) == 0) {
> -		rc = 0;
> -	} else {
> +	if (crypto_memneq(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE)) {
>  		dev_err(&chip->dev, "TPM: HMAC check failed\n");
>  		goto out;
>  	}
> +	rc = 0;
>  
>  	/* now do response decryption */
>  	if (auth->attrs & TPM2_SA_ENCRYPT) {
>  		/* need key and IV */
>  		tpm2_KDFa(auth->session_key, SHA256_DIGEST_SIZE
> -- 
> 2.50.1
> 

I think we might want to also backport this to stables.

BR, Jarkko

Re: [PATCH v2 2/2] tpm: Use HMAC-SHA256 library instead of open-coded HMAC

[Jarkko Sakkinen]

On Fri, Aug 01, 2025 at 02:24:22PM -0700, Eric Biggers wrote:
> Now that there are easy-to-use HMAC-SHA256 library functions, use these
> in tpm2-sessions.c instead of open-coding the HMAC algorithm.
> 
> Note that the new implementation correctly handles keys longer than 64
> bytes (SHA256_BLOCK_SIZE), whereas the old implementation handled such
> keys incorrectly.  But it doesn't appear that such keys were being used.
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  drivers/char/tpm/tpm2-sessions.c | 98 +++++++++-----------------------
>  1 file changed, 27 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index 5fbd62ee50903..6d03c224e6b21 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -67,12 +67,11 @@
>  #include <linux/random.h>
>  #include <linux/scatterlist.h>
>  #include <linux/unaligned.h>
>  #include <crypto/kpp.h>
>  #include <crypto/ecdh.h>
> -#include <crypto/hash.h>
> -#include <crypto/hmac.h>
> +#include <crypto/sha2.h>
>  #include <crypto/utils.h>
>  
>  /* maximum number of names the TPM must remember for authorization */
>  #define AUTH_MAX_NAMES	3
>  
> @@ -383,55 +382,10 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_hmac_session);
>  #ifdef CONFIG_TCG_TPM2_HMAC
>  
>  static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
>  			       u32 *handle, u8 *name);
>  
> -/*
> - * It turns out the crypto hmac(sha256) is hard for us to consume
> - * because it assumes a fixed key and the TPM seems to change the key
> - * on every operation, so we weld the hmac init and final functions in
> - * here to give it the same usage characteristics as a regular hash
> - */
> -static void tpm2_hmac_init(struct sha256_ctx *sctx, u8 *key, u32 key_len)
> -{
> -	u8 pad[SHA256_BLOCK_SIZE];
> -	int i;
> -
> -	sha256_init(sctx);
> -	for (i = 0; i < sizeof(pad); i++) {
> -		if (i < key_len)
> -			pad[i] = key[i];
> -		else
> -			pad[i] = 0;
> -		pad[i] ^= HMAC_IPAD_VALUE;
> -	}
> -	sha256_update(sctx, pad, sizeof(pad));
> -}
> -
> -static void tpm2_hmac_final(struct sha256_ctx *sctx, u8 *key, u32 key_len,
> -			    u8 *out)
> -{
> -	u8 pad[SHA256_BLOCK_SIZE];
> -	int i;
> -
> -	for (i = 0; i < sizeof(pad); i++) {
> -		if (i < key_len)
> -			pad[i] = key[i];
> -		else
> -			pad[i] = 0;
> -		pad[i] ^= HMAC_OPAD_VALUE;
> -	}
> -
> -	/* collect the final hash;  use out as temporary storage */
> -	sha256_final(sctx, out);
> -
> -	sha256_init(sctx);
> -	sha256_update(sctx, pad, sizeof(pad));
> -	sha256_update(sctx, out, SHA256_DIGEST_SIZE);
> -	sha256_final(sctx, out);
> -}
> -
>  /*
>   * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE but
>   * otherwise standard tpm2_KDFa.  Note output is in bytes not bits.
>   */
>  static void tpm2_KDFa(u8 *key, u32 key_len, const char *label, u8 *u,
> @@ -439,20 +393,20 @@ static void tpm2_KDFa(u8 *key, u32 key_len, const char *label, u8 *u,
>  {
>  	u32 counter = 1;
>  	const __be32 bits = cpu_to_be32(bytes * 8);
>  
>  	while (bytes > 0) {
> -		struct sha256_ctx sctx;
> +		struct hmac_sha256_ctx hctx;
>  		__be32 c = cpu_to_be32(counter);
>  
> -		tpm2_hmac_init(&sctx, key, key_len);
> -		sha256_update(&sctx, (u8 *)&c, sizeof(c));
> -		sha256_update(&sctx, label, strlen(label)+1);
> -		sha256_update(&sctx, u, SHA256_DIGEST_SIZE);
> -		sha256_update(&sctx, v, SHA256_DIGEST_SIZE);
> -		sha256_update(&sctx, (u8 *)&bits, sizeof(bits));
> -		tpm2_hmac_final(&sctx, key, key_len, out);
> +		hmac_sha256_init_usingrawkey(&hctx, key, key_len);
> +		hmac_sha256_update(&hctx, (u8 *)&c, sizeof(c));
> +		hmac_sha256_update(&hctx, label, strlen(label) + 1);
> +		hmac_sha256_update(&hctx, u, SHA256_DIGEST_SIZE);
> +		hmac_sha256_update(&hctx, v, SHA256_DIGEST_SIZE);
> +		hmac_sha256_update(&hctx, (u8 *)&bits, sizeof(bits));
> +		hmac_sha256_final(&hctx, out);
>  
>  		bytes -= SHA256_DIGEST_SIZE;
>  		counter++;
>  		out += SHA256_DIGEST_SIZE;
>  	}
> @@ -592,10 +546,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
>  	off_t offset_s = TPM_HEADER_SIZE, offset_p;
>  	u8 *hmac = NULL;
>  	u32 attrs;
>  	u8 cphash[SHA256_DIGEST_SIZE];
>  	struct sha256_ctx sctx;
> +	struct hmac_sha256_ctx hctx;
>  
>  	if (!auth)
>  		return;
>  
>  	/* save the command code in BE format */
> @@ -703,18 +658,18 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
>  		sha256_update(&sctx, &buf->data[offset_s],
>  			      tpm_buf_length(buf) - offset_s);
>  	sha256_final(&sctx, cphash);
>  
>  	/* now calculate the hmac */
> -	tpm2_hmac_init(&sctx, auth->session_key, sizeof(auth->session_key)
> -		       + auth->passphrase_len);
> -	sha256_update(&sctx, cphash, sizeof(cphash));
> -	sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce));
> -	sha256_update(&sctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> -	sha256_update(&sctx, &auth->attrs, 1);
> -	tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
> -			+ auth->passphrase_len, hmac);
> +	hmac_sha256_init_usingrawkey(&hctx, auth->session_key,
> +				     sizeof(auth->session_key) +
> +					     auth->passphrase_len);
> +	hmac_sha256_update(&hctx, cphash, sizeof(cphash));
> +	hmac_sha256_update(&hctx, auth->our_nonce, sizeof(auth->our_nonce));
> +	hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> +	hmac_sha256_update(&hctx, &auth->attrs, 1);
> +	hmac_sha256_final(&hctx, hmac);
>  }
>  EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
>  
>  /**
>   * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
> @@ -750,10 +705,11 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
>  	struct tpm2_auth *auth = chip->auth;
>  	off_t offset_s, offset_p;
>  	u8 rphash[SHA256_DIGEST_SIZE];
>  	u32 attrs, cc;
>  	struct sha256_ctx sctx;
> +	struct hmac_sha256_ctx hctx;
>  	u16 tag = be16_to_cpu(head->tag);
>  	int parm_len, len, i, handles;
>  
>  	if (!auth)
>  		return rc;
> @@ -819,19 +775,19 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
>  	sha256_update(&sctx, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
>  	sha256_update(&sctx, &buf->data[offset_p], parm_len);
>  	sha256_final(&sctx, rphash);
>  
>  	/* now calculate the hmac */
> -	tpm2_hmac_init(&sctx, auth->session_key, sizeof(auth->session_key)
> -		       + auth->passphrase_len);
> -	sha256_update(&sctx, rphash, sizeof(rphash));
> -	sha256_update(&sctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> -	sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce));
> -	sha256_update(&sctx, &auth->attrs, 1);
> +	hmac_sha256_init_usingrawkey(&hctx, auth->session_key,
> +				     sizeof(auth->session_key) +
> +					     auth->passphrase_len);
> +	hmac_sha256_update(&hctx, rphash, sizeof(rphash));
> +	hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> +	hmac_sha256_update(&hctx, auth->our_nonce, sizeof(auth->our_nonce));
> +	hmac_sha256_update(&hctx, &auth->attrs, 1);
>  	/* we're done with the rphash, so put our idea of the hmac there */
> -	tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
> -			+ auth->passphrase_len, rphash);
> +	hmac_sha256_final(&hctx, rphash);
>  	if (crypto_memneq(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE)) {
>  		dev_err(&chip->dev, "TPM: HMAC check failed\n");
>  		goto out;
>  	}
>  	rc = 0;
> -- 
> 2.50.1
> 

This good stuff, thanks!

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

BR, Jarkko

Re: [PATCH v2 1/2] tpm: Compare HMAC values in constant time

[Eric Biggers]

On Tue, Aug 05, 2025 at 04:50:17PM +0300, Jarkko Sakkinen wrote:
> 
> I think we might want to also backport this to stables.
> 

That's what I did originally, but on v1 James complained about it being
characterized as a fix.

- Eric

Re: [PATCH v2 1/2] tpm: Compare HMAC values in constant time

[Jarkko Sakkinen]

On Tue, Aug 05, 2025 at 09:07:40AM -0700, Eric Biggers wrote:
> On Tue, Aug 05, 2025 at 04:50:17PM +0300, Jarkko Sakkinen wrote:
> > 
> > I think we might want to also backport this to stables.
> > 
> 
> That's what I did originally, but on v1 James complained about it being
> characterized as a fix.

Please put out v3 with backporting shenanigans and I can apply these.

> 
> - Eric

BR, Jarkko

Re: [PATCH v2 1/2] tpm: Compare HMAC values in constant time

[Eric Biggers]

On Sat, Aug 09, 2025 at 01:36:46PM +0300, Jarkko Sakkinen wrote:
> On Tue, Aug 05, 2025 at 09:07:40AM -0700, Eric Biggers wrote:
> > On Tue, Aug 05, 2025 at 04:50:17PM +0300, Jarkko Sakkinen wrote:
> > > 
> > > I think we might want to also backport this to stables.
> > > 
> > 
> > That's what I did originally, but on v1 James complained about it being
> > characterized as a fix.
> 
> Please put out v3 with backporting shenanigans and I can apply these.
> 

v1 had Fixes and Cc stable
(https://lore.kernel.org/r/20250731215255.113897-2-ebiggers@kernel.org/).
Again, I removed them in response to James' complaint about it being
characterized as a fix.  If you want them back, please go ahead and add
them back in when committing.  I'm not going to go around in circles.

- Eric

Re: [PATCH v2 1/2] tpm: Compare HMAC values in constant time

[Eric Biggers]

Hi Jarkko,

On Sat, Aug 09, 2025 at 10:38:39AM -0700, Eric Biggers wrote:
> On Sat, Aug 09, 2025 at 01:36:46PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Aug 05, 2025 at 09:07:40AM -0700, Eric Biggers wrote:
> > > On Tue, Aug 05, 2025 at 04:50:17PM +0300, Jarkko Sakkinen wrote:
> > > > 
> > > > I think we might want to also backport this to stables.
> > > > 
> > > 
> > > That's what I did originally, but on v1 James complained about it being
> > > characterized as a fix.
> > 
> > Please put out v3 with backporting shenanigans and I can apply these.
> > 
> 
> v1 had Fixes and Cc stable
> (https://lore.kernel.org/r/20250731215255.113897-2-ebiggers@kernel.org/).
> Again, I removed them in response to James' complaint about it being
> characterized as a fix.  If you want them back, please go ahead and add
> them back in when committing.  I'm not going to go around in circles.
> 
> - Eric

Could you let me know how you'd like to proceed here?  Thanks.

- Eric