Page MenuHome GnuPG

FIPS: Allow salt=NULL (or shorter salt) for HKDF
Closed, ResolvedPublic

Description

In rC07722d89bac1: kdf,fips: Modify HKDF test for FIPS mode., I fixed HKDF tests for shorter salts for HKDF in FIPS mode.

However, no salt is actually a valid practice for use of HKDF.

For concrete reference, I found this one: https://pages.nist.gov/ACVP/draft-hammett-acvp-kas-kdf-hkdf.html

So, I think that rejecting is not good.

Event Timeline

gniibe triaged this task as Normal priority.Jun 22 2022, 3:40 AM
gniibe created this task.

In rC76aad97dd312: fips: Reject shorter key for HMAC in FIPS mode., I added rejection, but it would be good to move the check to src/visibility.c to allow internal use.

diff --git a/cipher/md.c b/cipher/md.c
index 4f4fc9bf..34336b5c 100644
--- a/cipher/md.c
+++ b/cipher/md.c
@@ -903,9 +903,6 @@ prepare_macpads (gcry_md_hd_t a, const unsigned char *key, size_t keylen)
 {
   GcryDigestEntry *r;
 
-  if (fips_mode () && keylen < 14)
-    return GPG_ERR_INV_VALUE;
-
   if (!a->ctx->list)
     return GPG_ERR_DIGEST_ALGO; /* Might happen if no algo is enabled.  */
 
diff --git a/src/visibility.c b/src/visibility.c
index c98247d8..aee5bffb 100644
--- a/src/visibility.c
+++ b/src/visibility.c
@@ -946,6 +946,9 @@ gcry_mac_setkey (gcry_mac_hd_t hd, const void *key, size_t keylen)
   if (!fips_is_operational ())
     return gpg_error (fips_not_operational ());
 
+  if (fips_mode () && keylen < 14)
+    return GPG_ERR_INV_VALUE;
+
   return gpg_error (_gcry_mac_setkey (hd, key, keylen));
 }
 
diff --git a/tests/t-kdf.c b/tests/t-kdf.c
index 4596c5c7..508e4bbe 100644
--- a/tests/t-kdf.c
+++ b/tests/t-kdf.c
@@ -1875,17 +1875,7 @@ check_hkdf (void)
                        info, infolen,
                        expectedlen, out);
   if (err)
-    {
-      if (in_fips_mode && saltlen < 14)
-        {
-          if (verbose)
-            fprintf (stderr,
-                     "  shorter salt (%lu) rejected correctly in fips mode\n",
-                     saltlen);
-        }
-      else
-        fail ("HKDF test %d failed: %s\n", count, gpg_strerror (err));
-    }
+    fail ("HKDF test %d failed: %s\n", count, gpg_strerror (err));
   else if (memcmp (out, expected, expectedlen))
     {
       fail ("HKDF test %d failed: mismatch\n", count);
gniibe renamed this task from FIPS: Allow salt=NULL for HKDF to FIPS: Allow salt=NULL (or shorter salt) for HKDF.Jun 22 2022, 3:47 AM
gniibe updated the task description. (Show Details)
gniibe added projects: FIPS, libgcrypt.

Considering again, I concluded the patch above should be applied.
The use of SALT in HKDF may be not secret and there are valid use cases with no last or shorter salt. It's different to the use case of HMAC, where KEY is secret.

The change allows internal use of HMAC with shorter key.

The change has a side effects that it will reject setting other MAC keys (than HMAC). But it should be rejected, too.

For CMAC and GMAC, it's about setting underlying cipher key, so, no problem.

For KMAC (which is not yet implemented in libgcrypt), same check (rejection for < 112-bit) should be applied.

Key length requirements for KDFs are specified in SP 800-131Ar2 (https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf), which is linked from SP 800-140Dr1 (https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-140Dr1.pdf) in section "6.2.1 Transitions".

Section 8 "Deriving Additional Keys from a Cryptographic Key" covers KDFs and their required key lengths.

The length of the key-derivation key shall be at least 112 bits.

Second, it lists all acceptable pseudo-random functions for use in KDFs in table 7, which includes HMAC using any approved hash function. It does, however, expand on this below the table, where it says:

The use of HMAC-based KDFs is acceptable using a hash function specified in FIPS 180 or FIPS 202 with a key whose length is at least 112 bits.

As for requirements for the salt value, see SP 800-56Cr2 (https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Cr2.pdf), section 8.2 "The Choice of a Salt Value". Specifically, it says

This Recommendation does not require the use of a randomly selected salt value. In particular, if there are no means to select a salt value and share it with all of the participants during a key-establishment transaction, then this Recommendation specifies that a predetermined default (e.g., all-zero) byte string be used as the salt value.

werner changed the task status from Open to Testing.Sep 22 2022, 10:54 AM
werner removed a project: Restricted Project.

I read the document (SP 800-131Ar2) again. I think that it would be irrelevant for PKDF2, because it's password KDF, not deriving additional keys from a Cryptographic Key.

I read the document (SP 800-131Ar2) again. I think that it would be irrelevant for PKDF2, because it's password KDF, not deriving additional keys from a Cryptographic Key.

Right. I think the 857e6f467d0fc9fd858a73d84122695425970075 should be reverted.

In regards to this issue, we were also notified that the MD API using gcry_md_setkey() can be used to calculate HMACs and it does not have the needed input key length limitation. From the discussion here I read that we would like to keep the internal usage still available so my proposal would be to to add similar check as in gcry_mac_setkey() into the above function. Together with the revert, it is available in the following merge request:

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

Let me describe the changes recorded in this task.