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