Hi,
the sampling loop in jent_gen_entropy() can potentially run indefinitely
w/o making any forward progress, namely if only stuck samples are taken
for whatever reason.
There's a straight-forward way to make the entropy collection more robust,
namely to terminate the loop and report an error if this happens. This
patchset here implements that.
Applies to herbert/cryptodev-2.6.git master.
Thanks!
Nicolai
Nicolai Stange (3):
crypto: drbg - ignore jitterentropy errors if not in FIPS mode
crypto: jitter - don't limit ->health_failure check to FIPS mode
crypto: jitter - quit sample collection loop upon RCT failure
crypto/drbg.c | 7 +++++--
crypto/jitterentropy-kcapi.c | 6 ------
crypto/jitterentropy.c | 6 +-----
crypto/jitterentropy.h | 1 -
4 files changed, 6 insertions(+), 14 deletions(-)
--
2.26.2
A subsequent patch will make the jitterentropy RNG to unconditionally
report health test errors back to callers, independent of whether
fips_enabled is set or not. The DRBG needs access to a functional
jitterentropy instance only in FIPS mode (because it's the only SP800-90B
compliant entropy source as it currently stands). Thus, it is perfectly
fine for the DRBGs to obtain entropy from the jitterentropy source only
on a best effort basis if fips_enabled is off.
Make the DRBGs to ignore jitterentropy failures if fips_enabled is not set.
Signed-off-by: Nicolai Stange <[email protected]>
---
crypto/drbg.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 5977a72afb03..177983b6ae38 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1193,11 +1193,14 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
pr_devel("DRBG: (re)seeding with %u bytes of entropy\n",
entropylen);
} else {
- /* Get seed from Jitter RNG */
+ /*
+ * Get seed from Jitter RNG, failures are
+ * fatal only in FIPS mode.
+ */
ret = crypto_rng_get_bytes(drbg->jent,
entropy + entropylen,
entropylen);
- if (ret) {
+ if (fips_enabled && ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
/*
--
2.26.2
The jitterentropy's Repetition Count Test (RCT) as well as the Adaptive
Proportion Test (APT) are run unconditionally on any collected samples.
However, their result, i.e. ->health_failure, will only get checked if
fips_enabled is set, c.f. the jent_health_failure() wrapper.
I would argue that a RCT or APT failure indicates that something's
seriously off and that this should always be reported as an error,
independently of whether FIPS mode is enabled or not: it should be up to
callers whether or not and how to handle jitterentropy failures.
Make jent_health_failure() to unconditionally return ->health_failure,
independent of whether fips_enabled is set.
Note that fips_enabled isn't accessed from the jitterentropy code anymore
now. Remove the linux/fips.h include as well as the jent_fips_enabled()
wrapper.
Signed-off-by: Nicolai Stange <[email protected]>
---
crypto/jitterentropy-kcapi.c | 6 ------
crypto/jitterentropy.c | 4 ----
crypto/jitterentropy.h | 1 -
3 files changed, 11 deletions(-)
diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index e8a4165a1874..2d115bec15ae 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -40,7 +40,6 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/slab.h>
-#include <linux/fips.h>
#include <linux/time.h>
#include <crypto/internal/rng.h>
@@ -60,11 +59,6 @@ void jent_zfree(void *ptr)
kfree_sensitive(ptr);
}
-int jent_fips_enabled(void)
-{
- return fips_enabled;
-}
-
void jent_panic(char *s)
{
panic("%s", s);
diff --git a/crypto/jitterentropy.c b/crypto/jitterentropy.c
index 788d90749715..24e087c3f526 100644
--- a/crypto/jitterentropy.c
+++ b/crypto/jitterentropy.c
@@ -298,10 +298,6 @@ static int jent_stuck(struct rand_data *ec, __u64 current_delta)
*/
static int jent_health_failure(struct rand_data *ec)
{
- /* Test is only enabled in FIPS mode */
- if (!jent_fips_enabled())
- return 0;
-
return ec->health_failure;
}
diff --git a/crypto/jitterentropy.h b/crypto/jitterentropy.h
index c83fff32d130..b7397b617ef0 100644
--- a/crypto/jitterentropy.h
+++ b/crypto/jitterentropy.h
@@ -2,7 +2,6 @@
extern void *jent_zalloc(unsigned int len);
extern void jent_zfree(void *ptr);
-extern int jent_fips_enabled(void);
extern void jent_panic(char *s);
extern void jent_memcpy(void *dest, const void *src, unsigned int n);
extern void jent_get_nstime(__u64 *out);
--
2.26.2
The jitterentropy collection loop in jent_gen_entropy() can in principle
run indefinitely without making any progress if it only receives stuck
measurements as determined by jent_stuck(). After 31 consecutive stuck
samples, the Repetition Count Test (RCT) would fail anyway and the
jitterentropy RNG instances moved into ->health_failure == 1 state.
jent_gen_entropy()'s caller, jent_read_entropy() would then check for
this ->health_failure condition and return an error if found set. It
follows that there's absolutely no point in continuing the collection loop
in jent_gen_entropy() once the RCT has failed.
Make the jitterentropy collection loop more robust by terminating it upon
jent_health_failure() so that it won't continue to run indefinitely without
making any progress.
Signed-off-by: Nicolai Stange <[email protected]>
---
crypto/jitterentropy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/jitterentropy.c b/crypto/jitterentropy.c
index 24e087c3f526..8f5283f28ed3 100644
--- a/crypto/jitterentropy.c
+++ b/crypto/jitterentropy.c
@@ -547,7 +547,7 @@ static void jent_gen_entropy(struct rand_data *ec)
/* priming of the ->prev_time value */
jent_measure_jitter(ec);
- while (1) {
+ while (!jent_health_failure(ec)) {
/* If a stuck measurement is received, repeat measurement */
if (jent_measure_jitter(ec))
continue;
--
2.26.2
Am Dienstag, 30. November 2021, 15:10:07 CET schrieb Nicolai Stange:
Hi Nicolai,
> A subsequent patch will make the jitterentropy RNG to unconditionally
> report health test errors back to callers, independent of whether
> fips_enabled is set or not. The DRBG needs access to a functional
> jitterentropy instance only in FIPS mode (because it's the only SP800-90B
> compliant entropy source as it currently stands). Thus, it is perfectly
> fine for the DRBGs to obtain entropy from the jitterentropy source only
> on a best effort basis if fips_enabled is off.
>
> Make the DRBGs to ignore jitterentropy failures if fips_enabled is not set.
>
> Signed-off-by: Nicolai Stange <[email protected]>
Reviewed-by: Stephan Mueller <[email protected]>
Thanks
Stephan
> ---
> crypto/drbg.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 5977a72afb03..177983b6ae38 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -1193,11 +1193,14 @@ static int drbg_seed(struct drbg_state *drbg, struct
> drbg_string *pers, pr_devel("DRBG: (re)seeding with %u bytes of entropy\n",
> entropylen);
> } else {
> - /* Get seed from Jitter RNG */
> + /*
> + * Get seed from Jitter RNG, failures are
> + * fatal only in FIPS mode.
> + */
> ret = crypto_rng_get_bytes(drbg->jent,
> entropy + entropylen,
> entropylen);
> - if (ret) {
> + if (fips_enabled && ret) {
> pr_devel("DRBG: jent failed with %d\n", ret);
>
> /*
Ciao
Stephan
Am Dienstag, 30. November 2021, 15:10:08 CET schrieb Nicolai Stange:
Hi Nicolai,
> The jitterentropy's Repetition Count Test (RCT) as well as the Adaptive
> Proportion Test (APT) are run unconditionally on any collected samples.
> However, their result, i.e. ->health_failure, will only get checked if
> fips_enabled is set, c.f. the jent_health_failure() wrapper.
>
> I would argue that a RCT or APT failure indicates that something's
> seriously off and that this should always be reported as an error,
> independently of whether FIPS mode is enabled or not: it should be up to
> callers whether or not and how to handle jitterentropy failures.
>
> Make jent_health_failure() to unconditionally return ->health_failure,
> independent of whether fips_enabled is set.
>
> Note that fips_enabled isn't accessed from the jitterentropy code anymore
> now. Remove the linux/fips.h include as well as the jent_fips_enabled()
> wrapper.
>
> Signed-off-by: Nicolai Stange <[email protected]>
Reviewed-by: Stephan Mueller <[email protected]>
Thanks
Stephan
> ---
> crypto/jitterentropy-kcapi.c | 6 ------
> crypto/jitterentropy.c | 4 ----
> crypto/jitterentropy.h | 1 -
> 3 files changed, 11 deletions(-)
>
> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> index e8a4165a1874..2d115bec15ae 100644
> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -40,7 +40,6 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> -#include <linux/fips.h>
> #include <linux/time.h>
> #include <crypto/internal/rng.h>
>
> @@ -60,11 +59,6 @@ void jent_zfree(void *ptr)
> kfree_sensitive(ptr);
> }
>
> -int jent_fips_enabled(void)
> -{
> - return fips_enabled;
> -}
> -
> void jent_panic(char *s)
> {
> panic("%s", s);
> diff --git a/crypto/jitterentropy.c b/crypto/jitterentropy.c
> index 788d90749715..24e087c3f526 100644
> --- a/crypto/jitterentropy.c
> +++ b/crypto/jitterentropy.c
> @@ -298,10 +298,6 @@ static int jent_stuck(struct rand_data *ec, __u64
> current_delta) */
> static int jent_health_failure(struct rand_data *ec)
> {
> - /* Test is only enabled in FIPS mode */
> - if (!jent_fips_enabled())
> - return 0;
> -
> return ec->health_failure;
> }
>
> diff --git a/crypto/jitterentropy.h b/crypto/jitterentropy.h
> index c83fff32d130..b7397b617ef0 100644
> --- a/crypto/jitterentropy.h
> +++ b/crypto/jitterentropy.h
> @@ -2,7 +2,6 @@
>
> extern void *jent_zalloc(unsigned int len);
> extern void jent_zfree(void *ptr);
> -extern int jent_fips_enabled(void);
> extern void jent_panic(char *s);
> extern void jent_memcpy(void *dest, const void *src, unsigned int n);
> extern void jent_get_nstime(__u64 *out);
Ciao
Stephan
Am Dienstag, 30. November 2021, 15:10:09 CET schrieb Nicolai Stange:
Hi Nicolai,
> The jitterentropy collection loop in jent_gen_entropy() can in principle
> run indefinitely without making any progress if it only receives stuck
> measurements as determined by jent_stuck(). After 31 consecutive stuck
> samples, the Repetition Count Test (RCT) would fail anyway and the
> jitterentropy RNG instances moved into ->health_failure == 1 state.
> jent_gen_entropy()'s caller, jent_read_entropy() would then check for
> this ->health_failure condition and return an error if found set. It
> follows that there's absolutely no point in continuing the collection loop
> in jent_gen_entropy() once the RCT has failed.
>
> Make the jitterentropy collection loop more robust by terminating it upon
> jent_health_failure() so that it won't continue to run indefinitely without
> making any progress.
>
> Signed-off-by: Nicolai Stange <[email protected]>
Reviewed-by: Stephan Mueller <[email protected]>
Thanks
Stephan
> ---
> crypto/jitterentropy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/jitterentropy.c b/crypto/jitterentropy.c
> index 24e087c3f526..8f5283f28ed3 100644
> --- a/crypto/jitterentropy.c
> +++ b/crypto/jitterentropy.c
> @@ -547,7 +547,7 @@ static void jent_gen_entropy(struct rand_data *ec)
> /* priming of the ->prev_time value */
> jent_measure_jitter(ec);
>
> - while (1) {
> + while (!jent_health_failure(ec)) {
> /* If a stuck measurement is received, repeat measurement */
> if (jent_measure_jitter(ec))
> continue;
Ciao
Stephan
On Tue, Nov 30, 2021 at 03:10:06PM +0100, Nicolai Stange wrote:
> Hi,
>
> the sampling loop in jent_gen_entropy() can potentially run indefinitely
> w/o making any forward progress, namely if only stuck samples are taken
> for whatever reason.
>
> There's a straight-forward way to make the entropy collection more robust,
> namely to terminate the loop and report an error if this happens. This
> patchset here implements that.
>
> Applies to herbert/cryptodev-2.6.git master.
>
> Thanks!
>
> Nicolai
>
> Nicolai Stange (3):
> crypto: drbg - ignore jitterentropy errors if not in FIPS mode
> crypto: jitter - don't limit ->health_failure check to FIPS mode
> crypto: jitter - quit sample collection loop upon RCT failure
>
> crypto/drbg.c | 7 +++++--
> crypto/jitterentropy-kcapi.c | 6 ------
> crypto/jitterentropy.c | 6 +-----
> crypto/jitterentropy.h | 1 -
> 4 files changed, 6 insertions(+), 14 deletions(-)
>
> --
> 2.26.2
All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt