[PATCH] random: prime last_data value per fips requirements

[PATCH] random: prime last_data value per fips requirements

[Jarod Wilson]

The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
first use, either on system startup or after an RNDCLEARPOOL ioctl, we
need to take an initial random sample, store it internally in last_data,
then pass along the value after that to the requester, so that consistency
checks aren't being run against stale and possibly known data.

CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: "David S. Miller" <davem@davemloft.net>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: Matt Mackall <mpm@selenic.com>
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/char/random.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b86eae9..24d17b8 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -437,6 +437,7 @@ struct entropy_store {
 	int entropy_count;
 	int entropy_total;
 	unsigned int initialized:1;
+	bool last_data_init;
 	__u8 last_data[EXTRACT_SIZE];
 };
 
@@ -967,6 +968,15 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 		if (fips_enabled) {
 			unsigned long flags;
 
+			/* prime last_data value if need be, per fips 140-2 */
+			if (!r->last_data_init) {
+				spin_lock_irqsave(&r->lock, flags);
+				memcpy(r->last_data, tmp, EXTRACT_SIZE);
+				r->last_data_init = true;
+				spin_unlock_irqrestore(&r->lock, flags);
+				continue;
+			}
+
 			spin_lock_irqsave(&r->lock, flags);
 			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
 				panic("Hardware RNG duplicated output!\n");
@@ -1086,6 +1096,7 @@ static void init_std_data(struct entropy_store *r)
 
 	r->entropy_count = 0;
 	r->entropy_total = 0;
+	r->last_data_init = false;
 	mix_pool_bytes(r, &now, sizeof(now), NULL);
 	for (i = r->poolinfo->POOLBYTES; i > 0; i -= sizeof(rv)) {
 		if (!arch_get_random_long(&rv))
-- 
1.7.1

Re: [PATCH] random: prime last_data value per fips requirements

[Neil Horman]

On Mon, Nov 05, 2012 at 04:00:10PM -0500, Jarod Wilson wrote:
> The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
> first use, either on system startup or after an RNDCLEARPOOL ioctl, we
> need to take an initial random sample, store it internally in last_data,
> then pass along the value after that to the requester, so that consistency
> checks aren't being run against stale and possibly known data.
> 
> CC: Herbert Xu <herbert@gondor.apana.org.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Neil Horman <nhorman@tuxdriver.com>
> CC: Matt Mackall <mpm@selenic.com>
> CC: linux-crypto@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
>  drivers/char/random.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index b86eae9..24d17b8 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -437,6 +437,7 @@ struct entropy_store {
>  	int entropy_count;
>  	int entropy_total;
>  	unsigned int initialized:1;
> +	bool last_data_init;
>  	__u8 last_data[EXTRACT_SIZE];
>  };
>  
> @@ -967,6 +968,15 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
>  		if (fips_enabled) {
>  			unsigned long flags;
>  
> +			/* prime last_data value if need be, per fips 140-2 */
> +			if (!r->last_data_init) {
> +				spin_lock_irqsave(&r->lock, flags);
> +				memcpy(r->last_data, tmp, EXTRACT_SIZE);
> +				r->last_data_init = true;
> +				spin_unlock_irqrestore(&r->lock, flags);
> +				continue;
Continue?  Is that left over from earlier work?  Or did you have some other
purpose in mind for it?

Also, not that its in a hot path or anything, but it might be nice to
consolodate this code such that you only lock and unlock r->flags once instead
of twice here.

Best
Neil

> +			}
> +
>  			spin_lock_irqsave(&r->lock, flags);
>  			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
>  				panic("Hardware RNG duplicated output!\n");
> @@ -1086,6 +1096,7 @@ static void init_std_data(struct entropy_store *r)
>  
>  	r->entropy_count = 0;
>  	r->entropy_total = 0;
> +	r->last_data_init = false;
>  	mix_pool_bytes(r, &now, sizeof(now), NULL);
>  	for (i = r->poolinfo->POOLBYTES; i > 0; i -= sizeof(rv)) {
>  		if (!arch_get_random_long(&rv))
> -- 
> 1.7.1
> 
> 

Re: [PATCH] random: prime last_data value per fips requirements

[Jarod Wilson]

On Tue, Nov 06, 2012 at 07:05:23AM -0500, Neil Horman wrote:
> On Mon, Nov 05, 2012 at 04:00:10PM -0500, Jarod Wilson wrote:
> > The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
> > first use, either on system startup or after an RNDCLEARPOOL ioctl, we
> > need to take an initial random sample, store it internally in last_data,
> > then pass along the value after that to the requester, so that consistency
> > checks aren't being run against stale and possibly known data.
> > 
> > CC: Herbert Xu <herbert@gondor.apana.org.au>
> > CC: "David S. Miller" <davem@davemloft.net>
> > CC: Neil Horman <nhorman@tuxdriver.com>
> > CC: Matt Mackall <mpm@selenic.com>
> > CC: linux-crypto@vger.kernel.org
> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > ---
> >  drivers/char/random.c |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/char/random.c b/drivers/char/random.c
> > index b86eae9..24d17b8 100644
> > --- a/drivers/char/random.c
> > +++ b/drivers/char/random.c
> > @@ -437,6 +437,7 @@ struct entropy_store {
> >  	int entropy_count;
> >  	int entropy_total;
> >  	unsigned int initialized:1;
> > +	bool last_data_init;
> >  	__u8 last_data[EXTRACT_SIZE];
> >  };
> >  
> > @@ -967,6 +968,15 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
> >  		if (fips_enabled) {
> >  			unsigned long flags;
> >  
> > +			/* prime last_data value if need be, per fips 140-2 */
> > +			if (!r->last_data_init) {
> > +				spin_lock_irqsave(&r->lock, flags);
> > +				memcpy(r->last_data, tmp, EXTRACT_SIZE);
> > +				r->last_data_init = true;
> > +				spin_unlock_irqrestore(&r->lock, flags);
> > +				continue;
> Continue?  Is that left over from earlier work?  Or did you have some other
> purpose in mind for it?

The continue takes you back to the top of the while loop for another
extract_buf() call, but continue could simply be replaced with another
extract_buf() call, so we don't have to restart the loop and check
last_data_init again. Otherwise, we're going to fail the memcmp and panic,
because tmp and r->last_data will be identical.

> Also, not that its in a hot path or anything, but it might be nice to
> consolodate this code such that you only lock and unlock r->flags once instead
> of twice here.

I thought about that, but figured it would be more trouble and possibly
more code to execute than it was worth in the normal case. But I can spin
up a v2 that tries to be a bit cleaner here.

-- 
Jarod Wilson
jarod@redhat.com

[PATCH v2] random: prime last_data value per fips requirements

[Jarod Wilson]

The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
first use, either on system startup or after an RNDCLEARPOOL ioctl, we
need to take an initial random sample, store it internally in last_data,
then pass along the value after that to the requester, so that consistency
checks aren't being run against stale and possibly known data.

v2: streamline code flow a bit, eliminating extra loop and spinlock in the
case where we need to prime, and account for the extra primer bits.

CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: "David S. Miller" <davem@davemloft.net>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: Matt Mackall <mpm@selenic.com>
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/char/random.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b86eae9..6bdd57f 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -437,6 +437,7 @@ struct entropy_store {
 	int entropy_count;
 	int entropy_total;
 	unsigned int initialized:1;
+	bool last_data_init;
 	__u8 last_data[EXTRACT_SIZE];
 };
 
@@ -957,6 +958,10 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 	ssize_t ret = 0, i;
 	__u8 tmp[EXTRACT_SIZE];
 
+	/* if last_data isn't primed, we need EXTRACT_SIZE extra bytes */
+	if (fips_enabled && !r->last_data_init)
+		nbytes += EXTRACT_SIZE;
+
 	trace_extract_entropy(r->name, nbytes, r->entropy_count, _RET_IP_);
 	xfer_secondary_pool(r, nbytes);
 	nbytes = account(r, nbytes, min, reserved);
@@ -968,6 +973,15 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 			unsigned long flags;
 
 			spin_lock_irqsave(&r->lock, flags);
+
+			/* prime last_data value if need be, per fips 140-2 */
+			if (!r->last_data_init) {
+				memcpy(r->last_data, tmp, EXTRACT_SIZE);
+				r->last_data_init = true;
+				nbytes -= EXTRACT_SIZE;
+				extract_buf(r, tmp);
+			}
+
 			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
 				panic("Hardware RNG duplicated output!\n");
 			memcpy(r->last_data, tmp, EXTRACT_SIZE);
@@ -1086,6 +1100,7 @@ static void init_std_data(struct entropy_store *r)
 
 	r->entropy_count = 0;
 	r->entropy_total = 0;
+	r->last_data_init = false;
 	mix_pool_bytes(r, &now, sizeof(now), NULL);
 	for (i = r->poolinfo->POOLBYTES; i > 0; i -= sizeof(rv)) {
 		if (!arch_get_random_long(&rv))
-- 
1.7.1

[PATCH v3] random: prime last_data value per fips requirements

[Jarod Wilson]

The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
first use, either on system startup or after an RNDCLEARPOOL ioctl, we
need to take an initial random sample, store it internally in last_data,
then pass along the value after that to the requester, so that consistency
checks aren't being run against stale and possibly known data.

v2: streamline code flow a bit, eliminating extra loop and spinlock in the
case where we need to prime, and account for the extra primer bits.

v3: extract_buf() can't be called with spinlock already held, so bring
back some extra lock/unlock calls.

CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: "David S. Miller" <davem@davemloft.net>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: Matt Mackall <mpm@selenic.com>
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/char/random.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b86eae9..d0139df 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -437,6 +437,7 @@ struct entropy_store {
 	int entropy_count;
 	int entropy_total;
 	unsigned int initialized:1;
+	bool last_data_init;
 	__u8 last_data[EXTRACT_SIZE];
 };
 
@@ -957,6 +958,10 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 	ssize_t ret = 0, i;
 	__u8 tmp[EXTRACT_SIZE];
 
+	/* if last_data isn't primed, we need EXTRACT_SIZE extra bytes */
+	if (fips_enabled && !r->last_data_init)
+		nbytes += EXTRACT_SIZE;
+
 	trace_extract_entropy(r->name, nbytes, r->entropy_count, _RET_IP_);
 	xfer_secondary_pool(r, nbytes);
 	nbytes = account(r, nbytes, min, reserved);
@@ -967,6 +972,17 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 		if (fips_enabled) {
 			unsigned long flags;
 
+
+			/* prime last_data value if need be, per fips 140-2 */
+			if (!r->last_data_init) {
+				spin_lock_irqsave(&r->lock, flags);
+				memcpy(r->last_data, tmp, EXTRACT_SIZE);
+				r->last_data_init = true;
+				nbytes -= EXTRACT_SIZE;
+				spin_unlock_irqrestore(&r->lock, flags);
+				extract_buf(r, tmp);
+			}
+
 			spin_lock_irqsave(&r->lock, flags);
 			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
 				panic("Hardware RNG duplicated output!\n");
@@ -1086,6 +1102,7 @@ static void init_std_data(struct entropy_store *r)
 
 	r->entropy_count = 0;
 	r->entropy_total = 0;
+	r->last_data_init = false;
 	mix_pool_bytes(r, &now, sizeof(now), NULL);
 	for (i = r->poolinfo->POOLBYTES; i > 0; i -= sizeof(rv)) {
 		if (!arch_get_random_long(&rv))
-- 
1.7.1

Re: [PATCH v3] random: prime last_data value per fips requirements

[Neil Horman]

On Tue, Nov 06, 2012 at 10:42:42AM -0500, Jarod Wilson wrote:
> The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
> first use, either on system startup or after an RNDCLEARPOOL ioctl, we
> need to take an initial random sample, store it internally in last_data,
> then pass along the value after that to the requester, so that consistency
> checks aren't being run against stale and possibly known data.
> 
> v2: streamline code flow a bit, eliminating extra loop and spinlock in the
> case where we need to prime, and account for the extra primer bits.
> 
> v3: extract_buf() can't be called with spinlock already held, so bring
> back some extra lock/unlock calls.
> 
> CC: Herbert Xu <herbert@gondor.apana.org.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Neil Horman <nhorman@tuxdriver.com>
> CC: Matt Mackall <mpm@selenic.com>
> CC: linux-crypto@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
>  drivers/char/random.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index b86eae9..d0139df 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -437,6 +437,7 @@ struct entropy_store {
>  	int entropy_count;
>  	int entropy_total;
>  	unsigned int initialized:1;
> +	bool last_data_init;
>  	__u8 last_data[EXTRACT_SIZE];
>  };
>  
> @@ -957,6 +958,10 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
>  	ssize_t ret = 0, i;
>  	__u8 tmp[EXTRACT_SIZE];
>  
> +	/* if last_data isn't primed, we need EXTRACT_SIZE extra bytes */
> +	if (fips_enabled && !r->last_data_init)
> +		nbytes += EXTRACT_SIZE;
> +
>  	trace_extract_entropy(r->name, nbytes, r->entropy_count, _RET_IP_);
>  	xfer_secondary_pool(r, nbytes);
>  	nbytes = account(r, nbytes, min, reserved);
> @@ -967,6 +972,17 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
>  		if (fips_enabled) {
>  			unsigned long flags;
>  
> +
> +			/* prime last_data value if need be, per fips 140-2 */
> +			if (!r->last_data_init) {
> +				spin_lock_irqsave(&r->lock, flags);
> +				memcpy(r->last_data, tmp, EXTRACT_SIZE);
> +				r->last_data_init = true;
> +				nbytes -= EXTRACT_SIZE;
> +				spin_unlock_irqrestore(&r->lock, flags);
> +				extract_buf(r, tmp);
> +			}
> +
>  			spin_lock_irqsave(&r->lock, flags);
>  			if (!memcmp(tmp, r->last_data, EXTRACT_SIZE))
>  				panic("Hardware RNG duplicated output!\n");
> @@ -1086,6 +1102,7 @@ static void init_std_data(struct entropy_store *r)
>  
>  	r->entropy_count = 0;
>  	r->entropy_total = 0;
> +	r->last_data_init = false;
>  	mix_pool_bytes(r, &now, sizeof(now), NULL);
>  	for (i = r->poolinfo->POOLBYTES; i > 0; i -= sizeof(rv)) {
>  		if (!arch_get_random_long(&rv))
> -- 
> 1.7.1
> 
> 
Thanks Jarod.
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH v3] random: prime last_data value per fips requirements

[Theodore Ts'o]

On Tue, Nov 06, 2012 at 10:42:42AM -0500, Jarod Wilson wrote:
> The value stored in last_data must be primed for FIPS 140-2 purposes. Upon
> first use, either on system startup or after an RNDCLEARPOOL ioctl, we
> need to take an initial random sample, store it internally in last_data,
> then pass along the value after that to the requester, so that consistency
> checks aren't being run against stale and possibly known data.
> 
> v2: streamline code flow a bit, eliminating extra loop and spinlock in the
> case where we need to prime, and account for the extra primer bits.
> 
> v3: extract_buf() can't be called with spinlock already held, so bring
> back some extra lock/unlock calls.
> 
> CC: Herbert Xu <herbert@gondor.apana.org.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Neil Horman <nhorman@tuxdriver.com>
> CC: Matt Mackall <mpm@selenic.com>
> CC: linux-crypto@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>

Thanks, applied to the /dev/random git tree.

						- Ted