Hi Stephan, Stephan Mueller <smueller@xxxxxxxxxx> writes: > Am Freitag, 28. Januar 2022, 15:14:39 CET schrieb Nicolai Stange: > >> Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> writes: >> > On Fri, Jan 14, 2022 at 10:09:02AM +0100, Nicolai Stange wrote: >> >> This looks all good to me, but as !->fips_allowed tests aren't skipped >> >> over anymore now, it would perhaps make sense to make their failure >> >> non-fatal in FIPS mode. Because in FIPS mode a failure could mean a >> >> panic and some of the existing TVs might not pass because of e.g. some >> >> key length checks or so active only for fips_enabled... >> > >> > You mean a buggy non-FIPS algorithm that fails when tested in >> > FIPS mode? I guess we could skip the panic in that case if >> > everyone is happy with that. Stephan? >> >> One more thing I just realized: dracut's fips module ([1]) modprobes >> tcrypt (*) and failure is considered fatal, i.e. the system would not >> boot up. >> >> First of all this would mean that tcrypt_test() needs to ignore >> -ECANCELED return values from alg_test() in FIPS mode, in addition to >> the -EINVAL it is already prepared for. >> >> However, chances are that some of the !fips_allowed algorithms looped >> over by tcrypt are not available (i.e. not enabled at build time) and as >> this change here makes alg_test() to unconditionally attempt a test >> execution now, this would fail with -ENOENT AFAICS. >> >> One way to work around this is to make tcrypt_test() to ignore -ENOENT >> in addition to -EINVAL and -ECANCELED. >> >> It might be undesirable though that the test executions triggered from >> tcrypt would still instantiate/load a ton of !fips_allowed algorithms at >> boot, most of which will effectively be inaccessible (because they're >> not used as FIPS_INTERNAL arguments to fips_allowed == 1 template >> instances). >> >> So how about making alg_test() to skip the !fips_allowed tests in FIPS >> mode as before, but to return -ECANCELED and eventually set >> FIPS_INTERNAL as implemented with this patch here. >> >> This would imply that FIPS_INTERNAL algorithms by themselves remain >> untested, but I think this might be Ok as they would be usable only as >> template arguments in fips_allowed instantiations. That is, they will >> still receive some form of testing when the larger construction they're >> part of gets tested. >> >> For example, going with the "dh" example, where "dh" and "ffdhe3072(dh)" >> would have fips_allowed unset and set respecively, ffdhe3072(dh) as >> a whole would get tested, but not the "dh" argument individually. >> >> Stephan, would this approach work from a FIPS 140-3 perspective? > > Are we sure that we always will have power-up tests of the compound algorithms > when we disable the lower-level algorithm testing? Yes. The compound algorithms having ->fips_allowed == 1 and alg_test_null entries are: authenc(hmac(sha1),ctr(aes)) authenc(hmac(sha1),rfc3686(ctr(aes))) authenc(hmac(sha256),ctr(aes)) authenc(hmac(sha256),rfc3686(ctr(aes))) authenc(hmac(sha384),ctr(aes)) authenc(hmac(sha384),rfc3686(ctr(aes))) authenc(hmac(sha512),ctr(aes)) authenc(hmac(sha512),rfc3686(ctr(aes))) The hmac(sha*), ctr(aes) and rfc3686(ctr(aes)) all have ->fips_allowed == 1 and proper non-alg_test_null test entries. So no change here. cbc(paes) ctr(paes) ecb(paes) ofb(paes) xts(paes) xts4096(paes) xts512(paes) cts(cbc(paes)) As ecb(paes) has only a alg_test_null() entry, no test would have been performed at all before this change for these. So no change here either. ecb(cipher_null) No change here either. pkcs1pad(rsa,sha224) pkcs1pad(rsa,sha384) pkcs1pad(rsa,sha512) The sha* and rsa all have ->fips_allowed == 1 and proper non-alg_test_null test entries. So no change here. > > For example, consider the DH work you are preparing: we currently have a self > test for dh - which then will be marked as FIPS_INTERNAL and not executed. > Would we now have self tests for modpXXX(dh) or ffdheXXX(dh)? Yes, exactly. > If not, how would it be guaranteed that DH is tested? > > The important part is that the algorithm testing is guaranteed. I see a number > of alg_test_null in testmgr.c. I see the potential that some algorithms do not > get tested at all when we skip FIPS_INTERNAL algorithms. See above. But of course one needs to be careful not to add ->fips_allowed + alg_test_null entries for compound algorithms where any of the template arguments isn't approved in the future. > From a FIPS perspective it is permissible that compound algo power up tests > are claimed to cover respective lower-level algos. Perfect. Thanks, Nicolai -- SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg), GF: Ivo Totev
