linux-kernel - Re: [PATCH v1] crypto: Make the DRBG compliant with NIST SP800-90A rev1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7acf0d4a63f7c94d8355101dd03cbfeb58c05d17.camel@chronox.de>
Date:   Wed, 23 Jun 2021 21:10:51 +0200
From:   Stephan Mueller <smueller@...onox.de>
To:     Mickaël Salaün <mic@...ikod.net>,
        James Morris <jamorris@...ux.microsoft.com>
Cc:     David Miller <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        John Haxby <john.haxby@...cle.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Simo Sorce <simo@...hat.com>, linux-crypto@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Mickaël Salaün <mic@...ux.microsoft.com>,
        hpa@...or.com, tytso@....edu
Subject: Re: [PATCH v1] crypto: Make the DRBG compliant with NIST SP800-90A
 rev1

Am Mittwoch, dem 23.06.2021 um 20:04 +0200 schrieb Mickaël Salaün:
> 
> On 23/06/2021 19:27, Stephan Müller wrote:
> > Am Mittwoch, 23. Juni 2021, 19:00:29 CEST schrieb James Morris:
> > 
> > Hi James,
> > 
> > > On Wed, 23 Jun 2021, Stephan Mueller wrote:
> > > > > These changes replace the use of the Linux RNG with the Jitter RNG,
> > > > > which is NIST SP800-90B compliant, to get a proper entropy input and a
> > > > > nonce as defined by FIPS.
> > > > 
> > > > Can you please help me understand what is missing in the current code
> > > > which
> > > > seemingly already has achieved this goal?
> > > 
> > > The advice we have is that if an attacker knows the internal state of the
> > > CPU, then the output of the Jitter RNG can be predicted.
> > 
> > Thank you for the hint. And I think such goal is worthwhile (albeit I have
> > to 
> > admit that if an attacker is able to gain the internal state of a CPU, I
> > would 
> > assume we have more pressing problems that a bit of entropy).
> > 
> > Anyways, the current code does:
> > 
> > - in regular mode: seed the DRBG with 384 bits of data from get_random_bytes
> > 
> > - in FIPS mode: seed the DRBG with 384 bits of data from get_random_bytes 
> > concatenated with 384 bits from the Jitter RNG
> > 
> > 
> > If I understand the suggested changes right, I would see the following
> > changes 
> > in the patch:
> > 
> > - in the regular case: 640 bits from get_random_bytes
> 
> Why 640 bits?

 		if (!reseed)
 			entropylen = ((entropylen + 1) / 2) * 3;

-> Entropylen is 384 in case of a security strength of 256 bits.

Your code does the following if the Jitter RNG is not allocated (i.e. in non-
fips mode):

ret = drbg_get_random_bytes(drbg, entropy, entropylen + strength);

so: entropylen + strength = 384 + 256, no?

> 
> > 
> > - in FIPS mode: 256 bits of data from get_random_bytes concatenated with 384
> > bits from the Jitter RNG
> 
> In both cases there are 256 bits for the entropy input and 128 bits for
> the nonce.

I see in the code path with the Jitter RNG:

ret = crypto_rng_get_bytes(drbg->jent, entropy,
 						   entropylen);

--> 384 bits from the Jitter RNG

ret = drbg_get_random_bytes(drbg, entropy + entropylen,
+						    strength);

--> 256 bits from get_random_bytes

What am I missing here?


>  If Jitter RNG is not available, then urandom is used instead,
> which means that the system is not FIPS compliant.

Agreed, the existing does does exactly the same with the exception that it
pulls 384 bits from get_random_bytes instead of 640 in non-FIPS mode (i.e.
when the Jitter RNG is not allocated).

In FIPS mode, the current code draws 384 bits from get_random_bytes and
separately 384 bits from the Jitter RNG. So, each data block from either
entropy source could completely satisfy the SP800-90A requirement.

> 
> This follows the SP800-90Ar1, section 8.6.7: [a nonce shall be] "A value
> with at least (security_strength/2) bits of entropy".

Agreed, but what is your code doing different than the existing code?
> 
> > 
> > So, I am not fully sure what the benefit of the difference is: in FIPS mode 
> > (where the Jitter RNG is used), the amount of data pulled from 
> > get_random_bytes seems to be now reduced.
> 
> We can increase the amount of data pulled from get_random_bytes (how to
> decide the amount?), but as we understand the document, this should be
> part of the personalization string and additional input, not the nonce.

There is no need to have a personalization string or additional input. Note,
those two values are intended to be provided by a caller or some other
environment.

If you want to stuff more seed into the DRBG, you simply enlarge the seed
buffer and pull more from the entropy sources. I have no objections doing
that. All I am trying to point out is that the 90A standard does not require
more entropy than 256 bits during initialization plus 128 bits of nonce == 384
bits of data. But the DRBGs all allow providing more data as seed.

> I guess it may not change much according to the implementation, as for
> the order of random and entropy concatenation, but these changes align
> with the specifications and it should help FIPS certifications.

Are you saying the order of data from the entropy sources matters in the
entropy buffer? I have not seen that in the standard, but if you say this is
the goal, then allow me to understand which order you want to see?

The current code contains the order of:

<384 bits get_random_bytes> || <384 bits Jitter RNG>

Thanks
Stephan
> 
> > 
> > Maybe I miss a point here, but I currently fail to understand why the
> > changes 
> > should be an improvement compared to the current case.
> > 
> > Ciao
> > Stephan
> > 
> >