On Fri, 17 May 2024 at 03:59, James Bottomley
<[email protected]> wrote:
>
> On Thu, 2024-05-16 at 20:25 -0400, Nícolas F. R. A. Prado wrote:
...
> > KernelCI has identified a new warning and I tracked it down to this
> > commit. It
> > was observed on the following platforms:
> > * mt8183-kukui-jacuzzi-juniper-sku16
> > * sc7180-trogdor-kingoftown
> > (but probably affects all platforms that have a tpm driver with async
> > probe)
> >
> > [ 2.175146] Call trace:
> > [ 2.177587] __request_module+0x188/0x1f4
> > [ 2.181596] crypto_alg_mod_lookup+0x178/0x21c
> > [ 2.186042] crypto_alloc_tfm_node+0x58/0x114
> > [ 2.190396] crypto_alloc_shash+0x24/0x30
> > [ 2.194404] drbg_init_hash_kernel+0x28/0xdc
> > [ 2.198673] drbg_kcapi_seed+0x21c/0x420
> > [ 2.202593] crypto_rng_reset+0x84/0xb4
> > [ 2.206425] crypto_get_default_rng+0xa4/0xd8
> > [ 2.210779] ecc_gen_privkey+0x58/0xd0
> > [ 2.214526] ecdh_set_secret+0x90/0x198
> > [ 2.218360] tpm_buf_append_salt+0x164/0x2dc
>
> This looks like a misconfiguration. The kernel is trying to load the
> ecdh module, but it should have been selected as built in by this in
> drivers/char/tpm/Kconfig:
>
> config TCG_TPM2_HMAC
> bool "Use HMAC and encrypted transactions on the TPM bus"
> default y
> select CRYPTO_ECDH
> select CRYPTO_LIB_AESCFB
> select CRYPTO_LIB_SHA256
>
The module request is not for ECDH itself but for the DRBG it attempts
to use to generate the secret.
Given that CRYPTO_ECDH does not strictly require a DRBG in principle,
but does in this particular case, I think it makes sense to select
CRYPTO_DRBG here (or depend on it being builtin), rather than updating
the Kconfig rules for CRYPTO_ECDH itself.
On Fri May 17, 2024 at 10:20 AM EEST, Ard Biesheuvel wrote:
> On Fri, 17 May 2024 at 03:59, James Bottomley
> <[email protected]> wrote:
> >
> > On Thu, 2024-05-16 at 20:25 -0400, Nícolas F. R. A. Prado wrote:
> ...
> > > KernelCI has identified a new warning and I tracked it down to this
> > > commit. It
> > > was observed on the following platforms:
> > > * mt8183-kukui-jacuzzi-juniper-sku16
> > > * sc7180-trogdor-kingoftown
> > > (but probably affects all platforms that have a tpm driver with async
> > > probe)
> > >
> > > [ 2.175146] Call trace:
> > > [ 2.177587] __request_module+0x188/0x1f4
> > > [ 2.181596] crypto_alg_mod_lookup+0x178/0x21c
> > > [ 2.186042] crypto_alloc_tfm_node+0x58/0x114
> > > [ 2.190396] crypto_alloc_shash+0x24/0x30
> > > [ 2.194404] drbg_init_hash_kernel+0x28/0xdc
> > > [ 2.198673] drbg_kcapi_seed+0x21c/0x420
> > > [ 2.202593] crypto_rng_reset+0x84/0xb4
> > > [ 2.206425] crypto_get_default_rng+0xa4/0xd8
> > > [ 2.210779] ecc_gen_privkey+0x58/0xd0
> > > [ 2.214526] ecdh_set_secret+0x90/0x198
> > > [ 2.218360] tpm_buf_append_salt+0x164/0x2dc
> >
> > This looks like a misconfiguration. The kernel is trying to load the
> > ecdh module, but it should have been selected as built in by this in
> > drivers/char/tpm/Kconfig:
> >
> > config TCG_TPM2_HMAC
> > bool "Use HMAC and encrypted transactions on the TPM bus"
> > default y
> > select CRYPTO_ECDH
> > select CRYPTO_LIB_AESCFB
> > select CRYPTO_LIB_SHA256
> >
>
> The module request is not for ECDH itself but for the DRBG it attempts
> to use to generate the secret.
>
> Given that CRYPTO_ECDH does not strictly require a DRBG in principle,
> but does in this particular case, I think it makes sense to select
> CRYPTO_DRBG here (or depend on it being builtin), rather than updating
> the Kconfig rules for CRYPTO_ECDH itself.
I can spin a new PR if James can make a fix.
All previous 4 PR's for 6.10 were applied to Linus' tree so my queue
is empty. Need to have both fixes and stable-tags to save my bandwidth.
Maybe for transcript just two first lines denoting that it was
__request_module() will do. That and adding CONFIG_DRBG will take
it away should be enough for the full disclosure, right?
BR, Jarkko
On Fri, 2024-05-17 at 09:20 +0200, Ard Biesheuvel wrote:
> On Fri, 17 May 2024 at 03:59, James Bottomley
> <[email protected]> wrote:
> >
> > On Thu, 2024-05-16 at 20:25 -0400, Nícolas F. R. A. Prado wrote:
> ...
> > > KernelCI has identified a new warning and I tracked it down to
> > > this
> > > commit. It
> > > was observed on the following platforms:
> > > * mt8183-kukui-jacuzzi-juniper-sku16
> > > * sc7180-trogdor-kingoftown
> > > (but probably affects all platforms that have a tpm driver with
> > > async
> > > probe)
> > >
> > > [ 2.175146] Call trace:
> > > [ 2.177587] __request_module+0x188/0x1f4
> > > [ 2.181596] crypto_alg_mod_lookup+0x178/0x21c
> > > [ 2.186042] crypto_alloc_tfm_node+0x58/0x114
> > > [ 2.190396] crypto_alloc_shash+0x24/0x30
> > > [ 2.194404] drbg_init_hash_kernel+0x28/0xdc
> > > [ 2.198673] drbg_kcapi_seed+0x21c/0x420
> > > [ 2.202593] crypto_rng_reset+0x84/0xb4
> > > [ 2.206425] crypto_get_default_rng+0xa4/0xd8
> > > [ 2.210779] ecc_gen_privkey+0x58/0xd0
> > > [ 2.214526] ecdh_set_secret+0x90/0x198
> > > [ 2.218360] tpm_buf_append_salt+0x164/0x2dc
> >
> > This looks like a misconfiguration. The kernel is trying to load
> > the
> > ecdh module, but it should have been selected as built in by this
> > in
> > drivers/char/tpm/Kconfig:
> >
> > config TCG_TPM2_HMAC
> > bool "Use HMAC and encrypted transactions on the TPM bus"
> > default y
> > select CRYPTO_ECDH
> > select CRYPTO_LIB_AESCFB
> > select CRYPTO_LIB_SHA256
> >
>
> The module request is not for ECDH itself but for the DRBG it
> attempts
> to use to generate the secret.
>
> Given that CRYPTO_ECDH does not strictly require a DRBG in principle,
> but does in this particular case, I think it makes sense to select
> CRYPTO_DRBG here (or depend on it being builtin), rather than
> updating the Kconfig rules for CRYPTO_ECDH itself.
Thanks for the analysis. If I look at how CRYPTO_ECC does it, that
selects CRYPTO_RNG_DEFAULT which pulls in CRYPTO_DRBG, so the fix would
be the attached. Does that look right to you Ard? And does it work
Nicolas?
James
---8>8>8><8<8<8---
From 8c60ffd959eaa65627aca6596c35bb9cbfd9bee6 Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Fri, 17 May 2024 06:29:31 -0700
Subject: [PATCH] tpm: Fix sessions cryptography requirement for Random Numbers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The ECDH code in tpm2-sessions.c requires an initial random number
generator to generate the key pair. If the configuration doesn't have
CONFIG_RNG_DEFAULT, it will try to pull this in as a module (which is
impossible for the early kernel boot where the TPM starts). Fix this
by selecting the required RNG.
Reported-by: Nícolas F. R. A. Prado <[email protected]>
Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
Signed-off-by: James Bottomley <[email protected]>
---
drivers/char/tpm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 4f83ee7021d0..12065eddb922 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -31,6 +31,7 @@ config TCG_TPM2_HMAC
bool "Use HMAC and encrypted transactions on the TPM bus"
default y
select CRYPTO_ECDH
+ select CRYTPO_RNG_DEFAULT
select CRYPTO_LIB_AESCFB
select CRYPTO_LIB_SHA256
help
--
2.35.3
On Fri, 17 May 2024 at 15:35, James Bottomley
<[email protected]> wrote:
>
> On Fri, 2024-05-17 at 09:20 +0200, Ard Biesheuvel wrote:
> > On Fri, 17 May 2024 at 03:59, James Bottomley
> > <[email protected]> wrote:
> > >
> > > On Thu, 2024-05-16 at 20:25 -0400, Nícolas F. R. A. Prado wrote:
> > ...
> > > > KernelCI has identified a new warning and I tracked it down to
> > > > this
> > > > commit. It
> > > > was observed on the following platforms:
> > > > * mt8183-kukui-jacuzzi-juniper-sku16
> > > > * sc7180-trogdor-kingoftown
> > > > (but probably affects all platforms that have a tpm driver with
> > > > async
> > > > probe)
> > > >
> > > > [ 2.175146] Call trace:
> > > > [ 2.177587] __request_module+0x188/0x1f4
> > > > [ 2.181596] crypto_alg_mod_lookup+0x178/0x21c
> > > > [ 2.186042] crypto_alloc_tfm_node+0x58/0x114
> > > > [ 2.190396] crypto_alloc_shash+0x24/0x30
> > > > [ 2.194404] drbg_init_hash_kernel+0x28/0xdc
> > > > [ 2.198673] drbg_kcapi_seed+0x21c/0x420
> > > > [ 2.202593] crypto_rng_reset+0x84/0xb4
> > > > [ 2.206425] crypto_get_default_rng+0xa4/0xd8
> > > > [ 2.210779] ecc_gen_privkey+0x58/0xd0
> > > > [ 2.214526] ecdh_set_secret+0x90/0x198
> > > > [ 2.218360] tpm_buf_append_salt+0x164/0x2dc
> > >
> > > This looks like a misconfiguration. The kernel is trying to load
> > > the
> > > ecdh module, but it should have been selected as built in by this
> > > in
> > > drivers/char/tpm/Kconfig:
> > >
> > > config TCG_TPM2_HMAC
> > > bool "Use HMAC and encrypted transactions on the TPM bus"
> > > default y
> > > select CRYPTO_ECDH
> > > select CRYPTO_LIB_AESCFB
> > > select CRYPTO_LIB_SHA256
> > >
> >
> > The module request is not for ECDH itself but for the DRBG it
> > attempts
> > to use to generate the secret.
> >
> > Given that CRYPTO_ECDH does not strictly require a DRBG in principle,
> > but does in this particular case, I think it makes sense to select
> > CRYPTO_DRBG here (or depend on it being builtin), rather than
> > updating the Kconfig rules for CRYPTO_ECDH itself.
>
> Thanks for the analysis. If I look at how CRYPTO_ECC does it, that
> selects CRYPTO_RNG_DEFAULT which pulls in CRYPTO_DRBG, so the fix would
> be the attached. Does that look right to you Ard?
No it doesn't - it's CRYPTO_RNG_DEFAULT not CRYTPO_RNG_DEFAULT :-)
With that fixed,
Acked-by: Ard Biesheuvel <[email protected]>
> And does it work
> Nicolas?
>
> James
>
> ---8>8>8><8<8<8---
>
> From 8c60ffd959eaa65627aca6596c35bb9cbfd9bee6 Mon Sep 17 00:00:00 2001
> From: James Bottomley <[email protected]>
> Date: Fri, 17 May 2024 06:29:31 -0700
> Subject: [PATCH] tpm: Fix sessions cryptography requirement for Random Numbers
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The ECDH code in tpm2-sessions.c requires an initial random number
> generator to generate the key pair. If the configuration doesn't have
> CONFIG_RNG_DEFAULT, it will try to pull this in as a module (which is
> impossible for the early kernel boot where the TPM starts). Fix this
> by selecting the required RNG.
>
> Reported-by: Nícolas F. R. A. Prado <[email protected]>
> Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
> Signed-off-by: James Bottomley <[email protected]>
> ---
> drivers/char/tpm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 4f83ee7021d0..12065eddb922 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -31,6 +31,7 @@ config TCG_TPM2_HMAC
> bool "Use HMAC and encrypted transactions on the TPM bus"
> default y
> select CRYPTO_ECDH
> + select CRYTPO_RNG_DEFAULT
> select CRYPTO_LIB_AESCFB
> select CRYPTO_LIB_SHA256
> help
> --
> 2.35.3
>
>
On Fri, 2024-05-17 at 15:43 +0200, Ard Biesheuvel wrote:
> On Fri, 17 May 2024 at 15:35, James Bottomley
> <[email protected]> wrote:
[...]
> > Thanks for the analysis. If I look at how CRYPTO_ECC does it, that
> > selects CRYPTO_RNG_DEFAULT which pulls in CRYPTO_DRBG, so the fix
> > would be the attached. Does that look right to you Ard?
>
> No it doesn't - it's CRYPTO_RNG_DEFAULT not CRYTPO_RNG_DEFAULT :-)
>
> With that fixed,
>
> Acked-by: Ard Biesheuvel <[email protected]>
Erm, oops, sorry about that; so attached is the update.
James
---8>8>8><8<8<8---
From 2ac337a33e6416ef806e2c692b9239d193e8468f Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Fri, 17 May 2024 06:29:31 -0700
Subject: [PATCH] tpm: Fix sessions cryptography requirement for Random Numbers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The ECDH code in tpm2-sessions.c requires an initial random number
generator to generate the key pair. If the configuration doesn't have
CONFIG_RNG_DEFAULT, it will try to pull this in as a module (which is
impossible for the early kernel boot where the TPM starts). Fix this
by selecting the required RNG.
Reported-by: Nícolas F. R. A. Prado <[email protected]>
Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
Acked-by: Ard Biesheuvel <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
---
drivers/char/tpm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 4f83ee7021d0..ecdd3db4be2b 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -31,6 +31,7 @@ config TCG_TPM2_HMAC
bool "Use HMAC and encrypted transactions on the TPM bus"
default y
select CRYPTO_ECDH
+ select CRYPTO_RNG_DEFAULT
select CRYPTO_LIB_AESCFB
select CRYPTO_LIB_SHA256
help
--
2.35.3
On Fri, May 17, 2024 at 07:25:40AM -0700, James Bottomley wrote:
> On Fri, 2024-05-17 at 15:43 +0200, Ard Biesheuvel wrote:
> > On Fri, 17 May 2024 at 15:35, James Bottomley
> > <[email protected]> wrote:
> [...]
> > > Thanks for the analysis.? If I look at how CRYPTO_ECC does it, that
> > > selects CRYPTO_RNG_DEFAULT which pulls in CRYPTO_DRBG, so the fix
> > > would be the attached.? Does that look right to you Ard?
> >
> > No it doesn't - it's CRYPTO_RNG_DEFAULT not CRYTPO_RNG_DEFAULT :-)
> >
> > With that fixed,
> >
> > Acked-by: Ard Biesheuvel <[email protected]>
>
> Erm, oops, sorry about that; so attached is the update.
>
> James
>
> ---8>8>8><8<8<8---
>
> From 2ac337a33e6416ef806e2c692b9239d193e8468f Mon Sep 17 00:00:00 2001
> From: James Bottomley <[email protected]>
> Date: Fri, 17 May 2024 06:29:31 -0700
> Subject: [PATCH] tpm: Fix sessions cryptography requirement for Random Numbers
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The ECDH code in tpm2-sessions.c requires an initial random number
> generator to generate the key pair. If the configuration doesn't have
> CONFIG_RNG_DEFAULT, it will try to pull this in as a module (which is
> impossible for the early kernel boot where the TPM starts). Fix this
> by selecting the required RNG.
>
> Reported-by: N?colas F. R. A. Prado <[email protected]>
> Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
> Acked-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: James Bottomley <[email protected]>
> ---
> drivers/char/tpm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 4f83ee7021d0..ecdd3db4be2b 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -31,6 +31,7 @@ config TCG_TPM2_HMAC
> bool "Use HMAC and encrypted transactions on the TPM bus"
> default y
> select CRYPTO_ECDH
> + select CRYPTO_RNG_DEFAULT
> select CRYPTO_LIB_AESCFB
> select CRYPTO_LIB_SHA256
> help
> --
> 2.35.3
>
>
Hi James,
thanks for the patch. But I actually already had that config enabled builtin. I
also had ECDH and DRBG which have been suggested previously:
CONFIG_CRYPTO_RNG_DEFAULT=y
CONFIG_CRYPTO_DRBG_MENU=y
CONFIG_CRYPTO_DRBG_HMAC=y
# CONFIG_CRYPTO_DRBG_HASH is not set
# CONFIG_CRYPTO_DRBG_CTR is not set
CONFIG_CRYPTO_DRBG=y
CONFIG_CRYPTO_ECDH=y
I've pasted my full config here: http://0x0.st/XPN_.txt
Adding a debug print I see that the module that the code tries to load is
"crypto-hmac(sha512)". I would have expected to see
MODULE_ALIAS_CRYPTO("hmac(sha512)");
in crypto/drbg.c, but I don't see it anywhere in the tree. Maybe it is missing?
Thanks,
N?colas
On Fri May 17, 2024 at 7:22 PM EEST, Nícolas F. R. A. Prado wrote:
> On Fri, May 17, 2024 at 07:25:40AM -0700, James Bottomley wrote:
> > On Fri, 2024-05-17 at 15:43 +0200, Ard Biesheuvel wrote:
> > > On Fri, 17 May 2024 at 15:35, James Bottomley
> > > <[email protected]> wrote:
> > [...]
> > > > Thanks for the analysis. If I look at how CRYPTO_ECC does it, that
> > > > selects CRYPTO_RNG_DEFAULT which pulls in CRYPTO_DRBG, so the fix
> > > > would be the attached. Does that look right to you Ard?
> > >
> > > No it doesn't - it's CRYPTO_RNG_DEFAULT not CRYTPO_RNG_DEFAULT :-)
> > >
> > > With that fixed,
> > >
> > > Acked-by: Ard Biesheuvel <[email protected]>
> >
> > Erm, oops, sorry about that; so attached is the update.
> >
> > James
> >
> > ---8>8>8><8<8<8---
> >
> > From 2ac337a33e6416ef806e2c692b9239d193e8468f Mon Sep 17 00:00:00 2001
> > From: James Bottomley <[email protected]>
> > Date: Fri, 17 May 2024 06:29:31 -0700
> > Subject: [PATCH] tpm: Fix sessions cryptography requirement for Random Numbers
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > The ECDH code in tpm2-sessions.c requires an initial random number
> > generator to generate the key pair. If the configuration doesn't have
> > CONFIG_RNG_DEFAULT, it will try to pull this in as a module (which is
> > impossible for the early kernel boot where the TPM starts). Fix this
> > by selecting the required RNG.
> >
> > Reported-by: Nícolas F. R. A. Prado <[email protected]>
> > Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
> > Acked-by: Ard Biesheuvel <[email protected]>
> > Signed-off-by: James Bottomley <[email protected]>
> > ---
> > drivers/char/tpm/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> > index 4f83ee7021d0..ecdd3db4be2b 100644
> > --- a/drivers/char/tpm/Kconfig
> > +++ b/drivers/char/tpm/Kconfig
> > @@ -31,6 +31,7 @@ config TCG_TPM2_HMAC
> > bool "Use HMAC and encrypted transactions on the TPM bus"
> > default y
> > select CRYPTO_ECDH
> > + select CRYPTO_RNG_DEFAULT
> > select CRYPTO_LIB_AESCFB
> > select CRYPTO_LIB_SHA256
> > help
> > --
> > 2.35.3
> >
> >
>
> Hi James,
>
> thanks for the patch. But I actually already had that config enabled builtin. I
> also had ECDH and DRBG which have been suggested previously:
>
> CONFIG_CRYPTO_RNG_DEFAULT=y
>
> CONFIG_CRYPTO_DRBG_MENU=y
> CONFIG_CRYPTO_DRBG_HMAC=y
> # CONFIG_CRYPTO_DRBG_HASH is not set
> # CONFIG_CRYPTO_DRBG_CTR is not set
> CONFIG_CRYPTO_DRBG=y
>
> CONFIG_CRYPTO_ECDH=y
>
> I've pasted my full config here: http://0x0.st/XPN_.txt
>
> Adding a debug print I see that the module that the code tries to load is
> "crypto-hmac(sha512)". I would have expected to see
>
> MODULE_ALIAS_CRYPTO("hmac(sha512)");
>
> in crypto/drbg.c, but I don't see it anywhere in the tree. Maybe it is missing?
1. Bug fixes need to be submitted as described in
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
2. The patch is missing the transcript:
https://lore.kernel.org/linux-integrity/[email protected]/
There's nothing to review at this point.
>
> Thanks,
> Nícolas
BR, Jarkko
On Fri, May 17, 2024 at 07:48:48PM +0300, Jarkko Sakkinen wrote:
> On Fri May 17, 2024 at 7:22 PM EEST, N?colas F. R. A. Prado wrote:
> > On Fri, May 17, 2024 at 07:25:40AM -0700, James Bottomley wrote:
> > > On Fri, 2024-05-17 at 15:43 +0200, Ard Biesheuvel wrote:
> > > > On Fri, 17 May 2024 at 15:35, James Bottomley
> > > > <[email protected]> wrote:
> > > [...]
> > > > > Thanks for the analysis.? If I look at how CRYPTO_ECC does it, that
> > > > > selects CRYPTO_RNG_DEFAULT which pulls in CRYPTO_DRBG, so the fix
> > > > > would be the attached.? Does that look right to you Ard?
> > > >
> > > > No it doesn't - it's CRYPTO_RNG_DEFAULT not CRYTPO_RNG_DEFAULT :-)
> > > >
> > > > With that fixed,
> > > >
> > > > Acked-by: Ard Biesheuvel <[email protected]>
> > >
> > > Erm, oops, sorry about that; so attached is the update.
> > >
> > > James
> > >
> > > ---8>8>8><8<8<8---
> > >
> > > From 2ac337a33e6416ef806e2c692b9239d193e8468f Mon Sep 17 00:00:00 2001
> > > From: James Bottomley <[email protected]>
> > > Date: Fri, 17 May 2024 06:29:31 -0700
> > > Subject: [PATCH] tpm: Fix sessions cryptography requirement for Random Numbers
> > > MIME-Version: 1.0
> > > Content-Type: text/plain; charset=UTF-8
> > > Content-Transfer-Encoding: 8bit
> > >
> > > The ECDH code in tpm2-sessions.c requires an initial random number
> > > generator to generate the key pair. If the configuration doesn't have
> > > CONFIG_RNG_DEFAULT, it will try to pull this in as a module (which is
> > > impossible for the early kernel boot where the TPM starts). Fix this
> > > by selecting the required RNG.
> > >
> > > Reported-by: N?colas F. R. A. Prado <[email protected]>
> > > Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
> > > Acked-by: Ard Biesheuvel <[email protected]>
> > > Signed-off-by: James Bottomley <[email protected]>
> > > ---
> > > drivers/char/tpm/Kconfig | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> > > index 4f83ee7021d0..ecdd3db4be2b 100644
> > > --- a/drivers/char/tpm/Kconfig
> > > +++ b/drivers/char/tpm/Kconfig
> > > @@ -31,6 +31,7 @@ config TCG_TPM2_HMAC
> > > bool "Use HMAC and encrypted transactions on the TPM bus"
> > > default y
> > > select CRYPTO_ECDH
> > > + select CRYPTO_RNG_DEFAULT
> > > select CRYPTO_LIB_AESCFB
> > > select CRYPTO_LIB_SHA256
> > > help
> > > --
> > > 2.35.3
> > >
> > >
> >
> > Hi James,
> >
> > thanks for the patch. But I actually already had that config enabled builtin. I
> > also had ECDH and DRBG which have been suggested previously:
> >
> > CONFIG_CRYPTO_RNG_DEFAULT=y
> >
> > CONFIG_CRYPTO_DRBG_MENU=y
> > CONFIG_CRYPTO_DRBG_HMAC=y
> > # CONFIG_CRYPTO_DRBG_HASH is not set
> > # CONFIG_CRYPTO_DRBG_CTR is not set
> > CONFIG_CRYPTO_DRBG=y
> >
> > CONFIG_CRYPTO_ECDH=y
> >
> > I've pasted my full config here: http://0x0.st/XPN_.txt
> >
> > Adding a debug print I see that the module that the code tries to load is
> > "crypto-hmac(sha512)". I would have expected to see
> >
> > MODULE_ALIAS_CRYPTO("hmac(sha512)");
> >
> > in crypto/drbg.c, but I don't see it anywhere in the tree. Maybe it is missing?
>
This is "normal" behavior when the crypto API instantiates a template:
1. drbg.c asks for "hmac(sha512)"
2. The crypto API looks for a direct implementation of "hmac(sha512)".
This includes requesting a module with alias "crypto-hmac(sha512)".
3. If none is found, the "hmac" template is instantiated instead.
There are two possible fixes for the bug. Either fix ecc_gen_privkey() to just
use get_random_bytes() instead of the weird crypto API RNG, or make
drbg_init_hash_kernel() pass the CRYPTO_NOLOAD flag to crypto_alloc_shash().
Or if the TPM driver could be changed to not need to generate an ECC private key
at probe time, that would also avoid this problem.
- Eric
On Sat May 18, 2024 at 7:31 AM EEST, Eric Biggers wrote:
> This is "normal" behavior when the crypto API instantiates a template:
>
> 1. drbg.c asks for "hmac(sha512)"
>
> 2. The crypto API looks for a direct implementation of "hmac(sha512)".
> This includes requesting a module with alias "crypto-hmac(sha512)".
>
> 3. If none is found, the "hmac" template is instantiated instead.
>
> There are two possible fixes for the bug. Either fix ecc_gen_privkey() to just
> use get_random_bytes() instead of the weird crypto API RNG, or make
> drbg_init_hash_kernel() pass the CRYPTO_NOLOAD flag to crypto_alloc_shash().
>
> Or if the TPM driver could be changed to not need to generate an ECC private key
> at probe time, that would also avoid this problem.
Issues:
- IMA extends PCR's. This requires encrypted communications path.
- HWRNG uses auth session (see tpm2_get_radom()).
- TPM trusted keys
Null key is required before any other legit use in initialization.
Even something like
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -36,6 +36,8 @@ config TCG_TPM2_HMAC
bool "Use HMAC and encrypted transactions on the TPM bus"
default y
+ select CRYPTO_DRBG
select CRYPTO_ECDH
+ select CRYPTO_HMAC
+ select CRYPTO_SHA512
select CRYPTO_LIB_AESCFB
select CRYPTO_LIB_SHA256
help
would be more decent.
>
> - Eric
BR, Jarkko