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? Thanks! Nicolai [1] https://git.kernel.org/pub/scm/boot/dracut/dracut.git/tree/modules.d/01fips/fips.sh#n106 (*) I'm not sure why this is being done, but it is what it is. -- SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg), GF: Ivo Totev
