[PATCH] tpm: Disable TPM2_TCG_HMAC by default

[PATCH] tpm: Disable TPM2_TCG_HMAC by default

[Jarkko Sakkinen]

After reading all the feedback, right now disabling the TPM2_TCG_HMAC
is the right call.

Other views discussed:

A. Having a kernel command-line parameter or refining the feature
   otherwise. This goes to the area of improvements.  E.g., one
   example is my own idea where the null key specific code would be
   replaced with a persistent handle parameter (which can be
   *unambigously* defined as part of attestation process when
   done correctly).

B. Removing the code. I don't buy this because that is same as saying
   that HMAC encryption cannot work at all (if really nitpicking) in
   any form. Also I disagree on the view that the feature could not
   be refined to something more reasoable.

Also, both A and B are worst options in terms of backporting.

Thuss, this is the best possible choice.

Cc: stable@vger.kernel.or # v6.10+
Fixes: d2add27cf2b8 ("tpm: Add NULL primary creation")
Suggested-by: Chris Fenner <cfenn@google.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
PS. I did not post this last week because that would have been most
likely the most counter-productive action to taken. It's better
sometimes to take a bit of time to think (which can be seen that
I've given also more reasonable weight to my own eaerlier
proposals).

I also accept further changes, if there is e.g., inconsistency
with TCG_TPM_HMAC setting or similar (obviously).
---
 drivers/char/tpm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index dddd702b2454..3e4684f6b4af 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -29,7 +29,7 @@ if TCG_TPM
 
 config TCG_TPM2_HMAC
 	bool "Use HMAC and encrypted transactions on the TPM bus"
-	default X86_64
+	default n
 	select CRYPTO_ECDH
 	select CRYPTO_LIB_AESCFB
 	select CRYPTO_LIB_SHA256
-- 
2.39.5

Re: [PATCH] tpm: Disable TPM2_TCG_HMAC by default

[Jonathan McDowell]

On Mon, Aug 25, 2025 at 11:32:23PM +0300, Jarkko Sakkinen wrote:
>After reading all the feedback, right now disabling the TPM2_TCG_HMAC
>is the right call.
>
>Other views discussed:
>
>A. Having a kernel command-line parameter or refining the feature
>   otherwise. This goes to the area of improvements.  E.g., one
>   example is my own idea where the null key specific code would be
>   replaced with a persistent handle parameter (which can be
>   *unambigously* defined as part of attestation process when
>   done correctly).
>
>B. Removing the code. I don't buy this because that is same as saying
>   that HMAC encryption cannot work at all (if really nitpicking) in
>   any form. Also I disagree on the view that the feature could not
>   be refined to something more reasoable.
>
>Also, both A and B are worst options in terms of backporting.
>
>Thus, this is the best possible choice.

I think this is reasonable; it's adding runtime overhead and not adding 
enough benefit to be the default upstream.

Reviewed-By: Jonathan McDowell <noodles@earth.li>

>Cc: stable@vger.kernel.or # v6.10+
>Fixes: d2add27cf2b8 ("tpm: Add NULL primary creation")
>Suggested-by: Chris Fenner <cfenn@google.com>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
>PS. I did not post this last week because that would have been most
>likely the most counter-productive action to taken. It's better
>sometimes to take a bit of time to think (which can be seen that
>I've given also more reasonable weight to my own eaerlier
>proposals).
>
>I also accept further changes, if there is e.g., inconsistency
>with TCG_TPM_HMAC setting or similar (obviously).
>---
> drivers/char/tpm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>index dddd702b2454..3e4684f6b4af 100644
>--- a/drivers/char/tpm/Kconfig
>+++ b/drivers/char/tpm/Kconfig
>@@ -29,7 +29,7 @@ if TCG_TPM
>
> config TCG_TPM2_HMAC
> 	bool "Use HMAC and encrypted transactions on the TPM bus"
>-	default X86_64
>+	default n
> 	select CRYPTO_ECDH
> 	select CRYPTO_LIB_AESCFB
> 	select CRYPTO_LIB_SHA256
>-- 
>2.39.5

J.

-- 
] https://www.earth.li/~noodles/ []  Is this real - that's the first   [
]  PGP/GPG Key @ the.earth.li    []    thing I think every morning.    [
] via keyserver, web or email.   []                                    [
] RSA: 4096/0x94FA372B2DA8B985   []                                    [

Re: [PATCH] tpm: Disable TPM2_TCG_HMAC by default

[Jarkko Sakkinen]

On Thu, Sep 18, 2025 at 07:56:53PM +0100, Jonathan McDowell wrote:
> On Mon, Aug 25, 2025 at 11:32:23PM +0300, Jarkko Sakkinen wrote:
> > After reading all the feedback, right now disabling the TPM2_TCG_HMAC
> > is the right call.
> > 
> > Other views discussed:
> > 
> > A. Having a kernel command-line parameter or refining the feature
> >   otherwise. This goes to the area of improvements.  E.g., one
> >   example is my own idea where the null key specific code would be
> >   replaced with a persistent handle parameter (which can be
> >   *unambigously* defined as part of attestation process when
> >   done correctly).
> > 
> > B. Removing the code. I don't buy this because that is same as saying
> >   that HMAC encryption cannot work at all (if really nitpicking) in
> >   any form. Also I disagree on the view that the feature could not
> >   be refined to something more reasoable.
> > 
> > Also, both A and B are worst options in terms of backporting.
> > 
> > Thus, this is the best possible choice.
> 
> I think this is reasonable; it's adding runtime overhead and not adding
> enough benefit to be the default upstream.

Yes, I think this is a balanced change. I agree what you say and at the
same time this gives more space to refine it something usable. Right now
it is much harder to tackle those issue, as it is part of the default
config. By looking at things from this angle, the change is also
benefical for the feature itself (in the long run).

> Reviewed-By: Jonathan McDowell <noodles@earth.li>

Thank you! I appreciate this and will append this to the commit.

BR, Jarkko

Re: [PATCH] tpm: Disable TPM2_TCG_HMAC by default

[Chris Fenner]

Agreed, the feature needs some work in order to provide meaningful
security value, and disabling it by default facilitates that work.

Reviewed-By: Chris Fenner <cfenn@google.com>

On Thu, Sep 18, 2025 at 12:12 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 07:56:53PM +0100, Jonathan McDowell wrote:
> > On Mon, Aug 25, 2025 at 11:32:23PM +0300, Jarkko Sakkinen wrote:
> > > After reading all the feedback, right now disabling the TPM2_TCG_HMAC
> > > is the right call.
> > >
> > > Other views discussed:
> > >
> > > A. Having a kernel command-line parameter or refining the feature
> > >   otherwise. This goes to the area of improvements.  E.g., one
> > >   example is my own idea where the null key specific code would be
> > >   replaced with a persistent handle parameter (which can be
> > >   *unambigously* defined as part of attestation process when
> > >   done correctly).
> > >
> > > B. Removing the code. I don't buy this because that is same as saying
> > >   that HMAC encryption cannot work at all (if really nitpicking) in
> > >   any form. Also I disagree on the view that the feature could not
> > >   be refined to something more reasoable.
> > >
> > > Also, both A and B are worst options in terms of backporting.
> > >
> > > Thus, this is the best possible choice.
> >
> > I think this is reasonable; it's adding runtime overhead and not adding
> > enough benefit to be the default upstream.
>
> Yes, I think this is a balanced change. I agree what you say and at the
> same time this gives more space to refine it something usable. Right now
> it is much harder to tackle those issue, as it is part of the default
> config. By looking at things from this angle, the change is also
> benefical for the feature itself (in the long run).
>
> > Reviewed-By: Jonathan McDowell <noodles@earth.li>
>
> Thank you! I appreciate this and will append this to the commit.
>
> BR, Jarkko

Re: [PATCH] tpm: Disable TPM2_TCG_HMAC by default

[Jarkko Sakkinen]

On Thu, Sep 18, 2025 at 12:50:57PM -0700, Chris Fenner wrote:
> Agreed, the feature needs some work in order to provide meaningful
> security value, and disabling it by default facilitates that work.
> 
> Reviewed-By: Chris Fenner <cfenn@google.com>

Thanks!

BR, Jarkko