Hello,

Using the function gcry_kdf_derive as an example (as David Sugar kindly
suggested), here are my patches against master, as of today.

I didn't touch the code for the rejection by argument with an error
GPG_ERR_INV_VALUE, where it does not compute the value under FIPS mode.
(If computation should be done, we need to change the _gcry_kdf_derive
function not to reject by GPG_ERR_INV_VALUE, but let it continue the
computing, setting the service indicator.)

In t-kdf.c, I'm not sure if checking the computed value makes sense in
check_fips_gcry_kdf_derive for GCRY_KDF_SCRYPT.  (I modified the case of
GCRY_KDF_SCRYPT, so that checking goes well.)

Please tell us your comments/opinions/whatever.
-- 
diff --git a/cipher/kdf.c b/cipher/kdf.c
index b4c5f83a..c6d2a10c 100644
--- a/cipher/kdf.c
+++ b/cipher/kdf.c
@@ -261,6 +261,9 @@ _gcry_kdf_derive (const void *passphrase, size_t passphraselen,
       goto leave;
     }
 
+  if (fips_mode ())
+    /* Clear the FIPS service indicator.  */
+    _gcry_thread_context_set_fsi (0);
 
   switch (algo)
     {
@@ -319,6 +322,9 @@ _gcry_kdf_derive (const void *passphrase, size_t passphraselen,
       break;
     }
 
+  if (ec == 0 && fips_mode ())
+    _gcry_fips_check_kdf_compliant (algo, subalgo);
+
  leave:
   return ec;
 }
diff --git a/src/fips.c b/src/fips.c
index 58fb69df..2c400438 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -363,6 +363,14 @@ _gcry_fips_test_operational (void)
   return result;
 }
 
+gpg_err_code_t
+_gcry_fips_indicator (unsigned long *p)
+{
+
+  *p = _gcry_thread_context_get_fsi ();
+  return 0;
+}
+
 int
 _gcry_fips_indicator_cipher (va_list arg_ptr)
 {
@@ -1191,3 +1199,20 @@ _gcry_fips_noreturn (void)
   abort ();
   /*NOTREACHED*/
 }
+
+void
+_gcry_fips_check_kdf_compliant (int algo, int subalgo)
+{
+  switch (algo)
+    {
+    case GCRY_KDF_PBKDF2:
+      /* It's valid.  */
+      break;
+
+    default:
+      /* Record the reason of failure, in the indicator.
+       * Or putting GPG_ERR_NOT_SUPPORTED would be enough.  */
+      _gcry_thread_context_set_fsi ((unsigned long)((algo << 16) | subalgo));
+      break;
+    }
+}
diff --git a/src/g10lib.h b/src/g10lib.h
index adf03e86..30117d0e 100644
--- a/src/g10lib.h
+++ b/src/g10lib.h
@@ -468,6 +468,8 @@ void _gcry_fips_signal_error (const char *srcfile,
            _gcry_fips_signal_error (__FILE__, __LINE__, NULL, 1, (a))
 #endif
 
+gpg_err_code_t _gcry_fips_indicator (unsigned long *);
+
 int _gcry_fips_indicator_cipher (va_list arg_ptr);
 int _gcry_fips_indicator_mac (va_list arg_ptr);
 int _gcry_fips_indicator_md (va_list arg_ptr);
@@ -493,6 +495,7 @@ gpg_err_code_t _gcry_fips_run_selftests (int extended);
 void _gcry_fips_noreturn (void);
 #define fips_noreturn()  (_gcry_fips_noreturn ())
 
+void _gcry_fips_check_kdf_compliant (int algo, int subalgo);
 
 
 #endif /* G10LIB_H */
diff --git a/src/gcrypt.h.in b/src/gcrypt.h.in
index 52e9bcea..af02580b 100644
--- a/src/gcrypt.h.in
+++ b/src/gcrypt.h.in
@@ -335,7 +335,8 @@ enum gcry_ctl_cmds
     GCRYCTL_FIPS_SERVICE_INDICATOR_MAC = 85,
     GCRYCTL_FIPS_SERVICE_INDICATOR_MD = 86,
     GCRYCTL_FIPS_SERVICE_INDICATOR_PK_FLAGS = 87,
-    GCRYCTL_MD_CUSTOMIZE = 88
+    GCRYCTL_MD_CUSTOMIZE = 88,
+    GCRYCTL_FIPS_SERVICE_INDICATOR = 89
   };
 
 /* Perform various operations defined by CMD. */
@@ -1974,6 +1975,9 @@ void gcry_log_debugsxp (const char *text, gcry_sexp_t sexp);
 
 char *gcry_get_config (int mode, const char *what);
 
+/* Convinience macro to access the FIPS service indicator.  */
+#define gcry_get_fips_service_indicator(p) gcry_control (GCRYCTL_FIPS_SERVICE_INDICATOR,p)
+
 /* Log levels used by the internal logging facility. */
 enum gcry_log_levels
   {
diff --git a/src/global.c b/src/global.c
index 593ea406..7a1fcc96 100644
--- a/src/global.c
+++ b/src/global.c
@@ -787,6 +787,10 @@ _gcry_vcontrol (enum gcry_ctl_cmds cmd, va_list arg_ptr)
       rc = _gcry_fips_run_selftests (1);
       break;
 
+    case GCRYCTL_FIPS_SERVICE_INDICATOR:
+      rc = _gcry_fips_indicator (va_arg (arg_ptr, unsigned long *));
+      break;
+
     case GCRYCTL_FIPS_SERVICE_INDICATOR_CIPHER:
       /* Get FIPS Service Indicator for a given symmetric algorithm and
        * optional mode. Returns GPG_ERR_NO_ERROR if algorithm is allowed or
diff --git a/tests/t-kdf.c b/tests/t-kdf.c
index 10f64a7c..836cf74a 100644
--- a/tests/t-kdf.c
+++ b/tests/t-kdf.c
@@ -1927,6 +1927,159 @@ check_fips_indicators (void)
 }
 
 
+static void
+check_fips_gcry_kdf_derive(void)
+{
+  static struct {
+    const char *p;   /* Passphrase.  */
+    size_t plen;     /* Length of P. */
+    int algo;
+    int subalgo;
+    const char *salt;
+    size_t saltlen;
+    unsigned long iterations;
+    int dklen;       /* Requested key length.  */
+    const char *dk;  /* Derived key.  */
+    int expect_error;
+  } tv[] = {
+    {
+      "passwordPASSWORDpassword", 24,
+      GCRY_KDF_PBKDF2, GCRY_MD_SHA1,
+      "saltSALTsaltSALTsaltSALTsaltSALTsalt", 36,
+      4096,
+      25,
+      "\x3d\x2e\xec\x4f\xe4\x1c\x84\x9b\x80\xc8"
+      "\xd8\x36\x62\xc0\xe4\x4a\x8b\x29\x1a\x96"
+      "\x4c\xf2\xf0\x70\x38",
+      0
+    },
+    {
+      "pleaseletmein", 13,
+      GCRY_KDF_SCRYPT, 16384,
+      "SodiumChloride", 14,
+      1,
+      64,
+      "\x70\x23\xbd\xcb\x3a\xfd\x73\x48\x46\x1c\x06\xcd\x81\xfd\x38\xeb"
+      "\xfd\xa8\xfb\xba\x90\x4f\x8e\x3e\xa9\xb5\x43\xf6\x54\x5d\xa1\xf2"
+      "\xd5\x43\x29\x55\x61\x3f\x0f\xcf\x62\xd4\x97\x05\x24\x2a\x9a\xf9"
+      "\xe6\x1e\x85\xdc\x0d\x65\x1e\x40\xdf\xcf\x01\x7b\x45\x57\x58\x87",
+      1 /* By checking service indicator, forbidden because unallowed algo */
+    },
+    {
+      "passwor", 7,
+      GCRY_KDF_PBKDF2, GCRY_MD_SHA1,
+      "saltSALTsaltSALTsaltSALTsaltSALTsalt", 36,
+      4096,
+      25,
+      "\x3d\x2e\xec\x4f\xe4\x1c\x84\x9b\x80\xc8"
+      "\xd8\x36\x62\xc0\xe4\x4a\x8b\x29\x1a\x96"
+      "\x4c\xf2\xf0\x70\x38", /* this is wrong but we don't care because
+       it should fail anyway */
+      2 /* forbidden because passphrase len is too small */
+    },
+    {
+      "passwordPASSWORDpassword", 24,
+      GCRY_KDF_PBKDF2, GCRY_MD_SHA1,
+      "saltSALTsaltSAL", 15,
+      4096,
+      25,
+      "\x3d\x2e\xec\x4f\xe4\x1c\x84\x9b\x80\xc8"
+      "\xd8\x36\x62\xc0\xe4\x4a\x8b\x29\x1a\x96"
+      "\x4c\xf2\xf0\x70\x38", /* this is wrong but we don't care because
+       it should fail anyway */
+      2 /* forbidden because salt len is too small */
+    },
+    {
+      "passwordPASSWORDpassword", 24,
+      GCRY_KDF_PBKDF2, GCRY_MD_SHA1,
+      "saltSALTsaltSALTsaltSALTsaltSALTsalt", 36,
+      999,
+      25,
+      "\x3d\x2e\xec\x4f\xe4\x1c\x84\x9b\x80\xc8"
+      "\xd8\x36\x62\xc0\xe4\x4a\x8b\x29\x1a\x96"
+      "\x4c\xf2\xf0\x70\x38", /* this is wrong but we don't care because
+       it should fail anyway */
+      2 /* forbidden because too few iterations */
+    },
+    {
+      "passwordPASSWORDpassword", 24,
+      GCRY_KDF_PBKDF2, GCRY_MD_SHA1,
+      "saltSALTsaltSALTsaltSALTsaltSALTsalt", 36,
+      4096,
+      13,
+      "\x3d\x2e\xec\x4f\xe4\x1c\x84\x9b\x80\xc8"
+      "\xd8\x36\x62", /* this is wrong but we don't care because
+       it should fail anyway */
+      2 /* forbidden because key size too small */
+    },
+  };
+
+  int tvidx;
+  gpg_error_t err;
+  unsigned char outbuf[100];
+  int i;
+
+  for (tvidx=0; tvidx < DIM(tv); tvidx++)
+    {
+      if (verbose)
+        fprintf (stderr, "checking gcry_kdf_derive test vector %d algo %d for fips\n",
+                 tvidx, tv[tvidx].algo);
+      assert (tv[tvidx].dklen <= sizeof outbuf);
+      err = gcry_kdf_derive (tv[tvidx].p, tv[tvidx].plen,
+                             tv[tvidx].algo, tv[tvidx].subalgo,
+                             tv[tvidx].salt, tv[tvidx].saltlen,
+                             tv[tvidx].iterations, tv[tvidx].dklen, outbuf);
+
+      if (err)
+        {
+          if (tv[tvidx].expect_error == 0 || tv[tvidx].expect_error == 1)
+            fail ("gcry_kdf_derive test %d unexpectedly returned an error in FIPS mode: %s\n",
+                  tvidx, gpg_strerror (err));
+        }
+      else
+        {
+          unsigned long fips_service_indicator;
+
+          gcry_get_fips_service_indicator (&fips_service_indicator);
+
+          /* Failure by an error expected.  Something goes wrong.  */
+          if (tv[tvidx].expect_error == 2)
+            {
+              fail ("gcry_kdf_derive test %d unexpectedly succeed in FIPS mode.\n",
+                    tvidx);
+              continue;
+            }
+
+          /* Success with fips_service_indicator == 0 expected.  */
+          if (tv[tvidx].expect_error == 0 && fips_service_indicator)
+            {
+              fail ("gcry_kdf_derive test %d unexpectedly set %08lx in FIPS mode.\n",
+                    tvidx, fips_service_indicator);
+              continue;
+            }
+
+#define COMPUTATION_MAY_NOT_BE_DONE_IN_THIS_CASE 0 /* Yes, it computes.  */
+#if COMPUTATION_MAY_NOT_BE_DONE_IN_THIS_CASE
+          /* Failure with fips_service_indicator != 0 expected.  */
+          if (fips_service_indicator && tv[tvidx].expect_error == 1)
+            continue;
+#else
+          /* It's a failure, but let us check the value.  */
+#endif
+
+          if (memcmp (outbuf, tv[tvidx].dk, tv[tvidx].dklen))
+            {
+              fail ("gcry_kdf_derive test %d failed: mismatch\n", tvidx);
+              fputs ("got:", stderr);
+              for (i=0; i < tv[tvidx].dklen; i++)
+                fprintf (stderr, " %02x", outbuf[i]);
+              putc ('\n', stderr);
+            }
+        }
+    }
+}
+
+
 int
 main (int argc, char **argv)
 {
@@ -2008,7 +2161,9 @@ main (int argc, char **argv)
       check_onestep_kdf ();
       check_hkdf ();
       if (in_fips_mode)
-        check_fips_indicators();
+        check_fips_indicators ();
+      if (in_fips_mode)
+        check_fips_gcry_kdf_derive ();
     }
 
   return error_count ? 1 : 0;
_______________________________________________
Gcrypt-devel mailing list
Gcrypt-devel@gnupg.org
https://lists.gnupg.org/mailman/listinfo/gcrypt-devel

Reply via email to