Skip to contentSkip to conversation

crypto: tcrypt: add option to not exit on success

PATCH In review Jarod Wilson 3 participants 17 messages May 11, 2009 – May 27, 2009 Crypto API

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 crypto: tcrypt: add option to not exit on success

At present, the tcrypt module always exits with an -EAGAIN upon successfully completing all the tests its been asked to run. There are cases where it would be much simpler to verify all tests passed if tcrypt simply stayed loaded (i.e. returned 0). Specifically, in fips mode, all self-tests need to be run from the initrd, and its much simpler to check the ret from modprobe for success than to scrape dmesg. To make this doable, I've simply added a module param to allow this behavior, leaving the default behavior more or less the same as before, although now we're tracking all success/failure rets as well.

The tcrypt_test() portion of the patch is dependent on my earlier pair of patches that skip non-fips algs in fips mode, at least to achieve the fully intended behavior.

---
 crypto/tcrypt.c |  168 +++++++++++++++++++++++++++++++------------------------
 1 files changed, 94 insertions(+), 74 deletions(-)
crypto/tcrypt.c +94 -74
index 9e4974e..dee729c 100644
@@ -27,6 +27,7 @@
 #include <linux/timex.h>
 #include <linux/interrupt.h>
 #include "tcrypt.h"
+#include "internal.h"
 
 /*
  * Need slab memory for testing (size in number of pages).
@@ -45,6 +46,7 @@
 static unsigned int sec;
 
 static int mode;
+static int noexit;
 static char *tvmem[TVMEMSIZE];
 
 static char *check[] = {
@@ -468,248 +470,255 @@ static void test_available(void)
 
 static inline int tcrypt_test(const char *alg)
 {
-	return alg_test(alg, alg, 0, 0);
+	int ret;
+
+	ret = alg_test(alg, alg, 0, 0);
+	/* non-fips algs return -EINVAL in fips mode */
+	if (fips_enabled && ret == -EINVAL)
+		ret = 0;
+	return ret;
 }
 
-static void do_test(int m)
+static int do_test(int m)
 {
 	int i;
+	int ret = 0;
 
 	switch (m) {
 	case 0:
 		for (i = 1; i < 200; i++)
-			do_test(i);
+			ret += do_test(i);
 		break;
 
 	case 1:
-		tcrypt_test("md5");
+		ret += tcrypt_test("md5");
 		break;
 
 	case 2:
-		tcrypt_test("sha1");
+		ret += tcrypt_test("sha1");
 		break;
 
 	case 3:
-		tcrypt_test("ecb(des)");
-		tcrypt_test("cbc(des)");
+		ret += tcrypt_test("ecb(des)");
+		ret += tcrypt_test("cbc(des)");
 		break;
 
 	case 4:
-		tcrypt_test("ecb(des3_ede)");
-		tcrypt_test("cbc(des3_ede)");
+		ret += tcrypt_test("ecb(des3_ede)");
+		ret += tcrypt_test("cbc(des3_ede)");
 		break;
 
 	case 5:
-		tcrypt_test("md4");
+		ret += tcrypt_test("md4");
 		break;
 
 	case 6:
-		tcrypt_test("sha256");
+		ret += tcrypt_test("sha256");
 		break;
 
 	case 7:
-		tcrypt_test("ecb(blowfish)");
-		tcrypt_test("cbc(blowfish)");
+		ret += tcrypt_test("ecb(blowfish)");
+		ret += tcrypt_test("cbc(blowfish)");
 		break;
 
 	case 8:
-		tcrypt_test("ecb(twofish)");
-		tcrypt_test("cbc(twofish)");
+		ret += tcrypt_test("ecb(twofish)");
+		ret += tcrypt_test("cbc(twofish)");
 		break;
 
 	case 9:
-		tcrypt_test("ecb(serpent)");
+		ret += tcrypt_test("ecb(serpent)");
 		break;
 
 	case 10:
-		tcrypt_test("ecb(aes)");
-		tcrypt_test("cbc(aes)");
-		tcrypt_test("lrw(aes)");
-		tcrypt_test("xts(aes)");
-		tcrypt_test("ctr(aes)");
-		tcrypt_test("rfc3686(ctr(aes))");
+		ret += tcrypt_test("ecb(aes)");
+		ret += tcrypt_test("cbc(aes)");
+		ret += tcrypt_test("lrw(aes)");
+		ret += tcrypt_test("xts(aes)");
+		ret += tcrypt_test("ctr(aes)");
+		ret += tcrypt_test("rfc3686(ctr(aes))");
 		break;
 
 	case 11:
-		tcrypt_test("sha384");
+		ret += tcrypt_test("sha384");
 		break;
 
 	case 12:
-		tcrypt_test("sha512");
+		ret += tcrypt_test("sha512");
 		break;
 
 	case 13:
-		tcrypt_test("deflate");
+		ret += tcrypt_test("deflate");
 		break;
 
 	case 14:
-		tcrypt_test("ecb(cast5)");
+		ret += tcrypt_test("ecb(cast5)");
 		break;
 
 	case 15:
-		tcrypt_test("ecb(cast6)");
+		ret += tcrypt_test("ecb(cast6)");
 		break;
 
 	case 16:
-		tcrypt_test("ecb(arc4)");
+		ret += tcrypt_test("ecb(arc4)");
 		break;
 
 	case 17:
-		tcrypt_test("michael_mic");
+		ret += tcrypt_test("michael_mic");
 		break;
 
 	case 18:
-		tcrypt_test("crc32c");
+		ret += tcrypt_test("crc32c");
 		break;
 
 	case 19:
-		tcrypt_test("ecb(tea)");
+		ret += tcrypt_test("ecb(tea)");
 		break;
 
 	case 20:
-		tcrypt_test("ecb(xtea)");
+		ret += tcrypt_test("ecb(xtea)");
 		break;
 
 	case 21:
-		tcrypt_test("ecb(khazad)");
+		ret += tcrypt_test("ecb(khazad)");
 		break;
 
 	case 22:
-		tcrypt_test("wp512");
+		ret += tcrypt_test("wp512");
 		break;
 
 	case 23:
-		tcrypt_test("wp384");
+		ret += tcrypt_test("wp384");
 		break;
 
 	case 24:
-		tcrypt_test("wp256");
+		ret += tcrypt_test("wp256");
 		break;
 
 	case 25:
-		tcrypt_test("ecb(tnepres)");
+		ret += tcrypt_test("ecb(tnepres)");
 		break;
 
 	case 26:
-		tcrypt_test("ecb(anubis)");
-		tcrypt_test("cbc(anubis)");
+		ret += tcrypt_test("ecb(anubis)");
+		ret += tcrypt_test("cbc(anubis)");
 		break;
 
 	case 27:
-		tcrypt_test("tgr192");
+		ret += tcrypt_test("tgr192");
 		break;
 
 	case 28:
 
-		tcrypt_test("tgr160");
+		ret += tcrypt_test("tgr160");
 		break;
 
 	case 29:
-		tcrypt_test("tgr128");
+		ret += tcrypt_test("tgr128");
 		break;
 
 	case 30:
-		tcrypt_test("ecb(xeta)");
+		ret += tcrypt_test("ecb(xeta)");
 		break;
 
 	case 31:
-		tcrypt_test("pcbc(fcrypt)");
+		ret += tcrypt_test("pcbc(fcrypt)");
 		break;
 
 	case 32:
-		tcrypt_test("ecb(camellia)");
-		tcrypt_test("cbc(camellia)");
+		ret += tcrypt_test("ecb(camellia)");
+		ret += tcrypt_test("cbc(camellia)");
 		break;
 	case 33:
-		tcrypt_test("sha224");
+		ret += tcrypt_test("sha224");
 		break;
 
 	case 34:
-		tcrypt_test("salsa20");
+		ret += tcrypt_test("salsa20");
 		break;
 
 	case 35:
-		tcrypt_test("gcm(aes)");
+		ret += tcrypt_test("gcm(aes)");
 		break;
 
 	case 36:
-		tcrypt_test("lzo");
+		ret += tcrypt_test("lzo");
 		break;
 
 	case 37:
-		tcrypt_test("ccm(aes)");
+		ret += tcrypt_test("ccm(aes)");
 		break;
 
 	case 38:
-		tcrypt_test("cts(cbc(aes))");
+		ret += tcrypt_test("cts(cbc(aes))");
 		break;
 
         case 39:
-		tcrypt_test("rmd128");
+		ret += tcrypt_test("rmd128");
 		break;
 
         case 40:
-		tcrypt_test("rmd160");
+		ret += tcrypt_test("rmd160");
 		break;
 
 	case 41:
-		tcrypt_test("rmd256");
+		ret += tcrypt_test("rmd256");
 		break;
 
 	case 42:
-		tcrypt_test("rmd320");
+		ret += tcrypt_test("rmd320");
 		break;
 
 	case 43:
-		tcrypt_test("ecb(seed)");
+		ret += tcrypt_test("ecb(seed)");
 		break;
 
 	case 44:
-		tcrypt_test("zlib");
+		ret += tcrypt_test("zlib");
 		break;
 
 	case 45:
-		tcrypt_test("rfc4309(ccm(aes))");
+		ret += tcrypt_test("rfc4309(ccm(aes))");
 		break;
 
 	case 100:
-		tcrypt_test("hmac(md5)");
+		ret += tcrypt_test("hmac(md5)");
 		break;
 
 	case 101:
-		tcrypt_test("hmac(sha1)");
+		ret += tcrypt_test("hmac(sha1)");
 		break;
 
 	case 102:
-		tcrypt_test("hmac(sha256)");
+		ret += tcrypt_test("hmac(sha256)");
 		break;
 
 	case 103:
-		tcrypt_test("hmac(sha384)");
+		ret += tcrypt_test("hmac(sha384)");
 		break;
 
 	case 104:
-		tcrypt_test("hmac(sha512)");
+		ret += tcrypt_test("hmac(sha512)");
 		break;
 
 	case 105:
-		tcrypt_test("hmac(sha224)");
+		ret += tcrypt_test("hmac(sha224)");
 		break;
 
 	case 106:
-		tcrypt_test("xcbc(aes)");
+		ret += tcrypt_test("xcbc(aes)");
 		break;
 
 	case 107:
-		tcrypt_test("hmac(rmd128)");
+		ret += tcrypt_test("hmac(rmd128)");
 		break;
 
 	case 108:
-		tcrypt_test("hmac(rmd160)");
+		ret += tcrypt_test("hmac(rmd160)");
 		break;
 
 	case 150:
-		tcrypt_test("ansi_cprng");
+		ret += tcrypt_test("ansi_cprng");
 		break;
 
 	case 200:
@@ -873,6 +882,8 @@ static void do_test(int m)
 		test_available();
 		break;
 	}
+
+	return ret;
 }
 
 static int __init tcrypt_mod_init(void)
@@ -886,15 +897,21 @@ static int __init tcrypt_mod_init(void)
 			goto err_free_tv;
 	}
 
-	do_test(mode);
+	err = do_test(mode);
+	if (err) {
+		printk(KERN_ERR "tcrypt: one or more tests failed!\n");
+		goto err_free_tv;
+	}
 
-	/* We intentionaly return -EAGAIN to prevent keeping
-	 * the module. It does all its work from init()
-	 * and doesn't offer any runtime functionality 
+	/* We intentionaly return -EAGAIN to prevent keeping the module,
+	 * unless it was loaded w/the noexit param set. It does all its
+	 * work from init() and doesn't offer any runtime functionality,
+	 * but in some cases, checking for a successful load is helpful.
 	 * => we don't need it in the memory, do we?
 	 *                                        -- mludvig
 	 */
-	err = -EAGAIN;
+	if (!noexit)
+		err = -EAGAIN;
 
 err_free_tv:
 	for (i = 0; i < TVMEMSIZE && tvmem[i]; i++)
@@ -916,6 +933,9 @@ module_param(mode, int, 0);
 module_param(sec, uint, 0);
 MODULE_PARM_DESC(sec, "Length in seconds of speed tests "
 		      "(defaults to zero which uses CPU cycles instead)");
+module_param(noexit, int, 0);
+MODULE_PARM_DESC(noexit, "Whether or not the module should simply exit "
+		 "after running tests. (default is to exit).");
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Quick & dirty crypto testing module");
Signature · 2 lines
Jarod Wilson
[email protected]
#1 lore raw
Quoted 10 lines · On Monday 11 May 2009 10:06:32 Jarod Wilson wrote:

At present, the tcrypt module always exits with an -EAGAIN upon successfully completing all the tests its been asked to run. There are cases where it would be much simpler to verify all tests passed if tcrypt simply stayed loaded (i.e. returned 0). Specifically, in fips mode, all self-tests need to be run from the initrd, and its much simpler to check the ret from modprobe for success than to scrape dmesg. To make this doable, I've simply added a module param to allow this behavior, leaving the default behavior more or less the same as before, although now we're tracking all success/failure rets as well.

I've been reminded that a self-test failure in fips mode means an immediate panic, so modprobe never sees the ret in that case, but if the module load failed for other reasons, a non-zero return value from modprobe is possible w/o traversing the code paths that trigger a self-test failure panic. For one, if the tcrypt module were to go missing for some reason, modprobe would have a non-zero ret, and the initrd would need to handle panicking the system.

Would there be any objections to dropping the noexit parameter entirely and just making its behavior the default? It would make all users regardless of fips mode notice failures more readily.

Signature · 2 lines
Jarod Wilson
[email protected]
#2 lore raw
Quoted 20 lines · Jarod Wilson
Quoted 10 lines · On Monday 11 May 2009 10:06:32 Jarod Wilson wrote:

At present, the tcrypt module always exits with an -EAGAIN upon successfully completing all the tests its been asked to run. There are cases where it would be much simpler to verify all tests passed if tcrypt simply stayed loaded (i.e. returned 0). Specifically, in fips mode, all self-tests need to be run from the initrd, and its much simpler to check the ret from modprobe for success than to scrape dmesg. To make this doable, I've simply added a module param to allow this behavior, leaving the default behavior more or less the same as before, although now we're tracking all success/failure rets as well.

I've been reminded that a self-test failure in fips mode means an immediate panic, so modprobe never sees the ret in that case, but if the module load failed for other reasons, a non-zero return value from modprobe is possible w/o traversing the code paths that trigger a self-test failure panic. For one, if the tcrypt module were to go missing for some reason, modprobe would have a non-zero ret, and the initrd would need to handle panicking the system.

Quoted 4 lines

Would there be any objections to dropping the noexit parameter entirely and just making its behavior the default? It would make all users regardless of fips mode notice failures more readily.

I think thats a fine idea. Theres no reason that a user of the tcrypt module can't manually rmmod it when the testing is done. Doing it that way just seems more sane to me to begin with anyway.

Neil

Quoted 4 lines
Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#3 lore raw
Quoted 8 lines · Neil Horman
Quoted 5 lines

Would there be any objections to dropping the noexit parameter entirely and just making its behavior the default? It would make all users regardless of fips mode notice failures more readily.

I think thats a fine idea. Theres no reason that a user of the tcrypt module can't manually rmmod it when the testing is done. Doing it that way just seems more sane to me to begin with anyway.

No, tcrypt is only a relic for correctness testing. Its main purpose these days is for speed testing. Having to rmmod it is silly.

There's really no need to load tcrypt for correctness testing anymore.

Cheers,

Signature · 4 lines
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
#4 lore raw
Quoted 10 lines · Herbert Xu
Quoted 8 lines · Neil Horman
Quoted 5 lines

Would there be any objections to dropping the noexit parameter entirely and just making its behavior the default? It would make all users regardless of fips mode notice failures more readily.

I think thats a fine idea. Theres no reason that a user of the tcrypt module can't manually rmmod it when the testing is done. Doing it that way just seems more sane to me to begin with anyway.

Quoted 7 lines

No, tcrypt is only a relic for correctness testing. Its main purpose these days is for speed testing. Having to rmmod it is silly.

There's really no need to load tcrypt for correctness testing anymore.

Not really sure I agree with the logic here. I agree that its pretty clear that its major value is for quickly testing all the algorithms in a system, but universally failing the loading of the module simply to save a few milliseconds seems like a poor choice. In so doing you create an alias effect, as jarod noted between a non-existent module and a module that failed to load. The aliasing can be resolved, if you want to parse dmesg, but if speed is the issue at hand, that parsing is a significant impact. If you allow the module to load properly, then for the cost of an rmmod, you can tell simply from the exit code of modprobe:
1) If the module was found
2) If the tests passed

And if the rmmod is simply to expensive for whatever reason, then for the cost of a few k of ram taken up by the module, you can choose not to unload it.

Of course, if tcrypt is really as much of a relic as you say, perhaps that is an argument for removing the module entirely. Perhaps the testmgr interface could be exported to userspace and the tcrypt tests be packaged as a userspace suite.

Regards
Neil

Quoted 7 lines

Cheers,

Signature · 4 lines
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
maintainer
#5 lore raw
Quoted 12 lines · Neil Horman

Not really sure I agree with the logic here. I agree that its pretty clear that its major value is for quickly testing all the algorithms in a system, but universally failing the loading of the module simply to save a few milliseconds seems like a poor choice. In so doing you create an alias effect, as jarod noted between a non-existent module and a module that failed to load. The aliasing can be resolved, if you want to parse dmesg, but if speed is the issue at hand, that parsing is a significant impact. If you allow the module to load properly, then for the cost of an rmmod, you can tell simply from the exit code of modprobe:
1) If the module was found
2) If the tests passed

As I said, tcrypt should no longer be used to test algorithms. Algorithms are tested as they load, with the result available in /proc/crypto. tcrypt is just a relic that will be going away.

Do not rely on it.

Cheers,

Signature · 4 lines
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
#6 lore raw
Quoted 14 lines · On Wednesday 13 May 2009 07:38:19 Herbert Xu wrote:
Quoted 12 lines · Neil Horman

Not really sure I agree with the logic here. I agree that its pretty clear that its major value is for quickly testing all the algorithms in a system, but universally failing the loading of the module simply to save a few milliseconds seems like a poor choice. In so doing you create an alias effect, as jarod noted between a non-existent module and a module that failed to load. The aliasing can be resolved, if you want to parse dmesg, but if speed is the issue at hand, that parsing is a significant impact. If you allow the module to load properly, then for the cost of an rmmod, you can tell simply from the exit code of modprobe:
1) If the module was found
2) If the tests passed

Quoted 5 lines

As I said, tcrypt should no longer be used to test algorithms. Algorithms are tested as they load, with the result available in /proc/crypto. tcrypt is just a relic that will be going away.

Do not rely on it.

Hm... FIPS has the requirement that we test all algs before we use any algs, self-tests on demand before first use for each alg is insufficient. At first blush, I'm not seeing how we ensure this happens. How can we trigger a cbc(des3_ede) self-test from userspace? I see that modprobe'ing des.ko runs the base des and des3_ede self-tests, but modprobe'ing cbc.ko doesn't lead to any self-tests being run.

Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#7 lore raw
Quoted 8 lines · Jarod Wilson

Hm... FIPS has the requirement that we test all algs before we use any algs, self-tests on demand before first use for each alg is insufficient. At first blush, I'm not seeing how we ensure this happens. How can we trigger a cbc(des3_ede) self-test from userspace? I see that modprobe'ing des.ko runs the base des and des3_ede self-tests, but modprobe'ing cbc.ko doesn't lead to any self-tests being run.

Once we have a user-space interface crypto API you will be able to instantiate any given algorithm.

For now I suggest that you create your own module to instantiate these FIPS algorithms. Or just load tcrypt and ignore the exit status, or make tcrypt return 0 if we're in FIPS mode.

Cheers,

Signature · 4 lines
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
#8 lore raw
Quoted 10 lines · On Wednesday 13 May 2009 09:27:52 Herbert Xu wrote:
Quoted 8 lines · Jarod Wilson

Hm... FIPS has the requirement that we test all algs before we use any algs, self-tests on demand before first use for each alg is insufficient. At first blush, I'm not seeing how we ensure this happens. How can we trigger a cbc(des3_ede) self-test from userspace? I see that modprobe'ing des.ko runs the base des and des3_ede self-tests, but modprobe'ing cbc.ko doesn't lead to any self-tests being run.

Quoted 6 lines

Once we have a user-space interface crypto API you will be able to instantiate any given algorithm.

For now I suggest that you create your own module to instantiate these FIPS algorithms. Or just load tcrypt and ignore the exit status, or make tcrypt return 0 if we're in FIPS mode.

The latter option is more or less what the patch at the start of this thread did, although via a param to tcrypt, not keying off the fips flag. If I were to modify the patch to drop the mod param usage, and instead key off the fips flag to not exit, would that be acceptable for committing until such time as the userspace interface is ready?

Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#9 lore raw
Quoted 6 lines · Jarod Wilson

The latter option is more or less what the patch at the start of this thread did, although via a param to tcrypt, not keying off the fips flag. If I were to modify the patch to drop the mod param usage, and instead key off the fips flag to not exit, would that be acceptable for committing until such time as the userspace interface is ready?

Yes that sounds like the way to go.

Thanks,

Signature · 4 lines
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
#10 lore raw

PATCH v2 1/1 crypto: tcrypt: do not exit on success in fips mode

Quoted 9 lines · On Wednesday 13 May 2009 10:03:29 Herbert Xu wrote:
Quoted 6 lines · Jarod Wilson

The latter option is more or less what the patch at the start of this thread did, although via a param to tcrypt, not keying off the fips flag. If I were to modify the patch to drop the mod param usage, and instead key off the fips flag to not exit, would that be acceptable for committing until such time as the userspace interface is ready?

Yes that sounds like the way to go.

Okay, here it is.

At present, the tcrypt module always exits with an -EAGAIN upon successfully completing all the tests its been asked to run. In fips mode, integrity checking is done by running all self-tests from the initrd, and its much simpler to check the ret from modprobe for success than to scrape dmesg and/or /proc/crypto. Simply stay loaded, giving modprobe a retval of 0, if self-tests all pass and we're in fips mode.

A side-effect of tracking success/failure for fips mode is that in non-fips mode, self-test failures will return the actual failure return codes, rather than always returning -EAGAIN, which seems more correct anyway.

The tcrypt_test() portion of the patch is dependent on my earlier pair of patches that skip non-fips algs in fips mode, at least to achieve the fully intended behavior.

Nb: testing this patch against the cryptodev tree revealed a test failure for sha384, which I have yet to look into...

---
 crypto/tcrypt.c |  164 ++++++++++++++++++++++++++++++-------------------------
 1 files changed, 90 insertions(+), 74 deletions(-)
crypto/tcrypt.c +90 -74
index 9e4974e..d59ba50 100644
@@ -27,6 +27,7 @@
 #include <linux/timex.h>
 #include <linux/interrupt.h>
 #include "tcrypt.h"
+#include "internal.h"
 
 /*
  * Need slab memory for testing (size in number of pages).
@@ -468,248 +469,255 @@ static void test_available(void)
 
 static inline int tcrypt_test(const char *alg)
 {
-	return alg_test(alg, alg, 0, 0);
+	int ret;
+
+	ret = alg_test(alg, alg, 0, 0);
+	/* non-fips algs return -EINVAL in fips mode */
+	if (fips_enabled && ret == -EINVAL)
+		ret = 0;
+	return ret;
 }
 
-static void do_test(int m)
+static int do_test(int m)
 {
 	int i;
+	int ret = 0;
 
 	switch (m) {
 	case 0:
 		for (i = 1; i < 200; i++)
-			do_test(i);
+			ret += do_test(i);
 		break;
 
 	case 1:
-		tcrypt_test("md5");
+		ret += tcrypt_test("md5");
 		break;
 
 	case 2:
-		tcrypt_test("sha1");
+		ret += tcrypt_test("sha1");
 		break;
 
 	case 3:
-		tcrypt_test("ecb(des)");
-		tcrypt_test("cbc(des)");
+		ret += tcrypt_test("ecb(des)");
+		ret += tcrypt_test("cbc(des)");
 		break;
 
 	case 4:
-		tcrypt_test("ecb(des3_ede)");
-		tcrypt_test("cbc(des3_ede)");
+		ret += tcrypt_test("ecb(des3_ede)");
+		ret += tcrypt_test("cbc(des3_ede)");
 		break;
 
 	case 5:
-		tcrypt_test("md4");
+		ret += tcrypt_test("md4");
 		break;
 
 	case 6:
-		tcrypt_test("sha256");
+		ret += tcrypt_test("sha256");
 		break;
 
 	case 7:
-		tcrypt_test("ecb(blowfish)");
-		tcrypt_test("cbc(blowfish)");
+		ret += tcrypt_test("ecb(blowfish)");
+		ret += tcrypt_test("cbc(blowfish)");
 		break;
 
 	case 8:
-		tcrypt_test("ecb(twofish)");
-		tcrypt_test("cbc(twofish)");
+		ret += tcrypt_test("ecb(twofish)");
+		ret += tcrypt_test("cbc(twofish)");
 		break;
 
 	case 9:
-		tcrypt_test("ecb(serpent)");
+		ret += tcrypt_test("ecb(serpent)");
 		break;
 
 	case 10:
-		tcrypt_test("ecb(aes)");
-		tcrypt_test("cbc(aes)");
-		tcrypt_test("lrw(aes)");
-		tcrypt_test("xts(aes)");
-		tcrypt_test("ctr(aes)");
-		tcrypt_test("rfc3686(ctr(aes))");
+		ret += tcrypt_test("ecb(aes)");
+		ret += tcrypt_test("cbc(aes)");
+		ret += tcrypt_test("lrw(aes)");
+		ret += tcrypt_test("xts(aes)");
+		ret += tcrypt_test("ctr(aes)");
+		ret += tcrypt_test("rfc3686(ctr(aes))");
 		break;
 
 	case 11:
-		tcrypt_test("sha384");
+		ret += tcrypt_test("sha384");
 		break;
 
 	case 12:
-		tcrypt_test("sha512");
+		ret += tcrypt_test("sha512");
 		break;
 
 	case 13:
-		tcrypt_test("deflate");
+		ret += tcrypt_test("deflate");
 		break;
 
 	case 14:
-		tcrypt_test("ecb(cast5)");
+		ret += tcrypt_test("ecb(cast5)");
 		break;
 
 	case 15:
-		tcrypt_test("ecb(cast6)");
+		ret += tcrypt_test("ecb(cast6)");
 		break;
 
 	case 16:
-		tcrypt_test("ecb(arc4)");
+		ret += tcrypt_test("ecb(arc4)");
 		break;
 
 	case 17:
-		tcrypt_test("michael_mic");
+		ret += tcrypt_test("michael_mic");
 		break;
 
 	case 18:
-		tcrypt_test("crc32c");
+		ret += tcrypt_test("crc32c");
 		break;
 
 	case 19:
-		tcrypt_test("ecb(tea)");
+		ret += tcrypt_test("ecb(tea)");
 		break;
 
 	case 20:
-		tcrypt_test("ecb(xtea)");
+		ret += tcrypt_test("ecb(xtea)");
 		break;
 
 	case 21:
-		tcrypt_test("ecb(khazad)");
+		ret += tcrypt_test("ecb(khazad)");
 		break;
 
 	case 22:
-		tcrypt_test("wp512");
+		ret += tcrypt_test("wp512");
 		break;
 
 	case 23:
-		tcrypt_test("wp384");
+		ret += tcrypt_test("wp384");
 		break;
 
 	case 24:
-		tcrypt_test("wp256");
+		ret += tcrypt_test("wp256");
 		break;
 
 	case 25:
-		tcrypt_test("ecb(tnepres)");
+		ret += tcrypt_test("ecb(tnepres)");
 		break;
 
 	case 26:
-		tcrypt_test("ecb(anubis)");
-		tcrypt_test("cbc(anubis)");
+		ret += tcrypt_test("ecb(anubis)");
+		ret += tcrypt_test("cbc(anubis)");
 		break;
 
 	case 27:
-		tcrypt_test("tgr192");
+		ret += tcrypt_test("tgr192");
 		break;
 
 	case 28:
 
-		tcrypt_test("tgr160");
+		ret += tcrypt_test("tgr160");
 		break;
 
 	case 29:
-		tcrypt_test("tgr128");
+		ret += tcrypt_test("tgr128");
 		break;
 
 	case 30:
-		tcrypt_test("ecb(xeta)");
+		ret += tcrypt_test("ecb(xeta)");
 		break;
 
 	case 31:
-		tcrypt_test("pcbc(fcrypt)");
+		ret += tcrypt_test("pcbc(fcrypt)");
 		break;
 
 	case 32:
-		tcrypt_test("ecb(camellia)");
-		tcrypt_test("cbc(camellia)");
+		ret += tcrypt_test("ecb(camellia)");
+		ret += tcrypt_test("cbc(camellia)");
 		break;
 	case 33:
-		tcrypt_test("sha224");
+		ret += tcrypt_test("sha224");
 		break;
 
 	case 34:
-		tcrypt_test("salsa20");
+		ret += tcrypt_test("salsa20");
 		break;
 
 	case 35:
-		tcrypt_test("gcm(aes)");
+		ret += tcrypt_test("gcm(aes)");
 		break;
 
 	case 36:
-		tcrypt_test("lzo");
+		ret += tcrypt_test("lzo");
 		break;
 
 	case 37:
-		tcrypt_test("ccm(aes)");
+		ret += tcrypt_test("ccm(aes)");
 		break;
 
 	case 38:
-		tcrypt_test("cts(cbc(aes))");
+		ret += tcrypt_test("cts(cbc(aes))");
 		break;
 
         case 39:
-		tcrypt_test("rmd128");
+		ret += tcrypt_test("rmd128");
 		break;
 
         case 40:
-		tcrypt_test("rmd160");
+		ret += tcrypt_test("rmd160");
 		break;
 
 	case 41:
-		tcrypt_test("rmd256");
+		ret += tcrypt_test("rmd256");
 		break;
 
 	case 42:
-		tcrypt_test("rmd320");
+		ret += tcrypt_test("rmd320");
 		break;
 
 	case 43:
-		tcrypt_test("ecb(seed)");
+		ret += tcrypt_test("ecb(seed)");
 		break;
 
 	case 44:
-		tcrypt_test("zlib");
+		ret += tcrypt_test("zlib");
 		break;
 
 	case 45:
-		tcrypt_test("rfc4309(ccm(aes))");
+		ret += tcrypt_test("rfc4309(ccm(aes))");
 		break;
 
 	case 100:
-		tcrypt_test("hmac(md5)");
+		ret += tcrypt_test("hmac(md5)");
 		break;
 
 	case 101:
-		tcrypt_test("hmac(sha1)");
+		ret += tcrypt_test("hmac(sha1)");
 		break;
 
 	case 102:
-		tcrypt_test("hmac(sha256)");
+		ret += tcrypt_test("hmac(sha256)");
 		break;
 
 	case 103:
-		tcrypt_test("hmac(sha384)");
+		ret += tcrypt_test("hmac(sha384)");
 		break;
 
 	case 104:
-		tcrypt_test("hmac(sha512)");
+		ret += tcrypt_test("hmac(sha512)");
 		break;
 
 	case 105:
-		tcrypt_test("hmac(sha224)");
+		ret += tcrypt_test("hmac(sha224)");
 		break;
 
 	case 106:
-		tcrypt_test("xcbc(aes)");
+		ret += tcrypt_test("xcbc(aes)");
 		break;
 
 	case 107:
-		tcrypt_test("hmac(rmd128)");
+		ret += tcrypt_test("hmac(rmd128)");
 		break;
 
 	case 108:
-		tcrypt_test("hmac(rmd160)");
+		ret += tcrypt_test("hmac(rmd160)");
 		break;
 
 	case 150:
-		tcrypt_test("ansi_cprng");
+		ret += tcrypt_test("ansi_cprng");
 		break;
 
 	case 200:
@@ -873,6 +881,8 @@ static void do_test(int m)
 		test_available();
 		break;
 	}
+
+	return ret;
 }
 
 static int __init tcrypt_mod_init(void)
@@ -886,15 +896,21 @@ static int __init tcrypt_mod_init(void)
 			goto err_free_tv;
 	}
 
-	do_test(mode);
+	err = do_test(mode);
+	if (err) {
+		printk(KERN_ERR "tcrypt: one or more tests failed!\n");
+		goto err_free_tv;
+	}
 
-	/* We intentionaly return -EAGAIN to prevent keeping
-	 * the module. It does all its work from init()
-	 * and doesn't offer any runtime functionality 
+	/* We intentionaly return -EAGAIN to prevent keeping the module,
+	 * unless we're running in fips mode. It does all its work from
+	 * init() and doesn't offer any runtime functionality, but in
+	 * the fips case, checking for a successful load is helpful.
 	 * => we don't need it in the memory, do we?
 	 *                                        -- mludvig
 	 */
-	err = -EAGAIN;
+	if (!fips_enabled)
+		err = -EAGAIN;
 
 err_free_tv:
 	for (i = 0; i < TVMEMSIZE && tvmem[i]; i++)
Signature · 2 lines
Jarod Wilson
[email protected]
Acked-by on v2 1/1
#11 lore raw

Re: [PATCH v2] crypto: tcrypt: do not exit on success in fips mode

Quoted 26 lines · Jarod Wilson
Quoted 9 lines · On Wednesday 13 May 2009 10:03:29 Herbert Xu wrote:
Quoted 6 lines · Jarod Wilson

The latter option is more or less what the patch at the start of this thread did, although via a param to tcrypt, not keying off the fips flag. If I were to modify the patch to drop the mod param usage, and instead key off the fips flag to not exit, would that be acceptable for committing until such time as the userspace interface is ready?

Yes that sounds like the way to go.

Okay, here it is.

At present, the tcrypt module always exits with an -EAGAIN upon successfully completing all the tests its been asked to run. In fips mode, integrity checking is done by running all self-tests from the initrd, and its much simpler to check the ret from modprobe for success than to scrape dmesg and/or /proc/crypto. Simply stay loaded, giving modprobe a retval of 0, if self-tests all pass and we're in fips mode.

A side-effect of tracking success/failure for fips mode is that in non-fips mode, self-test failures will return the actual failure return codes, rather than always returning -EAGAIN, which seems more correct anyway.

Quoted 9 lines

The tcrypt_test() portion of the patch is dependent on my earlier pair of patches that skip non-fips algs in fips mode, at least to achieve the fully intended behavior.

Nb: testing this patch against the cryptodev tree revealed a test failure for sha384, which I have yet to look into...

Quoted 1 line
maintainer Applied
#12 lore raw

Re: [PATCH v2] crypto: tcrypt: do not exit on success in fips mode

Quoted 4 lines · Neil Horman
Quoted 3 lines

Applied to cryptodev. Thanks!

Signature · 4 lines
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
#13 lore raw
In reply to Herbert Xu
Quoted 10 lines · Herbert Xu
Quoted 8 lines · Jarod Wilson

Hm... FIPS has the requirement that we test all algs before we use any algs, self-tests on demand before first use for each alg is insufficient. At first blush, I'm not seeing how we ensure this happens. How can we trigger a cbc(des3_ede) self-test from userspace? I see that modprobe'ing des.ko runs the base des and des3_ede self-tests, but modprobe'ing cbc.ko doesn't lead to any self-tests being run.

Quoted 3 lines

Once we have a user-space interface crypto API you will be able to instantiate any given algorithm.

Thats a good idea. Jarod, didn't you create a generic netlink socket family module that created just such an interface for testing purposes? That might be worth polishing and submitting to provide that user space crypto api

Neil

Quoted 1 line
#14 lore raw
Quoted 11 lines · On Wednesday 13 May 2009 10:02:25 Neil Horman wrote:
Quoted 10 lines · Herbert Xu
Quoted 8 lines · Jarod Wilson

Hm... FIPS has the requirement that we test all algs before we use any algs, self-tests on demand before first use for each alg is insufficient. At first blush, I'm not seeing how we ensure this happens. How can we trigger a cbc(des3_ede) self-test from userspace? I see that modprobe'ing des.ko runs the base des and des3_ede self-tests, but modprobe'ing cbc.ko doesn't lead to any self-tests being run.

Quoted 5 lines
Quoted 3 lines

Once we have a user-space interface crypto API you will be able to instantiate any given algorithm.

Thats a good idea. Jarod, didn't you create a generic netlink socket family module that created just such an interface for testing purposes?

Indeed. Works quite well for my somewhat specific needs...

Quoted 2 lines

That might be
worth polishing and submitting to provide that user space crypto api

It would likely need a LOT of polish, and I'm not sure if its at all close to what we have (Herbert has?) in mind.... At the moment, it consists of:

1) a kernel module, which duplicates many of the functions in testmgr, more or less, but gets its input over a generic netlink socket, rather than from a static array, and sends the result back out over the same socket.

2) a userspace binary that feeds very specifically formatted vectors to the kernel via the netlink socket, listens for the result and spits it out -- it doesn't actually verify the correctness of the result, but adding support for passing in an expected result and checking against it would be trivial.

I guess the place to start would be to ask exactly what sort of user-space interface to the crypto API did we have in mind here? i.e., what sort of user-space<->kernel-space communication is envisioned, and what sort of functionality should be present?

I could see the desire for a simpler interface that doesn't rely upon a specific userspace binary -- something sysfs or proc-based -- but netlink is reasonably flexible...

Signature · 2 lines
Jarod Wilson
[email protected]
maintainer
#15 lore raw
Quoted 4 lines · Jarod Wilson

It would likely need a LOT of polish, and I'm not sure if its at all close to what we have (Herbert has?) in mind.... At the moment, it consists of:

The interface I had in mind can be found in the recent discussions in the linux-crypto archives.

Cheers,

Signature · 4 lines
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
#16 lore raw
Quoted 8 lines · On Wednesday 13 May 2009 19:45:50 Herbert Xu wrote:
Quoted 4 lines · Jarod Wilson

It would likely need a LOT of polish, and I'm not sure if its at all close to what we have (Herbert has?) in mind.... At the moment, it consists of:

The interface I had in mind can be found in the recent discussions in the linux-crypto archives.

Ah, sorry, I found it. So pretty much absolutely nothing like the netlink-based stuff I've done. :) I'll go further familiarize myself with the proposed patch...

Signature · 2 lines
Jarod Wilson
[email protected]