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

From: "Mickaël Salaün" <mic@digikod.net>
To: "Stephan Müller" <smueller@chronox.de>,
	"James Morris" <jamorris@linux.microsoft.com>
Cc: "David Miller" <davem@davemloft.net>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"John Haxby" <john.haxby@oracle.com>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"Simo Sorce" <simo@redhat.com>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Mickaël Salaün" <mic@linux.microsoft.com>,
	hpa@zytor.com, tytso@mit.edu
Subject: Re: [PATCH v1] crypto: Make the DRBG compliant with NIST SP800-90A rev1
Date: Wed, 23 Jun 2021 20:04:00 +0200	[thread overview]
Message-ID: <9ca2fdb4-8cee-3667-c90a-358255fb8f54@digikod.net> (raw)
In-Reply-To: <8811360.37IJKxs2K1@positron.chronox.de>

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?

> 
> - 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. If Jitter RNG is not available, then urandom is used instead,
which means that the system is not FIPS compliant.

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

> 
> 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.
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.

> 
> 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
> 
> 