Re: FIPS 140 service indicator revamp
NIIBE Yutaka via Gcrypt-devel <[email protected]>
| Newsgroups | gmane.comp.encryption.gpg.libgcrypt.devel |
|---|---|
| Message-ID | <[email protected]> |
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. -- _______________________________________________ Gcrypt-devel mailing list [email protected] https://lists.gnupg.org/mailman/listinfo/gcrypt-devel
fips-140-service-indicator-revamp-gniibe-00.patch
(text/x-diff, 9 KB)
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;