Page MenuHome GnuPG

Fix tests in FIPS mode
Open, HighPublic

Description

At this moment, libgcrypt tests fail in FIPS mode because they are not updated to match FIPS behavior (mostly secmem and SHA1).

The attached patch resolves the issue and makes the tests passing in FIPS mode.

This is not an issue of high priority, but having tests working in FIPS mode gives more confidence in the functionality of libgcrypt in FIPS mode.

Event Timeline

werner triaged this task as High priority.Tue, Jul 6, 3:33 PM

Thank you for your report.

I'll fix master so that it doesn't fail when FIPS mode is enabled.

Firstly, I'm going to fix the first part (tests/basic.c for checking with gcry_md_test_algo),
then, more fixes for tests/basic.c, and proceed to other parts.

I applied rC297d31294333: tests: Fix messages to STDERR when FIPS mode is enabled.. Please note that your intention to change check_digests is right, but your patch actually didn't; When a MD algo is not supported, gcry_md_test_algo returns != 0 (an error code), and it "continues" to next entry (before the change).

That reminds me that we we should replace libgcrypt's internal debug functions by those from gpgrt. We have a dependency for gpgrt anyway and thus we should avoid code duplication. Sure we will keep the existsing public functions but that is easy given that gpgrt comes with gpgrt_logv since 1.28 which we can make mandatory (currently libgcrypt requires 1.27 (from 2017, with 1.28 is from 2018)

Thank you for checking and for revised patch. I tested your patch and it works fine for the basic test up until this failure with the crcalgo:

basic: algo 316, crcalgo: 3, gcry_md_open failed: Invalid digest algorithm
basic: algo 317, crcalgo: 3, gcry_md_open failed: Invalid digest algorithm

These are GCRY_MD_SHAKE128 and GCRY_MD_SHAKE256, but the md used here is actually GCRY_MD_RMD160 which is hardcoded and not allowed in FIPS.

I used the following patch to workaround this issue:

diff --git a/tests/basic.c b/tests/basic.c
index a2c25766..07bb14aa 100644
--- a/tests/basic.c
+++ b/tests/basic.c
@@ -10503,7 +10503,7 @@ check_one_md (int algo, const char *data, int len, const char *expect, int elen,
 
       if (*data == '!' && !data[1])
 	{
-	  int crcalgo = GCRY_MD_RMD160;
+	  int crcalgo = GCRY_MD_SHA256;
 	  gcry_md_hd_t crc1, crc2;
 	  size_t startlen;
 	  size_t piecelen;

Not sure if it is the only or best solution, but it passes and as far as I see, the test is not dependent on this particular digest algorithm.

That crcalgo can be any digest algorithm and SHA256 seems best option to me.

Update: still ./basic --fips fails (for me), because of GCM (18 errors).
Need to fix T4873: Enable AES GCM in FIPS mode.

With `/etc/gcrypt/fips_enabled/', make check fails by:

FAIL: t-secmem
FAIL: t-mpi-point
FAIL: curves
FAIL: basic
FAIL: keygen
FAIL: pubkey
FAIL: keygrip
FAIL: random
FAIL: dsa-rfc6979
FAIL: t-ed25519
FAIL: t-cv25519
FAIL: t-x448
FAIL: t-ed448
FAIL: basic-disable-all-hwf
FAIL: benchmark
FAIL: bench-slope

I was so far testing with changes on top of our patches.

One of the fixes for the t-secmem was submitted in the initial comment. There are couple of more in Fedora repository disabling the gcm/poly1305 tests in fips mode:

https://src.fedoraproject.org/rpms/libgcrypt/blob/rawhide/f/libgcrypt-1.8.4-tests-fipsmode.patch

There is also the changeset for dsa-rfc6979 and benchmark, which will require some better handling in upstream, because the NIST P-192 is not allowed in FIPS mode:

https://src.fedoraproject.org/rpms/libgcrypt/blob/rawhide/f/libgcrypt-1.7.3-ecc-test-fix.patch

The t-x448 should be skipped in FIPS mode too.

Let me know if you want me to have a look into these patches and propose a upstreamable patch or you want to take care of that.

I went through the patches above + what I suggested in previous comments, tested everything against both upstream and libgcrypt in Fedora in FIPS mode. There were slight differences, some cases were already fixed in master, some needed to upstream some of our changes, but the result is 10 patches working in both FIPS and non-fips mode, hopefully enough annotated. If not, please, ask for clarifications.

Where I am not completely sure is the commit cipher: Do not use of non-approved digests in FIPS mode, which disables non-FIPS approved MD mechanisms (and prevents messing up FIPS state if something tries to use it in non-enforced mode). Having a better look, it might need similar logic as in cipher.c already. The _gcry_md_init() already disables the non-FIPS digests, but md_open does not consult this flags.disabled anywhere, but just blindly calls md_enable(), which jumps into this "trap". It might be enough to check for the flags.disabled in here, but I am really not sure what all consequences is this going to have.

As a start, I applied your patches.