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 <ebigg...@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 <jar...@kernel.org> BR, Jarkko