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

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

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Starting from November 2020, the first revision of NIST SP800-90A (June
2015) is required for FIPS 140-2.  One of the changes brought by this
first revision is that nonces used for seeding (instantiation) and
re-seeding must come from entropy sources compliant with NIST SP800-90B
(cf. NIST SP800-90A rev1, section 8.6.7).  However, this seeding is
currently done with the Linux RNG (i.e. in-kernel /dev/urandom) that
uses ChaCha20, a non-approved algorithm.
Cf. https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-90Ar1.pdf

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.

However, only using the Jitter RNG may not provide adequate security as
it could be possible for an attacker to know the state of the CPU and
predict this RNG output.  To avoid this threat, we are making this both
FIPS compliant and secure thanks to the use of Linux RNG as a random
source (but not entropy per se) for the personalization string
(instantiation) and the additional input (re-seeding).  These extra
inputs have a length equal to the DRBG strength.  The original
user-supplied personalization string and additional input are still used
but potentially truncated to fit with the 2**35 limit (cf. NIST
SP800-90A rev1, table 2 and 3).

This new DRBG uses the same random and entropy sources as the current
version but in a way that makes is compliant with FIPS 140-2.

Cc: David S. Miller <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Morris <jamorris@linux.microsoft.com>
Cc: John Haxby <john.haxby@oracle.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Simo Sorce <simo@redhat.com>
Cc: Stephan Müller <smueller@chronox.de>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210623120751.3033390-1-mic@digikod.net
---

Do you prefer to truncate the user-supplied personalization string and
the additional input, or to return an error if they are greater than
2**27 (instead of 2**35)?

Another solution to avoid truncating the personalization string and the
additional input would be to hash them with SHA-512 and concatenate the
resulting fixed-size buffers.
---
 crypto/drbg.c         | 77 ++++++++++++++++++++++++++++++++-----------
 include/crypto/drbg.h |  2 +-
 2 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 1b4587e0ddad..b817a831815e 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1119,9 +1119,10 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 		     bool reseed)
 {
 	int ret;
-	unsigned char entropy[((32 + 16) * 2)];
-	unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
-	struct drbg_string data1;
+	unsigned char entropy[((32 * 2) + 16)];
+	const unsigned int strength = drbg_sec_strength(drbg->core->flags);
+	unsigned int entropylen = strength;
+	struct drbg_string data1, data2;
 	LIST_HEAD(seedlist);
 
 	/* 9.1 / 9.2 / 9.3.1 step 3 */
@@ -1147,21 +1148,32 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 		BUG_ON(!entropylen);
 		if (!reseed)
 			entropylen = ((entropylen + 1) / 2) * 3;
-		BUG_ON((entropylen * 2) > sizeof(entropy));
-
-		/* Get seed from in-kernel /dev/urandom */
-		ret = drbg_get_random_bytes(drbg, entropy, entropylen);
-		if (ret)
-			goto out;
+		/*
+		 * Check that a minimal automatic personalization string
+		 * (instantiation) or additional input (re-seeding) of strength
+		 * length fits in.
+		 */
+		BUG_ON((entropylen + strength) > sizeof(entropy));
 
 		if (!drbg->jent) {
-			drbg_string_fill(&data1, entropy, entropylen);
-			pr_devel("DRBG: (re)seeding with %u bytes of entropy\n",
-				 entropylen);
+			/*
+			 * Get entropy, nonce, personalization string or
+			 * additional input from in-kernel /dev/urandom
+			 */
+			ret = drbg_get_random_bytes(drbg, entropy, entropylen + strength);
+			if (ret)
+				goto out;
+
+			drbg_string_fill(&data1, entropy, entropylen + strength);
+			pr_devel("DRBG: (re)seeding with %u bytes of random\n",
+				 entropylen + strength);
 		} else {
-			/* Get seed from Jitter RNG */
-			ret = crypto_rng_get_bytes(drbg->jent,
-						   entropy + entropylen,
+			/*
+			 * Get entropy (strength length), concatenated with a
+			 * nonce (half strength length) when instantiating,
+			 * both from the SP800-90B compliant Jitter RNG.
+			 */
+			ret = crypto_rng_get_bytes(drbg->jent, entropy,
 						   entropylen);
 			if (ret) {
 				pr_devel("DRBG: jent failed with %d\n", ret);
@@ -1184,9 +1196,25 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 					goto out;
 			}
 
-			drbg_string_fill(&data1, entropy, entropylen * 2);
-			pr_devel("DRBG: (re)seeding with %u bytes of entropy\n",
-				 entropylen * 2);
+			/*
+			 * To improve security while still be compliant with
+			 * SP800-90A rev1, automatically append a minimal
+			 * personalization string (instantiation) or additional
+			 * input (re-seeding) of strength length from in-kernel
+			 * /dev/urandom (random source).  This may then replace
+			 * a (small) part of the supplied pers according to
+			 * drbg_max_addtl().
+			 */
+			ret = drbg_get_random_bytes(drbg, entropy + entropylen,
+						    strength);
+			if (ret)
+				goto out;
+
+			drbg_string_fill(&data1, entropy, entropylen + strength);
+
+			pr_devel("DRBG: (re)seeding with %u bytes of entropy "
+				 "and %u bytes of random\n", entropylen,
+				 strength);
 		}
 	}
 	list_add_tail(&data1.list, &seedlist);
@@ -1197,7 +1225,16 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 	 * contents whether it is appropriate
 	 */
 	if (pers && pers->buf && 0 < pers->len) {
-		list_add_tail(&pers->list, &seedlist);
+		const size_t available = drbg_max_addtl(drbg) - pers->len;
+
+		data2 = *pers;
+		/*
+		 * Make sure that the drbg_max_addtl() limit is still respected
+		 * according to the automatically appended random values.
+		 */
+		if (available < strength)
+			data2.len -= strength - available;
+		list_add_tail(&data2.list, &seedlist);
 		pr_devel("DRBG: using personalization string\n");
 	}
 
@@ -1209,7 +1246,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 	ret = __drbg_seed(drbg, &seedlist, reseed);
 
 out:
-	memzero_explicit(entropy, entropylen * 2);
+	memzero_explicit(entropy, sizeof(entropy));
 
 	return ret;
 }
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index c4165126937e..7fcff8d2289e 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -168,7 +168,7 @@ static inline size_t drbg_max_request_bytes(struct drbg_state *drbg)
 
 static inline size_t drbg_max_addtl(struct drbg_state *drbg)
 {
-	/* SP800-90A requires 2**35 bytes additional info str / pers str */
+	/* SP800-90A requires 2**35 bits of additional info str / pers str */
 #if (__BITS_PER_LONG == 32)
 	/*
 	 * SP800-90A allows smaller maximum numbers to be returned -- we

base-commit: 13311e74253fe64329390df80bed3f07314ddd61
-- 
2.32.0

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

[Stephan Mueller]

Am Mittwoch, dem 23.06.2021 um 14:07 +0200 schrieb Mickaël Salaün:
> From: Mickaël Salaün <mic@linux.microsoft.com>
> 
> Starting from November 2020, the first revision of NIST SP800-90A (June
> 2015) is required for FIPS 140-2.

Starting from November 7, 2020, SP800-90B is mandated. SP800-90A was always
required since 2015 which sunset the ANSI X9.31 DRNG.


>   One of the changes brought by this
> first revision is that nonces used for seeding (instantiation) and
> re-seeding must come from entropy sources compliant with NIST SP800-90B
> (cf. NIST SP800-90A rev1, section 8.6.7).

Nonces do not need to be derived from SP800-90B entropy sources. The
construction methods 2 through 4 clearly allow a nonce to be derived from a
deterministic source.

>   However, this seeding is
> currently done with the Linux RNG (i.e. in-kernel /dev/urandom) that
> uses ChaCha20, a non-approved algorithm.
> Cf.   
> https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-90Ar1.pdf
> 

I do not think this statement is correct.

With the patches that went into the DRBG in kernel version 5.8, the DRBG
seeding is performed as follows when the kernel is booted in FIPS mode
(fips=1):

- get a buffer of 384 bits from get_random_bytes

- get a buffer of 384 bits from the Jitter RNG

Both buffers are concatenated and used to seed the DRBG.

So, in total, the DRBG receives a buffer of 768 bits.

Assume for your FIPS work that /dev/urandom cannot be claimed to have entropy,
you can only look at the Jitter RNG. This entropy source alone provides 384
bits which *may* be interpreted as follows:

- 256 bits forming the entropy string as mandated by SP800-90A

- 128 bits forming the nonce as mandated by SP800-90A following 8.6.7 bullet 1
with the deviation that it uses a 90B entropy source instead of an approved
random bit generator (a deviation that is allowed according to 8.6.7).

You also may interpret the 384 bits as follows:

- 384 bits as mandated by the current draft from January this year of SP800-
90C mandating that the initial seed is s+128 bits


> 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?
> 
> However, only using the Jitter RNG may not provide adequate security as
> it could be possible for an attacker to know the state of the CPU and
> predict this RNG output.  To avoid this threat, we are making this both
> FIPS compliant and secure thanks to the use of Linux RNG as a random
> source (but not entropy per se) for the personalization string

Why do you need a personalization string beyond the 384 bits coming from
get_random_bytes as mentioned above?

> (instantiation) and the additional input (re-seeding).

Why do you need a personalization string beyond the 384 bits coming from
get_random_bytes as mentioned above?

But maybe this is only a terminology issue where the existing 384 bits drawn
from get_random_bytes are called personalization string / additional info
string.

>   These extra
> inputs have a length equal to the DRBG strength.  The original
> user-supplied personalization string and additional input are still used
> but potentially truncated to fit with the 2**35 limit (cf. NIST
> SP800-90A rev1, table 2 and 3).


> 
> This new DRBG uses the same random and entropy sources as the current
> version but in a way that makes is compliant with FIPS 140-2.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Morris <jamorris@linux.microsoft.com>
> Cc: John Haxby <john.haxby@oracle.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Simo Sorce <simo@redhat.com>
> Cc: Stephan Müller <smueller@chronox.de>
> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
> Link: https://lore.kernel.org/r/20210623120751.3033390-1-mic@digikod.net
> ---
> 
> Do you prefer to truncate the user-supplied personalization string and
> the additional input, or to return an error if they are greater than
> 2**27 (instead of 2**35)?
> 
> Another solution to avoid truncating the personalization string and the
> additional input would be to hash them with SHA-512 and concatenate the
> resulting fixed-size buffers.
> ---
>  crypto/drbg.c         | 77 ++++++++++++++++++++++++++++++++-----------
>  include/crypto/drbg.h |  2 +-
>  2 files changed, 58 insertions(+), 21 deletions(-)
> 
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 1b4587e0ddad..b817a831815e 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -1119,9 +1119,10 @@ static int drbg_seed(struct drbg_state *drbg, struct
> drbg_string *pers,
>                      bool reseed)
>  {
>         int ret;
> -       unsigned char entropy[((32 + 16) * 2)];
> -       unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
> -       struct drbg_string data1;
> +       unsigned char entropy[((32 * 2) + 16)];
> +       const unsigned int strength = drbg_sec_strength(drbg->core->flags);
> +       unsigned int entropylen = strength;
> +       struct drbg_string data1, data2;
>         LIST_HEAD(seedlist);
>  
>         /* 9.1 / 9.2 / 9.3.1 step 3 */
> @@ -1147,21 +1148,32 @@ static int drbg_seed(struct drbg_state *drbg, struct
> drbg_string *pers,
>                 BUG_ON(!entropylen);
>                 if (!reseed)
>                         entropylen = ((entropylen + 1) / 2) * 3;
> -               BUG_ON((entropylen * 2) > sizeof(entropy));
> -
> -               /* Get seed from in-kernel /dev/urandom */
> -               ret = drbg_get_random_bytes(drbg, entropy, entropylen);
> -               if (ret)
> -                       goto out;
> +               /*
> +                * Check that a minimal automatic personalization string
> +                * (instantiation) or additional input (re-seeding) of
> strength
> +                * length fits in.
> +                */
> +               BUG_ON((entropylen + strength) > sizeof(entropy));
>  
>                 if (!drbg->jent) {
> -                       drbg_string_fill(&data1, entropy, entropylen);
> -                       pr_devel("DRBG: (re)seeding with %u bytes of
> entropy\n",
> -                                entropylen);
> +                       /*
> +                        * Get entropy, nonce, personalization string or
> +                        * additional input from in-kernel /dev/urandom
> +                        */

If we are in this branch, we will never be using an SP800-90B entropy source
and thus I am not sure changes are warranted.

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



> +                       if (ret)
> +                               goto out;
> +
> +                       drbg_string_fill(&data1, entropy, entropylen +
> strength);
> +                       pr_devel("DRBG: (re)seeding with %u bytes of
> random\n",
> +                                entropylen + strength);
>                 } else {
> -                       /* Get seed from Jitter RNG */
> -                       ret = crypto_rng_get_bytes(drbg->jent,
> -                                                  entropy + entropylen,
> +                       /*
> +                        * Get entropy (strength length), concatenated with
> a
> +                        * nonce (half strength length) when instantiating,
> +                        * both from the SP800-90B compliant Jitter RNG.
> +                        */
> +                       ret = crypto_rng_get_bytes(drbg->jent, entropy,
>                                                    entropylen);

So, here you would get 384 bits from the Jitter RNG.

>                         if (ret) {
>                                 pr_devel("DRBG: jent failed with %d\n",
> ret);
> @@ -1184,9 +1196,25 @@ static int drbg_seed(struct drbg_state *drbg, struct
> drbg_string *pers,
>                                         goto out;
>                         }
>  
> -                       drbg_string_fill(&data1, entropy, entropylen * 2);
> -                       pr_devel("DRBG: (re)seeding with %u bytes of
> entropy\n",
> -                                entropylen * 2);
> +                       /*
> +                        * To improve security while still be compliant with
> +                        * SP800-90A rev1, automatically append a minimal
> +                        * personalization string (instantiation) or
> additional
> +                        * input (re-seeding) of strength length from in-
> kernel
> +                        * /dev/urandom (random source).  This may then
> replace
> +                        * a (small) part of the supplied pers according to
> +                        * drbg_max_addtl().
> +                        */
> +                       ret = drbg_get_random_bytes(drbg, entropy +
> entropylen,
> +                                                   strength);

Here you would get 256 bits from get_random_bytes.

So, compared to the original code, you now just draw 256 bits of data instead
of 384 bits of data.

With these code changes, I fail to see what benefits these changes bring
compared to the existing code.

> +                       if (ret)
> +                               goto out;
> +
> +                       drbg_string_fill(&data1, entropy, entropylen +
> strength);
> +
> +                       pr_devel("DRBG: (re)seeding with %u bytes of entropy
> "
> +                                "and %u bytes of random\n", entropylen,
> +                                strength);
>                 }
>         }
>         list_add_tail(&data1.list, &seedlist);
> @@ -1197,7 +1225,16 @@ static int drbg_seed(struct drbg_state *drbg, struct
> drbg_string *pers,
>          * contents whether it is appropriate
>          */
>         if (pers && pers->buf && 0 < pers->len) {
> -               list_add_tail(&pers->list, &seedlist);
> +               const size_t available = drbg_max_addtl(drbg) - pers->len;

I am uneasy about subtractions without sanity checks and I am not sure what is
the purpose of this work here. Truncating the personalization string here is
wrong IMHO. If there is a need that the personalization string should be
checked for size, the code receiving the personalization string to begin with
should make the check, i.e. drbg_kcapi_seed and drbg_kcapi_random. But a
truncation here is not right as this changes the data provided by the caller.


Thanks
Stephan

> +
> +               data2 = *pers;
> +               /*
> +                * Make sure that the drbg_max_addtl() limit is still
> respected
> +                * according to the automatically appended random values.
> +                */
> +               if (available < strength)
> +                       data2.len -= strength - available;
> +               list_add_tail(&data2.list, &seedlist);
>                 pr_devel("DRBG: using personalization string\n");
>         }
>  
> @@ -1209,7 +1246,7 @@ static int drbg_seed(struct drbg_state *drbg, struct
> drbg_string *pers,
>         ret = __drbg_seed(drbg, &seedlist, reseed);
>  
>  out:
> -       memzero_explicit(entropy, entropylen * 2);
> +       memzero_explicit(entropy, sizeof(entropy));
>  
>         return ret;
>  }
> diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
> index c4165126937e..7fcff8d2289e 100644
> --- a/include/crypto/drbg.h
> +++ b/include/crypto/drbg.h
> @@ -168,7 +168,7 @@ static inline size_t drbg_max_request_bytes(struct
> drbg_state *drbg)
>  
>  static inline size_t drbg_max_addtl(struct drbg_state *drbg)
>  {
> -       /* SP800-90A requires 2**35 bytes additional info str / pers str */
> +       /* SP800-90A requires 2**35 bits of additional info str / pers str
> */
>  #if (__BITS_PER_LONG == 32)
>         /*
>          * SP800-90A allows smaller maximum numbers to be returned -- we
> 
> base-commit: 13311e74253fe64329390df80bed3f07314ddd61

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

[James Morris]

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.



-- 
James Morris
<jamorris@linux.microsoft.com>

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

[Stephan Müller]

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

- in FIPS mode: 256 bits of data from get_random_bytes concatenated with 384 
bits from the Jitter RNG

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.

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

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

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

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

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

[Stephan Mueller]

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

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

[H. Peter Anvin]

This one really does keep coming back like yesterday's herring, doesn't it...

On June 23, 2021 10:00:29 AM PDT, James Morris <jamorris@linux.microsoft.com> wrote:
>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.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

[Mickaël Salaün]

On 23/06/2021 21:10, Stephan Mueller wrote:
> 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?

Correct (for entropy + nonce + pers), I thought you were referring to
just the entropy + nonce part. This change is not needed for FIPS of
course but I changed it too to use the same algorithm and lengths as
when Jitter RNG is available.

Do you prefer to keep the current lengths (384 bits) when there is no
Jitter RNG? It seems to me that this change makes sense and could only
strengthen this case though.

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

OK, it is just a misunderstanding of what is where. To simplify the code
(with a fixed-length entropy array) I used the "entropy" array to store
the entropy + nonce + the automatically generated personalization string
or additional input. The user-supplied personalization string or
additional input is stored (unchanged) in the pers drbg_string. I
(briefly) explained this in the comments.

Another difference brought by this change is that the Jitter RNG data
(i.e. entropy source) is used at the beginning (of the entropy array)
and the urandom data (i.e. random source) at the end. This strictly
follows the algorithm described in SP800-90Ar1 sections 8.6.1, 8.6.2,
10.1.1.2 and 10.1.1.3 .

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

What is the rational of using 384 (strength * 1.5) bits to draw from
get_random_bytes?

We noticed that (as explained above) the order of these sources doesn't
follows SP800-90Ar1.
It is not clear which part and source is used for the entropy, nonce and
(maybe) personalization string.
If the random source is indeed the personalization string or the
additional input, then the pers length may not fit with the maximum
lengths specified in SP800-90Ar1 table 2 and 3.

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

It just fits with strength/2 for the length of the nonce.

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

Right, but the intent here is to strictly follow SP800-90Ar1 (hence the
use of Jitter RNG for entropy and nonce) but to still use the urandom
source, which seems to only fit in the personalization string according
to SP800-90Ar1.

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

Agreed, the user-supplied personalization string can be used to add more
data. We also want to harden the default use (i.e. without user-provided
personalization string) by automatically using non-entropy source
(urandom) though.

> 
>> 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>
   ^                              ^
   personalization string?        entropy+nonce

As described in SP800-90Ar1 section 10.1.1.2:
seed_material = entropy_input || nonce || personalization_string

As described in SP800-90Ar1 section 10.1.1.3:
seed_material = 0x01 || V || entropy_input || additional_input

And SP800-90Ar1 section 5 defining "X || Y" as "Concatenation of two
strings X and Y. X and Y are either both bitstrings, or both byte strings".

According to that, we would like at least:
<384 bits Jitter RNG> || <256 bits get_random_bytes>
and also enforcing the maximum length for the whole personalization
string and the additional input.

Thanks,
 Mickaël

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

[Stephan Mueller]

Am Donnerstag, dem 24.06.2021 um 12:13 +0200 schrieb Mickaël Salaün:
> 
> On 23/06/2021 21:10, Stephan Mueller wrote:
> > 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?
> 
> Correct (for entropy + nonce + pers), I thought you were referring to
> just the entropy + nonce part. This change is not needed for FIPS of
> course but I changed it too to use the same algorithm and lengths as
> when Jitter RNG is available.
> 
> Do you prefer to keep the current lengths (384 bits) when there is no
> Jitter RNG? It seems to me that this change makes sense and could only
> strengthen this case though.
> 
> > 
> > > 
> > > > 
> > > > - 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?
> 
> OK, it is just a misunderstanding of what is where. To simplify the code
> (with a fixed-length entropy array) I used the "entropy" array to store
> the entropy + nonce + the automatically generated personalization string
> or additional input. The user-supplied personalization string or
> additional input is stored (unchanged) in the pers drbg_string. I
> (briefly) explained this in the comments.
> 
> Another difference brought by this change is that the Jitter RNG data
> (i.e. entropy source) is used at the beginning (of the entropy array)
> and the urandom data (i.e. random source) at the end. This strictly
> follows the algorithm described in SP800-90Ar1 sections 8.6.1, 8.6.2,
> 10.1.1.2 and 10.1.1.3 .
> 
> > 
> > 
> > >  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.
> 
> What is the rational of using 384 (strength * 1.5) bits to draw from
> get_random_bytes?
> 
> We noticed that (as explained above) the order of these sources doesn't
> follows SP800-90Ar1.
> It is not clear which part and source is used for the entropy, nonce and
> (maybe) personalization string.
> If the random source is indeed the personalization string or the
> additional input, then the pers length may not fit with the maximum
> lengths specified in SP800-90Ar1 table 2 and 3.
> 
> > 
> > > 
> > > 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?
> 
> It just fits with strength/2 for the length of the nonce.
> 
> > > 
> > > > 
> > > > 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.
> 
> Right, but the intent here is to strictly follow SP800-90Ar1 (hence the
> use of Jitter RNG for entropy and nonce) but to still use the urandom
> source, which seems to only fit in the personalization string according
> to SP800-90Ar1.
> 
> > 
> > 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.
> 
> Agreed, the user-supplied personalization string can be used to add more
> data. We also want to harden the default use (i.e. without user-provided
> personalization string) by automatically using non-entropy source
> (urandom) though.
> 
> > 
> > > 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>
>    ^                              ^
>    personalization string?        entropy+nonce
> 
> As described in SP800-90Ar1 section 10.1.1.2:
> seed_material = entropy_input || nonce || personalization_string
> 
> As described in SP800-90Ar1 section 10.1.1.3:
> seed_material = 0x01 || V || entropy_input || additional_input
> 
> And SP800-90Ar1 section 5 defining "X || Y" as "Concatenation of two
> strings X and Y. X and Y are either both bitstrings, or both byte strings".
> 
> According to that, we would like at least:
> <384 bits Jitter RNG> || <256 bits get_random_bytes>
> and also enforcing the maximum length for the whole personalization
> string and the additional input.

And I think here we found the misconception.

- SP800-90A section 8.7.1 clearly marks that a personalization string is
optional. If it is not provided, the personalization string is treated as a
zero-bit string in the formulas you mentioned above. This is also consistent
with the CAVP testing of DRBGs conducted by NIST which allows testing of the
DRBG without a personalization string. See [1] as an example (search for DRBG
and click on it to see the tested options - there you see the testing with
zero personalization string).

- SP800-90A section 9.1 specifies that the entropy_input is: "The maximum
length of the entropy_input is implementation dependent, but shall be less
than or equal to the specified maximum length for the selected DRBG
mechanism". For all DRBGs except the CTR DRBG without derivation function the
maximum length is 2^35 as defined in table 2. As we do not have a CTR DRBG
without derivation function, we can ignore that special case.

With these considerations, the current DRBG code

- does not mandate a personalization string or even create one by itself but
allows the caller to specify one

- applies an entropy_input len of 512 bits during initial seeding

- applies a nonce of 128 bits during initial seeding

entropy_input == <384 bits get_random_bytes> || <256 bits Jitter RNG>

nonce = 128 bits Jitter RNG


You asked why 384 bits from get_random_bytes. Well, the answer is exactly to
cover your initial concern also raised by James Morris: some folks may not
trust the Jitter RNG. Yet we need it here as it only provides 90B compliance.
But folks who dislike it can treat its data providing no entropy and assume
get_random_bytes provides the entropy.

As both, the get_random_bytes and Jitter RNG data is concatenated, we do not
destroy entropy by this operation. This implies we cover different view points
at the same time.

[1]
https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/details?validation=33056

Ciao
Stephan

> 
> Thanks,
>  Mickaël

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

[Mickaël Salaün]

On 24/06/2021 13:50, Stephan Mueller wrote:
> Am Donnerstag, dem 24.06.2021 um 12:13 +0200 schrieb Mickaël Salaün:
>>
>> On 23/06/2021 21:10, Stephan Mueller wrote:
>>> 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?
>>
>> Correct (for entropy + nonce + pers), I thought you were referring to
>> just the entropy + nonce part. This change is not needed for FIPS of
>> course but I changed it too to use the same algorithm and lengths as
>> when Jitter RNG is available.
>>
>> Do you prefer to keep the current lengths (384 bits) when there is no
>> Jitter RNG? It seems to me that this change makes sense and could only
>> strengthen this case though.
>>
>>>
>>>>
>>>>>
>>>>> - 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?
>>
>> OK, it is just a misunderstanding of what is where. To simplify the code
>> (with a fixed-length entropy array) I used the "entropy" array to store
>> the entropy + nonce + the automatically generated personalization string
>> or additional input. The user-supplied personalization string or
>> additional input is stored (unchanged) in the pers drbg_string. I
>> (briefly) explained this in the comments.
>>
>> Another difference brought by this change is that the Jitter RNG data
>> (i.e. entropy source) is used at the beginning (of the entropy array)
>> and the urandom data (i.e. random source) at the end. This strictly
>> follows the algorithm described in SP800-90Ar1 sections 8.6.1, 8.6.2,
>> 10.1.1.2 and 10.1.1.3 .
>>
>>>
>>>
>>>>  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.
>>
>> What is the rational of using 384 (strength * 1.5) bits to draw from
>> get_random_bytes?
>>
>> We noticed that (as explained above) the order of these sources doesn't
>> follows SP800-90Ar1.
>> It is not clear which part and source is used for the entropy, nonce and
>> (maybe) personalization string.
>> If the random source is indeed the personalization string or the
>> additional input, then the pers length may not fit with the maximum
>> lengths specified in SP800-90Ar1 table 2 and 3.
>>
>>>
>>>>
>>>> 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?
>>
>> It just fits with strength/2 for the length of the nonce.
>>
>>>>
>>>>>
>>>>> 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.
>>
>> Right, but the intent here is to strictly follow SP800-90Ar1 (hence the
>> use of Jitter RNG for entropy and nonce) but to still use the urandom
>> source, which seems to only fit in the personalization string according
>> to SP800-90Ar1.
>>
>>>
>>> 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.
>>
>> Agreed, the user-supplied personalization string can be used to add more
>> data. We also want to harden the default use (i.e. without user-provided
>> personalization string) by automatically using non-entropy source
>> (urandom) though.
>>
>>>
>>>> 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>
>>    ^                              ^
>>    personalization string?        entropy+nonce
>>
>> As described in SP800-90Ar1 section 10.1.1.2:
>> seed_material = entropy_input || nonce || personalization_string
>>
>> As described in SP800-90Ar1 section 10.1.1.3:
>> seed_material = 0x01 || V || entropy_input || additional_input
>>
>> And SP800-90Ar1 section 5 defining "X || Y" as "Concatenation of two
>> strings X and Y. X and Y are either both bitstrings, or both byte strings".
>>
>> According to that, we would like at least:
>> <384 bits Jitter RNG> || <256 bits get_random_bytes>
>> and also enforcing the maximum length for the whole personalization
>> string and the additional input.
> 
> And I think here we found the misconception.
> 
> - SP800-90A section 8.7.1 clearly marks that a personalization string is
> optional. If it is not provided, the personalization string is treated as a
> zero-bit string in the formulas you mentioned above. This is also consistent
> with the CAVP testing of DRBGs conducted by NIST which allows testing of the
> DRBG without a personalization string. See [1] as an example (search for DRBG
> and click on it to see the tested options - there you see the testing with
> zero personalization string).
> 
> - SP800-90A section 9.1 specifies that the entropy_input is: "The maximum
> length of the entropy_input is implementation dependent, but shall be less
> than or equal to the specified maximum length for the selected DRBG
> mechanism". For all DRBGs except the CTR DRBG without derivation function the
> maximum length is 2^35 as defined in table 2. As we do not have a CTR DRBG
> without derivation function, we can ignore that special case.
> 
> With these considerations, the current DRBG code
> 
> - does not mandate a personalization string or even create one by itself but
> allows the caller to specify one

OK

> 
> - applies an entropy_input len of 512 bits during initial seeding
> 
> - applies a nonce of 128 bits during initial seeding
> 
> entropy_input == <384 bits get_random_bytes> || <256 bits Jitter RNG>

We think that using "<384 bits get_random_bytes> || " makes this DRBG
non-compliant with SP800-90A rev1 because get_random_bytes doesn't use a
vetted conditioning component (but ChaCha20 instead):

SP800-90Ar1, section 8.6.5 says "A DRBG mechanism requires an approved
randomness source during instantiation and reseeding [...]. An approved
randomness source is an entropy source that conforms to [SP 800-90B], or
an RBG that conforms to [SP 800-90C] − either a DRBG or an NRBG".
The FIPS 140-2 Implementation Guidance
(https://csrc.nist.gov/csrc/media/projects/cryptographic-module-validation-program/documents/fips140-2/fips1402ig.pdf),
section 7.19 says "As of November 7, 2020, all newly submitted modules
requiring an entropy evaluation must demonstrate compliance to SP 800-90B".
In resolution 3 it says "all processing of the raw data output from the
noise sources that happens before it is ultimately output from the
entropy source *shall* occur within a conditioning chain". Data from
get_random_bytes may come from multiple noise sources, but they are
hashed with ChaCha20.
In resolution 6 it says "a vetted conditioning component may optionally
take a finite amount of supplemental data [...] in addition to the data
from the primary noise source", which would be OK if get_random_bytes
used a vetted algorithm, but it is not the case for now.


> 
> nonce = 128 bits Jitter RNG

OK

> 
> 
> You asked why 384 bits from get_random_bytes. Well, the answer is exactly to
> cover your initial concern also raised by James Morris: some folks may not
> trust the Jitter RNG. Yet we need it here as it only provides 90B compliance.
> But folks who dislike it can treat its data providing no entropy and assume
> get_random_bytes provides the entropy.

Agreed, this is the reason we want to keep using urandom, but not as the
entropy input.

> 
> As both, the get_random_bytes and Jitter RNG data is concatenated, we do not
> destroy entropy by this operation. This implies we cover different view points
> at the same time.

The entropy is not destroyed but doesn't seems compliant with the
specification (and IG).

> 
> [1]
> https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/details?validation=33056
> 
> Ciao
> Stephan
> 
>>
>> Thanks,
>>  Mickaël
> 
> 

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

[Stephan Müller]

Am Freitag, 25. Juni 2021, 13:09:26 CEST schrieb Mickaël Salaün:

Hi Mickaël,

[...]
> 
> > - applies an entropy_input len of 512 bits during initial seeding
> > 
> > - applies a nonce of 128 bits during initial seeding
> > 
> > entropy_input == <384 bits get_random_bytes> || <256 bits Jitter RNG>
> 
> We think that using "<384 bits get_random_bytes> || " makes this DRBG
> non-compliant with SP800-90A rev1 because get_random_bytes doesn't use a
> vetted conditioning component (but ChaCha20 instead):
> 
> SP800-90Ar1, section 8.6.5 says "A DRBG mechanism requires an approved
> randomness source during instantiation and reseeding [...]. An approved
> randomness source is an entropy source that conforms to [SP 800-90B], or
> an RBG that conforms to [SP 800-90C] − either a DRBG or an NRBG".
> The FIPS 140-2 Implementation Guidance
> (https://csrc.nist.gov/csrc/media/projects/cryptographic-module-validation-p
> rogram/documents/fips140-2/fips1402ig.pdf), section 7.19 says "As of
> November 7, 2020, all newly submitted modules requiring an entropy
> evaluation must demonstrate compliance to SP 800-90B". In resolution 3 it
> says "all processing of the raw data output from the noise sources that
> happens before it is ultimately output from the entropy source *shall*
> occur within a conditioning chain". Data from get_random_bytes may come
> from multiple noise sources, but they are hashed with ChaCha20.
> In resolution 6 it says "a vetted conditioning component may optionally
> take a finite amount of supplemental data [...] in addition to the data
> from the primary noise source", which would be OK if get_random_bytes
> used a vetted algorithm, but it is not the case for now.

You cite the right references, I think the interpretation is too strict.

The specifications require that

a) The DRBG must be seeded by a 90B entropy source

b) The DRBG must be initially seeded with 256 bits of entropy plus some 128 
bit nonce

We cover a) with the Jitter RNG and b) by pulling 384 bits from it.

The standard does not forbit:

c) the entropy string may contain data from another origin or it contains a 
larger buffer

d) the actual entropy distribution in the entropy string being not an 
equidistribution over the entire entropy string

Bullet d) implies that it is perfectly fine to have entropy distribution begin 
loopsided in the entropy string.

Bullet c) implies that other data can be provided with the entropy string.

With that, to be 90A/B compliant, you interpret that the Jitter RNG provides 
all entropy you need and credit the entropy from get_random_bytes with zero 
bits of entropy.


Note, if you look into the implementation of the DRBG seeding, the different 
input strings like entropy string or data without entropy like personalization 
string are simply concatenated and handed to the DRBG. As the Jitter RNG and 
get_random_bytes data is also concatenated, it follows the concepts of 90A.

If you look into the draft 90C standard, it explicitly allows concatenation of 
data from an entropy source that you credit with entropy and data without 
entropy - see the crediting of entropy of multiple entropy sources defined 
with "Method 1" and "Method 2" in the current 90C draft.

This ultimately allows us to have an entropy string that is concatenated from 
different entropy sources. If you have an entropy source that is not 90B 
compliant, you have to credit it with zero bits of entropy in the entropy 
analysis. Thus, only the entropy source(s) compliant to 90B must provide the 
entire entropy as mandated by 90A.

After having several discussions with the Entropy Working group sponsored by 
NIST that included also representatives from the NIST crypto technology group, 
there was no concern regarding such approach.

This approach you see in the current DRBG seeding code is now taken for 
different FIPS validations including FIPS validations that I work on as a FIPS 
tester as part of my duties working for a FIPS lab. My colleagues have 
reviewed the current kernel DRBG seeding strategy and approved of it for other 
FIPS validations.

Ciao
Stephan

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

[Mickaël Salaün]

On 25/06/2021 15:50, Stephan Müller wrote:
> Am Freitag, 25. Juni 2021, 13:09:26 CEST schrieb Mickaël Salaün:
> 
> Hi Mickaël,
> 
> [...]
>>
>>> - applies an entropy_input len of 512 bits during initial seeding
>>>
>>> - applies a nonce of 128 bits during initial seeding
>>>
>>> entropy_input == <384 bits get_random_bytes> || <256 bits Jitter RNG>
>>
>> We think that using "<384 bits get_random_bytes> || " makes this DRBG
>> non-compliant with SP800-90A rev1 because get_random_bytes doesn't use a
>> vetted conditioning component (but ChaCha20 instead):
>>
>> SP800-90Ar1, section 8.6.5 says "A DRBG mechanism requires an approved
>> randomness source during instantiation and reseeding [...]. An approved
>> randomness source is an entropy source that conforms to [SP 800-90B], or
>> an RBG that conforms to [SP 800-90C] − either a DRBG or an NRBG".
>> The FIPS 140-2 Implementation Guidance
>> (https://csrc.nist.gov/csrc/media/projects/cryptographic-module-validation-p
>> rogram/documents/fips140-2/fips1402ig.pdf), section 7.19 says "As of
>> November 7, 2020, all newly submitted modules requiring an entropy
>> evaluation must demonstrate compliance to SP 800-90B". In resolution 3 it
>> says "all processing of the raw data output from the noise sources that
>> happens before it is ultimately output from the entropy source *shall*
>> occur within a conditioning chain". Data from get_random_bytes may come
>> from multiple noise sources, but they are hashed with ChaCha20.
>> In resolution 6 it says "a vetted conditioning component may optionally
>> take a finite amount of supplemental data [...] in addition to the data
>> from the primary noise source", which would be OK if get_random_bytes
>> used a vetted algorithm, but it is not the case for now.
> 
> You cite the right references, I think the interpretation is too strict.
> 
> The specifications require that
> 
> a) The DRBG must be seeded by a 90B entropy source
> 
> b) The DRBG must be initially seeded with 256 bits of entropy plus some 128 
> bit nonce
> 
> We cover a) with the Jitter RNG and b) by pulling 384 bits from it.
> 
> The standard does not forbit:
> 
> c) the entropy string may contain data from another origin or it contains a 
> larger buffer
> 
> d) the actual entropy distribution in the entropy string being not an 
> equidistribution over the entire entropy string
> 
> Bullet d) implies that it is perfectly fine to have entropy distribution begin 
> loopsided in the entropy string.
> 
> Bullet c) implies that other data can be provided with the entropy string.
> 
> With that, to be 90A/B compliant, you interpret that the Jitter RNG provides 
> all entropy you need and credit the entropy from get_random_bytes with zero 
> bits of entropy.
> 
> 
> Note, if you look into the implementation of the DRBG seeding, the different 
> input strings like entropy string or data without entropy like personalization 
> string are simply concatenated and handed to the DRBG. As the Jitter RNG and 
> get_random_bytes data is also concatenated, it follows the concepts of 90A.
> 
> If you look into the draft 90C standard, it explicitly allows concatenation of 
> data from an entropy source that you credit with entropy and data without 
> entropy - see the crediting of entropy of multiple entropy sources defined 
> with "Method 1" and "Method 2" in the current 90C draft.
> 
> This ultimately allows us to have an entropy string that is concatenated from 
> different entropy sources. If you have an entropy source that is not 90B 
> compliant, you have to credit it with zero bits of entropy in the entropy 
> analysis. Thus, only the entropy source(s) compliant to 90B must provide the 
> entire entropy as mandated by 90A.

Thanks for your detailed explanation Stephan. We agree that data from
get_random_bytes is not accounted as entropy, but the question is: is it
still in line with the specification because it uses an algorithm not
compliant to SP800-90B (i.e. ChaCha20 is not a vetted conditioning
component)? Cf. IG 7.19 resolution 6 from 08/28/2020 and IG 7.20 from
05/04/2021.

> 
> After having several discussions with the Entropy Working group sponsored by 
> NIST that included also representatives from the NIST crypto technology group, 
> there was no concern regarding such approach.
> 
> This approach you see in the current DRBG seeding code is now taken for 
> different FIPS validations including FIPS validations that I work on as a FIPS 
> tester as part of my duties working for a FIPS lab. My colleagues have 
> reviewed the current kernel DRBG seeding strategy and approved of it for other 
> FIPS validations.

Good to know. We are worried that a new FIPS validation (started after
November 7, 2020) could failed because of the new SP800-90B requirement.
This issue was pointed out by a lab. It seems that the specification is
still open to different interpretations.

Regards,
 Mickaël