On Thu, Apr 30, 2009 at 05:13:25PM -0400, Jarod Wilson wrote: > On Wednesday 29 April 2009 08:46:47 Jarod Wilson wrote: > > On Wednesday 29 April 2009 06:50:35 Neil Horman wrote: > > > On Tue, Apr 28, 2009 at 09:18:22PM -0400, Jarod Wilson wrote: > > > > Per the NIST AESAVS document, Appendix A[1], it isn't possible to > > > > have automated self-tests for counter-mode AES, but people are > > > > misled to believe something is wrong by the message that says there > > > > is no test for ctr(aes). Simply suppress all 'no test for ctr(aes*' > > > > messages if fips_enabled is set to avoid confusion. > > > > > > > > Dependent on earlier patch 'crypto: catch base cipher self-test > > > > failures in fips mode', which adds the test_done label. > > > > > > > > [1] http://csrc.nist.gov/groups/STM/cavp/documents/aes/AESAVS.pdf > [...] > > > From the way I read the document, anything operating in a counter mode > > > will have > > > an unpredictable output (given the counter operation isn't specified). > > > While > > > the above works, I'm not sure that it fully covers the various ccm modes > > > available (ccm_base and rfc4309). > > > > I believe Appendix A only applies for straight up counter-mode aes, > > ccm_base and rfc4309 actually have well-defined counter operations. > > We've already got self-tests for ccm(aes) and a pending patch for > > rfc4309(ccm(aes), and since they don't start w/'ctr(aes', they > > wouldn't be caught by that (admittedly hacky) check even if we > > didn't have test vectors for them. > > > > > Perhaps instead it would be better to add a > > > TFM mask flag indicating that the selected transform included a > > > unpredictable > > > component or counter input (marking the alg as being unsuitable for > > > automatic > > > testing without knoweldge of the inner workings of that counter. Then > > > you could > > > just test for that flag? > > > > Yeah, I thought about a flag too, but it seemed potentially a lot of > > overhead for what might well be restricted to ctr(aes*). It might've > > been relevant for ctr(des3_ede) or ctr(des), but they're not on the > > fips approved algo/mode list, so I took the easy way out. I'm game to > > go the flag route if need be though. > > Neil and I talked a bit more off-list about the best approach to take, and > after a bit of trial and error, we came up with the idea to simply add an > 'untestable' flag to the alg_test_desc struct, a corresponding entry for > ctr(aes) in alg_test_descs, and a hook to check for untestable algs in > alg_find_test(). > > Works well enough in local testing, eliminates the use of strncmp, and > results in far more granular control over marking which algs are > explicitly untestable and shouldn't have warnings printed for. At the > moment, I've gone for suppressing the warnings regardless of whether > we're in fips mode or not, but adding a different warning (er, info) > message in the untestable case works too, if that's preferred. The > errnos used seem appropriate, but I might have missed more appropriate > ones somewhere. > > nb: this patch applies atop my earlier '[PATCH v2] crypto: catch base > cipher self-test failures in fips mode'. > > Signed-off-by: Jarod Wilson <[email protected]> > > --- > crypto/testmgr.c | 21 +++++++++++++++++++-- > 1 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/crypto/testmgr.c b/crypto/testmgr.c > index f39c148..b78278c 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 untestable; > > union { > struct aead_test_suite aead; > @@ -1518,6 +1519,13 @@ static const struct alg_test_desc alg_test_descs[] = { > } > } > }, { > + /* > + * Automated ctr(aes) tests aren't possible per Appendix A of > + * http://csrc.nist.gov/groups/STM/cavp/documents/aes/AESAVS.pdf > + */ > + .alg = "ctr(aes)", > + .untestable = 1, > + }, { > .alg = "cts(cbc(aes))", > .test = alg_test_skcipher, > .suite = { > @@ -2198,10 +2206,13 @@ static int alg_find_test(const char *alg) > continue; > } > > + if (alg_test_descs[i].untestable) > + return -ENODATA; > + > return i; > } > > - return -1; > + return -ENOSYS; > } > > int alg_test(const char *driver, const char *alg, u32 type, u32 mask) > @@ -2237,7 +2248,13 @@ test_done: > return rc; > > notest: > - printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver); > + switch (i) { > + case -ENODATA: > + break; > + case -ENOSYS: > + default: > + printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver); > + } > return 0; > } > EXPORT_SYMBOL_GPL(alg_test); > > > -- > Jarod Wilson > [email protected] >
Acked-by: Neil Horman <[email protected]> -- 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
