diff mbox series

[v10,2/7] KEYS: trusted: allow use of kernel RNG for key material

Message ID 20220513145705.2080323-3-a.fatoum@pengutronix.de (mailing list archive)
State New
Headers show
Series KEYS: trusted: Introduce support for NXP CAAM-based trusted keys | expand

Commit Message

Ahmad Fatoum May 13, 2022, 2:57 p.m. UTC
The two existing trusted key sources don't make use of the kernel RNG,
but instead let the hardware doing the sealing/unsealing also
generate the random key material. However, both users and future
backends may want to place less trust into the quality of the trust
source's random number generator and instead reuse the kernel entropy
pool, which can be seeded from multiple entropy sources.

Make this possible by adding a new trusted.rng parameter,
that will force use of the kernel RNG. In its absence, it's up
to the trust source to decide, which random numbers to use,
maintaining the existing behavior.

Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
Acked-by: Sumit Garg <sumit.garg@linaro.org>
Acked-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Reviewed-by: David Gstir <david@sigma-star.at>
Reviewed-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Tested-by: Michael Walle <michael@walle.cc> # on ls1028a (non-E and E)
Tested-by: John Ernberg <john.ernberg@actia.se> # iMX8QXP
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v9 -> v10:
  - added Michael's and John's Tested-by
v8 -> v9:
  - No changes
v7 -> v8:
  - add Pankaj's Tested-by
v6 -> v7:
  - No changes
v5 -> v6:
  - Squash with follow-up patch enabling trust sources to use
    kernel RNG if they don't define their own .get_random
  - Collected Jarkko's Reviewed-by
v4 -> v5:
  - Changed trusted.kernel_rng bool option into a string trusted.rng option
    (Jarkko)
  - Typo fix in commit message (Jarkko)
v3 -> v4:
  - Collected Acked-by's, Reviewed-by's and Tested-by
v2 -> v3:
  - No change
v1 -> v2:
 - Allow users to force use of kernel RNG (Jarkko)

To: James Bottomley <jejb@linux.ibm.com>
To: Jarkko Sakkinen <jarkko@kernel.org>
To: Mimi Zohar <zohar@linux.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Pankaj Gupta <pankaj.gupta@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jan Luebbe <j.luebbe@pengutronix.de>
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: David Gstir <david@sigma-star.at>
Cc: Richard Weinberger <richard@nod.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Cc: Michael Walle <michael@walle.cc>
Cc: John Ernberg <john.ernberg@actia.se>
Cc: keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
---
 .../admin-guide/kernel-parameters.txt         | 10 ++++++
 .../security/keys/trusted-encrypted.rst       | 20 ++++++-----
 include/keys/trusted-type.h                   |  2 +-
 security/keys/trusted-keys/trusted_core.c     | 35 ++++++++++++++++++-
 4 files changed, 57 insertions(+), 10 deletions(-)

Comments

Mimi Zohar May 17, 2022, 3:52 p.m. UTC | #1
On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
>  static int __init init_trusted(void)
>  {
> +       int (*get_random)(unsigned char *key, size_t key_len);
>         int i, ret = 0;
>  
>         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
> @@ -322,6 +333,28 @@ static int __init init_trusted(void)
>                             strlen(trusted_key_sources[i].name)))
>                         continue;
>  
> +               /*
> +                * We always support trusted.rng="kernel" and "default" as
> +                * well as trusted.rng=$trusted.source if the trust source
> +                * defines its own get_random callback.
> +                */
 
While TEE trusted keys support was upstreamed, there was a lot of
discussion about using kernel RNG.  One of the concerns was lack of or
insuffiencent entropy during early boot on embedded devices.  This
concern needs to be clearly documented in both Documentation/admin-
guide/kernel-parameters.txt and Documentation/security/keys/trusted-
encrypted.rst.

thanks,

Mimi

> +               get_random = trusted_key_sources[i].ops->get_random;
> +               if (trusted_rng && strcmp(trusted_rng, "default")) {
> +                       if (!strcmp(trusted_rng, "kernel")) {
> +                               get_random = kernel_get_random;
> +                       } else if (strcmp(trusted_rng, trusted_key_sources[i].name) ||
> +                                  !get_random) {
> +                               pr_warn("Unsupported RNG. Supported: kernel");
> +                               if (get_random)
> +                                       pr_cont(", %s", trusted_key_sources[i].name);
> +                               pr_cont(", default\n");
> +                               return -EINVAL;
> +                       }
> +               }
> +
> +               if (!get_random)
> +                       get_random = kernel_get_random;
> +
>                 static_call_update(trusted_key_init,
>                                    trusted_key_sources[i].ops->init);
Ahmad Fatoum May 17, 2022, 4:25 p.m. UTC | #2
Hello Mimi,

[Cc'ing RNG maintainers in case they want to chime in]

On 17.05.22 17:52, Mimi Zohar wrote:
> On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
>>  static int __init init_trusted(void)
>>  {
>> +       int (*get_random)(unsigned char *key, size_t key_len);
>>         int i, ret = 0;
>>  
>>         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
>> @@ -322,6 +333,28 @@ static int __init init_trusted(void)
>>                             strlen(trusted_key_sources[i].name)))
>>                         continue;
>>  
>> +               /*
>> +                * We always support trusted.rng="kernel" and "default" as
>> +                * well as trusted.rng=$trusted.source if the trust source
>> +                * defines its own get_random callback.
>> +                */
>  
> While TEE trusted keys support was upstreamed, there was a lot of
> discussion about using kernel RNG.  One of the concerns was lack of or
> insuffiencent entropy during early boot on embedded devices.  This
> concern needs to be clearly documented in both Documentation/admin-
> guide/kernel-parameters.txt and Documentation/security/keys/trusted-
> encrypted.rst.

If a user decides to use kernel RNG for trusted keys, wait_for_random_bytes()
called first thing in the used get_random_bytes_wait() will (quoting
documentation) "wait for the input pool to be seeded and thus [is] guaranteed
to supply cryptographically secure random numbers."

Does this address your concerns about Kernel RNG use?

Thanks,
Ahmad

> 
> thanks,
> 
> Mimi
> 
>> +               get_random = trusted_key_sources[i].ops->get_random;
>> +               if (trusted_rng && strcmp(trusted_rng, "default")) {
>> +                       if (!strcmp(trusted_rng, "kernel")) {
>> +                               get_random = kernel_get_random;
>> +                       } else if (strcmp(trusted_rng, trusted_key_sources[i].name) ||
>> +                                  !get_random) {
>> +                               pr_warn("Unsupported RNG. Supported: kernel");
>> +                               if (get_random)
>> +                                       pr_cont(", %s", trusted_key_sources[i].name);
>> +                               pr_cont(", default\n");
>> +                               return -EINVAL;
>> +                       }
>> +               }
>> +
>> +               if (!get_random)
>> +                       get_random = kernel_get_random;
>> +
>>                 static_call_update(trusted_key_init,
>>                                    trusted_key_sources[i].ops->init);
> 
>
Jason A. Donenfeld May 17, 2022, 5:27 p.m. UTC | #3
On Fri, May 13, 2022 at 04:57:00PM +0200, Ahmad Fatoum wrote:
> +	trusted.rng=	[KEYS]
> +			Format: <string>
> +			The RNG used to generate key material for trusted keys.
> +			Can be one of:
> +			- "kernel"
> +			- the same value as trusted.source: "tpm" or "tee"
> +			- "default"
> +			If not specified, "default" is used. In this case,
> +			the RNG's choice is left to each individual trust source.
> +

As a general mechanism, I object to this. The kernel's RNG must be
trusted in the first place for key material. That's the whole point of
it.

However, it sounds like you're not proposing a general mechanism, but
just something particular to this "trusted keys" business. In that case,
this should be a module flag, and thus not documented here, but rather
some place namespaced to your trusted keys stuff. "trusted_keys.preferred_rng={whatever}"

Jason
Jason A. Donenfeld May 17, 2022, 5:38 p.m. UTC | #4
On Tue, May 17, 2022 at 11:52:55AM -0400, Mimi Zohar wrote:
> On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
> >  static int __init init_trusted(void)
> >  {
> > +       int (*get_random)(unsigned char *key, size_t key_len);
> >         int i, ret = 0;
> >  
> >         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
> > @@ -322,6 +333,28 @@ static int __init init_trusted(void)
> >                             strlen(trusted_key_sources[i].name)))
> >                         continue;
> >  
> > +               /*
> > +                * We always support trusted.rng="kernel" and "default" as
> > +                * well as trusted.rng=$trusted.source if the trust source
> > +                * defines its own get_random callback.
> > +                */
>  
> While TEE trusted keys support was upstreamed, there was a lot of
> discussion about using kernel RNG.  One of the concerns was lack of or
> insuffiencent entropy during early boot on embedded devices.  This
> concern needs to be clearly documented in both Documentation/admin-
> guide/kernel-parameters.txt and Documentation/security/keys/trusted-
> encrypted.rst.

Sounds like FUD. Use `get_random_bytes_wait()`, and you'll be fine.

Jason
Jason A. Donenfeld May 17, 2022, 5:40 p.m. UTC | #5
Hi Ahmad,

On Tue, May 17, 2022 at 06:25:08PM +0200, Ahmad Fatoum wrote:
> Hello Mimi,
> 
> [Cc'ing RNG maintainers in case they want to chime in]

Thanks for adding me to this thread.

> On 17.05.22 17:52, Mimi Zohar wrote:
> > On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
> >>  static int __init init_trusted(void)
> >>  {
> >> +       int (*get_random)(unsigned char *key, size_t key_len);
> >>         int i, ret = 0;
> >>  
> >>         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
> >> @@ -322,6 +333,28 @@ static int __init init_trusted(void)
> >>                             strlen(trusted_key_sources[i].name)))
> >>                         continue;
> >>  
> >> +               /*
> >> +                * We always support trusted.rng="kernel" and "default" as
> >> +                * well as trusted.rng=$trusted.source if the trust source
> >> +                * defines its own get_random callback.
> >> +                */
> >  
> > While TEE trusted keys support was upstreamed, there was a lot of
> > discussion about using kernel RNG.  One of the concerns was lack of or
> > insuffiencent entropy during early boot on embedded devices.  This
> > concern needs to be clearly documented in both Documentation/admin-
> > guide/kernel-parameters.txt and Documentation/security/keys/trusted-
> > encrypted.rst.
> 
> If a user decides to use kernel RNG for trusted keys, wait_for_random_bytes()
> called first thing in the used get_random_bytes_wait() will (quoting
> documentation) "wait for the input pool to be seeded and thus [is] guaranteed
> to supply cryptographically secure random numbers."
> 
> Does this address your concerns about Kernel RNG use?

Indeed if get_random_bytes_wait() or wait_for_random_bytes() is called,
then the RNG will just block until it's accumulated 256 bits of
estimated entropy. The RNG will also make use of whatever hwrng or
cpu rng capabilities are available, and mix those in to augment its own
output.

Jason
Ahmad Fatoum May 17, 2022, 5:52 p.m. UTC | #6
Hello Jason,

On 17.05.22 19:27, Jason A. Donenfeld wrote:
> On Fri, May 13, 2022 at 04:57:00PM +0200, Ahmad Fatoum wrote:
>> +	trusted.rng=	[KEYS]
>> +			Format: <string>
>> +			The RNG used to generate key material for trusted keys.
>> +			Can be one of:
>> +			- "kernel"
>> +			- the same value as trusted.source: "tpm" or "tee"
>> +			- "default"
>> +			If not specified, "default" is used. In this case,
>> +			the RNG's choice is left to each individual trust source.
>> +
> 
> As a general mechanism, I object to this. The kernel's RNG must be
> trusted in the first place for key material. That's the whole point of
> it.
> 
> However, it sounds like you're not proposing a general mechanism, but
> just something particular to this "trusted keys" business.

The two currently upstream trust sources (trusted key backends) each provide
their own RNG callback. This series adds a third backend that uses kernel RNG
and additionally provides users of the two existing trust sources the option
to benefit from kernel RNG as well.

> this should be a module flag, and thus not documented here, but rather
> some place namespaced to your trusted keys stuff. "trusted_keys.preferred_rng={whatever}"

The trusted keys module is trusted.ko and directly before my added lines is
the trusted.source=  documentation, so I think this is already at the correct place.

Thanks,
Ahmad

> 
> Jason
>
Jason A. Donenfeld May 17, 2022, 6 p.m. UTC | #7
Hi Ahmad,

On Tue, May 17, 2022 at 07:52:51PM +0200, Ahmad Fatoum wrote:
> Hello Jason,
> 
> On 17.05.22 19:27, Jason A. Donenfeld wrote:
> > On Fri, May 13, 2022 at 04:57:00PM +0200, Ahmad Fatoum wrote:
> >> +	trusted.rng=	[KEYS]
> >> +			Format: <string>
> >> +			The RNG used to generate key material for trusted keys.
> >> +			Can be one of:
> >> +			- "kernel"
> >> +			- the same value as trusted.source: "tpm" or "tee"
> >> +			- "default"
> >> +			If not specified, "default" is used. In this case,
> >> +			the RNG's choice is left to each individual trust source.
> >> +
> > 
> > As a general mechanism, I object to this. The kernel's RNG must be
> > trusted in the first place for key material. That's the whole point of
> > it.
> > 
> > However, it sounds like you're not proposing a general mechanism, but
> > just something particular to this "trusted keys" business.
> 
> The two currently upstream trust sources (trusted key backends) each provide
> their own RNG callback. This series adds a third backend that uses kernel RNG
> and additionally provides users of the two existing trust sources the option
> to benefit from kernel RNG as well.
> 
> > this should be a module flag, and thus not documented here, but rather
> > some place namespaced to your trusted keys stuff. "trusted_keys.preferred_rng={whatever}"
> 
> The trusted keys module is trusted.ko and directly before my added lines is
> the trusted.source=  documentation, so I think this is already at the correct place.

My apologies; I should have looked at the file itself instead of just
relying on git line context. You're right, the module itself is called
trusted.ko. This is confusing (shouldn't it be trusted_keys or
something?) , but what you propose sounds consistent from a namespacing
perspective with what's already there.

Jason
Jarkko Sakkinen May 17, 2022, 6:10 p.m. UTC | #8
On Tue, 2022-05-17 at 19:27 +0200, Jason A. Donenfeld wrote:
> On Fri, May 13, 2022 at 04:57:00PM +0200, Ahmad Fatoum wrote:
> > +       trusted.rng=    [KEYS]
> > +                       Format: <string>
> > +                       The RNG used to generate key material for trusted keys.
> > +                       Can be one of:
> > +                       - "kernel"
> > +                       - the same value as trusted.source: "tpm" or "tee"
> > +                       - "default"
> > +                       If not specified, "default" is used. In this case,
> > +                       the RNG's choice is left to each individual trust source.
> > +
> 
> As a general mechanism, I object to this. The kernel's RNG must be
> trusted in the first place for key material. That's the whole point of
> it.

I would relax this  a bit: kernel's RNG must be implicitly must be
trusted. If the parameter is used, you make an explicit choice that
you are aware of the trust.

If this was opt-out parameter, instead of opt-in, I would get your
argument.

> However, it sounds like you're not proposing a general mechanism, but
> just something particular to this "trusted keys" business. In that case,
> this should be a module flag, and thus not documented here, but rather
> some place namespaced to your trusted keys stuff. "trusted_keys.preferred_rng={whatever}"

However, I think this a good proposal. Let's make it a module parameter
instead.

> Jason

BR, Jarkko
Ahmad Fatoum May 17, 2022, 6:20 p.m. UTC | #9
Hello Jarkko,

On 17.05.22 20:10, Jarkko Sakkinen wrote:
> On Tue, 2022-05-17 at 19:27 +0200, Jason A. Donenfeld wrote:
>> On Fri, May 13, 2022 at 04:57:00PM +0200, Ahmad Fatoum wrote:
>>> +       trusted.rng=    [KEYS]
>>> +                       Format: <string>
>>> +                       The RNG used to generate key material for trusted keys.
>>> +                       Can be one of:
>>> +                       - "kernel"
>>> +                       - the same value as trusted.source: "tpm" or "tee"
>>> +                       - "default"
>>> +                       If not specified, "default" is used. In this case,
>>> +                       the RNG's choice is left to each individual trust source.
>>> +
>>
>> As a general mechanism, I object to this. The kernel's RNG must be
>> trusted in the first place for key material. That's the whole point of
>> it.
> 
> I would relax this  a bit: kernel's RNG must be implicitly must be
> trusted. If the parameter is used, you make an explicit choice that
> you are aware of the trust.
> 
> If this was opt-out parameter, instead of opt-in, I would get your
> argument.
> 
>> However, it sounds like you're not proposing a general mechanism, but
>> just something particular to this "trusted keys" business. In that case,
>> this should be a module flag, and thus not documented here, but rather
>> some place namespaced to your trusted keys stuff. "trusted_keys.preferred_rng={whatever}"
> 
> However, I think this a good proposal. Let's make it a module parameter
> instead.

It's already a module parameter.

> 
>> Jason
> 
> BR, Jarkko
Jason A. Donenfeld May 17, 2022, 6:20 p.m. UTC | #10
Hi Jarkko,


On Tue, May 17, 2022 at 09:10:57PM +0300, Jarkko Sakkinen wrote:
> On Tue, 2022-05-17 at 19:27 +0200, Jason A. Donenfeld wrote:
> > On Fri, May 13, 2022 at 04:57:00PM +0200, Ahmad Fatoum wrote:
> > > +       trusted.rng=    [KEYS]
> > > +                       Format: <string>
> > > +                       The RNG used to generate key material for trusted keys.
> > > +                       Can be one of:
> > > +                       - "kernel"
> > > +                       - the same value as trusted.source: "tpm" or "tee"
> > > +                       - "default"
> > > +                       If not specified, "default" is used. In this case,
> > > +                       the RNG's choice is left to each individual trust source.
> > > +
> > 
> > As a general mechanism, I object to this. The kernel's RNG must be
> > trusted in the first place for key material. That's the whole point of
> > it.
> 
> I would relax this  a bit: kernel's RNG must be implicitly must be

Sorry, I didn't mean to seem unrelaxed. What I meant was that as a
general mechanism, it doesn't make sense, but it isn't a general
mechanism, it's a particular one for trusted_keys, which led me to
question why it seemed to have such a general name like "trusted.rng".
Ahmad pointed out that the trusted prefix actually isn't general. It's
what trusted_keys compiles its module as. So just a misunderstanding. It
appears to be a module param after all. Sorry for the noise!

Jason
Mimi Zohar May 17, 2022, 6:21 p.m. UTC | #11
On Tue, 2022-05-17 at 19:38 +0200, Jason A. Donenfeld wrote:
> On Tue, May 17, 2022 at 11:52:55AM -0400, Mimi Zohar wrote:
> > On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
> > >  static int __init init_trusted(void)
> > >  {
> > > +       int (*get_random)(unsigned char *key, size_t key_len);
> > >         int i, ret = 0;
> > >  
> > >         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
> > > @@ -322,6 +333,28 @@ static int __init init_trusted(void)
> > >                             strlen(trusted_key_sources[i].name)))
> > >                         continue;
> > >  
> > > +               /*
> > > +                * We always support trusted.rng="kernel" and "default" as
> > > +                * well as trusted.rng=$trusted.source if the trust source
> > > +                * defines its own get_random callback.
> > > +                */
> >  
> > While TEE trusted keys support was upstreamed, there was a lot of
> > discussion about using kernel RNG.  One of the concerns was lack of or
> > insuffiencent entropy during early boot on embedded devices.  This
> > concern needs to be clearly documented in both Documentation/admin-
> > guide/kernel-parameters.txt and Documentation/security/keys/trusted-
> > encrypted.rst.
> 
> Sounds like FUD. Use `get_random_bytes_wait()`, and you'll be fine.

As per the original discussion, there's also certification requirements
[1].

Mimi

[1] https://lkml.org/lkml/2019/10/9/53
Ahmad Fatoum May 17, 2022, 6:27 p.m. UTC | #12
Hello Jason,

On 17.05.22 20:00, Jason A. Donenfeld wrote:
> On Tue, May 17, 2022 at 07:52:51PM +0200, Ahmad Fatoum wrote:
>> The trusted keys module is trusted.ko and directly before my added lines is
>> the trusted.source=  documentation, so I think this is already at the correct place.
> 
> My apologies; I should have looked at the file itself instead of just
> relying on git line context. You're right, the module itself is called
> trusted.ko. This is confusing (shouldn't it be trusted_keys or
> something?)

Encrypted keys are encrypted-keys.ko, so ye, trusted.ko is a bit unfortunate..

> , but what you propose sounds consistent from a namespacing
> perspective with what's already there.

Thanks!
Ahmad

> 
> Jason
>
Jason A. Donenfeld May 17, 2022, 6:30 p.m. UTC | #13
Hi Mimi,

On Tue, May 17, 2022 at 02:21:08PM -0400, Mimi Zohar wrote:
> On Tue, 2022-05-17 at 19:38 +0200, Jason A. Donenfeld wrote:
> > On Tue, May 17, 2022 at 11:52:55AM -0400, Mimi Zohar wrote:
> > > On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
> > > >  static int __init init_trusted(void)
> > > >  {
> > > > +       int (*get_random)(unsigned char *key, size_t key_len);
> > > >         int i, ret = 0;
> > > >  
> > > >         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
> > > > @@ -322,6 +333,28 @@ static int __init init_trusted(void)
> > > >                             strlen(trusted_key_sources[i].name)))
> > > >                         continue;
> > > >  
> > > > +               /*
> > > > +                * We always support trusted.rng="kernel" and "default" as
> > > > +                * well as trusted.rng=$trusted.source if the trust source
> > > > +                * defines its own get_random callback.
> > > > +                */
> > >  
> > > While TEE trusted keys support was upstreamed, there was a lot of
> > > discussion about using kernel RNG.  One of the concerns was lack of or
> > > insuffiencent entropy during early boot on embedded devices.  This
> > > concern needs to be clearly documented in both Documentation/admin-
> > > guide/kernel-parameters.txt and Documentation/security/keys/trusted-
> > > encrypted.rst.
> > 
> > Sounds like FUD. Use `get_random_bytes_wait()`, and you'll be fine.
> 
> As per the original discussion, there's also certification requirements
> [1].

As per countless conversations on this mailing list -- which I really
really really hope you will not attempt to drown me in again -- I'm not
too keen on the certification requirements. Let's just leave that
conversation there.

There *is* a cryptographic design reason why you might want certain keys
generated on a TPM rather than in the kernel though: so that the keys
can be marked as unexportable and never leave the hardware. In that case
-- I assume -- the kernel just operates on a handle to the key, rather
than possessing the key material itself. And this is probably a good
thing. (On the other hand, people who think the TPM might be backdoored
may prefer the kernel's open source RNG, which in theory is in a
position to aggregate entropy from many sources, so that one being
backdoored isn't a problem. So maybe that's the purpose of having this
switch?)

So to the extent that this driver (I haven't looked deeply at it) is
doing the thing where a TPM generates the key and just returns a
handle to it, that sounds good. But if actually you're implementing some
wrapper around a hardware rng, it'd be convenient if there was instead a
hw_random driver for this, so it can be one of the many sources that the
kernel rng aggregates.

Apologies in advance if I've missed the mark here; I'm not very familiar
with this thread or what it's driving at. If the simple question was
just "is get_random_bytes_wait() good to use?" the answer is just "yes"
and I can disappear and stop confusing things. :)

Jason
Mimi Zohar May 17, 2022, 7:49 p.m. UTC | #14
On Tue, 2022-05-17 at 20:30 +0200, Jason A. Donenfeld wrote:
> Hi Mimi,
> 
> On Tue, May 17, 2022 at 02:21:08PM -0400, Mimi Zohar wrote:
> > On Tue, 2022-05-17 at 19:38 +0200, Jason A. Donenfeld wrote:
> > > On Tue, May 17, 2022 at 11:52:55AM -0400, Mimi Zohar wrote:
> > > > On Fri, 2022-05-13 at 16:57 +0200, Ahmad Fatoum wrote:
> > > > >  static int __init init_trusted(void)
> > > > >  {
> > > > > +       int (*get_random)(unsigned char *key, size_t key_len);
> > > > >         int i, ret = 0;
> > > > >  
> > > > >         for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
> > > > > @@ -322,6 +333,28 @@ static int __init init_trusted(void)
> > > > >                             strlen(trusted_key_sources[i].name)))
> > > > >                         continue;
> > > > >  
> > > > > +               /*
> > > > > +                * We always support trusted.rng="kernel" and "default" as
> > > > > +                * well as trusted.rng=$trusted.source if the trust source
> > > > > +                * defines its own get_random callback.
> > > > > +                */
> > > >  
> > > > While TEE trusted keys support was upstreamed, there was a lot of
> > > > discussion about using kernel RNG.  One of the concerns was lack of or
> > > > insuffiencent entropy during early boot on embedded devices.  This
> > > > concern needs to be clearly documented in both Documentation/admin-
> > > > guide/kernel-parameters.txt and Documentation/security/keys/trusted-
> > > > encrypted.rst.
> > > 
> > > Sounds like FUD. Use `get_random_bytes_wait()`, and you'll be fine.
> > 
> > As per the original discussion, there's also certification requirements
> > [1].
> 
> As per countless conversations on this mailing list -- which I really
> really really hope you will not attempt to drown me in again -- I'm not
> too keen on the certification requirements. Let's just leave that
> conversation there.
> 
> There *is* a cryptographic design reason why you might want certain keys
> generated on a TPM rather than in the kernel though: so that the keys
> can be marked as unexportable and never leave the hardware. In that case
> -- I assume -- the kernel just operates on a handle to the key, rather
> than possessing the key material itself. And this is probably a good
> thing. (On the other hand, people who think the TPM might be backdoored
> may prefer the kernel's open source RNG, which in theory is in a
> position to aggregate entropy from many sources, so that one being
> backdoored isn't a problem. So maybe that's the purpose of having this
> switch?)
> 
> So to the extent that this driver (I haven't looked deeply at it) is
> doing the thing where a TPM generates the key and just returns a
> handle to it, that sounds good. But if actually you're implementing some
> wrapper around a hardware rng, it'd be convenient if there was instead a
> hw_random driver for this, so it can be one of the many sources that the
> kernel rng aggregates.
> 
> Apologies in advance if I've missed the mark here; I'm not very familiar
> with this thread or what it's driving at. If the simple question was
> just "is get_random_bytes_wait() good to use?" the answer is just "yes"
> and I can disappear and stop confusing things. :)

My apologies for your having been brought into this discussion without
having properly reviewed and summarized the previous thread.   As you
saw there is a long history.

Jarrko, Ahmad,  "Trusted" keys, by definition, are based on the TPM
RNG.  If CAAM trusted key support wants to use kernel RNG by default,
that's fine.  However defining and allowing a boot command line option
to use kernel RNG instead of the TPM RNG, needs to be configurable.

thanks,

Mimi
Ahmad Fatoum May 18, 2022, 4:31 a.m. UTC | #15
Hello Mimi,

On 17.05.22 21:49, Mimi Zohar wrote:
> On Tue, 2022-05-17 at 20:30 +0200, Jason A. Donenfeld wrote:
>> Hi Mimi,
>>
>> On Tue, May 17, 2022 at 02:21:08PM -0400, Mimi Zohar wrote:
>>> On Tue, 2022-05-17 at 19:38 +0200, Jason A. Donenfeld wrote:
>> Apologies in advance if I've missed the mark here; I'm not very familiar
>> with this thread or what it's driving at. If the simple question was
>> just "is get_random_bytes_wait() good to use?" the answer is just "yes"
>> and I can disappear and stop confusing things. :)
> 
> My apologies for your having been brought into this discussion without
> having properly reviewed and summarized the previous thread.   As you
> saw there is a long history.
> 
> Jarrko, Ahmad,  "Trusted" keys, by definition, are based on the TPM
> RNG.  If CAAM trusted key support wants to use kernel RNG by default,
> that's fine.  However defining and allowing a boot command line option
> to use kernel RNG instead of the TPM RNG, needs to be configurable.

The use of kernel RNG for TPM Trusted Keys is already opt-in. The default
is trusted.rng=default, which maintains existing behavior. Users who want
to use kernel RNG instead need to explicitly specify trusted.rng=kernel.

What more is needed?

Cheers,
Ahmad



> 
> thanks,
> 
> Mimi
> 
>
diff mbox series

Patch

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3f1cc5e317ed..4deed1908a75 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5963,6 +5963,16 @@ 
 			first trust source as a backend which is initialized
 			successfully during iteration.
 
+	trusted.rng=	[KEYS]
+			Format: <string>
+			The RNG used to generate key material for trusted keys.
+			Can be one of:
+			- "kernel"
+			- the same value as trusted.source: "tpm" or "tee"
+			- "default"
+			If not specified, "default" is used. In this case,
+			the RNG's choice is left to each individual trust source.
+
 	tsc=		Disable clocksource stability checks for TSC.
 			Format: <string>
 			[x86] reliable: mark tsc clocksource as reliable, this
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index f614dad7de12..2fe6fd1a2bbd 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -87,22 +87,26 @@  Key Generation
 Trusted Keys
 ------------
 
-New keys are created from random numbers generated in the trust source. They
-are encrypted/decrypted using a child key in the storage key hierarchy.
-Encryption and decryption of the child key must be protected by a strong
-access control policy within the trust source.
+New keys are created from random numbers. They are encrypted/decrypted using
+a child key in the storage key hierarchy. Encryption and decryption of the
+child key must be protected by a strong access control policy within the
+trust source. The random number generator in use differs according to the
+selected trust source:
 
-  *  TPM (hardware device) based RNG
+  *  TPM: hardware device based RNG
 
-     Strength of random numbers may vary from one device manufacturer to
-     another.
+     Keys are generated within the TPM. Strength of random numbers may vary
+     from one device manufacturer to another.
 
-  *  TEE (OP-TEE based on Arm TrustZone) based RNG
+  *  TEE: OP-TEE based on Arm TrustZone based RNG
 
      RNG is customizable as per platform needs. It can either be direct output
      from platform specific hardware RNG or a software based Fortuna CSPRNG
      which can be seeded via multiple entropy sources.
 
+Users may override this by specifying ``trusted.rng=kernel`` on the kernel
+command-line to override the used RNG with the kernel's random number pool.
+
 Encrypted Keys
 --------------
 
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index d89fa2579ac0..4eb64548a74f 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -64,7 +64,7 @@  struct trusted_key_ops {
 	/* Unseal a key. */
 	int (*unseal)(struct trusted_key_payload *p, char *datablob);
 
-	/* Get a randomized key. */
+	/* Optional: Get a randomized key. */
 	int (*get_random)(unsigned char *key, size_t key_len);
 
 	/* Exit key interface. */
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 7cdbd16aed30..9235fb7d0ec9 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -16,12 +16,17 @@ 
 #include <linux/key-type.h>
 #include <linux/module.h>
 #include <linux/parser.h>
+#include <linux/random.h>
 #include <linux/rcupdate.h>
 #include <linux/slab.h>
 #include <linux/static_call.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
 
+static char *trusted_rng = "default";
+module_param_named(rng, trusted_rng, charp, 0);
+MODULE_PARM_DESC(rng, "Select trusted key RNG");
+
 static char *trusted_key_source;
 module_param_named(source, trusted_key_source, charp, 0);
 MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
@@ -312,8 +317,14 @@  struct key_type key_type_trusted = {
 };
 EXPORT_SYMBOL_GPL(key_type_trusted);
 
+static int kernel_get_random(unsigned char *key, size_t key_len)
+{
+	return get_random_bytes_wait(key, key_len) ?: key_len;
+}
+
 static int __init init_trusted(void)
 {
+	int (*get_random)(unsigned char *key, size_t key_len);
 	int i, ret = 0;
 
 	for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
@@ -322,6 +333,28 @@  static int __init init_trusted(void)
 			    strlen(trusted_key_sources[i].name)))
 			continue;
 
+		/*
+		 * We always support trusted.rng="kernel" and "default" as
+		 * well as trusted.rng=$trusted.source if the trust source
+		 * defines its own get_random callback.
+		 */
+		get_random = trusted_key_sources[i].ops->get_random;
+		if (trusted_rng && strcmp(trusted_rng, "default")) {
+			if (!strcmp(trusted_rng, "kernel")) {
+				get_random = kernel_get_random;
+			} else if (strcmp(trusted_rng, trusted_key_sources[i].name) ||
+				   !get_random) {
+				pr_warn("Unsupported RNG. Supported: kernel");
+				if (get_random)
+					pr_cont(", %s", trusted_key_sources[i].name);
+				pr_cont(", default\n");
+				return -EINVAL;
+			}
+		}
+
+		if (!get_random)
+			get_random = kernel_get_random;
+
 		static_call_update(trusted_key_init,
 				   trusted_key_sources[i].ops->init);
 		static_call_update(trusted_key_seal,
@@ -329,7 +362,7 @@  static int __init init_trusted(void)
 		static_call_update(trusted_key_unseal,
 				   trusted_key_sources[i].ops->unseal);
 		static_call_update(trusted_key_get_random,
-				   trusted_key_sources[i].ops->get_random);
+				   get_random);
 		static_call_update(trusted_key_exit,
 				   trusted_key_sources[i].ops->exit);
 		migratable = trusted_key_sources[i].ops->migratable;