Refactor HMAC selftest into MAC selftest

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Refactor HMAC selftest into MAC selftest

NIIBE Yutaka
Hello,

For FIPS 140 things, I created a branch to review works from Red Hat:

    gniibe/fips-from-redhat
    https://dev.gnupg.org/source/libgcrypt/history/gniibe%252Ffips-from-redhat/

Reviewing libgcrypt-1.8.3-cmac-selftest.patch, before actually working
for that, I think that we need some improvement in our selftest.

Here is my proposal #1 to refactor HMAC selftest into MAC selftest, so
that we can add CMAC selftest (and GMAC) easily.  (Proposal #2 is
merging hmac-tests.c into mac-hmac.c (and removing hmac-tests.c).)

This patch does:

   Add new member 'selftest' in gcry_mac_spec_ops_t.
   Add an internal function _gcry_mac_selftest.
   Rename run_hmac_selftests in fips.c to run_mac_selftests.
   Remove the function _gcry_hmac_selftest in hmac-tests.c.

Any comments will be appriciated.

--------------------------
diff --git a/cipher/Makefile.am b/cipher/Makefile.am
index 4798d456..eca87d1a 100644
--- a/cipher/Makefile.am
+++ b/cipher/Makefile.am
@@ -26,7 +26,7 @@ AM_CFLAGS = $(GPG_ERROR_CFLAGS)
 
 AM_CCASFLAGS = $(NOEXECSTACK_FLAGS)
 
-EXTRA_DIST = gost-s-box.c
+EXTRA_DIST = gost-s-box.c hmac-tests.c
 
 CLEANFILES = gost-s-box
 DISTCLEANFILES = gost-sb.h
@@ -61,7 +61,6 @@ libcipher_la_SOURCES = \
  mac-hmac.c mac-cmac.c mac-gmac.c mac-poly1305.c \
  poly1305.c poly1305-internal.h \
  kdf.c kdf-internal.h \
- hmac-tests.c \
  bithelp.h  \
  bufhelp.h  \
  primegen.c  \
diff --git a/cipher/gost28147.c b/cipher/gost28147.c
index df16c3c6..1bafe317 100644
--- a/cipher/gost28147.c
+++ b/cipher/gost28147.c
@@ -542,6 +542,7 @@ static gcry_mac_spec_ops_t gost_imit_ops = {
   gost_imit_get_maclen,
   gost_imit_get_keylen,
   gost_imit_set_extra_info,
+  NULL
 };
 
 gcry_mac_spec_t _gcry_mac_type_spec_gost28147_imit =
diff --git a/cipher/hmac-tests.c b/cipher/hmac-tests.c
index 78d260a1..f5bac69c 100644
--- a/cipher/hmac-tests.c
+++ b/cipher/hmac-tests.c
@@ -1098,6 +1098,7 @@ selftests_sha3 (int hashalgo, int extended, selftest_report_func_t report)
 }
 
 
+#if 0
 /* Run a full self-test for ALGO and return 0 on success.  */
 static gpg_err_code_t
 run_selftests (int algo, int extended, selftest_report_func_t report)
@@ -1138,7 +1139,6 @@ run_selftests (int algo, int extended, selftest_report_func_t report)
 
 
 
-
 /* Run the selftests for HMAC with digest algorithm ALGO with optional
    reporting function REPORT.  */
 gpg_error_t
@@ -1158,3 +1158,4 @@ _gcry_hmac_selftest (int algo, int extended, selftest_report_func_t report)
     }
   return gpg_error (ec);
 }
+#endif
diff --git a/cipher/mac-cmac.c b/cipher/mac-cmac.c
index 120fa3df..f4d0ce59 100644
--- a/cipher/mac-cmac.c
+++ b/cipher/mac-cmac.c
@@ -157,6 +157,7 @@ static gcry_mac_spec_ops_t cmac_ops = {
   cmac_verify,
   cmac_get_maclen,
   cmac_get_keylen,
+  NULL,
   NULL
 };
 
diff --git a/cipher/mac-gmac.c b/cipher/mac-gmac.c
index aa78c7e3..b9805eea 100644
--- a/cipher/mac-gmac.c
+++ b/cipher/mac-gmac.c
@@ -150,6 +150,7 @@ static gcry_mac_spec_ops_t gmac_ops = {
   gmac_verify,
   gmac_get_maclen,
   gmac_get_keylen,
+  NULL,
   NULL
 };
 
diff --git a/cipher/mac-hmac.c b/cipher/mac-hmac.c
index d0cc5775..2dca4253 100644
--- a/cipher/mac-hmac.c
+++ b/cipher/mac-hmac.c
@@ -222,6 +222,49 @@ hmac_get_keylen (int algo)
     }
 }
 
+#include "hmac-tests.c"
+
+static gpg_err_code_t
+hmac_selftest (int algo, int extended, selftest_report_func_t report)
+{
+  gpg_err_code_t ec;
+
+  switch (algo)
+    {
+    case GCRY_MAC_HMAC_SHA1:
+      ec = selftests_sha1 (extended, report);
+      break;
+    case GCRY_MAC_HMAC_SHA224:
+      ec = selftests_sha224 (extended, report);
+      break;
+    case GCRY_MAC_HMAC_SHA256:
+      ec = selftests_sha256 (extended, report);
+      break;
+    case GCRY_MAC_HMAC_SHA384:
+      ec = selftests_sha384 (extended, report);
+      break;
+    case GCRY_MAC_HMAC_SHA512:
+      ec = selftests_sha512 (extended, report);
+      break;
+
+    case GCRY_MAC_HMAC_SHA3_224:
+    case GCRY_MAC_HMAC_SHA3_256:
+    case GCRY_MAC_HMAC_SHA3_384:
+    case GCRY_MAC_HMAC_SHA3_512:
+      {
+        int md_algo = map_mac_algo_to_md (algo);
+        ec = selftests_sha3 (md_algo, extended, report);
+      }
+      break;
+
+    default:
+      ec = GPG_ERR_MAC_ALGO;
+      break;
+    }
+
+  return ec;
+}
+
 
 static const gcry_mac_spec_ops_t hmac_ops = {
   hmac_open,
@@ -234,7 +277,8 @@ static const gcry_mac_spec_ops_t hmac_ops = {
   hmac_verify,
   hmac_get_maclen,
   hmac_get_keylen,
-  NULL
+  NULL,
+  hmac_selftest
 };
 
 
diff --git a/cipher/mac-internal.h b/cipher/mac-internal.h
index 8c13520b..d907a46f 100644
--- a/cipher/mac-internal.h
+++ b/cipher/mac-internal.h
@@ -20,6 +20,7 @@
 #include <config.h>
 
 #include "g10lib.h"
+#include "cipher-proto.h"
 #include "gost.h"
 
 
@@ -81,6 +82,7 @@ typedef struct gcry_mac_spec_ops
   gcry_mac_get_maclen_func_t get_maclen;
   gcry_mac_get_keylen_func_t get_keylen;
   gcry_mac_set_extra_info_t set_extra_info;
+  selftest_func_t selftest;
 } gcry_mac_spec_ops_t;
 
 
diff --git a/cipher/mac-poly1305.c b/cipher/mac-poly1305.c
index 39ba790f..d27a31c6 100644
--- a/cipher/mac-poly1305.c
+++ b/cipher/mac-poly1305.c
@@ -323,7 +323,8 @@ static gcry_mac_spec_ops_t poly1305mac_ops = {
   poly1305mac_verify,
   poly1305mac_get_maclen,
   poly1305mac_get_keylen,
-  NULL
+  NULL,
+  NULL,
 };
 
 
diff --git a/cipher/mac.c b/cipher/mac.c
index 933be74c..e274b356 100644
--- a/cipher/mac.c
+++ b/cipher/mac.c
@@ -781,3 +781,28 @@ _gcry_mac_algo_info (int algo, int what, void *buffer, size_t * nbytes)
 
   return rc;
 }
+
+
+/* Run the self-tests for the MAC.  */
+gpg_error_t
+_gcry_mac_selftest (int algo, int extended, selftest_report_func_t report)
+{
+  gcry_err_code_t ec;
+  gcry_mac_spec_t *spec;
+
+  spec = spec_from_algo (algo);
+  if (spec && !spec->flags.disabled && spec->ops && spec->ops->selftest)
+    ec = spec->ops->selftest (algo, extended, report);
+  else
+    {
+      ec = GPG_ERR_MAC_ALGO;
+      if (report)
+        report ("mac", algo, "module",
+                spec && !spec->flags.disabled?
+                "no selftest available" :
+                spec? "algorithm disabled" :
+                "algorithm not found");
+    }
+
+  return gpg_error (ec);
+}
diff --git a/src/fips.c b/src/fips.c
index 1ac7f477..94ffbd20 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -493,21 +493,21 @@ run_digest_selftests (int extended)
 }
 
 
-/* Run self-tests for all HMAC algorithms.  Return 0 on success. */
+/* Run self-tests for MAC algorithms.  Return 0 on success. */
 static int
-run_hmac_selftests (int extended)
+run_mac_selftests (int extended)
 {
   static int algos[] =
     {
-      GCRY_MD_SHA1,
-      GCRY_MD_SHA224,
-      GCRY_MD_SHA256,
-      GCRY_MD_SHA384,
-      GCRY_MD_SHA512,
-      GCRY_MD_SHA3_224,
-      GCRY_MD_SHA3_256,
-      GCRY_MD_SHA3_384,
-      GCRY_MD_SHA3_512,
+      GCRY_MAC_HMAC_SHA1,
+      GCRY_MAC_HMAC_SHA224,
+      GCRY_MAC_HMAC_SHA256,
+      GCRY_MAC_HMAC_SHA384,
+      GCRY_MAC_HMAC_SHA512,
+      GCRY_MAC_HMAC_SHA3_224,
+      GCRY_MAC_HMAC_SHA3_256,
+      GCRY_MAC_HMAC_SHA3_384,
+      GCRY_MAC_HMAC_SHA3_512,
       0
     };
   int idx;
@@ -516,8 +516,8 @@ run_hmac_selftests (int extended)
 
   for (idx=0; algos[idx]; idx++)
     {
-      err = _gcry_hmac_selftest (algos[idx], extended, reporter);
-      reporter ("hmac", algos[idx], NULL,
+      err = _gcry_mac_selftest (algos[idx], extended, reporter);
+      reporter ("mac", algos[idx], NULL,
                 err? gpg_strerror (err):NULL);
       if (err)
         anyerr = 1;
@@ -678,7 +678,7 @@ _gcry_fips_run_selftests (int extended)
   if (run_digest_selftests (extended))
     goto leave;
 
-  if (run_hmac_selftests (extended))
+  if (run_mac_selftests (extended))
     goto leave;
 
   /* Run random tests before the pubkey tests because the latter
--

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel
Reply | Threaded
Open this post in threaded view
|

Re: Refactor HMAC selftest into MAC selftest

GnuPG - Libgcrypt - Dev mailing list
On Wed, 16 Dec 2020 12:19, NIIBE Yutaka said:

> This patch does:
>
>    Add new member 'selftest' in gcry_mac_spec_ops_t.
>    Add an internal function _gcry_mac_selftest.
>    Rename run_hmac_selftests in fips.c to run_mac_selftests.
>    Remove the function _gcry_hmac_selftest in hmac-tests.c.

Okay with me.

FWIW: Actually it would be better if we could factor out the hmac code
from the md functions and divert hmac requests in gcry_md_* directly to
gcry_mac_* functions.  However, that is a too intrusive change for now
and thus we need to stick to the structure we have.


Shalom-Salam,

   Werner

--
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel

signature.asc (233 bytes) Download Attachment