Mailing List Archive

Refactor HMAC selftest into MAC selftest
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
Gcrypt-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel
Re: Refactor HMAC selftest into MAC selftest [ In reply to ]
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.