On Fri, 17 May 2024 at 15:35, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, 2024-05-17 at 09:20 +0200, Ard Biesheuvel wrote:
> > On Fri, 17 May 2024 at 03:59, James Bottomley
> > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> 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 <ardb@xxxxxxxxxx>
> And does it work
> Nicolas?
>
> James
>
> ---8>8>8><8<8<8---
>
> From 8c60ffd959eaa65627aca6596c35bb9cbfd9bee6 Mon Sep 17 00:00:00 2001
> From: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> 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 <nfraprado@xxxxxxxxxxxxx>
> Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
> Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> ---
> 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
>
>
