Re: [v2 PATCH] crypto: api - Disallow sha1 in FIPS-mode while allowing hmac(sha1)

[Stephan Mueller]

Am Freitag, 28. Januar 2022, 15:14:39 CET schrieb Nicolai Stange:

Hi Nicolai,

> 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?

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)? 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.

>From a FIPS perspective it is permissible that compound algo power up tests 
are claimed to cover respective lower-level algos.

> 
> 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.


Ciao
Stephan