2009-05-07 18:42:16

by Jarod Wilson

[permalink] [raw]
Subject: [PATCH 0/2] crypto: disallow non-approved algs in fips mode

At present, nothing is preventing the use of non-approved algorithms
in fips mode. I was initially working on a patch to make it easier
for all fips-approved algs to be tested using tcrypt, and realized
the changes I was making could also be used to prevent non-approved
algs in fips mode. Any approved alg *must* have self-tests, and thus
have an entry in testmgr.c's alg_test_descs[]. By adding a fips flag
to these entries, we can simply reject all algs that don't have this
flag when in fips mode by skipping their self-tests and returning
an -EINVAL to prevent them from being loaded. So with this change, I
can

1) 'modprobe tcrypt' and have all fips approved algs self-tested, and
*only* fips approved algs tested

2) 'modprobe md4' for example, and in fips mode, have the module load
rejected as invalid

Patch 1/2 adds the basic infra
Patch 2/2 marks the allowed algs

--
Jarod Wilson
[email protected]


2009-05-07 19:28:19

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 0/2] crypto: disallow non-approved algs in fips mode

On Thursday 07 May 2009 14:41:26 Jarod Wilson wrote:
> At present, nothing is preventing the use of non-approved algorithms
> in fips mode. I was initially working on a patch to make it easier
> for all fips-approved algs to be tested using tcrypt, and realized
> the changes I was making could also be used to prevent non-approved
> algs in fips mode. Any approved alg *must* have self-tests, and thus
> have an entry in testmgr.c's alg_test_descs[]. By adding a fips flag
> to these entries, we can simply reject all algs that don't have this
> flag when in fips mode by skipping their self-tests and returning
> an -EINVAL to prevent them from being loaded. So with this change, I
> can
>
> 1) 'modprobe tcrypt' and have all fips approved algs self-tested, and
> *only* fips approved algs tested
>
> 2) 'modprobe md4' for example, and in fips mode, have the module load
> rejected as invalid

Hrm. Minor correction... Only seeing module loads rejected as invalid
when patching this into a Red Hat Enterprise Linux 5.x kernel. With the
cryptodev tree, we do skip non-allowed algs as intended, but loading
modules for non-allowed algs still works...

--
Jarod Wilson
[email protected]

2009-05-07 19:28:56

by Jarod Wilson

[permalink] [raw]
Subject: [PATCH 1/2] crypto: add infra to skip disallowed algs in fips mode

Because all fips-allowed algorithms must be self-tested before they
can be used, they will all have entries in testmgr.c's alg_test_descs[].
If we add and set an additional flag for the allowed algorithms, we can
key off of it to prevent use of any algs that aren't allowed.

Signed-off-by: Jarod Wilson <[email protected]>

---
crypto/testmgr.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f4cc178..232f043 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -94,6 +94,7 @@ struct alg_test_desc {
const char *alg;
int (*test)(const struct alg_test_desc *desc, const char *driver,
u32 type, u32 mask);
+ int fips_allowed; /* set if alg is allowed in fips mode */

union {
struct aead_test_suite aead;
@@ -2285,6 +2286,9 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
if (i < 0)
goto notest;

+ if (fips_enabled && !alg_test_descs[i].fips_allowed)
+ goto non_fips_alg;
+
rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
goto test_done;
}
@@ -2293,6 +2297,9 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
if (i < 0)
goto notest;

+ if (fips_enabled && !alg_test_descs[i].fips_allowed)
+ goto non_fips_alg;
+
rc = alg_test_descs[i].test(alg_test_descs + i, driver,
type, mask);
test_done:
@@ -2308,5 +2315,7 @@ test_done:
notest:
printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);
return 0;
+non_fips_alg:
+ return -EINVAL;
}
EXPORT_SYMBOL_GPL(alg_test);

--
Jarod Wilson
[email protected]

2009-05-07 19:28:58

by Jarod Wilson

[permalink] [raw]
Subject: [PATCH 2/2] crypto: mark algs allowed in fips mode

Set the fips_allowed flag in testmgr.c's alg_test_descs[] for algs
that are allowed to be used when in fips mode.

One caveat: des isn't actually allowed anymore, but des (and thus also
ecb(des)) has to be permitted, because disallowing them results in
des3_ede being unable to properly register (see des module init func).

Also, crc32 isn't technically on the fips approved list, but I think
it gets used in various places that necessitate it being allowed.

This list is based on
http://csrc.nist.gov/groups/STM/cavp/index.html

Important note: allowed/approved here does NOT mean "validated", just
that its an alg that *could* be validated.

Signed-off-by: Jarod Wilson <[email protected]>

---
crypto/testmgr.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 232f043..f93b26d 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1433,6 +1433,7 @@ static const struct alg_test_desc alg_test_descs[] = {
{
.alg = "ansi_cprng",
.test = alg_test_cprng,
+ .fips_allowed = 1,
.suite = {
.cprng = {
.vecs = ansi_cprng_aes_tv_template,
@@ -1442,6 +1443,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "cbc(aes)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1517,6 +1519,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "cbc(des3_ede)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1547,6 +1550,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ccm(aes)",
.test = alg_test_aead,
+ .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -1562,6 +1566,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "crc32c",
.test = alg_test_crc32c,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = crc32c_tv_template,
@@ -1571,6 +1576,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ctr(aes)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1616,6 +1622,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(aes)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1721,6 +1728,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(des)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1736,6 +1744,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(des3_ede)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1871,6 +1880,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "gcm(aes)",
.test = alg_test_aead,
+ .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -1913,6 +1923,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha1)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha1_tv_template,
@@ -1922,6 +1933,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha224)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha224_tv_template,
@@ -1931,6 +1943,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha256)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha256_tv_template,
@@ -1940,6 +1953,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha384)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha384_tv_template,
@@ -1949,6 +1963,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha512)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha512_tv_template,
@@ -2030,6 +2045,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "rfc3686(ctr(aes))",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -2045,6 +2061,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "rfc4309(ccm(aes))",
.test = alg_test_aead,
+ .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2107,6 +2124,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha1",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha1_tv_template,
@@ -2116,6 +2134,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha224",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha224_tv_template,
@@ -2125,6 +2144,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha256",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha256_tv_template,
@@ -2134,6 +2154,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha384",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha384_tv_template,
@@ -2143,6 +2164,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha512",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha512_tv_template,


--
Jarod Wilson
[email protected]

2009-05-08 02:12:37

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: add infra to skip disallowed algs in fips mode

On Thu, May 07, 2009 at 03:27:59PM -0400, Jarod Wilson wrote:
> Because all fips-allowed algorithms must be self-tested before they
> can be used, they will all have entries in testmgr.c's alg_test_descs[].
> If we add and set an additional flag for the allowed algorithms, we can
> key off of it to prevent use of any algs that aren't allowed.
>
> Signed-off-by: Jarod Wilson <[email protected]>
>
> ---
> crypto/testmgr.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index f4cc178..232f043 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -94,6 +94,7 @@ struct alg_test_desc {
> const char *alg;
> int (*test)(const struct alg_test_desc *desc, const char *driver,
> u32 type, u32 mask);
> + int fips_allowed; /* set if alg is allowed in fips mode */
>
> union {
> struct aead_test_suite aead;

Please merge this chunk with the second patch into one and have
it as the first patch. We want this to be bisectable.

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-05-08 04:51:07

by Jarod Wilson

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: add infra to skip disallowed algs in fips mode

On 05/07/2009 10:12 PM, Herbert Xu wrote:
> On Thu, May 07, 2009 at 03:27:59PM -0400, Jarod Wilson wrote:
>> Because all fips-allowed algorithms must be self-tested before they
>> can be used, they will all have entries in testmgr.c's alg_test_descs[].
>> If we add and set an additional flag for the allowed algorithms, we can
>> key off of it to prevent use of any algs that aren't allowed.
>>
>> Signed-off-by: Jarod Wilson <[email protected]>
>>
>> ---
>> crypto/testmgr.c | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
>> index f4cc178..232f043 100644
>> --- a/crypto/testmgr.c
>> +++ b/crypto/testmgr.c
>> @@ -94,6 +94,7 @@ struct alg_test_desc {
>> const char *alg;
>> int (*test)(const struct alg_test_desc *desc, const char *driver,
>> u32 type, u32 mask);
>> + int fips_allowed; /* set if alg is allowed in fips mode */
>>
>> union {
>> struct aead_test_suite aead;
>
> Please merge this chunk with the second patch into one and have
> it as the first patch. We want this to be bisectable.

D'oh. I actually was attempting to make it bisectable, but now I see
that w/fips mode enabled, you wouldn't be able to use any algs with
them split this way if you landed between 'em. I'll repost shortly.

--
Jarod Wilson
[email protected]


2009-05-08 04:55:56

by Jarod Wilson

[permalink] [raw]
Subject: [PATCH 1/2] crypto: mark algs allowed in fips mode

Set the fips_allowed flag in testmgr.c's alg_test_descs[] for algs
that are allowed to be used when in fips mode.

One caveat: des isn't actually allowed anymore, but des (and thus also
ecb(des)) has to be permitted, because disallowing them results in
des3_ede being unable to properly register (see des module init func).

Also, crc32 isn't technically on the fips approved list, but I think
it gets used in various places that necessitate it being allowed.

This list is based on
http://csrc.nist.gov/groups/STM/cavp/index.html

Important note: allowed/approved here does NOT mean "validated", just
that its an alg that *could* be validated.

Signed-off-by: Jarod Wilson <[email protected]>

---
crypto/testmgr.c | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f4cc178..51bae62 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -94,6 +94,7 @@ struct alg_test_desc {
const char *alg;
int (*test)(const struct alg_test_desc *desc, const char *driver,
u32 type, u32 mask);
+ int fips_allowed; /* set if alg is allowed in fips mode */

union {
struct aead_test_suite aead;
@@ -1432,6 +1433,7 @@ static const struct alg_test_desc alg_test_descs[] = {
{
.alg = "ansi_cprng",
.test = alg_test_cprng,
+ .fips_allowed = 1,
.suite = {
.cprng = {
.vecs = ansi_cprng_aes_tv_template,
@@ -1441,6 +1443,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "cbc(aes)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1516,6 +1519,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "cbc(des3_ede)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1546,6 +1550,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ccm(aes)",
.test = alg_test_aead,
+ .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -1561,6 +1566,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "crc32c",
.test = alg_test_crc32c,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = crc32c_tv_template,
@@ -1570,6 +1576,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ctr(aes)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1615,6 +1622,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(aes)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1720,6 +1728,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(des)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1735,6 +1744,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "ecb(des3_ede)",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -1870,6 +1880,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "gcm(aes)",
.test = alg_test_aead,
+ .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -1912,6 +1923,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha1)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha1_tv_template,
@@ -1921,6 +1933,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha224)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha224_tv_template,
@@ -1930,6 +1943,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha256)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha256_tv_template,
@@ -1939,6 +1953,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha384)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha384_tv_template,
@@ -1948,6 +1963,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "hmac(sha512)",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = hmac_sha512_tv_template,
@@ -2029,6 +2045,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "rfc3686(ctr(aes))",
.test = alg_test_skcipher,
+ .fips_allowed = 1,
.suite = {
.cipher = {
.enc = {
@@ -2044,6 +2061,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "rfc4309(ccm(aes))",
.test = alg_test_aead,
+ .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2106,6 +2124,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha1",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha1_tv_template,
@@ -2115,6 +2134,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha224",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha224_tv_template,
@@ -2124,6 +2144,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha256",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha256_tv_template,
@@ -2133,6 +2154,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha384",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha384_tv_template,
@@ -2142,6 +2164,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "sha512",
.test = alg_test_hash,
+ .fips_allowed = 1,
.suite = {
.hash = {
.vecs = sha512_tv_template,


--
Jarod Wilson
[email protected]


2009-05-08 05:00:18

by Jarod Wilson

[permalink] [raw]
Subject: [PATCH 2/2] crypto: skip algs not flagged fips_allowed in fips mode

Because all fips-allowed algorithms must be self-tested before they
can be used, they will all have entries in testmgr.c's alg_test_descs[].
Skip self-tests for any algs not flagged as fips_approved and return
-EINVAL when in fips mode.

Signed-off-by: Jarod Wilson <[email protected]>

---
crypto/testmgr.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 51bae62..f93b26d 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2308,6 +2308,9 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
if (i < 0)
goto notest;

+ if (fips_enabled && !alg_test_descs[i].fips_allowed)
+ goto non_fips_alg;
+
rc = alg_test_cipher(alg_test_descs + i, driver, type, mask);
goto test_done;
}
@@ -2316,6 +2319,9 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
if (i < 0)
goto notest;

+ if (fips_enabled && !alg_test_descs[i].fips_allowed)
+ goto non_fips_alg;
+
rc = alg_test_descs[i].test(alg_test_descs + i, driver,
type, mask);
test_done:
@@ -2331,5 +2337,7 @@ test_done:
notest:
printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver);
return 0;
+non_fips_alg:
+ return -EINVAL;
}
EXPORT_SYMBOL_GPL(alg_test);


--
Jarod Wilson
[email protected]


2009-05-15 05:17:21

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: mark algs allowed in fips mode

On Fri, May 08, 2009 at 12:55:51AM -0400, Jarod Wilson wrote:
> Set the fips_allowed flag in testmgr.c's alg_test_descs[] for algs
> that are allowed to be used when in fips mode.

Both patches applied. Thanks Jarod!
--
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