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

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Aug 01, 2025 at 03:20:52PM -0400, James Bottomley wrote:
> On Fri, 2025-08-01 at 12:03 -0700, Eric Biggers wrote:
> > On Fri, Aug 01, 2025 at 02:53:09PM -0400, James Bottomley wrote:
> > > On Fri, 2025-08-01 at 11:40 -0700, Eric Biggers wrote:
> > > > On Fri, Aug 01, 2025 at 02:03:47PM -0400, James Bottomley wrote:
> > > > > On Fri, 2025-08-01 at 10:11 -0700, Eric Biggers wrote:
> > > [...]
> > > > > > It's true that such attacks don't work with one-time keys. 
> > > > > > But here it's not necessarily a one-time key.  E.g.,
> > > > > > tpm2_get_random() sets a key, then authenticates multiple
> > > > > > messages using that key.
> > > > > 
> > > > > The nonces come one from us and one from the TPM.  I think ours
> > > > > doesn't change if the session is continued although it could,
> > > > > whereas the TPM one does, so the HMAC key is different for
> > > > > every communication of a continued session.
> > > > 
> > > > Again, tpm2_get_random() sets a HMAC key once and then uses it
> > > > multiple times.
> > > 
> > > No it doesn't.  If you actually read the code, you'd find it does
> > > what I say above.  Specifically  tpm_buf_fill_hmac_session() which
> > > is called inside that loop recalculates the hmac key from the
> > > nonces.  This recalculated key is what is used in
> > > tpm_buf_check_hmac_response(), and which is where the new tpm nonce
> > > is collected for the next
> > > iteration.
> > 
> > tpm_buf_fill_hmac_session() computes a HMAC value, but it doesn't
> > modify the HMAC key.  tpm2_parse_start_auth_session() is the only
> > place where the HMAC key is changed.  You may be confusing HMAC
> > values with keys.
> 
> Is this simply a semantic quibble about what gets called a key?  For
> each TPM command we compute a cphash across all the command parameters
> (and for each return a rphash).  This hash then forms a
> hmac(session_key, cphash | our_nonce | tpm_nonce | attrs).  The point
> being that although session_key is fixed across the session, the
> our_nonce and tpm_nonce can change with every iteration.  Since the
> cphash is over the ciphertext, it's the only bit you get to vary with a
> chosen ciphertext attack, so the other parameters effectively key the
> hmac.

No, it's not "simply a semantic quibble".  You're just wrong.

As I said earlier, our_nonce (which is not a key) does appear to make
MAC timing attacks not possible.  All the other fields appear to be
attacker-controlled, contrary to what you're claiming above.

Anyway, point taken: I'll drop the Fixes and Cc stable from the commit,
and include my own analysis of why MAC timing attacks don't appear to be
possible with this protocol.  Everything else in this thread has just
been a pointless distraction.

- Eric