FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
source. Up to kernel 4.8 drivers/char/random.c provided this continuous
self test. Afterwards it was moved to a location that is inconsistent
with the FIPS 140-2 requirements.
Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
seed. This patch resurrects the function drbg_fips_continous_test that
existed some time ago and applies it to the noise sources.
The Jitter RNG implements its own FIPS 140-2 self test and thus does not
need to be subjected to the test in the DRBG.
The patch contains a tiny fix to ensure proper zeroization in case of an
error during the Jitter RNG data gathering.
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/drbg.c | 83 +++++++++++++++++++++++++++++++++++++++++--
include/crypto/drbg.h | 4 +++
2 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2a5b16bb000c..6a2c3f7fbe1d 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
}
}
+/*
+ * FIPS 140-2 continuous self test for the noise source
+ * The test is performed on the noise source input data. Thus, the function
+ * implicitly knows the size of the buffer to be equal to the security
+ * strength.
+ *
+ * Note, this function disregards the nonce trailing the entropy data during
+ * initial seeding.
+ *
+ * drbg->drbg_mutex must have been taken.
+ *
+ * @drbg DRBG handle
+ * @entropy buffer of seed data to be checked
+ *
+ * return:
+ * 0 on success
+ * -EAGAIN on when the CTRNG is not yet primed
+ * < 0 on error
+ */
+static int drbg_fips_continuous_test(struct drbg_state *drbg,
+ const unsigned char *entropy)
+{
+#ifdef CONFIG_CRYPTO_FIPS
+ unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
+ int ret = 0;
+
+ /* skip test if we test the overall system */
+ if (list_empty(&drbg->test_data.list))
+ return true;
+ /* only perform test in FIPS mode */
+ if (!fips_enabled)
+ return true;
+
+ if (!drbg->fips_primed) {
+ /* Priming of FIPS test */
+ memcpy(drbg->prev, entropy, entropylen);
+ drbg->fips_primed = true;
+ /* return false due to priming, i.e. another round is needed */
+ return -EAGAIN;
+ }
+ ret = memcmp(drbg->prev, entropy, entropylen);
+ if (!ret)
+ panic("DRBG continuous self test failed\n");
+ memcpy(drbg->prev, entropy, entropylen);
+ /* the test shall pass when the two compared values are not equal */
+ return (ret != 0) ? 0 : -EFAULT;
+#else
+ return true;
+#endif /* CONFIG_CRYPTO_FIPS */
+}
+
/*
* Convert an integer into a byte representation of this integer.
* The byte representation is big-endian
@@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct *work)
seed_work);
unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
unsigned char entropy[32];
+ int ret;
BUG_ON(!entropylen);
BUG_ON(entropylen > sizeof(entropy));
- get_random_bytes(entropy, entropylen);
drbg_string_fill(&data, entropy, entropylen);
list_add_tail(&data.list, &seedlist);
mutex_lock(&drbg->drbg_mutex);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto unlock;
+ } while (ret);
+
/* If nonblocking pool is initialized, deactivate Jitter RNG */
crypto_free_rng(drbg->jent);
drbg->jent = NULL;
@@ -1030,6 +1088,7 @@ static void drbg_async_seed(struct work_struct *work)
if (drbg->seeded)
drbg->reseed_threshold = drbg_max_requests(drbg);
+unlock:
mutex_unlock(&drbg->drbg_mutex);
memzero_explicit(entropy, entropylen);
@@ -1081,7 +1140,12 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
BUG_ON((entropylen * 2) > sizeof(entropy));
/* Get seed from in-kernel /dev/urandom */
- get_random_bytes(entropy, entropylen);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto out;
+ } while (ret);
if (!drbg->jent) {
drbg_string_fill(&data1, entropy, entropylen);
@@ -1094,7 +1158,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
entropylen);
if (ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
- return ret;
+ goto out;
}
drbg_string_fill(&data1, entropy, entropylen * 2);
@@ -1121,6 +1185,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);
return ret;
@@ -1142,6 +1207,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
drbg->reseed_ctr = 0;
drbg->d_ops = NULL;
drbg->core = NULL;
+#ifdef CONFIG_CRYPTO_FIPS
+ kzfree(drbg->prev);
+ drbg->prev = NULL;
+ drbg->fips_primed = false;
+#endif
}
/*
@@ -1211,6 +1281,13 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
}
+#ifdef CONFIG_CRYPTO_FIPS
+ drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags), GFP_KERNEL);
+ if (!drbg->prev)
+ goto fini;
+ drbg->fips_primed = false;
+#endif
+
return 0;
fini:
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 3fb581bf3b87..7603f2d0e326 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -129,6 +129,10 @@ struct drbg_state {
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
+#ifdef CONFIG_CRYPTO_FIPS
+ bool fips_primed; /* Continuous test primed? */
+ unsigned char *prev; /* FIPS 140-2 continuous test value */
+#endif
struct work_struct seed_work; /* asynchronous seeding support */
struct crypto_rng *jent;
const struct drbg_state_ops *d_ops;
--
2.21.0
Am Donnerstag, 2. Mai 2019, 14:48:11 CEST schrieb Herbert Xu:
Hi Herbert,
> Hi Stephan:
>
> On Thu, May 02, 2019 at 02:40:51PM +0200, Stephan M?ller wrote:
> > +static int drbg_fips_continuous_test(struct drbg_state *drbg,
> > + const unsigned char *entropy)
> > +{
> > +#ifdef CONFIG_CRYPTO_FIPS
>
> Please use the IS_ENABLED macro from linux/kconfig.h so that all
> of the code gets compiler coverage.
Will do.
>
> Thanks,
Ciao
Stephan
Hi Stephan:
On Thu, May 02, 2019 at 02:40:51PM +0200, Stephan M?ller wrote:
>
> +static int drbg_fips_continuous_test(struct drbg_state *drbg,
> + const unsigned char *entropy)
> +{
> +#ifdef CONFIG_CRYPTO_FIPS
Please use the IS_ENABLED macro from linux/kconfig.h so that all
of the code gets compiler coverage.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Changes v2:
* use IS_ENABLED macro
* small fix in comment
---8<---
FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
source. Up to kernel 4.8 drivers/char/random.c provided this continuous
self test. Afterwards it was moved to a location that is inconsistent
with the FIPS 140-2 requirements.
Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
seed. This patch resurrects the function drbg_fips_continous_test that
existed some time ago and applies it to the noise sources.
The Jitter RNG implements its own FIPS 140-2 self test and thus does not
need to be subjected to the test in the DRBG.
The patch contains a tiny fix to ensure proper zeroization in case of an
error during the Jitter RNG data gathering.
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/drbg.c | 84 +++++++++++++++++++++++++++++++++++++++++--
include/crypto/drbg.h | 4 +++
2 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2a5b16bb000c..d6261262fda4 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
}
}
+/*
+ * FIPS 140-2 continuous self test for the noise source
+ * The test is performed on the noise source input data. Thus, the function
+ * implicitly knows the size of the buffer to be equal to the security
+ * strength.
+ *
+ * Note, this function disregards the nonce trailing the entropy data during
+ * initial seeding.
+ *
+ * drbg->drbg_mutex must have been taken.
+ *
+ * @drbg DRBG handle
+ * @entropy buffer of seed data to be checked
+ *
+ * return:
+ * 0 on success
+ * -EAGAIN on when the CTRNG is not yet primed
+ * < 0 on error
+ */
+static int drbg_fips_continuous_test(struct drbg_state *drbg,
+ const unsigned char *entropy)
+{
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
+ int ret = 0;
+
+ /* skip test if we test the overall system */
+ if (list_empty(&drbg->test_data.list))
+ return true;
+ /* only perform test in FIPS mode */
+ if (!fips_enabled)
+ return true;
+
+ if (!drbg->fips_primed) {
+ /* Priming of FIPS test */
+ memcpy(drbg->prev, entropy, entropylen);
+ drbg->fips_primed = true;
+ /* priming: another round is needed */
+ return -EAGAIN;
+ }
+ ret = memcmp(drbg->prev, entropy, entropylen);
+ if (!ret)
+ panic("DRBG continuous self test failed\n");
+ memcpy(drbg->prev, entropy, entropylen);
+ /* the test shall pass when the two values are not equal */
+ return (ret != 0) ? 0 : -EFAULT;
+#else
+ return true;
+#endif /* CONFIG_CRYPTO_FIPS */
+}
+
/*
* Convert an integer into a byte representation of this integer.
* The byte representation is big-endian
@@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct *work)
seed_work);
unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
unsigned char entropy[32];
+ int ret;
BUG_ON(!entropylen);
BUG_ON(entropylen > sizeof(entropy));
- get_random_bytes(entropy, entropylen);
drbg_string_fill(&data, entropy, entropylen);
list_add_tail(&data.list, &seedlist);
mutex_lock(&drbg->drbg_mutex);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto unlock;
+ } while (ret);
+
/* If nonblocking pool is initialized, deactivate Jitter RNG */
crypto_free_rng(drbg->jent);
drbg->jent = NULL;
@@ -1030,6 +1088,7 @@ static void drbg_async_seed(struct work_struct *work)
if (drbg->seeded)
drbg->reseed_threshold = drbg_max_requests(drbg);
+unlock:
mutex_unlock(&drbg->drbg_mutex);
memzero_explicit(entropy, entropylen);
@@ -1081,7 +1140,12 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
BUG_ON((entropylen * 2) > sizeof(entropy));
/* Get seed from in-kernel /dev/urandom */
- get_random_bytes(entropy, entropylen);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto out;
+ } while (ret);
if (!drbg->jent) {
drbg_string_fill(&data1, entropy, entropylen);
@@ -1094,7 +1158,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
entropylen);
if (ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
- return ret;
+ goto out;
}
drbg_string_fill(&data1, entropy, entropylen * 2);
@@ -1121,6 +1185,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);
return ret;
@@ -1142,6 +1207,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
drbg->reseed_ctr = 0;
drbg->d_ops = NULL;
drbg->core = NULL;
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ kzfree(drbg->prev);
+ drbg->prev = NULL;
+ drbg->fips_primed = false;
+#endif
}
/*
@@ -1211,6 +1281,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
}
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
+ GFP_KERNEL);
+ if (!drbg->prev)
+ goto fini;
+ drbg->fips_primed = false;
+#endif
+
return 0;
fini:
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 3fb581bf3b87..939051480c83 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -129,6 +129,10 @@ struct drbg_state {
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ bool fips_primed; /* Continuous test primed? */
+ unsigned char *prev; /* FIPS 140-2 continuous test value */
+#endif
struct work_struct seed_work; /* asynchronous seeding support */
struct crypto_rng *jent;
const struct drbg_state_ops *d_ops;
--
2.21.0
Changes v3:
* fix return code of drbg_fips_continuous_test in non-FIPS mode
---8<---
FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
source. Up to kernel 4.8 drivers/char/random.c provided this continuous
self test. Afterwards it was moved to a location that is inconsistent
with the FIPS 140-2 requirements.
Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
seed. This patch resurrects the function drbg_fips_continous_test that
existed some time ago and applies it to the noise sources.
The Jitter RNG implements its own FIPS 140-2 self test and thus does not
need to be subjected to the test in the DRBG.
The patch contains a tiny fix to ensure proper zeroization in case of an
error during the Jitter RNG data gathering.
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/drbg.c | 84 +++++++++++++++++++++++++++++++++++++++++--
include/crypto/drbg.h | 4 +++
2 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2a5b16bb000c..d6261262fda4 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
}
}
+/*
+ * FIPS 140-2 continuous self test for the noise source
+ * The test is performed on the noise source input data. Thus, the function
+ * implicitly knows the size of the buffer to be equal to the security
+ * strength.
+ *
+ * Note, this function disregards the nonce trailing the entropy data during
+ * initial seeding.
+ *
+ * drbg->drbg_mutex must have been taken.
+ *
+ * @drbg DRBG handle
+ * @entropy buffer of seed data to be checked
+ *
+ * return:
+ * 0 on success
+ * -EAGAIN on when the CTRNG is not yet primed
+ * < 0 on error
+ */
+static int drbg_fips_continuous_test(struct drbg_state *drbg,
+ const unsigned char *entropy)
+{
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
+ int ret = 0;
+
+ /* skip test if we test the overall system */
+ if (list_empty(&drbg->test_data.list))
+ return 0;
+ /* only perform test in FIPS mode */
+ if (!fips_enabled)
+ return 0;
+
+ if (!drbg->fips_primed) {
+ /* Priming of FIPS test */
+ memcpy(drbg->prev, entropy, entropylen);
+ drbg->fips_primed = true;
+ /* priming: another round is needed */
+ return -EAGAIN;
+ }
+ ret = memcmp(drbg->prev, entropy, entropylen);
+ if (!ret)
+ panic("DRBG continuous self test failed\n");
+ memcpy(drbg->prev, entropy, entropylen);
+ /* the test shall pass when the two values are not equal */
+ return (ret != 0) ? 0 : -EFAULT;
+#else
+ return true;
+#endif /* CONFIG_CRYPTO_FIPS */
+}
+
/*
* Convert an integer into a byte representation of this integer.
* The byte representation is big-endian
@@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct *work)
seed_work);
unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
unsigned char entropy[32];
+ int ret;
BUG_ON(!entropylen);
BUG_ON(entropylen > sizeof(entropy));
- get_random_bytes(entropy, entropylen);
drbg_string_fill(&data, entropy, entropylen);
list_add_tail(&data.list, &seedlist);
mutex_lock(&drbg->drbg_mutex);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto unlock;
+ } while (ret);
+
/* If nonblocking pool is initialized, deactivate Jitter RNG */
crypto_free_rng(drbg->jent);
drbg->jent = NULL;
@@ -1030,6 +1088,7 @@ static void drbg_async_seed(struct work_struct *work)
if (drbg->seeded)
drbg->reseed_threshold = drbg_max_requests(drbg);
+unlock:
mutex_unlock(&drbg->drbg_mutex);
memzero_explicit(entropy, entropylen);
@@ -1081,7 +1140,12 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
BUG_ON((entropylen * 2) > sizeof(entropy));
/* Get seed from in-kernel /dev/urandom */
- get_random_bytes(entropy, entropylen);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto out;
+ } while (ret);
if (!drbg->jent) {
drbg_string_fill(&data1, entropy, entropylen);
@@ -1094,7 +1158,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
entropylen);
if (ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
- return ret;
+ goto out;
}
drbg_string_fill(&data1, entropy, entropylen * 2);
@@ -1121,6 +1185,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);
return ret;
@@ -1142,6 +1207,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
drbg->reseed_ctr = 0;
drbg->d_ops = NULL;
drbg->core = NULL;
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ kzfree(drbg->prev);
+ drbg->prev = NULL;
+ drbg->fips_primed = false;
+#endif
}
/*
@@ -1211,6 +1281,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
}
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
+ GFP_KERNEL);
+ if (!drbg->prev)
+ goto fini;
+ drbg->fips_primed = false;
+#endif
+
return 0;
fini:
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 3fb581bf3b87..939051480c83 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -129,6 +129,10 @@ struct drbg_state {
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
+#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
+ bool fips_primed; /* Continuous test primed? */
+ unsigned char *prev; /* FIPS 140-2 continuous test value */
+#endif
struct work_struct seed_work; /* asynchronous seeding support */
struct crypto_rng *jent;
const struct drbg_state_ops *d_ops;
--
2.21.0
On Thu, May 02, 2019 at 06:38:12PM +0200, Stephan M?ller wrote:
> +static int drbg_fips_continuous_test(struct drbg_state *drbg,
> + const unsigned char *entropy)
> +{
> +#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
This should look like
if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
...
} else {
...
}
This way the compiler will see everything regardless of whether
FIPS is enabled or not.
> diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
> index 3fb581bf3b87..939051480c83 100644
> --- a/include/crypto/drbg.h
> +++ b/include/crypto/drbg.h
> @@ -129,6 +129,10 @@ struct drbg_state {
>
> bool seeded; /* DRBG fully seeded? */
> bool pr; /* Prediction resistance enabled? */
> +#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
> + bool fips_primed; /* Continuous test primed? */
> + unsigned char *prev; /* FIPS 140-2 continuous test value */
> +#endif
You can still use #ifdef here.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Am Freitag, 3. Mai 2019, 03:42:41 CEST schrieb Herbert Xu:
Hi Herbert,
> On Thu, May 02, 2019 at 06:38:12PM +0200, Stephan M?ller wrote:
> > +static int drbg_fips_continuous_test(struct drbg_state *drbg,
> > + const unsigned char *entropy)
> > +{
> > +#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
>
> This should look like
>
> if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
> ...
> } else {
> ...
> }
>
> This way the compiler will see everything regardless of whether
> FIPS is enabled or not.
>
> > diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
> > index 3fb581bf3b87..939051480c83 100644
> > --- a/include/crypto/drbg.h
> > +++ b/include/crypto/drbg.h
> > @@ -129,6 +129,10 @@ struct drbg_state {
> >
> > bool seeded; /* DRBG fully seeded? */
> > bool pr; /* Prediction resistance enabled? */
> >
> > +#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
> > + bool fips_primed; /* Continuous test primed? */
> > + unsigned char *prev; /* FIPS 140-2 continuous test value */
> > +#endif
>
> You can still use #ifdef here.
The variables would need to be defined unconditionally if we use a runtime
check in the C code. Is that what you want me to do?
Ciao
Stephan
On Fri, May 03, 2019 at 07:11:23AM +0200, Stephan Mueller wrote:
>
> > > diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
> > > index 3fb581bf3b87..939051480c83 100644
> > > --- a/include/crypto/drbg.h
> > > +++ b/include/crypto/drbg.h
> > > @@ -129,6 +129,10 @@ struct drbg_state {
> > >
> > > bool seeded; /* DRBG fully seeded? */
> > > bool pr; /* Prediction resistance enabled? */
> > >
> > > +#if IS_ENABLED(CONFIG_CRYPTO_FIPS)
> > > + bool fips_primed; /* Continuous test primed? */
> > > + unsigned char *prev; /* FIPS 140-2 continuous test value */
> > > +#endif
> >
> > You can still use #ifdef here.
>
> The variables would need to be defined unconditionally if we use a runtime
> check in the C code. Is that what you want me to do?
Yes please do that. If we wanted to we can get around this by
using accessor functions to hide them but DRBG without FIPS
doesn't make much sense anyway so let's just include them
unconditionally.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Changes v4:
* use if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) and compile CTRNG variables
unconditionally
---8<---
FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
source. Up to kernel 4.8 drivers/char/random.c provided this continuous
self test. Afterwards it was moved to a location that is inconsistent
with the FIPS 140-2 requirements.
Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
seed. This patch resurrects the function drbg_fips_continous_test that
existed some time ago and applies it to the noise sources.
The Jitter RNG implements its own FIPS 140-2 self test and thus does not
need to be subjected to the test in the DRBG.
The patch contains a tiny fix to ensure proper zeroization in case of an
error during the Jitter RNG data gathering.
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/drbg.c | 84 +++++++++++++++++++++++++++++++++++++++++--
include/crypto/drbg.h | 2 ++
2 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2a5b16bb000c..8328d7d9b42e 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
}
}
+/*
+ * FIPS 140-2 continuous self test for the noise source
+ * The test is performed on the noise source input data. Thus, the function
+ * implicitly knows the size of the buffer to be equal to the security
+ * strength.
+ *
+ * Note, this function disregards the nonce trailing the entropy data during
+ * initial seeding.
+ *
+ * drbg->drbg_mutex must have been taken.
+ *
+ * @drbg DRBG handle
+ * @entropy buffer of seed data to be checked
+ *
+ * return:
+ * 0 on success
+ * -EAGAIN on when the CTRNG is not yet primed
+ * < 0 on error
+ */
+static int drbg_fips_continuous_test(struct drbg_state *drbg,
+ const unsigned char *entropy)
+{
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
+ int ret = 0;
+
+ /* skip test if we test the overall system */
+ if (list_empty(&drbg->test_data.list))
+ return 0;
+ /* only perform test in FIPS mode */
+ if (!fips_enabled)
+ return 0;
+
+ if (!drbg->fips_primed) {
+ /* Priming of FIPS test */
+ memcpy(drbg->prev, entropy, entropylen);
+ drbg->fips_primed = true;
+ /* priming: another round is needed */
+ return -EAGAIN;
+ }
+ ret = memcmp(drbg->prev, entropy, entropylen);
+ if (!ret)
+ panic("DRBG continuous self test failed\n");
+ memcpy(drbg->prev, entropy, entropylen);
+ /* the test shall pass when the two values are not equal */
+ return (ret != 0) ? 0 : -EFAULT;
+ } else {
+ return 0;
+ }
+}
+
/*
* Convert an integer into a byte representation of this integer.
* The byte representation is big-endian
@@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct *work)
seed_work);
unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
unsigned char entropy[32];
+ int ret;
BUG_ON(!entropylen);
BUG_ON(entropylen > sizeof(entropy));
- get_random_bytes(entropy, entropylen);
drbg_string_fill(&data, entropy, entropylen);
list_add_tail(&data.list, &seedlist);
mutex_lock(&drbg->drbg_mutex);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto unlock;
+ } while (ret);
+
/* If nonblocking pool is initialized, deactivate Jitter RNG */
crypto_free_rng(drbg->jent);
drbg->jent = NULL;
@@ -1030,6 +1088,7 @@ static void drbg_async_seed(struct work_struct *work)
if (drbg->seeded)
drbg->reseed_threshold = drbg_max_requests(drbg);
+unlock:
mutex_unlock(&drbg->drbg_mutex);
memzero_explicit(entropy, entropylen);
@@ -1081,7 +1140,12 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
BUG_ON((entropylen * 2) > sizeof(entropy));
/* Get seed from in-kernel /dev/urandom */
- get_random_bytes(entropy, entropylen);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto out;
+ } while (ret);
if (!drbg->jent) {
drbg_string_fill(&data1, entropy, entropylen);
@@ -1094,7 +1158,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
entropylen);
if (ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
- return ret;
+ goto out;
}
drbg_string_fill(&data1, entropy, entropylen * 2);
@@ -1121,6 +1185,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);
return ret;
@@ -1142,6 +1207,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
drbg->reseed_ctr = 0;
drbg->d_ops = NULL;
drbg->core = NULL;
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ kzfree(drbg->prev);
+ drbg->prev = NULL;
+ drbg->fips_primed = false;
+ }
}
/*
@@ -1211,6 +1281,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
}
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
+ GFP_KERNEL);
+ if (!drbg->prev)
+ goto fini;
+ drbg->fips_primed = false;
+ }
+
return 0;
fini:
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 3fb581bf3b87..8c9af21efce1 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -129,6 +129,8 @@ struct drbg_state {
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
+ bool fips_primed; /* Continuous test primed? */
+ unsigned char *prev; /* FIPS 140-2 continuous test value */
struct work_struct seed_work; /* asynchronous seeding support */
struct crypto_rng *jent;
const struct drbg_state_ops *d_ops;
--
2.21.0
Hi
Le vendredi 03 mai 2019 à 21:58 +0200, Stephan Müller a écrit :
>
> FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
> source. Up to kernel 4.8 drivers/char/random.c provided this continuous
> self test. Afterwards it was moved to a location that is inconsistent
> with the FIPS 140-2 requirements.
>
Could you list the commit that move the self test and add that
information in the commit message.
> Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
> seed. This patch resurrects the function drbg_fips_continous_test that
> existed some time ago and applies it to the noise sources.
>
Please identify the commit it was resurrected from, for traceability
purpose.
Regards.
--
Yann Droneaud
OPTEYA
FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
source. Up to kernel 4.8 drivers/char/random.c provided this continuous
self test. Afterwards it was moved to a location that is inconsistent
with the FIPS 140-2 requirements. The relevant patch was
e192be9d9a30555aae2ca1dc3aad37cba484cd4a .
Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
seed. This patch resurrects the function drbg_fips_continous_test that
existed some time ago and applies it to the noise sources. The patch
that removed the drbg_fips_continous_test was
b3614763059b82c26bdd02ffcb1c016c1132aad0 .
The Jitter RNG implements its own FIPS 140-2 self test and thus does not
need to be subjected to the test in the DRBG.
The patch contains a tiny fix to ensure proper zeroization in case of an
error during the Jitter RNG data gathering.
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/drbg.c | 84 +++++++++++++++++++++++++++++++++++++++++--
include/crypto/drbg.h | 2 ++
2 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2a5b16bb000c..8328d7d9b42e 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
}
}
+/*
+ * FIPS 140-2 continuous self test for the noise source
+ * The test is performed on the noise source input data. Thus, the function
+ * implicitly knows the size of the buffer to be equal to the security
+ * strength.
+ *
+ * Note, this function disregards the nonce trailing the entropy data during
+ * initial seeding.
+ *
+ * drbg->drbg_mutex must have been taken.
+ *
+ * @drbg DRBG handle
+ * @entropy buffer of seed data to be checked
+ *
+ * return:
+ * 0 on success
+ * -EAGAIN on when the CTRNG is not yet primed
+ * < 0 on error
+ */
+static int drbg_fips_continuous_test(struct drbg_state *drbg,
+ const unsigned char *entropy)
+{
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
+ int ret = 0;
+
+ /* skip test if we test the overall system */
+ if (list_empty(&drbg->test_data.list))
+ return 0;
+ /* only perform test in FIPS mode */
+ if (!fips_enabled)
+ return 0;
+
+ if (!drbg->fips_primed) {
+ /* Priming of FIPS test */
+ memcpy(drbg->prev, entropy, entropylen);
+ drbg->fips_primed = true;
+ /* priming: another round is needed */
+ return -EAGAIN;
+ }
+ ret = memcmp(drbg->prev, entropy, entropylen);
+ if (!ret)
+ panic("DRBG continuous self test failed\n");
+ memcpy(drbg->prev, entropy, entropylen);
+ /* the test shall pass when the two values are not equal */
+ return (ret != 0) ? 0 : -EFAULT;
+ } else {
+ return 0;
+ }
+}
+
/*
* Convert an integer into a byte representation of this integer.
* The byte representation is big-endian
@@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct *work)
seed_work);
unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
unsigned char entropy[32];
+ int ret;
BUG_ON(!entropylen);
BUG_ON(entropylen > sizeof(entropy));
- get_random_bytes(entropy, entropylen);
drbg_string_fill(&data, entropy, entropylen);
list_add_tail(&data.list, &seedlist);
mutex_lock(&drbg->drbg_mutex);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto unlock;
+ } while (ret);
+
/* If nonblocking pool is initialized, deactivate Jitter RNG */
crypto_free_rng(drbg->jent);
drbg->jent = NULL;
@@ -1030,6 +1088,7 @@ static void drbg_async_seed(struct work_struct *work)
if (drbg->seeded)
drbg->reseed_threshold = drbg_max_requests(drbg);
+unlock:
mutex_unlock(&drbg->drbg_mutex);
memzero_explicit(entropy, entropylen);
@@ -1081,7 +1140,12 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
BUG_ON((entropylen * 2) > sizeof(entropy));
/* Get seed from in-kernel /dev/urandom */
- get_random_bytes(entropy, entropylen);
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ goto out;
+ } while (ret);
if (!drbg->jent) {
drbg_string_fill(&data1, entropy, entropylen);
@@ -1094,7 +1158,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
entropylen);
if (ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
- return ret;
+ goto out;
}
drbg_string_fill(&data1, entropy, entropylen * 2);
@@ -1121,6 +1185,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);
return ret;
@@ -1142,6 +1207,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
drbg->reseed_ctr = 0;
drbg->d_ops = NULL;
drbg->core = NULL;
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ kzfree(drbg->prev);
+ drbg->prev = NULL;
+ drbg->fips_primed = false;
+ }
}
/*
@@ -1211,6 +1281,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
}
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
+ GFP_KERNEL);
+ if (!drbg->prev)
+ goto fini;
+ drbg->fips_primed = false;
+ }
+
return 0;
fini:
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 3fb581bf3b87..8c9af21efce1 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -129,6 +129,8 @@ struct drbg_state {
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
+ bool fips_primed; /* Continuous test primed? */
+ unsigned char *prev; /* FIPS 140-2 continuous test value */
struct work_struct seed_work; /* asynchronous seeding support */
struct crypto_rng *jent;
const struct drbg_state_ops *d_ops;
--
2.21.0
Hi,
Le mardi 07 mai 2019 à 11:29 +0200, Stephan Müller a écrit :
> FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
> source. Up to kernel 4.8 drivers/char/random.c provided this continuous
> self test. Afterwards it was moved to a location that is inconsistent
> with the FIPS 140-2 requirements. The relevant patch was
> e192be9d9a30555aae2ca1dc3aad37cba484cd4a .
>
Please elaborate: in commit e192be9d9a30 ("random: replace non-blocking
pool with a Chacha20-based CRNG") the "self test" code was moved from
extract_entropy() to _extract_entropy(), which is used by
extract_entropy().
Only crng_initialize() call _extract_entropy() with fips = 0, regarless
of fips_enabled.
Is this the issue ?
Could crng_initialize() pass fips_enabled to _extract_entropy() instead
of 0 ?
> Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
> seed. This patch resurrects the function drbg_fips_continous_test that
> existed some time ago and applies it to the noise sources. The patch
> that removed the drbg_fips_continous_test was
> b3614763059b82c26bdd02ffcb1c016c1132aad0 .
>
Thanks for the commit.
> The Jitter RNG implements its own FIPS 140-2 self test and thus does not
> need to be subjected to the test in the DRBG.
>
> The patch contains a tiny fix to ensure proper zeroization in case of an
> error during the Jitter RNG data gathering.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> crypto/drbg.c | 84 +++++++++++++++++++++++++++++++++++++++++--
> include/crypto/drbg.h | 2 ++
> 2 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 2a5b16bb000c..8328d7d9b42e 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
> }
> }
>
> +/*
> + * FIPS 140-2 continuous self test for the noise source
> + * The test is performed on the noise source input data. Thus, the function
> + * implicitly knows the size of the buffer to be equal to the security
> + * strength.
> + *
> + * Note, this function disregards the nonce trailing the entropy data during
> + * initial seeding.
> + *
> + * drbg->drbg_mutex must have been taken.
> + *
> + * @drbg DRBG handle
> + * @entropy buffer of seed data to be checked
> + *
> + * return:
> + * 0 on success
> + * -EAGAIN on when the CTRNG is not yet primed
> + * < 0 on error
> + */
> +static int drbg_fips_continuous_test(struct drbg_state *drbg,
> + const unsigned char *entropy)
> +{
> + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
if (!IS_ENABLED(CONFIG_CRYPTO_FIPS))
return 0;
> + unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
> + int ret = 0;
> +
> + /* skip test if we test the overall system */
> + if (list_empty(&drbg->test_data.list))
> + return 0;
> + /* only perform test in FIPS mode */
> + if (!fips_enabled)
> + return 0;
> +
> + if (!drbg->fips_primed) {
> + /* Priming of FIPS test */
> + memcpy(drbg->prev, entropy, entropylen);
> + drbg->fips_primed = true;
> + /* priming: another round is needed */
> + return -EAGAIN;
> + }
> + ret = memcmp(drbg->prev, entropy, entropylen);
> + if (!ret)
> + panic("DRBG continuous self test failed\n");
Previous version from commit b3614763059b ("crypto: drbg - remove FIPS
140-2 continuous test") already has it, and so does the "self test" in
drivers/char/random.c, but do we really want to panic() in the
unlikely, but still possible, event of a duplicated output from the
PRNG ? The longer the system is up, the likelier this can happen ... if
one can wait for the end of the universe :)
> + memcpy(drbg->prev, entropy, entropylen);
> + /* the test shall pass when the two values are not equal */
> + return (ret != 0) ? 0 : -EFAULT;
Here, it's not possible to have ret == 0, since that would panic(), so
-EFAULT cannot be returned.
> + } else {
> + return 0;
> + }
> +}
> +
> /*
> * Convert an integer into a byte representation of this integer.
> * The byte representation is big-endian
> @@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct *work)
> seed_work);
> unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
> unsigned char entropy[32];
> + int ret;
>
> BUG_ON(!entropylen);
> BUG_ON(entropylen > sizeof(entropy));
> - get_random_bytes(entropy, entropylen);
>
> drbg_string_fill(&data, entropy, entropylen);
> list_add_tail(&data.list, &seedlist);
>
> mutex_lock(&drbg->drbg_mutex);
>
> + do {
> + get_random_bytes(entropy, entropylen);
> + ret = drbg_fips_continuous_test(drbg, entropy);
> + if (ret && ret != -EAGAIN)
> + goto unlock;
> + } while (ret);
> +
A function doing get_random_bytes() and continous_test() would be
useful to both sync and async seed function.
> /* If nonblocking pool is initialized, deactivate Jitter RNG */
> crypto_free_rng(drbg->jent);
> drbg->jent = NULL;
> @@ -1030,6 +1088,7 @@ static void drbg_async_seed(struct work_struct *work)
> if (drbg->seeded)
> drbg->reseed_threshold = drbg_max_requests(drbg);
>
> +unlock:
> mutex_unlock(&drbg->drbg_mutex);
>
> memzero_explicit(entropy, entropylen);
> @@ -1081,7 +1140,12 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
> BUG_ON((entropylen * 2) > sizeof(entropy));
>
> /* Get seed from in-kernel /dev/urandom */
> - get_random_bytes(entropy, entropylen);
> + do {
> + get_random_bytes(entropy, entropylen);
> + ret = drbg_fips_continuous_test(drbg, entropy);
> + if (ret && ret != -EAGAIN)
> + goto out;
> + } while (ret);
>
> if (!drbg->jent) {
> drbg_string_fill(&data1, entropy, entropylen);
> @@ -1094,7 +1158,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
> entropylen);
> if (ret) {
> pr_devel("DRBG: jent failed with %d\n", ret);
> - return ret;
> + goto out;
> }
>
> drbg_string_fill(&data1, entropy, entropylen * 2);
> @@ -1121,6 +1185,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);
>
> return ret;
> @@ -1142,6 +1207,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
> drbg->reseed_ctr = 0;
> drbg->d_ops = NULL;
> drbg->core = NULL;
> + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
> + kzfree(drbg->prev);
> + drbg->prev = NULL;
> + drbg->fips_primed = false;
> + }
> }
>
> /*
> @@ -1211,6 +1281,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
> drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
> }
>
> + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
> + drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
> + GFP_KERNEL);
> + if (!drbg->prev)
> + goto fini;
> + drbg->fips_primed = false;
> + }
> +
> return 0;
>
> fini:
> diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
> index 3fb581bf3b87..8c9af21efce1 100644
> --- a/include/crypto/drbg.h
> +++ b/include/crypto/drbg.h
> @@ -129,6 +129,8 @@ struct drbg_state {
>
> bool seeded; /* DRBG fully seeded? */
> bool pr; /* Prediction resistance enabled? */
> + bool fips_primed; /* Continuous test primed? */
> + unsigned char *prev; /* FIPS 140-2 continuous test value */
> struct work_struct seed_work; /* asynchronous seeding support */
> struct crypto_rng *jent;
> const struct drbg_state_ops *d_ops;
Regards.
--
Yann Droneaud
OPTEYA
Am Dienstag, 7. Mai 2019, 15:10:38 CEST schrieb Yann Droneaud:
Hi Yann,
> Hi,
>
> Le mardi 07 mai 2019 ? 11:29 +0200, Stephan M?ller a ?crit :
> > FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
> > source. Up to kernel 4.8 drivers/char/random.c provided this continuous
> > self test. Afterwards it was moved to a location that is inconsistent
> > with the FIPS 140-2 requirements. The relevant patch was
> > e192be9d9a30555aae2ca1dc3aad37cba484cd4a .
>
> Please elaborate: in commit e192be9d9a30 ("random: replace non-blocking
> pool with a Chacha20-based CRNG") the "self test" code was moved from
> extract_entropy() to _extract_entropy(), which is used by
> extract_entropy().
>
> Only crng_initialize() call _extract_entropy() with fips = 0, regarless
> of fips_enabled.
>
> Is this the issue ?
The issue is that _extract_entropy is invoked with the input_pool from the
ChaCha20 RNG during its initialization or reseed. So, this function is called
to extract data from the input_pool and inject it into the ChaCha20 RNG.
However, we need the test to be applied at the output of the ChaCha20 RNG (or
/dev/random).
>
> Could crng_initialize() pass fips_enabled to _extract_entropy() instead
> of 0 ?
This small change does not fix it. At the time the change to ChaCha20 was
applied, I provided a patch that moved the continuous test back to the
locations were we need it. But it was ignored.
Ciao
Stephan
Am Dienstag, 7. Mai 2019, 15:10:38 CEST schrieb Yann Droneaud:
Hi Yann,
> > + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
>
> if (!IS_ENABLED(CONFIG_CRYPTO_FIPS))
> return 0;
Changed
>
> > + unsigned short entropylen = drbg_sec_strength(drbg->core-
>flags);
> > + int ret = 0;
> > +
> > + /* skip test if we test the overall system */
> > + if (list_empty(&drbg->test_data.list))
> > + return 0;
> > + /* only perform test in FIPS mode */
> > + if (!fips_enabled)
> > + return 0;
> > +
> > + if (!drbg->fips_primed) {
> > + /* Priming of FIPS test */
> > + memcpy(drbg->prev, entropy, entropylen);
> > + drbg->fips_primed = true;
> > + /* priming: another round is needed */
> > + return -EAGAIN;
> > + }
> > + ret = memcmp(drbg->prev, entropy, entropylen);
> > + if (!ret)
> > + panic("DRBG continuous self test failed\n");
>
> Previous version from commit b3614763059b ("crypto: drbg - remove FIPS
> 140-2 continuous test") already has it, and so does the "self test" in
> drivers/char/random.c, but do we really want to panic() in the
> unlikely, but still possible, event of a duplicated output from the
> PRNG ? The longer the system is up, the likelier this can happen ... if
> one can wait for the end of the universe :)
Yes, we want that in FIPS mode.
The requirement is that the "crypto module" (i.e. the kernel crypto API) must
become unavailable if that issue happens.
Commonly this can only be achieved in two ways: either we have a kind of
global flag that effectively deactivates the kernel crypto API or we terminate
the kernel.
>
> > + memcpy(drbg->prev, entropy, entropylen);
> > + /* the test shall pass when the two values are not equal */
> > + return (ret != 0) ? 0 : -EFAULT;
>
> Here, it's not possible to have ret == 0, since that would panic(), so
> -EFAULT cannot be returned.
Agreed.
>
> > + } else {
> > + return 0;
> > + }
> > +}
> > +
> >
> > /*
> >
> > * Convert an integer into a byte representation of this integer.
> > * The byte representation is big-endian
> >
> > @@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct
> > *work)>
> > seed_work);
> >
> > unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
> > unsigned char entropy[32];
> >
> > + int ret;
> >
> > BUG_ON(!entropylen);
> > BUG_ON(entropylen > sizeof(entropy));
> >
> > - get_random_bytes(entropy, entropylen);
> >
> > drbg_string_fill(&data, entropy, entropylen);
> > list_add_tail(&data.list, &seedlist);
> >
> > mutex_lock(&drbg->drbg_mutex);
> >
> > + do {
> > + get_random_bytes(entropy, entropylen);
> > + ret = drbg_fips_continuous_test(drbg, entropy);
> > + if (ret && ret != -EAGAIN)
> > + goto unlock;
> > + } while (ret);
> > +
>
> A function doing get_random_bytes() and continous_test() would be
> useful to both sync and async seed function.
Changed
Thanks.
Ciao
Stephan
FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
source. Up to kernel 4.8 drivers/char/random.c provided this continuous
self test. Afterwards it was moved to a location that is inconsistent
with the FIPS 140-2 requirements. The relevant patch was
e192be9d9a30555aae2ca1dc3aad37cba484cd4a .
Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
seed. This patch resurrects the function drbg_fips_continous_test that
existed some time ago and applies it to the noise sources. The patch
that removed the drbg_fips_continous_test was
b3614763059b82c26bdd02ffcb1c016c1132aad0 .
The Jitter RNG implements its own FIPS 140-2 self test and thus does not
need to be subjected to the test in the DRBG.
The patch contains a tiny fix to ensure proper zeroization in case of an
error during the Jitter RNG data gathering.
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/drbg.c | 94 +++++++++++++++++++++++++++++++++++++++++--
include/crypto/drbg.h | 2 +
2 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2a5b16bb000c..b6929eb5f565 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
}
}
+/*
+ * FIPS 140-2 continuous self test for the noise source
+ * The test is performed on the noise source input data. Thus, the function
+ * implicitly knows the size of the buffer to be equal to the security
+ * strength.
+ *
+ * Note, this function disregards the nonce trailing the entropy data during
+ * initial seeding.
+ *
+ * drbg->drbg_mutex must have been taken.
+ *
+ * @drbg DRBG handle
+ * @entropy buffer of seed data to be checked
+ *
+ * return:
+ * 0 on success
+ * -EAGAIN on when the CTRNG is not yet primed
+ * < 0 on error
+ */
+static int drbg_fips_continuous_test(struct drbg_state *drbg,
+ const unsigned char *entropy)
+{
+ unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
+ int ret = 0;
+
+ if (!IS_ENABLED(CONFIG_CRYPTO_FIPS))
+ return 0;
+
+ /* skip test if we test the overall system */
+ if (list_empty(&drbg->test_data.list))
+ return 0;
+ /* only perform test in FIPS mode */
+ if (!fips_enabled)
+ return 0;
+
+ if (!drbg->fips_primed) {
+ /* Priming of FIPS test */
+ memcpy(drbg->prev, entropy, entropylen);
+ drbg->fips_primed = true;
+ /* priming: another round is needed */
+ return -EAGAIN;
+ }
+ ret = memcmp(drbg->prev, entropy, entropylen);
+ if (!ret)
+ panic("DRBG continuous self test failed\n");
+ memcpy(drbg->prev, entropy, entropylen);
+
+ /* the test shall pass when the two values are not equal */
+ return 0;
+}
+
/*
* Convert an integer into a byte representation of this integer.
* The byte representation is big-endian
@@ -998,6 +1049,22 @@ static inline int __drbg_seed(struct drbg_state *drbg, struct list_head *seed,
return ret;
}
+static inline int drbg_get_random_bytes(struct drbg_state *drbg,
+ unsigned char *entropy,
+ unsigned int entropylen)
+{
+ int ret;
+
+ do {
+ get_random_bytes(entropy, entropylen);
+ ret = drbg_fips_continuous_test(drbg, entropy);
+ if (ret && ret != -EAGAIN)
+ return ret;
+ } while (ret);
+
+ return 0;
+}
+
static void drbg_async_seed(struct work_struct *work)
{
struct drbg_string data;
@@ -1006,16 +1073,20 @@ static void drbg_async_seed(struct work_struct *work)
seed_work);
unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
unsigned char entropy[32];
+ int ret;
BUG_ON(!entropylen);
BUG_ON(entropylen > sizeof(entropy));
- get_random_bytes(entropy, entropylen);
drbg_string_fill(&data, entropy, entropylen);
list_add_tail(&data.list, &seedlist);
mutex_lock(&drbg->drbg_mutex);
+ ret = drbg_get_random_bytes(drbg, entropy, entropylen);
+ if (ret)
+ goto unlock;
+
/* If nonblocking pool is initialized, deactivate Jitter RNG */
crypto_free_rng(drbg->jent);
drbg->jent = NULL;
@@ -1030,6 +1101,7 @@ static void drbg_async_seed(struct work_struct *work)
if (drbg->seeded)
drbg->reseed_threshold = drbg_max_requests(drbg);
+unlock:
mutex_unlock(&drbg->drbg_mutex);
memzero_explicit(entropy, entropylen);
@@ -1081,7 +1153,9 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
BUG_ON((entropylen * 2) > sizeof(entropy));
/* Get seed from in-kernel /dev/urandom */
- get_random_bytes(entropy, entropylen);
+ ret = drbg_get_random_bytes(drbg, entropy, entropylen);
+ if (ret)
+ goto out;
if (!drbg->jent) {
drbg_string_fill(&data1, entropy, entropylen);
@@ -1094,7 +1168,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
entropylen);
if (ret) {
pr_devel("DRBG: jent failed with %d\n", ret);
- return ret;
+ goto out;
}
drbg_string_fill(&data1, entropy, entropylen * 2);
@@ -1121,6 +1195,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);
return ret;
@@ -1142,6 +1217,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
drbg->reseed_ctr = 0;
drbg->d_ops = NULL;
drbg->core = NULL;
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ kzfree(drbg->prev);
+ drbg->prev = NULL;
+ drbg->fips_primed = false;
+ }
}
/*
@@ -1211,6 +1291,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
}
+ if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
+ drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
+ GFP_KERNEL);
+ if (!drbg->prev)
+ goto fini;
+ drbg->fips_primed = false;
+ }
+
return 0;
fini:
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 3fb581bf3b87..8c9af21efce1 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -129,6 +129,8 @@ struct drbg_state {
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
+ bool fips_primed; /* Continuous test primed? */
+ unsigned char *prev; /* FIPS 140-2 continuous test value */
struct work_struct seed_work; /* asynchronous seeding support */
struct crypto_rng *jent;
const struct drbg_state_ops *d_ops;
--
2.21.0
Hi,
Le mercredi 08 mai 2019 à 16:19 +0200, Stephan Mueller a écrit :
> FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
> source. Up to kernel 4.8 drivers/char/random.c provided this continuous
> self test. Afterwards it was moved to a location that is inconsistent
> with the FIPS 140-2 requirements. The relevant patch was
> e192be9d9a30555aae2ca1dc3aad37cba484cd4a .
>
> Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
> seed. This patch resurrects the function drbg_fips_continous_test that
> existed some time ago and applies it to the noise sources. The patch
> that removed the drbg_fips_continous_test was
> b3614763059b82c26bdd02ffcb1c016c1132aad0 .
>
> The Jitter RNG implements its own FIPS 140-2 self test and thus does not
> need to be subjected to the test in the DRBG.
>
> The patch contains a tiny fix to ensure proper zeroization in case of an
> error during the Jitter RNG data gathering.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> crypto/drbg.c | 94 +++++++++++++++++++++++++++++++++++++++++--
> include/crypto/drbg.h | 2 +
> 2 files changed, 93 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 2a5b16bb000c..b6929eb5f565 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -219,6 +219,57 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
> }
> }
>
> +/*
> + * FIPS 140-2 continuous self test for the noise source
> + * The test is performed on the noise source input data. Thus, the function
> + * implicitly knows the size of the buffer to be equal to the security
> + * strength.
> + *
> + * Note, this function disregards the nonce trailing the entropy data during
> + * initial seeding.
> + *
> + * drbg->drbg_mutex must have been taken.
> + *
> + * @drbg DRBG handle
> + * @entropy buffer of seed data to be checked
> + *
> + * return:
> + * 0 on success
> + * -EAGAIN on when the CTRNG is not yet primed
> + * < 0 on error
> + */
> +static int drbg_fips_continuous_test(struct drbg_state *drbg,
> + const unsigned char *entropy)
> +{
> + unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
> + int ret = 0;
> +
> + if (!IS_ENABLED(CONFIG_CRYPTO_FIPS))
> + return 0;
> +
> + /* skip test if we test the overall system */
> + if (list_empty(&drbg->test_data.list))
> + return 0;
> + /* only perform test in FIPS mode */
> + if (!fips_enabled)
> + return 0;
> +
> + if (!drbg->fips_primed) {
> + /* Priming of FIPS test */
> + memcpy(drbg->prev, entropy, entropylen);
> + drbg->fips_primed = true;
> + /* priming: another round is needed */
> + return -EAGAIN;
> + }
> + ret = memcmp(drbg->prev, entropy, entropylen);
> + if (!ret)
> + panic("DRBG continuous self test failed\n");
> + memcpy(drbg->prev, entropy, entropylen);
> +
> + /* the test shall pass when the two values are not equal */
> + return 0;
> +}
> +
> /*
> * Convert an integer into a byte representation of this integer.
> * The byte representation is big-endian
> @@ -998,6 +1049,22 @@ static inline int __drbg_seed(struct drbg_state *drbg, struct list_head *seed,
> return ret;
> }
>
> +static inline int drbg_get_random_bytes(struct drbg_state *drbg,
> + unsigned char *entropy,
> + unsigned int entropylen)
> +{
> + int ret;
> +
> + do {
> + get_random_bytes(entropy, entropylen);
> + ret = drbg_fips_continuous_test(drbg, entropy);
> + if (ret && ret != -EAGAIN)
> + return ret;
> + } while (ret);
> +
> + return 0;
> +}
> +
> static void drbg_async_seed(struct work_struct *work)
> {
> struct drbg_string data;
> @@ -1006,16 +1073,20 @@ static void drbg_async_seed(struct work_struct *work)
> seed_work);
> unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
> unsigned char entropy[32];
> + int ret;
>
> BUG_ON(!entropylen);
> BUG_ON(entropylen > sizeof(entropy));
> - get_random_bytes(entropy, entropylen);
>
> drbg_string_fill(&data, entropy, entropylen);
> list_add_tail(&data.list, &seedlist);
>
> mutex_lock(&drbg->drbg_mutex);
>
> + ret = drbg_get_random_bytes(drbg, entropy, entropylen);
> + if (ret)
> + goto unlock;
> +
> /* If nonblocking pool is initialized, deactivate Jitter RNG */
> crypto_free_rng(drbg->jent);
> drbg->jent = NULL;
> @@ -1030,6 +1101,7 @@ static void drbg_async_seed(struct work_struct *work)
> if (drbg->seeded)
> drbg->reseed_threshold = drbg_max_requests(drbg);
>
> +unlock:
> mutex_unlock(&drbg->drbg_mutex);
>
> memzero_explicit(entropy, entropylen);
> @@ -1081,7 +1153,9 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
> BUG_ON((entropylen * 2) > sizeof(entropy));
>
> /* Get seed from in-kernel /dev/urandom */
> - get_random_bytes(entropy, entropylen);
> + ret = drbg_get_random_bytes(drbg, entropy, entropylen);
> + if (ret)
> + goto out;
>
> if (!drbg->jent) {
> drbg_string_fill(&data1, entropy, entropylen);
> @@ -1094,7 +1168,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
> entropylen);
> if (ret) {
> pr_devel("DRBG: jent failed with %d\n", ret);
> - return ret;
> + goto out;
> }
>
> drbg_string_fill(&data1, entropy, entropylen * 2);
> @@ -1121,6 +1195,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);
>
> return ret;
> @@ -1142,6 +1217,11 @@ static inline void drbg_dealloc_state(struct drbg_state *drbg)
> drbg->reseed_ctr = 0;
> drbg->d_ops = NULL;
> drbg->core = NULL;
> + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
> + kzfree(drbg->prev);
> + drbg->prev = NULL;
> + drbg->fips_primed = false;
> + }
> }
>
> /*
> @@ -1211,6 +1291,14 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
> drbg->scratchpad = PTR_ALIGN(drbg->scratchpadbuf, ret + 1);
> }
>
> + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
> + drbg->prev = kzalloc(drbg_sec_strength(drbg->core->flags),
> + GFP_KERNEL);
> + if (!drbg->prev)
> + goto fini;
> + drbg->fips_primed = false;
> + }
> +
> return 0;
>
> fini:
> diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
> index 3fb581bf3b87..8c9af21efce1 100644
> --- a/include/crypto/drbg.h
> +++ b/include/crypto/drbg.h
> @@ -129,6 +129,8 @@ struct drbg_state {
>
> bool seeded; /* DRBG fully seeded? */
> bool pr; /* Prediction resistance enabled? */
> + bool fips_primed; /* Continuous test primed? */
> + unsigned char *prev; /* FIPS 140-2 continuous test value */
> struct work_struct seed_work; /* asynchronous seeding support */
> struct crypto_rng *jent;
> const struct drbg_state_ops *d_ops;
Thanks.
Reviewed-by: Yann Droneaud <[email protected]>
--
Yann Droneaud
OPTEYA
On Wed, May 08, 2019 at 04:19:24PM +0200, Stephan Mueller wrote:
> FIPS 140-2 section 4.9.2 requires a continuous self test of the noise
> source. Up to kernel 4.8 drivers/char/random.c provided this continuous
> self test. Afterwards it was moved to a location that is inconsistent
> with the FIPS 140-2 requirements. The relevant patch was
> e192be9d9a30555aae2ca1dc3aad37cba484cd4a .
>
> Thus, the FIPS 140-2 CTRNG is added to the DRBG when it obtains the
> seed. This patch resurrects the function drbg_fips_continous_test that
> existed some time ago and applies it to the noise sources. The patch
> that removed the drbg_fips_continous_test was
> b3614763059b82c26bdd02ffcb1c016c1132aad0 .
>
> The Jitter RNG implements its own FIPS 140-2 self test and thus does not
> need to be subjected to the test in the DRBG.
>
> The patch contains a tiny fix to ensure proper zeroization in case of an
> error during the Jitter RNG data gathering.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> crypto/drbg.c | 94 +++++++++++++++++++++++++++++++++++++++++--
> include/crypto/drbg.h | 2 +
> 2 files changed, 93 insertions(+), 3 deletions(-)
Patch 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