Re: FIPS 140 service indicator revamp

>From d060dd58b82882dec0d8bfcc593536bc0083b4b1 Mon Sep 17 00:00:00 2001
From: NIIBE Yutaka <[email protected]>
Date: Fri, 20 Dec 2024 09:38:13 +0900
Subject: [PATCH] fips: Rejection by GCRYCTL_FIPS_REJECT_NON_FIPS, not by open
 flags.

* src/gcrypt.h.in (GCRY_CIPHER_FLAG_REJECT_NON_FIPS): Remove.
(GCRY_MD_FLAG_REJECT_NON_FIPS): Remove.
(GCRY_MAC_FLAG_REJECT_NON_FIPS): Remove.
* tests/t-fips-service-ind.c: Update tests with
GCRYCTL_FIPS_REJECT_NON_FIPS.
* cipher/cipher.c (_gcry_cipher_open_internal, cipher_setkey): Use
fips_check_rejection.
* cipher/mac.c (mac_open): Likewise.
* cipher/md.c (struct gcry_md_context): Remove reject_non_fips.
(md_open, md_enable): Use fips_check_rejection.
(_gcry_md_enable, md_copy): Likewise.

--

GnuPG-bug-id: 7338
Signed-off-by: NIIBE Yutaka <[email protected]>
---
 cipher/cipher.c            |  8 ++--
 cipher/mac.c               |  5 +--
 cipher/md.c                | 81 ++++++++++++++++++++++++++++++--------
 src/gcrypt.h.in            |  7 +---
 tests/t-fips-service-ind.c | 59 +++++++++++----------------
 5 files changed, 94 insertions(+), 66 deletions(-)

diff --git a/cipher/cipher.c b/cipher/cipher.c
index 7ffacf05..74dc2df7 100644
--- a/cipher/cipher.c
+++ b/cipher/cipher.c
@@ -510,7 +510,6 @@ _gcry_cipher_open_internal (gcry_cipher_hd_t *handle,
 			    int algo, int mode, unsigned int flags)
 {
   int secure = !!(flags & GCRY_CIPHER_SECURE);
-  int reject_non_fips = !!(flags & GCRY_CIPHER_FLAG_REJECT_NON_FIPS);
   gcry_cipher_spec_t *spec;
   gcry_cipher_hd_t h = NULL;
   gcry_err_code_t err;
@@ -526,7 +525,7 @@ _gcry_cipher_open_internal (gcry_cipher_hd_t *handle,
     err = GPG_ERR_CIPHER_ALGO;
   else if (!spec->flags.fips && fips_mode ())
     {
-      if (reject_non_fips)
+      if (fips_check_rejection (GCRY_FIPS_FLAG_REJECT_CIPHER))
         err = GPG_ERR_CIPHER_ALGO;
       else
         {
@@ -544,8 +543,7 @@ _gcry_cipher_open_internal (gcry_cipher_hd_t *handle,
 		     | GCRY_CIPHER_ENABLE_SYNC
 		     | GCRY_CIPHER_CBC_CTS
 		     | GCRY_CIPHER_CBC_MAC
-                     | GCRY_CIPHER_EXTENDED
-                     | GCRY_CIPHER_FLAG_REJECT_NON_FIPS))
+                     | GCRY_CIPHER_EXTENDED))
 	  || ((flags & GCRY_CIPHER_CBC_CTS) && (flags & GCRY_CIPHER_CBC_MAC))))
     err = GPG_ERR_CIPHER_ALGO;
 
@@ -776,7 +774,7 @@ cipher_setkey (gcry_cipher_hd_t c, byte *key, size_t keylen)
 	     Key Generation Requirements" for details.  */
 	  if (buf_eq_const (key, key + keylen, keylen))
             {
-              if ((c->flags & GCRY_CIPHER_FLAG_REJECT_NON_FIPS))
+              if (fips_check_rejection (GCRY_FIPS_FLAG_REJECT_CIPHER))
                 return GPG_ERR_WEAK_KEY;
               else
                 fips_service_indicator_mark_non_compliant ();
diff --git a/cipher/mac.c b/cipher/mac.c
index 0df48fd7..6bf966fd 100644
--- a/cipher/mac.c
+++ b/cipher/mac.c
@@ -519,7 +519,6 @@ mac_open (gcry_mac_hd_t * hd, int algo, unsigned int flags, gcry_ctx_t ctx)
   gcry_err_code_t err;
   gcry_mac_hd_t h;
   int secure = !!(flags & GCRY_MAC_FLAG_SECURE);
-  int reject_non_fips = !!(flags & GCRY_MAC_FLAG_REJECT_NON_FIPS);
 
   spec = spec_from_algo (algo);
   if (!spec)
@@ -528,7 +527,7 @@ mac_open (gcry_mac_hd_t * hd, int algo, unsigned int flags, gcry_ctx_t ctx)
     return GPG_ERR_MAC_ALGO;
   else if (!spec->flags.fips && fips_mode ())
     {
-      if (reject_non_fips)
+      if (fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MAC))
         return GPG_ERR_MAC_ALGO;
       else
         fips_service_indicator_mark_non_compliant ();
@@ -650,7 +649,7 @@ _gcry_mac_open (gcry_mac_hd_t * h, int algo, unsigned int flags,
   gcry_err_code_t rc;
   gcry_mac_hd_t hd = NULL;
 
-  if ((flags & ~(GCRY_MAC_FLAG_SECURE | GCRY_MAC_FLAG_REJECT_NON_FIPS)))
+  if ((flags & ~GCRY_MAC_FLAG_SECURE))
     rc = GPG_ERR_INV_ARG;
   else
     rc = mac_open (&hd, algo, flags, ctx);
diff --git a/cipher/md.c b/cipher/md.c
index 08a564ad..f600e7bb 100644
--- a/cipher/md.c
+++ b/cipher/md.c
@@ -275,7 +275,6 @@ struct gcry_md_context
     unsigned int finalized:1;
     unsigned int bugemu1:1;
     unsigned int hmac:1;
-    unsigned int reject_non_fips:1;
   } flags;
   size_t actual_handle_size;     /* Allocated size of this handle. */
   FILE  *debug;
@@ -509,7 +508,6 @@ md_open (gcry_md_hd_t *h, int algo, unsigned int flags)
       ctx->flags.secure = secure;
       ctx->flags.hmac = hmac;
       ctx->flags.bugemu1 = !!(flags & GCRY_MD_FLAG_BUGEMU1);
-      ctx->flags.reject_non_fips = !!(flags & GCRY_MD_FLAG_REJECT_NON_FIPS);
     }
 
   if (! err)
@@ -544,14 +542,11 @@ _gcry_md_open (gcry_md_hd_t *h, int algo, unsigned int flags)
 
   if ((flags & ~(GCRY_MD_FLAG_SECURE
                  | GCRY_MD_FLAG_HMAC
-                 | GCRY_MD_FLAG_REJECT_NON_FIPS
                  | GCRY_MD_FLAG_BUGEMU1)))
     rc = GPG_ERR_INV_ARG;
   else
     rc = md_open (&hd, algo, flags);
 
-  *h = rc? NULL : hd;
-
   if (!rc && fips_mode ())
     {
       GcryDigestEntry *entry = hd->ctx->list;
@@ -566,9 +561,26 @@ _gcry_md_open (gcry_md_hd_t *h, int algo, unsigned int flags)
         }
 
       if (!is_compliant_algo)
-        fips_service_indicator_mark_non_compliant ();
+        {
+          int reject = 0;
+
+          if (algo == GCRY_MD_MD5)
+            reject = fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_MD5);
+          else
+            reject = fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_OTHERS);
+
+          if (reject)
+            {
+              md_close (hd);
+              hd = NULL;
+              rc = GPG_ERR_DIGEST_ALGO;
+            }
+          else
+            fips_service_indicator_mark_non_compliant ();
+        }
     }
 
+  *h = rc? NULL : hd;
   return rc;
 }
 
@@ -581,12 +593,17 @@ md_enable (gcry_md_hd_t hd, int algorithm)
   const gcry_md_spec_t *spec;
   GcryDigestEntry *entry;
   gcry_err_code_t err = 0;
-  int reject_non_fips = h->flags.reject_non_fips;
+  int reject;
 
   for (entry = h->list; entry; entry = entry->next)
     if (entry->spec->algo == algorithm)
       return 0; /* Already enabled */
 
+  if (algorithm == GCRY_MD_MD5)
+    reject = fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_MD5);
+  else
+    reject = fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_OTHERS);
+
   spec = spec_from_algo (algorithm);
   if (!spec)
     {
@@ -598,7 +615,7 @@ md_enable (gcry_md_hd_t hd, int algorithm)
     err = GPG_ERR_DIGEST_ALGO;
 
   /* Any non-FIPS algorithm should go this way */
-  if (!err && reject_non_fips && !spec->flags.fips && fips_mode ())
+  if (!err && reject && !spec->flags.fips && fips_mode ())
     err = GPG_ERR_DIGEST_ALGO;
 
   if (!err && h->flags.hmac && spec->read == NULL)
@@ -657,7 +674,19 @@ _gcry_md_enable (gcry_md_hd_t hd, int algorithm)
         }
 
       if (!is_compliant_algo)
-        fips_service_indicator_mark_non_compliant ();
+        {
+          int reject = 0;
+
+          if (algorithm == GCRY_MD_MD5)
+            reject = fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_MD5);
+          else
+            reject = fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_OTHERS);
+
+          if (reject)
+            rc = GPG_ERR_DIGEST_ALGO;
+          else
+            fips_service_indicator_mark_non_compliant ();
+        }
     }
 
   return rc;
@@ -667,13 +696,14 @@ _gcry_md_enable (gcry_md_hd_t hd, int algorithm)
 static gcry_err_code_t
 md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
 {
-  gcry_err_code_t err = 0;
+  gcry_err_code_t rc = 0;
   struct gcry_md_context *a = ahd->ctx;
   struct gcry_md_context *b;
   GcryDigestEntry *ar, *br;
   gcry_md_hd_t bhd;
   size_t n;
   int is_compliant_algo = 1;
+  int reject = 0;
 
   if (ahd->bufpos)
     md_write (ahd, NULL, 0);
@@ -686,7 +716,7 @@ md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
 
   if (!bhd)
     {
-      err = gpg_err_code_from_syserror ();
+      rc = gpg_err_code_from_syserror ();
       goto leave;
     }
 
@@ -715,12 +745,20 @@ md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
         br = xtrymalloc (ar->actual_struct_size);
       if (!br)
         {
-          err = gpg_err_code_from_syserror ();
+          rc = gpg_err_code_from_syserror ();
           md_close (bhd);
           goto leave;
         }
 
-      is_compliant_algo &= spec->flags.fips;
+      if (!spec->flags.fips)
+        {
+          is_compliant_algo = 0;
+
+          if (spec->algo == GCRY_MD_MD5)
+            reject |= fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_MD5);
+          else
+            reject |= fips_check_rejection (GCRY_FIPS_FLAG_REJECT_MD_OTHERS);
+        }
 
       memcpy (br, ar, ar->actual_struct_size);
       br->next = b->list;
@@ -730,13 +768,22 @@ md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
   if (a->debug)
     md_start_debug (bhd, "unknown");
 
-  *b_hd = bhd;
+  if (!is_compliant_algo && fips_mode ())
+    {
+      if (reject)
+        {
+          rc = GPG_ERR_DIGEST_ALGO;
+          md_close (bhd);
+        }
+      else
+        fips_service_indicator_mark_non_compliant ();
+    }
 
-  if (!is_compliant_algo)
-    fips_service_indicator_mark_non_compliant ();
+  if (!rc)
+    *b_hd = bhd;
 
  leave:
-  return err;
+  return rc;
 }
 
 
diff --git a/src/gcrypt.h.in b/src/gcrypt.h.in
index 4a882c55..7bee45e9 100644
--- a/src/gcrypt.h.in
+++ b/src/gcrypt.h.in
@@ -993,8 +993,7 @@ enum gcry_cipher_flags
     GCRY_CIPHER_ENABLE_SYNC = 2,  /* Enable CFB sync mode. */
     GCRY_CIPHER_CBC_CTS     = 4,  /* Enable CBC cipher text stealing (CTS). */
     GCRY_CIPHER_CBC_MAC     = 8,  /* Enable CBC message auth. code (MAC).  */
-    GCRY_CIPHER_EXTENDED    = 16, /* Enable extended AES-WRAP.  */
-    GCRY_CIPHER_FLAG_REJECT_NON_FIPS = 32 /* Reject non-FIPS-compliant algo.  */
+    GCRY_CIPHER_EXTENDED    = 16  /* Enable extended AES-WRAP.  */
   };
 
 /* Methods used for AEAD IV generation. */
@@ -1320,7 +1319,6 @@ enum gcry_md_flags
   {
     GCRY_MD_FLAG_SECURE = 1,  /* Allocate all buffers in "secure" memory.  */
     GCRY_MD_FLAG_HMAC   = 2,  /* Make an HMAC out of this algorithm.  */
-    GCRY_MD_FLAG_REJECT_NON_FIPS = 4,  /* Reject non-FIPS-compliant algo.  */
     GCRY_MD_FLAG_BUGEMU1 = 0x0100
   };
 
@@ -1562,8 +1560,7 @@ enum gcry_mac_algos
 /* Flags used with the open function.  */
 enum gcry_mac_flags
   {
-    GCRY_MAC_FLAG_SECURE = 1,  /* Allocate all buffers in "secure" memory.  */
-    GCRY_MAC_FLAG_REJECT_NON_FIPS = 2   /* Reject non-FIPS-compliant algo.  */
+    GCRY_MAC_FLAG_SECURE = 1  /* Allocate all buffers in "secure" memory.  */
   };
 
 /* Create a MAC handle for algorithm ALGO.  FLAGS may be given as an bitwise OR
diff --git a/tests/t-fips-service-ind.c b/tests/t-fips-service-ind.c
index d092bfae..64e1e135 100644
--- a/tests/t-fips-service-ind.c
+++ b/tests/t-fips-service-ind.c
@@ -250,7 +250,7 @@ check_pk_hash_sign_verify (void)
 /* Check gcry_cipher_open, gcry_cipher_setkey, gcry_cipher_encrypt,
    gcry_cipher_decrypt, gcry_cipher_close API.  */
 static void
-check_cipher_o_s_e_d_c (void)
+check_cipher_o_s_e_d_c (int reject)
 {
   static struct {
     int algo;
@@ -258,18 +258,12 @@ check_cipher_o_s_e_d_c (void)
     int keylen;
     const char *expect;
     int expect_failure;
-    unsigned int flags;
   } tv[] = {
 #if USE_DES
       { GCRY_CIPHER_3DES,
 	"\xe3\x34\x7a\x6b\x0b\xc1\x15\x2c\x64\x2a\x25\xcb\xd3\xbc\x31\xab"
 	"\xfb\xa1\x62\xa8\x1f\x19\x7c\x15", 24,
         "\x3f\x1a\xb8\x83\x18\x8b\xb5\x97", 1 },
-      { GCRY_CIPHER_3DES,
-	"\xe3\x34\x7a\x6b\x0b\xc1\x15\x2c\x64\x2a\x25\xcb\xd3\xbc\x31\xab"
-	"\xfb\xa1\x62\xa8\x1f\x19\x7c\x15", 24,
-        "\x3f\x1a\xb8\x83\x18\x8b\xb5\x97",
-        1, GCRY_CIPHER_FLAG_REJECT_NON_FIPS },
 #endif
       { GCRY_CIPHER_AES,
 	"\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab\xf7\x15\x88\x09\xcf\x4f\x3c", 16,
@@ -297,12 +291,10 @@ check_cipher_o_s_e_d_c (void)
       assert (blklen != 0);
       assert (blklen <= ptlen);
       assert (blklen <= DIM (out));
-      err = gcry_cipher_open (&h, tv[tvidx].algo, GCRY_CIPHER_MODE_ECB,
-                              tv[tvidx].flags);
+      err = gcry_cipher_open (&h, tv[tvidx].algo, GCRY_CIPHER_MODE_ECB, 0);
       if (err)
         {
-          if (in_fips_mode && (tv[tvidx].flags & GCRY_CIPHER_FLAG_REJECT_NON_FIPS)
-              && tv[tvidx].expect_failure)
+          if (in_fips_mode && reject && tv[tvidx].expect_failure)
             /* Here, an error is expected */
             ;
           else
@@ -312,8 +304,7 @@ check_cipher_o_s_e_d_c (void)
         }
       else
         {
-          if (in_fips_mode && (tv[tvidx].flags & GCRY_CIPHER_FLAG_REJECT_NON_FIPS)
-              && tv[tvidx].expect_failure)
+          if (in_fips_mode && reject && tv[tvidx].expect_failure)
             /* This case, an error is expected, but we observed success */
             fail ("gcry_cipher_open test %d unexpectedly succeeded\n", tvidx);
         }
@@ -398,7 +389,7 @@ check_cipher_o_s_e_d_c (void)
 /* Check gcry_mac_open, gcry_mac_write, gcry_mac_write, gcry_mac_read,
    gcry_mac_close API.  */
 static void
-check_mac_o_w_r_c (void)
+check_mac_o_w_r_c (int reject)
 {
   static struct {
     int algo;
@@ -408,14 +399,10 @@ check_mac_o_w_r_c (void)
     int keylen;
     const char *expect;
     int expect_failure;
-    unsigned int flags;
   } tv[] = {
 #if USE_MD5
     { GCRY_MAC_HMAC_MD5, "hmac input abc", 14, "hmac key input", 14,
       "\x0d\x72\xd0\x60\xaf\x34\xf2\xca\x33\x58\xa9\xcc\xd3\x5a\xac\xb5", 1 },
-    { GCRY_MAC_HMAC_MD5, "hmac input abc", 14, "hmac key input", 14,
-      "\x0d\x72\xd0\x60\xaf\x34\xf2\xca\x33\x58\xa9\xcc\xd3\x5a\xac\xb5", 1,
-      GCRY_MAC_FLAG_REJECT_NON_FIPS },
 #endif
 #if USE_SHA1
     { GCRY_MAC_HMAC_SHA1, "hmac input abc", 14, "hmac key input", 14,
@@ -471,11 +458,10 @@ check_mac_o_w_r_c (void)
       expectlen = gcry_mac_get_algo_maclen (tv[tvidx].algo);
       assert (expectlen != 0);
       assert (expectlen <= DIM (mac));
-      err = gcry_mac_open (&h, tv[tvidx].algo, tv[tvidx].flags, NULL);
+      err = gcry_mac_open (&h, tv[tvidx].algo, 0, NULL);
       if (err)
         {
-          if (in_fips_mode && (tv[tvidx].flags & GCRY_MAC_FLAG_REJECT_NON_FIPS)
-              && tv[tvidx].expect_failure)
+          if (in_fips_mode && reject && tv[tvidx].expect_failure)
             /* Here, an error is expected */
             ;
           else
@@ -485,8 +471,7 @@ check_mac_o_w_r_c (void)
         }
       else
         {
-          if (in_fips_mode && (tv[tvidx].flags & GCRY_MAC_FLAG_REJECT_NON_FIPS)
-              && tv[tvidx].expect_failure)
+          if (in_fips_mode && reject && tv[tvidx].expect_failure)
             /* This case, an error is expected, but we observed success */
             fail ("gcry_mac_open test %d unexpectedly succeeded\n", tvidx);
         }
@@ -563,7 +548,7 @@ check_mac_o_w_r_c (void)
 /* Check gcry_md_open, gcry_md_write, gcry_md_write, gcry_md_read,
    gcry_md_close API.  */
 static void
-check_md_o_w_r_c (void)
+check_md_o_w_r_c (int reject)
 {
   static struct {
     int algo;
@@ -571,14 +556,10 @@ check_md_o_w_r_c (void)
     int datalen;
     const char *expect;
     int expect_failure;
-    unsigned int flags;
   } tv[] = {
 #if USE_MD5
     { GCRY_MD_MD5, "abc", 3,
       "\x90\x01\x50\x98\x3C\xD2\x4F\xB0\xD6\x96\x3F\x7D\x28\xE1\x7F\x72", 1 },
-    { GCRY_MD_MD5, "abc", 3,
-      "\x90\x01\x50\x98\x3C\xD2\x4F\xB0\xD6\x96\x3F\x7D\x28\xE1\x7F\x72", 1,
-      GCRY_MD_FLAG_REJECT_NON_FIPS },
 #endif
 #if USE_SHA1
     { GCRY_MD_SHA1, "abc", 3,
@@ -632,11 +613,10 @@ check_md_o_w_r_c (void)
 
       expectlen = gcry_md_get_algo_dlen (tv[tvidx].algo);
       assert (expectlen != 0);
-      err = gcry_md_open (&h, tv[tvidx].algo, tv[tvidx].flags);
+      err = gcry_md_open (&h, tv[tvidx].algo, 0);
       if (err)
         {
-          if (in_fips_mode && (tv[tvidx].flags & GCRY_MD_FLAG_REJECT_NON_FIPS)
-              && tv[tvidx].expect_failure)
+          if (in_fips_mode && reject && tv[tvidx].expect_failure)
             /* Here, an error is expected */
             ;
           else
@@ -646,8 +626,7 @@ check_md_o_w_r_c (void)
         }
       else
         {
-          if (in_fips_mode && (tv[tvidx].flags & GCRY_MD_FLAG_REJECT_NON_FIPS)
-              && tv[tvidx].expect_failure)
+          if (in_fips_mode && reject && tv[tvidx].expect_failure)
             /* This case, an error is expected, but we observed success */
             fail ("gcry_md_open test %d unexpectedly succeeded\n", tvidx);
         }
@@ -1143,10 +1122,18 @@ main (int argc, char **argv)
   check_kdf_derive ();
   check_hash_buffer ();
   check_hash_buffers ();
-  check_md_o_w_r_c ();
-  check_mac_o_w_r_c ();
-  check_cipher_o_s_e_d_c ();
+  check_md_o_w_r_c (0);
+  check_mac_o_w_r_c (0);
+  check_cipher_o_s_e_d_c (0);
   check_pk_hash_sign_verify ();
 
+  xgcry_control ((GCRYCTL_FIPS_REJECT_NON_FIPS,
+                  (GCRY_FIPS_FLAG_REJECT_MD_MD5
+                   | GCRY_FIPS_FLAG_REJECT_COMPAT110)));
+
+  check_md_o_w_r_c (1);
+  check_mac_o_w_r_c (1);
+  check_cipher_o_s_e_d_c (1);
+
   return !!error_count;
 }
-- 
2.39.5