Subject: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()


> From: Jarkko Sakkinen <[email protected]>
> Sent: Tuesday, October 8, 2019 7:54 PM
> To: Ken Goldman <[email protected]>
> Cc: Safford, David (GE Global Research, US) <[email protected]>; Mimi
> Zohar <[email protected]>; [email protected];
> [email protected]; open list:ASYMMETRIC KEYS
> <[email protected]>; open list:CRYPTO API <linux-
> [email protected]>; open list <[email protected]>
> Subject: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()
>
> On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote:
> > > The TPM library specification states that the TPM must comply with
> > > NIST
> > > SP800-90 A.
> > >
> > > https://trustedcomputinggroup.org/membership/certification/tpm-certi
> > > fied-products/
> > >
> > > shows that the TPMs get third party certification, Common Criteria EAL 4+.
> > >
> > > While it's theoretically possible that an attacker could compromise
> > > both the TPM vendors and the evaluation agencies, we do have EAL 4+
> > > assurance against both 1 and 2.
> >
> > Certifications do not equal to trust.
>
> And for trusted keys the least trust solution is to do generation with the kernel
> assets and sealing with TPM. With TEE the least trust solution is equivalent.
>
> Are you proposing that the kernel random number generation should be
> removed? That would be my conclusion of this discussion if I would agree any
> of this (I don't).
>
> /Jarkko

No one is suggesting that.

You are suggesting changing the documented behavior of trusted keys, and
that would cause problems for some of our use cases. While certification
may not in your mind be equal to trust, it is equal to compliance with
mandatory regulations.

Perhaps rather than arguing past each other, we should look into
providing users the ability to choose, as an argument to keyctl?

dave


2019-10-16 15:36:04

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()

On Wed, 2019-10-16 at 14:00 +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 14, 2019 at 12:29:57PM -0700, James Bottomley wrote:
> > The job of the in-kernel rng is simply to produce a mixed entropy
> > pool from which we can draw random numbers. The idea is that quite
> > a few attackers have identified the rng as being a weak point in
> > the security architecture of the kernel, so if we mix entropy from
> > all the sources we have, you have to compromise most of them to
> > gain some predictive power over the rng sequence.
>
> The documentation says that krng is suitable for key generation.
> Should the documentation changed to state that it is unsuitable?

How do you get that from the argument above? The krng is about the
best we have in terms of unpredictable key generation, so of course it
is suitable ... provided you give the entropy enough time to have
sufficient entropy. It's also not foolproof ... Bernstein did a
speculation about how you could compromise all our input sources for
entropy. However the more sources we have the more difficult the
compromise becomes.

> > The point is not how certified the TPM RNG is, the point is that
> > it's a single source and if we rely on it solely for some
> > applications, like trusted keys, then it gives the attackers a
> > single known point to go after. This may be impossible for script
> > kiddies, but it won't be for nation states ... are you going to
> > exclusively trust the random number you got from your chinese
> > certified TPM?
>
> I'd suggest approach where TPM RNG result is xored with krng result.

reversible ciphers are generally frowned upon in random number
generation, that's why the krng uses chacha20. In general I think we
shouldn't try to code our own mixing and instead should get the krng to
do it for us using whatever the algorithm du jour that the crypto guys
have blessed is. That's why I proposed adding the TPM output to the
krng as entropy input and then taking the output of the krng.

James

> > Remember also that the attack doesn't have to be to the TPM only,
> > it could be the pathway by which we get the random number, which
> > involves components outside of the TPM certification.
>
> Yeah, I do get this.
>
> /Jarkko
>

2019-10-17 12:52:50

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()

On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote:
> > reversible ciphers are generally frowned upon in random number
> > generation, that's why the krng uses chacha20. In general I think
> > we shouldn't try to code our own mixing and instead should get the
> > krng to do it for us using whatever the algorithm du jour that the
> > crypto guys have blessed is. That's why I proposed adding the TPM
> > output to the krng as entropy input and then taking the output of
> > the krng.
>
> It is already registered as hwrng. What else?

It only contributes entropy once at start of OS.

> Was the issue that it is only used as seed when the rng is init'd
> first? I haven't at this point gone to the internals of krng.

Basically it was similar to your xor patch except I got the kernel rng
to do the mixing, so it would use the chacha20 cipher at the moment
until they decide that's unsafe and change it to something else:

https://lore.kernel.org/linux-crypto/[email protected]/

It uses add_hwgenerator_randomness() to do the mixing. It also has an
unmixed source so that read of the TPM hwrng device works as expected.

James





2019-10-18 22:12:08

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()

On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote:
> On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote:
> > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote:
> > > reversible ciphers are generally frowned upon in random number
> > > generation, that's why the krng uses chacha20. In general I think
> > > we shouldn't try to code our own mixing and instead should get the
> > > krng to do it for us using whatever the algorithm du jour that the
> > > crypto guys have blessed is. That's why I proposed adding the TPM
> > > output to the krng as entropy input and then taking the output of
> > > the krng.
> >
> > It is already registered as hwrng. What else?
>
> It only contributes entropy once at start of OS.

Ok.

> > Was the issue that it is only used as seed when the rng is init'd
> > first? I haven't at this point gone to the internals of krng.
>
> Basically it was similar to your xor patch except I got the kernel rng
> to do the mixing, so it would use the chacha20 cipher at the moment
> until they decide that's unsafe and change it to something else:
>
> https://lore.kernel.org/linux-crypto/[email protected]/
>
> It uses add_hwgenerator_randomness() to do the mixing. It also has an
> unmixed source so that read of the TPM hwrng device works as expected.

Thinking that could this potentially racy? I.e. between the calls
something else could eat the entropy added?

/Jarkko

2019-10-21 11:40:21

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()

On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote:
> > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote:
> > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote:
> > > > reversible ciphers are generally frowned upon in random number
> > > > generation, that's why the krng uses chacha20. In general I think
> > > > we shouldn't try to code our own mixing and instead should get the
> > > > krng to do it for us using whatever the algorithm du jour that the
> > > > crypto guys have blessed is. That's why I proposed adding the TPM
> > > > output to the krng as entropy input and then taking the output of
> > > > the krng.
> > >
> > > It is already registered as hwrng. What else?
> >
> > It only contributes entropy once at start of OS.
>
> Ok.
>
> > > Was the issue that it is only used as seed when the rng is init'd
> > > first? I haven't at this point gone to the internals of krng.
> >
> > Basically it was similar to your xor patch except I got the kernel rng
> > to do the mixing, so it would use the chacha20 cipher at the moment
> > until they decide that's unsafe and change it to something else:
> >
> > https://lore.kernel.org/linux-crypto/[email protected]/
> >
> > It uses add_hwgenerator_randomness() to do the mixing. It also has an
> > unmixed source so that read of the TPM hwrng device works as expected.
>
> Thinking that could this potentially racy? I.e. between the calls
> something else could eat the entropy added?

Also, what is wrong just taking one value from krng and mixing
it with a value from TPM RNG where needed? That would be non-racy
too.

/Jarkko

2019-10-29 08:44:05

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()

On Mon, Oct 21, 2019 at 02:39:39PM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote:
> > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote:
> > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote:
> > > > > reversible ciphers are generally frowned upon in random number
> > > > > generation, that's why the krng uses chacha20. In general I think
> > > > > we shouldn't try to code our own mixing and instead should get the
> > > > > krng to do it for us using whatever the algorithm du jour that the
> > > > > crypto guys have blessed is. That's why I proposed adding the TPM
> > > > > output to the krng as entropy input and then taking the output of
> > > > > the krng.
> > > >
> > > > It is already registered as hwrng. What else?
> > >
> > > It only contributes entropy once at start of OS.
> >
> > Ok.
> >
> > > > Was the issue that it is only used as seed when the rng is init'd
> > > > first? I haven't at this point gone to the internals of krng.
> > >
> > > Basically it was similar to your xor patch except I got the kernel rng
> > > to do the mixing, so it would use the chacha20 cipher at the moment
> > > until they decide that's unsafe and change it to something else:
> > >
> > > https://lore.kernel.org/linux-crypto/[email protected]/
> > >
> > > It uses add_hwgenerator_randomness() to do the mixing. It also has an
> > > unmixed source so that read of the TPM hwrng device works as expected.
> >
> > Thinking that could this potentially racy? I.e. between the calls
> > something else could eat the entropy added?
>
> Also, what is wrong just taking one value from krng and mixing
> it with a value from TPM RNG where needed? That would be non-racy
> too.

I guess we can move forward with this?

/Jarkko

2019-10-31 22:47:51

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()

On Tue, Oct 29, 2019 at 07:58:16AM -0700, James Bottomley wrote:
> On Tue, 2019-10-29 at 10:42 +0200, Jarkko Sakkinen wrote:
> > On Mon, Oct 21, 2019 at 02:39:39PM +0300, Jarkko Sakkinen wrote:
> > > On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote:
> > > > On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote:
> > > > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote:
> > > > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley
> > > > > > wrote:
> > > > > > > reversible ciphers are generally frowned upon in random
> > > > > > > number
> > > > > > > generation, that's why the krng uses chacha20. In general
> > > > > > > I think
> > > > > > > we shouldn't try to code our own mixing and instead should
> > > > > > > get the
> > > > > > > krng to do it for us using whatever the algorithm du jour
> > > > > > > that the
> > > > > > > crypto guys have blessed is. That's why I proposed adding
> > > > > > > the TPM
> > > > > > > output to the krng as entropy input and then taking the
> > > > > > > output of
> > > > > > > the krng.
> > > > > >
> > > > > > It is already registered as hwrng. What else?
> > > > >
> > > > > It only contributes entropy once at start of OS.
> > > >
> > > > Ok.
> > > >
> > > > > > Was the issue that it is only used as seed when the rng is
> > > > > > init'd
> > > > > > first? I haven't at this point gone to the internals of krng.
> > > > >
> > > > > Basically it was similar to your xor patch except I got the
> > > > > kernel rng
> > > > > to do the mixing, so it would use the chacha20 cipher at the
> > > > > moment
> > > > > until they decide that's unsafe and change it to something
> > > > > else:
> > > > >
> > > > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@H
> > > > > ansenPartnership.com/
> > > > >
> > > > > It uses add_hwgenerator_randomness() to do the mixing. It also
> > > > > has an
> > > > > unmixed source so that read of the TPM hwrng device works as
> > > > > expected.
> > > >
> > > > Thinking that could this potentially racy? I.e. between the calls
> > > > something else could eat the entropy added?
> > >
> > > Also, what is wrong just taking one value from krng and mixing
> > > it with a value from TPM RNG where needed? That would be non-racy
> > > too.
> >
> > I guess we can move forward with this?
>
> Sure I suppose; can we can figure out how to get the mixing function du
> jour exposed?

Maybe it is best to reflect the whole issue in the context of the
Sumit's 2nd patch set, which adds ARM TEE support in order to move
forward.

/Jarkko