2009-09-16 16:05:04

by Neil Horman

[permalink] [raw]
Subject: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

Hey all-
Ok, so I've got a story behind this one. It was recently called to my
attention that the ansi cprng is missing an aspect of its compliance requrements
for FIPS-140. Specifically, its missing a behavior in its continuous test.
When the CPRNG produces random blocks, the firrst block that it produces must
never be returned to the user. Instead it must be saved and a second block must
be generated so that it can be compared to the first block before being returned
to the user.

I recently posted a patch to do this for the hardware RNG. Its fine to
do this there, since there are no expectations of a predictable result in that
RNG. The CPRNG however, provides a predictable random sequence for a given
input seed key and iteration. The above requirement messes with that
predictability however because it changes which block is returned on the zeroth
iteration to the user. Some test vectors expect this, some do not.

So the question is, how do I make this RNG fips compliant without
breaking some subset of users out there that rely on the predictability of the
CPRNG? The solution I've come up with is a dynamic flag. This patch series
adds two api calls to the crypto RNG api rng_set_flags and rng_get_flags, which
set flags with global meaning on instances of an rng. A given RNG can opt to
set the registered agorithm methods for these api calls or not. In the event
they don't a default handler is set for each that returns EOPNOTSUPPORT.

Using this new mechanism I've implemented these calls in ansi_cprng so
that setting the TEST_MODE flag disables the continuous check, allowing for the
zeroth block to get returned to the user, which lets us pass most of the
supplied test vectors (most notably the NIST provided vectors).

Signed-off-by: Neil Horman <[email protected]>




2009-09-16 16:11:20

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 1/3] add RNG api calls to set common flags

patch 1/3: Add flags infrastructure to rng api

This patch adds api calls for get/set flags calls to the crypto rng api. This
api allows algorithm implementations to register calls to respond to flag
settings that are global and common to all rng's. If a given algorithm has no
external flags that it needs to respond to, it can opt to not register any
calls, and as a result a default handler will be registered for each which
universally returns EOPNOTSUPPORT.

Signed-off-by: Neil Horman <[email protected]>


crypto/rng.c | 13 +++++++++++++
include/crypto/rng.h | 21 +++++++++++++++++++++
include/linux/crypto.h | 9 +++++++++
3 files changed, 43 insertions(+)

diff --git a/crypto/rng.c b/crypto/rng.c
index ba05e73..98f10b1 100644
--- a/crypto/rng.c
+++ b/crypto/rng.c
@@ -46,6 +46,17 @@ static int rngapi_reset(struct crypto_rng *tfm, u8 *seed, unsigned int slen)
return err;
}

+static int rngapi_set_flags(struct crypto_rng *tfm, u8 flags)
+{
+ return -EOPNOTSUPP;
+}
+
+static int rngapi_get_flags(struct crypto_rng *tfm, u8 *flags)
+{
+ return -EOPNOTSUPP;
+}
+
+
static int crypto_init_rng_ops(struct crypto_tfm *tfm, u32 type, u32 mask)
{
struct rng_alg *alg = &tfm->__crt_alg->cra_rng;
@@ -53,6 +64,8 @@ static int crypto_init_rng_ops(struct crypto_tfm *tfm, u32 type, u32 mask)

ops->rng_gen_random = alg->rng_make_random;
ops->rng_reset = rngapi_reset;
+ ops->rng_set_flags = alg->rng_set_flags ? : rngapi_set_flags;
+ ops->rng_get_flags = alg->rng_get_flags ? : rngapi_get_flags;

return 0;
}
diff --git a/include/crypto/rng.h b/include/crypto/rng.h
index c93f9b9..a639955 100644
--- a/include/crypto/rng.h
+++ b/include/crypto/rng.h
@@ -15,6 +15,17 @@

#include <linux/crypto.h>

+/*
+ * RNG behavioral flags
+ * CRYPTO_RNG_TEST_MODE
+ * places the RNG into a test mode for various certification tests. Some
+ * RNG's (most notably Deterministic RNGs) Can have internal tests which are
+ * required in normal operation mode, but affect the deterministic output
+ * of the RNG which throws some test vectors off, as they may not account for
+ * these tests. This flag allows us to disable the internal tests of an RNG.
+ */
+#define CRYPTO_RNG_TEST_MODE 0x01
+
extern struct crypto_rng *crypto_default_rng;

int crypto_get_default_rng(void);
@@ -72,4 +83,14 @@ static inline int crypto_rng_seedsize(struct crypto_rng *tfm)
return crypto_rng_alg(tfm)->seedsize;
}

+static inline int crypto_rng_set_flags(struct crypto_rng *tfm, u8 flags)
+{
+ return crypto_rng_alg(tfm)->rng_set_flags(tfm, flags);
+}
+
+static inline int crypto_rng_get_flags(struct crypto_rng *tfm, u8 *flags)
+{
+ return crypto_rng_alg(tfm)->rng_get_flags(tfm, flags);
+}
+
#endif
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index fd92988..6af8d2f 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -285,6 +285,10 @@ struct rng_alg {
unsigned int dlen);
int (*rng_reset)(struct crypto_rng *tfm, u8 *seed, unsigned int slen);

+ int (*rng_set_flags)(struct crypto_rng *tfm, u8 flags);
+
+ int (*rng_get_flags)(struct crypto_rng *tfm, u8 *flags);
+
unsigned int seedsize;
};

@@ -421,6 +425,11 @@ struct rng_tfm {
int (*rng_gen_random)(struct crypto_rng *tfm, u8 *rdata,
unsigned int dlen);
int (*rng_reset)(struct crypto_rng *tfm, u8 *seed, unsigned int slen);
+
+ int (*rng_set_flags)(struct crypto_rng *tfm, u8 flags);
+
+ int (*rng_get_flags)(struct crypto_rng *tfm, u8 *flags);
+
};

#define crt_ablkcipher crt_u.ablkcipher

2009-09-16 16:13:37

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 2/3] augment the testmgr code to set TEST_MODE flag on all rng instances

patch 2/3: Update testmgr code to place any rng it tests in TEST_MODE

This patch instructs the testmgr code to place all rng allocations that it makes
into test mode, so that in the event that it has internal mechanisms that may
affect the testing of the RNG, they won't affect the outcome of the test they
are preforming.

Signed-off-by: Neil Horman <[email protected]>


testmgr.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 6d5b746..89ea8c1 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1470,6 +1470,8 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
return PTR_ERR(rng);
}

+ crypto_rng_set_flags(rng, CRYPTO_RNG_TEST_MODE);
+
err = test_cprng(rng, desc->suite.cprng.vecs, desc->suite.cprng.count);

crypto_free_rng(rng);

2009-09-16 16:26:05

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 3/3] augment CPRNG to correctly implement continuous test for FIPS, and support TEST_MODE flags

patch 3/3: modify cprng to make contnuity check fips compliant and allow for a
disabling of the continuity test when the RNG is placed in FIPS mode

Signed-off-by: Neil Horman <[email protected]>


ansi_cprng.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 3aa6e38..c1ba5e8 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -33,6 +33,7 @@

#define PRNG_FIXED_SIZE 0x1
#define PRNG_NEED_RESET 0x2
+#define PRNG_DISABLE_CONT_TEST 0x3

/*
* Note: DT is our counter value
@@ -85,7 +86,7 @@ static void xor_vectors(unsigned char *in1, unsigned char *in2,
* Returns DEFAULT_BLK_SZ bytes of random data per call
* returns 0 if generation succeded, <0 if something went wrong
*/
-static int _get_more_prng_bytes(struct prng_context *ctx)
+static int _get_more_prng_bytes(struct prng_context *ctx, int test)
{
int i;
unsigned char tmp[DEFAULT_BLK_SZ];
@@ -130,6 +131,9 @@ static int _get_more_prng_bytes(struct prng_context *ctx)
* First check that we didn't produce the same
* random data that we did last time around through this
*/
+ if (ctx->flags & PRNG_DISABLE_CONT_TEST)
+ goto skip_test;
+
if (!memcmp(ctx->rand_data, ctx->last_rand_data,
DEFAULT_BLK_SZ)) {
if (fips_enabled) {
@@ -146,7 +150,7 @@ static int _get_more_prng_bytes(struct prng_context *ctx)
}
memcpy(ctx->last_rand_data, ctx->rand_data,
DEFAULT_BLK_SZ);
-
+skip_test:
/*
* Lastly xor the random data with I
* and encrypt that to obtain a new secret vector V
@@ -220,7 +224,7 @@ static int get_prng_bytes(char *buf, size_t nbytes, struct prng_context *ctx)

remainder:
if (ctx->rand_data_valid == DEFAULT_BLK_SZ) {
- if (_get_more_prng_bytes(ctx) < 0) {
+ if (_get_more_prng_bytes(ctx, 1) < 0) {
memset(buf, 0, nbytes);
err = -EINVAL;
goto done;
@@ -247,7 +251,7 @@ empty_rbuf:
*/
for (; byte_count >= DEFAULT_BLK_SZ; byte_count -= DEFAULT_BLK_SZ) {
if (ctx->rand_data_valid == DEFAULT_BLK_SZ) {
- if (_get_more_prng_bytes(ctx) < 0) {
+ if (_get_more_prng_bytes(ctx, 1) < 0) {
memset(buf, 0, nbytes);
err = -EINVAL;
goto done;
@@ -306,7 +310,6 @@ static int reset_prng_context(struct prng_context *ctx,
memset(ctx->rand_data, 0, DEFAULT_BLK_SZ);
memset(ctx->last_rand_data, 0, DEFAULT_BLK_SZ);

- ctx->rand_data_valid = DEFAULT_BLK_SZ;

ret = crypto_cipher_setkey(ctx->tfm, prng_key, klen);
if (ret) {
@@ -317,6 +320,18 @@ static int reset_prng_context(struct prng_context *ctx,

ret = 0;
ctx->flags &= ~PRNG_NEED_RESET;
+
+ /*
+ * If we don't disable the continuity test
+ * we need to seed the n=0 iteration for test
+ * comparison, so we get a first block here that
+ * we never return to the user
+ */
+ ctx->rand_data_valid = DEFAULT_BLK_SZ;
+ if (!(ctx->flags & PRNG_DISABLE_CONT_TEST))
+ _get_more_prng_bytes(ctx, 0);
+
+ ctx->rand_data_valid = DEFAULT_BLK_SZ;
out:
spin_unlock_bh(&ctx->prng_lock);
return ret;
@@ -384,6 +399,35 @@ static int cprng_reset(struct crypto_rng *tfm, u8 *seed, unsigned int slen)
return 0;
}

+ /*
+ * These are the registered functions which export behavioral flags
+ * to the crypto api. Most rng's don't have flags, but some might, like
+ * the cprng which implements a 'test mode' for validation of vecotors
+ * which disables the internal continuity tests
+ */
+static int cprng_set_flags(struct crypto_rng *tfm, u8 flags)
+{
+ struct prng_context *prng = crypto_rng_ctx(tfm);
+
+ if (flags & CRYPTO_RNG_TEST_MODE)
+ prng->flags |= PRNG_DISABLE_CONT_TEST;
+
+ return 0;
+}
+
+ static int cprng_get_flags(struct crypto_rng *tfm, u8 *flags)
+{
+ struct prng_context *prng = crypto_rng_ctx(tfm);
+
+ *flags = 0;
+
+ if (prng->flags & PRNG_DISABLE_CONT_TEST)
+ *flags |= CRYPTO_RNG_TEST_MODE;
+
+ return 0;
+}
+
+
static struct crypto_alg rng_alg = {
.cra_name = "stdrng",
.cra_driver_name = "ansi_cprng",
@@ -399,6 +443,8 @@ static struct crypto_alg rng_alg = {
.rng = {
.rng_make_random = cprng_get_random,
.rng_reset = cprng_reset,
+ .rng_set_flags = cprng_set_flags,
+ .rng_get_flags = cprng_get_flags,
.seedsize = DEFAULT_PRNG_KSZ + 2*DEFAULT_BLK_SZ,
}
}

2009-09-16 20:59:03

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On 09/16/2009 12:04 PM, Neil Horman wrote:
> Hey all-
> Ok, so I've got a story behind this one. It was recently called to my
> attention that the ansi cprng is missing an aspect of its compliance requrements
> for FIPS-140. Specifically, its missing a behavior in its continuous test.
> When the CPRNG produces random blocks, the firrst block that it produces must
> never be returned to the user. Instead it must be saved and a second block must
> be generated so that it can be compared to the first block before being returned
> to the user.
>
> I recently posted a patch to do this for the hardware RNG. Its fine to
> do this there, since there are no expectations of a predictable result in that
> RNG. The CPRNG however, provides a predictable random sequence for a given
> input seed key and iteration. The above requirement messes with that
> predictability however because it changes which block is returned on the zeroth
> iteration to the user. Some test vectors expect this, some do not.
>
> So the question is, how do I make this RNG fips compliant without
> breaking some subset of users out there that rely on the predictability of the
> CPRNG? The solution I've come up with is a dynamic flag. This patch series
> adds two api calls to the crypto RNG api rng_set_flags and rng_get_flags, which
> set flags with global meaning on instances of an rng. A given RNG can opt to
> set the registered agorithm methods for these api calls or not. In the event
> they don't a default handler is set for each that returns EOPNOTSUPPORT.
>
> Using this new mechanism I've implemented these calls in ansi_cprng so
> that setting the TEST_MODE flag disables the continuous check, allowing for the
> zeroth block to get returned to the user, which lets us pass most of the
> supplied test vectors (most notably the NIST provided vectors).

Neil and I discussed this whole mess off-list, and I'm in agreement that
this is the cleanest solution to the problem, despite the relative
complexity it adds to the base rng code.

Will reply to each part individually for tracking purposes, but ACK for
all three parts.

--
Jarod Wilson
[email protected]

2009-09-16 20:59:01

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 1/3] add RNG api calls to set common flags

On 09/16/2009 12:11 PM, Neil Horman wrote:
> patch 1/3: Add flags infrastructure to rng api
>
> This patch adds api calls for get/set flags calls to the crypto rng api. This
> api allows algorithm implementations to register calls to respond to flag
> settings that are global and common to all rng's. If a given algorithm has no
> external flags that it needs to respond to, it can opt to not register any
> calls, and as a result a default handler will be registered for each which
> universally returns EOPNOTSUPPORT.
>
> Signed-off-by: Neil Horman<[email protected]>

Acked-by: Jarod Wilson <[email protected]>

--
Jarod Wilson
[email protected]

2009-09-16 21:00:02

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 2/3] augment the testmgr code to set TEST_MODE flag on all rng instances

On 09/16/2009 12:13 PM, Neil Horman wrote:
> patch 2/3: Update testmgr code to place any rng it tests in TEST_MODE
>
> This patch instructs the testmgr code to place all rng allocations that it makes
> into test mode, so that in the event that it has internal mechanisms that may
> affect the testing of the RNG, they won't affect the outcome of the test they
> are preforming.
>
> Signed-off-by: Neil Horman<[email protected]>

With these same additions, my fips crypto test code arrives at expected
results still as well.

Acked-by: Jarod Wilson <[email protected]>


--
Jarod Wilson
[email protected]

2009-09-16 21:00:33

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 3/3] augment CPRNG to correctly implement continuous test for FIPS, and support TEST_MODE flags

On 09/16/2009 12:25 PM, Neil Horman wrote:
> patch 3/3: modify cprng to make contnuity check fips compliant and allow for a
> disabling of the continuity test when the RNG is placed in FIPS mode
>
> Signed-off-by: Neil Horman<[email protected]>

Acked-by: Jarod Wilson <[email protected]>

--
Jarod Wilson
[email protected]

2009-09-17 03:37:31

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On Wed, Sep 16, 2009 at 12:04:56PM -0400, Neil Horman wrote:
>
> So the question is, how do I make this RNG fips compliant without
> breaking some subset of users out there that rely on the predictability of the
> CPRNG? The solution I've come up with is a dynamic flag. This patch series

What user apart from the test vector relies on the predictability?
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-09-17 12:30:24

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On 09/16/2009 11:37 PM, Herbert Xu wrote:
> On Wed, Sep 16, 2009 at 12:04:56PM -0400, Neil Horman wrote:
>>
>> So the question is, how do I make this RNG fips compliant without
>> breaking some subset of users out there that rely on the predictability of the
>> CPRNG? The solution I've come up with is a dynamic flag. This patch series
>
> What user apart from the test vector relies on the predictability?

As far as I know, only the internal self-tests and fips testing rely on
the predictability without the first value consumed internally. However,
in theory, being able to disable the continuity check could also be of
benefit to some throughput-intensive operation, particularly on an
embedded system.

The monte carlo self-tests could obviously compensate for the internally
consumed value without altering the end result, but the single-iteration
tests could not. We're using self-test vectors that were published by
NIST right now, so I'd rather avoid having to alter them.

--
Jarod Wilson
[email protected]

2009-09-17 12:43:51

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On Wed, Sep 16, 2009 at 10:37:29PM -0500, Herbert Xu wrote:
> On Wed, Sep 16, 2009 at 12:04:56PM -0400, Neil Horman wrote:
> >
> > So the question is, how do I make this RNG fips compliant without
> > breaking some subset of users out there that rely on the predictability of the
> > CPRNG? The solution I've come up with is a dynamic flag. This patch series
>
> What user apart from the test vector relies on the predictability?
As Jarod mentioned, currently only the NIST certification vectors and, as a
result our testmgr vectors require disabling of the internal continuity test,
but to generalize from that, I would imagine that any set of certification
vectors that exist in the wild, may or may not assume the presence of the oth
iteration consumption, and this patch gives us the flexability to make use of
those. I was thinking that this api extension could also be used for various
debugging purposes (additional flags could be created to enable internal
debugging, etc).

Neil

> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2009-09-17 15:39:51

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On Thu, Sep 17, 2009 at 08:43:51AM -0400, Neil Horman wrote:
>
> As Jarod mentioned, currently only the NIST certification vectors and, as a
> result our testmgr vectors require disabling of the internal continuity test,
> but to generalize from that, I would imagine that any set of certification
> vectors that exist in the wild, may or may not assume the presence of the oth
> iteration consumption, and this patch gives us the flexability to make use of
> those. I was thinking that this api extension could also be used for various
> debugging purposes (additional flags could be created to enable internal
> debugging, etc).

My gut feeling would be to just get rid of the test vectors.

But if you really want to keep them, please do it like CTR and
RFC3686. That is, have the raw RNG tested with the current vectors,
and implement the FIPS version as a wrapper on top of it to remove
the required bits.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-09-17 17:08:26

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On Thu, Sep 17, 2009 at 08:39:51AM -0700, Herbert Xu wrote:
> On Thu, Sep 17, 2009 at 08:43:51AM -0400, Neil Horman wrote:
> >
> > As Jarod mentioned, currently only the NIST certification vectors and, as a
> > result our testmgr vectors require disabling of the internal continuity test,
> > but to generalize from that, I would imagine that any set of certification
> > vectors that exist in the wild, may or may not assume the presence of the oth
> > iteration consumption, and this patch gives us the flexability to make use of
> > those. I was thinking that this api extension could also be used for various
> > debugging purposes (additional flags could be created to enable internal
> > debugging, etc).
>
> My gut feeling would be to just get rid of the test vectors.
>
> But if you really want to keep them, please do it like CTR and
> RFC3686. That is, have the raw RNG tested with the current vectors,
> and implement the FIPS version as a wrapper on top of it to remove
> the required bits.
>

Just so that I'm clear on what your suggesting, you're approach would be to
register two algs in ansi_cprng, a 'raw' cprng, and a 'fips compliant cprng'
underneath that used the raw cprng as a base, but implemented the continuity
test underneath it? If so, yeah, I can get behind that idea. I'll spin a new
set of patches shortly.

Neil


2009-09-17 20:16:22

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On Thu, Sep 17, 2009 at 01:08:24PM -0400, Neil Horman wrote:
>
> Just so that I'm clear on what your suggesting, you're approach would be to
> register two algs in ansi_cprng, a 'raw' cprng, and a 'fips compliant cprng'
> underneath that used the raw cprng as a base, but implemented the continuity
> test underneath it? If so, yeah, I can get behind that idea. I'll spin a new
> set of patches shortly.

Yes, exactly like how we structure the raw CTR and RFC3686 which
is CTR tailored for IPsec.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-09-17 20:21:09

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On 09/17/2009 04:16 PM, Herbert Xu wrote:
> On Thu, Sep 17, 2009 at 01:08:24PM -0400, Neil Horman wrote:
>>
>> Just so that I'm clear on what your suggesting, you're approach would be to
>> register two algs in ansi_cprng, a 'raw' cprng, and a 'fips compliant cprng'
>> underneath that used the raw cprng as a base, but implemented the continuity
>> test underneath it? If so, yeah, I can get behind that idea. I'll spin a new
>> set of patches shortly.
>
> Yes, exactly like how we structure the raw CTR and RFC3686 which
> is CTR tailored for IPsec.

Yeah, I like that solution as well, does feel less dirty. So
essentially, in fips mode, we'd wind up using fips(ansi_cprng) or
similar, while the self-tests are done against raw ansi_cprng, correct?

--
Jarod Wilson
[email protected]

2009-09-17 20:23:06

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] enhance RNG api with flags to allow for different operational modes

On Thu, Sep 17, 2009 at 04:18:24PM -0400, Jarod Wilson wrote:
>
> Yeah, I like that solution as well, does feel less dirty. So
> essentially, in fips mode, we'd wind up using fips(ansi_cprng) or
> similar, while the self-tests are done against raw ansi_cprng, correct?

Exactly.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-09-18 18:33:00

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 0/1] enhance RNG api with flags to allow for different operational modes (v2)

Hey all-
Ok, so I've got a story behind this one. It was recently called to my
attention that the ansi cprng is missing an aspect of its compliance requrements
for FIPS-140. Specifically, its missing a behavior in its continuous test.
When the CPRNG produces random blocks, the firrst block that it produces must
never be returned to the user. Instead it must be saved and a second block must
be generated so that it can be compared to the first block before being returned
to the user.

I recently posted a patch to do this for the hardware RNG. Its fine to
do this there, since there are no expectations of a predictable result in that
RNG. The CPRNG however, provides a predictable random sequence for a given
input seed key and iteration. The above requirement messes with that
predictability however because it changes which block is returned on the zeroth
iteration to the user. Some test vectors expect this, some do not.

So the question is, how do I make this RNG fips compliant without
breaking some subset of users out there that rely on the predictability of the
CPRNG? The solution we've discussed here is the use of a wrapper algorithm. We
implement fips(ansi_cprng), which is exactly like the ansi_cprng, except that it
implements the continuous test on top of it.

Signed-off-by: Neil Horman <[email protected]>

2009-09-18 18:34:48

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH 1/1] add fips(ansi_cprng) (v2)

Patch to add fips(ansi_cprng) alg, which is ansi_cprng plus a continuous test

Signed-off-by: Neil Horman <[email protected]>

ansi_cprng.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 70 insertions(+), 9 deletions(-)

diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 3aa6e38..027176a 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -85,7 +85,7 @@ static void xor_vectors(unsigned char *in1, unsigned char *in2,
* Returns DEFAULT_BLK_SZ bytes of random data per call
* returns 0 if generation succeded, <0 if something went wrong
*/
-static int _get_more_prng_bytes(struct prng_context *ctx)
+static int _get_more_prng_bytes(struct prng_context *ctx, int cont_test)
{
int i;
unsigned char tmp[DEFAULT_BLK_SZ];
@@ -132,7 +132,7 @@ static int _get_more_prng_bytes(struct prng_context *ctx)
*/
if (!memcmp(ctx->rand_data, ctx->last_rand_data,
DEFAULT_BLK_SZ)) {
- if (fips_enabled) {
+ if (cont_test) {
panic("cprng %p Failed repetition check!\n",
ctx);
}
@@ -185,7 +185,8 @@ static int _get_more_prng_bytes(struct prng_context *ctx)
}

/* Our exported functions */
-static int get_prng_bytes(char *buf, size_t nbytes, struct prng_context *ctx)
+static int get_prng_bytes(char *buf, size_t nbytes, struct prng_context *ctx,
+ int do_cont_test)
{
unsigned char *ptr = buf;
unsigned int byte_count = (unsigned int)nbytes;
@@ -220,7 +221,7 @@ static int get_prng_bytes(char *buf, size_t nbytes, struct prng_context *ctx)

remainder:
if (ctx->rand_data_valid == DEFAULT_BLK_SZ) {
- if (_get_more_prng_bytes(ctx) < 0) {
+ if (_get_more_prng_bytes(ctx, do_cont_test) < 0) {
memset(buf, 0, nbytes);
err = -EINVAL;
goto done;
@@ -247,7 +248,7 @@ empty_rbuf:
*/
for (; byte_count >= DEFAULT_BLK_SZ; byte_count -= DEFAULT_BLK_SZ) {
if (ctx->rand_data_valid == DEFAULT_BLK_SZ) {
- if (_get_more_prng_bytes(ctx) < 0) {
+ if (_get_more_prng_bytes(ctx, do_cont_test) < 0) {
memset(buf, 0, nbytes);
err = -EINVAL;
goto done;
@@ -356,7 +357,15 @@ static int cprng_get_random(struct crypto_rng *tfm, u8 *rdata,
{
struct prng_context *prng = crypto_rng_ctx(tfm);

- return get_prng_bytes(rdata, dlen, prng);
+ return get_prng_bytes(rdata, dlen, prng, 0);
+}
+
+static int fips_cprng_get_random(struct crypto_rng *tfm, u8 *rdata,
+ unsigned int dlen)
+{
+ struct prng_context *prng = crypto_rng_ctx(tfm);
+
+ return get_prng_bytes(rdata, dlen, prng, 1);
}

/*
@@ -384,6 +393,26 @@ static int cprng_reset(struct crypto_rng *tfm, u8 *seed, unsigned int slen)
return 0;
}

+static int fips_cprng_reset(struct crypto_rng *tfm, u8 *seed, unsigned int slen)
+{
+ u8 rdata[DEFAULT_BLK_SZ];
+ int rc;
+
+ struct prng_context *prng = crypto_rng_ctx(tfm);
+
+ rc = cprng_reset(tfm, seed, slen);
+
+ if (!rc)
+ goto out;
+
+ /* this primes our continuity test */
+ rc = get_prng_bytes(rdata, DEFAULT_BLK_SZ, prng, 0);
+ prng->rand_data_valid = DEFAULT_BLK_SZ;
+
+out:
+ return rc;
+}
+
static struct crypto_alg rng_alg = {
.cra_name = "stdrng",
.cra_driver_name = "ansi_cprng",
@@ -404,19 +433,51 @@ static struct crypto_alg rng_alg = {
}
};

+#ifdef CONFIG_CRYPTO_FIPS
+static struct crypto_alg fips_rng_alg = {
+ .cra_name = "fips(ansi_cprng)",
+ .cra_driver_name = "fips_ansi_cprng",
+ .cra_priority = 300,
+ .cra_flags = CRYPTO_ALG_TYPE_RNG,
+ .cra_ctxsize = sizeof(struct prng_context),
+ .cra_type = &crypto_rng_type,
+ .cra_module = THIS_MODULE,
+ .cra_list = LIST_HEAD_INIT(rng_alg.cra_list),
+ .cra_init = cprng_init,
+ .cra_exit = cprng_exit,
+ .cra_u = {
+ .rng = {
+ .rng_make_random = fips_cprng_get_random,
+ .rng_reset = fips_cprng_reset,
+ .seedsize = DEFAULT_PRNG_KSZ + 2*DEFAULT_BLK_SZ,
+ }
+ }
+};
+#endif

/* Module initalization */
static int __init prng_mod_init(void)
{
- if (fips_enabled)
- rng_alg.cra_priority += 200;
+ int rc = 0;

- return crypto_register_alg(&rng_alg);
+ rc = crypto_register_alg(&rng_alg);
+#ifdef CONFIG_CRYPTO_FIPS
+ if (rc)
+ goto out;
+
+ rc = crypto_register_alg(&fips_rng_alg);
+
+out:
+#endif
+ return rc;
}

static void __exit prng_mod_fini(void)
{
crypto_unregister_alg(&rng_alg);
+#ifdef CONFIG_CRYPTO_FIPS
+ crypto_unregister_alg(&fips_rng_alg);
+#endif
return;
}


2009-09-19 01:13:02

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 1/1] add fips(ansi_cprng) (v2)

On 09/18/2009 02:34 PM, Neil Horman wrote:
> Patch to add fips(ansi_cprng) alg, which is ansi_cprng plus a continuous test
>
> Signed-off-by: Neil Horman <[email protected]>

That turned out to be a lot less messy than my puny mind was thinking it might be.
The solution actually looks quite elegant, especially like how much cleaner the
priming of the continuity test in fips mode is.

Acked-by: Jarod Wilson <[email protected]>


--
Jarod Wilson
[email protected]


2009-10-19 02:56:09

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/1] add fips(ansi_cprng) (v2)

On Fri, Sep 18, 2009 at 02:34:46PM -0400, Neil Horman wrote:
> Patch to add fips(ansi_cprng) alg, which is ansi_cprng plus a continuous test
>
> Signed-off-by: Neil Horman <[email protected]>

Patch applied to cryptodev. Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt