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

[Nicolai Stange]

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