This marks GCRY_MD_CRC32, GCRY_MD_CRC24_RFC2440 and GCRY_MD_CRC32_RFC1510 as approved.

CRCs are not cryptographic algorithms, and we should not make any claim on them from a FIPS perspective, especially not that they are approved. That's also the feedback we're getting from our lab on this change.

Please drop the CRCs.

Right, we have received the same feedback from our cert lab but I haven't found time to update the bug yet. Here are the updated patches:

These look good to me.

One potential pitfall here is that SHAKE-128 and SHAKE-256 must not be available for use in signature operations. That's because https://csrc.nist.gov/CSRC/media/Projects/cryptographic-module-validation-program/documents/fips%20140-3/FIPS%20140-3%20IG.pdf section C.C disallows the use of SHAKE in higher-level algorithms:

The SHAKE128 and SHAKE256 extendable-output functions may only be used as the standalone algorithms.

Good catch. A similar problem might arise with SHA384 according to section D.R which states

The new validations, or any revalidations that extend the module’s sunset date, submitted more than one year
after the publication date of this IG shall only use SHA-1, SHA-256 or SHA-512 in Hash_DRBG and
HMAC_DRBG. The revalidations not extending the module’s sunset date are not subject to this requirement.

where "one year after publication date of this IG" == May 16, 2023

It might be easiest to just drop SHAKE and SHA384 from the allowed-list for now.

EDIT: On second looks sha384 might actually better be blocked directly in random/random-drbg.c

After consulting with our certs lab and studying the code I think SHAKE should not be a problem for now. All of the _gcry_digest_spec_shakeXXX seem to neither have an mdlen nor a read() function. pk_sign and pk_verify seem to both call md_read() which should fail because of the missing read function, kdf checks _gcry_md_get_algo_dlen() which should also disallow SHAKE.

We came to the same conclusion -- the SHAKE digests are not usable for sign/verify operations the way how it is implemented now. But it would be more clear if we would have explicit allow-list.

For the DRBG and SHA384 I filled T6393 but from my reading, there is no code path to use the SHA384 as there is no conversion from the string flags.

Going through the code once more, there is one typo to be fixed:

+_gcry_fips_indicator_md (va_list arg_ptr)
+{
+  enum gcry_md_algos alg = va_arg (arg_ptr, enum gcry_cipher_algos);

should say

+_gcry_fips_indicator_md (va_list arg_ptr)
+{
+  enum gcry_md_algos alg = va_arg (arg_ptr, enum gcry_md_algos);

otherwise ack.

Actually, the same issue is in the mac case, which I missed on first couple of reviews:

-  enum gcry_mac_algos alg = va_arg (arg_ptr, enum gcry_cipher_algos);
+  enum gcry_mac_algos alg = va_arg (arg_ptr, enum gcry_mac_algos);

The updated version is available in the following MR with the other proposed FIPS changes:

https://gitlab.com/redhat-crypto/libgcrypt/libgcrypt-mirror/-/merge_requests/14/

Right, thanks for the review! Updated patches below.