diff mbox series

[v3] crypto: DRBG - add FIPS 140-2 CTRNG for noise source

Message ID 5352150.0CmBXKFm2E@positron.chronox.de (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series [v3] crypto: DRBG - add FIPS 140-2 CTRNG for noise source | expand

Commit Message

Stephan Mueller May 2, 2019, 4:38 p.m. UTC
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 <smueller@chronox.de>
---
 crypto/drbg.c         | 84 +++++++++++++++++++++++++++++++++++++++++--
 include/crypto/drbg.h |  4 +++
 2 files changed, 85 insertions(+), 3 deletions(-)

Comments

Herbert Xu May 3, 2019, 1:42 a.m. UTC | #1
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,
Stephan Mueller May 3, 2019, 5:11 a.m. UTC | #2
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
Herbert Xu May 3, 2019, 6:08 a.m. UTC | #3
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,
diff mbox series

Patch

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;