From 8a2d1b22ede5eeca4d104bb027b84f3ecfc69549 Mon Sep 17 00:00:00 2001 From: Clemens Lang Date: Thu, 11 May 2023 12:51:59 +0200 Subject: [PATCH] DH: Disable FIPS 186-4 type parameters in FIPS mode For DH parameter and key pair generation/verification, the DSA procedures specified in FIPS 186-4 are used. With the release of FIPS 186-5 and the removal of DSA, the approved status of these groups is in peril. Once the transition for DSA ends (this transition will be 1 year long and start once CMVP has published the guidance), no more submissions claiming DSA will be allowed. Hence, FIPS 186-type parameters will also be automatically non-approved. In the FIPS provider, disable validation of any DH parameters that are not well-known groups, and remove DH parameter generation completely. Adjust tests to use well-known groups or larger DH groups where this change would now cause failures, and skip tests that are expected to fail due to this change. Related: rhbz#2169757, rhbz#2169757 Signed-off-by: Clemens Lang --- crypto/dh/dh_backend.c | 10 ++++ crypto/dh/dh_check.c | 12 ++-- crypto/dh/dh_gen.c | 12 +++- crypto/dh/dh_key.c | 13 ++-- crypto/dh/dh_pmeth.c | 10 +++- providers/implementations/keymgmt/dh_kmgmt.c | 5 ++ test/endecode_test.c | 4 +- test/evp_libctx_test.c | 2 +- test/helpers/predefined_dhparams.c | 62 ++++++++++++++++++++ test/helpers/predefined_dhparams.h | 1 + test/recipes/80-test_cms.t | 4 +- test/recipes/80-test_ssl_old.t | 3 + 12 files changed, 118 insertions(+), 20 deletions(-) diff --git a/crypto/dh/dh_backend.c b/crypto/dh/dh_backend.c index 726843fd30..24c65ca84f 100644 --- a/crypto/dh/dh_backend.c +++ b/crypto/dh/dh_backend.c @@ -53,6 +53,16 @@ int ossl_dh_params_fromdata(DH *dh, const OSSL_PARAM params[]) if (!dh_ffc_params_fromdata(dh, params)) return 0; +#ifdef FIPS_MODULE + if (!ossl_dh_is_named_safe_prime_group(dh)) { + ERR_raise_data(ERR_LIB_DH, DH_R_BAD_FFC_PARAMETERS, + "FIPS 186-4 type domain parameters no longer allowed in" + " FIPS mode, since the required validation routines" + " were removed from FIPS 186-5"); + return 0; + } +#endif + param_priv_len = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_DH_PRIV_LEN); if (param_priv_len != NULL diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c index 0b391910d6..75581ca347 100644 --- a/crypto/dh/dh_check.c +++ b/crypto/dh/dh_check.c @@ -57,13 +57,15 @@ int DH_check_params(const DH *dh, int *ret) nid = DH_get_nid((DH *)dh); if (nid != NID_undef) return 1; + /* - * OR - * (2b) FFC domain params conform to FIPS-186-4 explicit domain param - * validity tests. + * FIPS 186-4 explicit domain parameters are no longer supported in FIPS mode. */ - return ossl_ffc_params_FIPS186_4_validate(dh->libctx, &dh->params, - FFC_PARAM_TYPE_DH, ret, NULL); + ERR_raise_data(ERR_LIB_DH, DH_R_BAD_FFC_PARAMETERS, + "FIPS 186-4 type domain parameters no longer allowed in" + " FIPS mode, since the required validation routines were" + " removed from FIPS 186-5"); + return 0; } #else int DH_check_params(const DH *dh, int *ret) diff --git a/crypto/dh/dh_gen.c b/crypto/dh/dh_gen.c index aec6b85316..9c55121067 100644 --- a/crypto/dh/dh_gen.c +++ b/crypto/dh/dh_gen.c @@ -38,18 +38,26 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int generator, int ossl_dh_generate_ffc_parameters(DH *dh, int type, int pbits, int qbits, BN_GENCB *cb) { - int ret, res; + int ret = 0; #ifndef FIPS_MODULE + int res; + if (type == DH_PARAMGEN_TYPE_FIPS_186_2) ret = ossl_ffc_params_FIPS186_2_generate(dh->libctx, &dh->params, FFC_PARAM_TYPE_DH, pbits, qbits, &res, cb); else -#endif ret = ossl_ffc_params_FIPS186_4_generate(dh->libctx, &dh->params, FFC_PARAM_TYPE_DH, pbits, qbits, &res, cb); +#else + /* In FIPS mode, we no longer support FIPS 186-4 domain parameters */ + ERR_raise_data(ERR_LIB_DH, DH_R_BAD_FFC_PARAMETERS, + "FIPS 186-4 type domain parameters no longer allowed in" + " FIPS mode, since the required generation routines were" + " removed from FIPS 186-5"); +#endif if (ret > 0) dh->dirty_cnt++; return ret; diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c index 4e9705beef..14c0b0b6b3 100644 --- a/crypto/dh/dh_key.c +++ b/crypto/dh/dh_key.c @@ -308,8 +308,12 @@ static int generate_key(DH *dh) goto err; } else { #ifdef FIPS_MODULE - if (dh->params.q == NULL) - goto err; + ERR_raise_data(ERR_LIB_DH, DH_R_BAD_FFC_PARAMETERS, + "FIPS 186-4 type domain parameters no longer" + " allowed in FIPS mode, since the required" + " generation routines were removed from FIPS" + " 186-5"); + goto err; #else if (dh->params.q == NULL) { /* secret exponent length, must satisfy 2^(l-1) <= p */ @@ -330,9 +334,7 @@ static int generate_key(DH *dh) if (!BN_clear_bit(priv_key, 0)) goto err; } - } else -#endif - { + } else { /* Do a partial check for invalid p, q, g */ if (!ossl_ffc_params_simple_validate(dh->libctx, &dh->params, FFC_PARAM_TYPE_DH, NULL)) @@ -348,6 +350,7 @@ static int generate_key(DH *dh) priv_key)) goto err; } +#endif } } diff --git a/crypto/dh/dh_pmeth.c b/crypto/dh/dh_pmeth.c index f201eede0d..30f90d15be 100644 --- a/crypto/dh/dh_pmeth.c +++ b/crypto/dh/dh_pmeth.c @@ -305,13 +305,17 @@ static DH *ffc_params_generate(OSSL_LIB_CTX *libctx, DH_PKEY_CTX *dctx, prime_len, subprime_len, &res, pcb); else -# endif - /* For FIPS we always use the DH_PARAMGEN_TYPE_FIPS_186_4 generator */ - if (dctx->paramgen_type >= DH_PARAMGEN_TYPE_FIPS_186_2) rv = ossl_ffc_params_FIPS186_4_generate(libctx, &ret->params, FFC_PARAM_TYPE_DH, prime_len, subprime_len, &res, pcb); +# else + /* In FIPS mode, we no longer support FIPS 186-4 domain parameters */ + ERR_raise_data(ERR_LIB_DH, DH_R_BAD_FFC_PARAMETERS, + "FIPS 186-4 type domain parameters no longer allowed in" + " FIPS mode, since the required generation routines were" + " removed from FIPS 186-5"); +# endif if (rv <= 0) { DH_free(ret); return NULL; diff --git a/providers/implementations/keymgmt/dh_kmgmt.c b/providers/implementations/keymgmt/dh_kmgmt.c index 9a7dde7c66..b3e7bca5ac 100644 --- a/providers/implementations/keymgmt/dh_kmgmt.c +++ b/providers/implementations/keymgmt/dh_kmgmt.c @@ -414,6 +414,11 @@ static int dh_validate(const void *keydata, int selection, int checktype) if ((selection & DH_POSSIBLE_SELECTIONS) == 0) return 1; /* nothing to validate */ +#ifdef FIPS_MODULE + /* In FIPS provider, always check the domain parameters to disallow + * operations on keys with FIPS 186-4 params. */ + selection |= OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS; +#endif if ((selection & OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS) != 0) { /* * Both of these functions check parameters. DH_check_params_ex() diff --git a/test/endecode_test.c b/test/endecode_test.c index e3f7b81f69..1b63daaed5 100644 --- a/test/endecode_test.c +++ b/test/endecode_test.c @@ -80,10 +80,10 @@ static EVP_PKEY *make_template(const char *type, OSSL_PARAM *genparams) * for testing only. Use a minimum key size of 2048 for security purposes. */ if (strcmp(type, "DH") == 0) - return get_dh512(keyctx); + return get_dh2048(keyctx); if (strcmp(type, "X9.42 DH") == 0) - return get_dhx512(keyctx); + return get_dhx_ffdhe2048(keyctx); # endif /* diff --git a/test/evp_libctx_test.c b/test/evp_libctx_test.c index 2448c35a14..92d484fb12 100644 --- a/test/evp_libctx_test.c +++ b/test/evp_libctx_test.c @@ -188,7 +188,7 @@ static int do_dh_param_keygen(int tstid, const BIGNUM **bn) if (!TEST_ptr(gen_ctx = EVP_PKEY_CTX_new_from_pkey(libctx, pkey_parm, NULL)) || !TEST_int_gt(EVP_PKEY_keygen_init(gen_ctx), 0) - || !TEST_int_eq(EVP_PKEY_keygen(gen_ctx, &pkey), expected)) + || !TEST_int_eq(EVP_PKEY_keygen(gen_ctx, &pkey) == 1, expected)) goto err; if (expected) { diff --git a/test/helpers/predefined_dhparams.c b/test/helpers/predefined_dhparams.c index 4bdadc4143..e5186e4b4a 100644 --- a/test/helpers/predefined_dhparams.c +++ b/test/helpers/predefined_dhparams.c @@ -116,6 +116,68 @@ EVP_PKEY *get_dhx512(OSSL_LIB_CTX *libctx) dhx512_q, sizeof(dhx512_q)); } +EVP_PKEY *get_dhx_ffdhe2048(OSSL_LIB_CTX *libctx) +{ + /* This is RFC 7919 ffdhe2048, since Red Hat removes support for + * non-well-known groups in FIPS mode. */ + static unsigned char dhx_p[] = { + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xad, 0xf8, 0x54, 0x58, + 0xa2, 0xbb, 0x4a, 0x9a, 0xaf, 0xdc, 0x56, 0x20, 0x27, 0x3d, 0x3c, 0xf1, + 0xd8, 0xb9, 0xc5, 0x83, 0xce, 0x2d, 0x36, 0x95, 0xa9, 0xe1, 0x36, 0x41, + 0x14, 0x64, 0x33, 0xfb, 0xcc, 0x93, 0x9d, 0xce, 0x24, 0x9b, 0x3e, 0xf9, + 0x7d, 0x2f, 0xe3, 0x63, 0x63, 0x0c, 0x75, 0xd8, 0xf6, 0x81, 0xb2, 0x02, + 0xae, 0xc4, 0x61, 0x7a, 0xd3, 0xdf, 0x1e, 0xd5, 0xd5, 0xfd, 0x65, 0x61, + 0x24, 0x33, 0xf5, 0x1f, 0x5f, 0x06, 0x6e, 0xd0, 0x85, 0x63, 0x65, 0x55, + 0x3d, 0xed, 0x1a, 0xf3, 0xb5, 0x57, 0x13, 0x5e, 0x7f, 0x57, 0xc9, 0x35, + 0x98, 0x4f, 0x0c, 0x70, 0xe0, 0xe6, 0x8b, 0x77, 0xe2, 0xa6, 0x89, 0xda, + 0xf3, 0xef, 0xe8, 0x72, 0x1d, 0xf1, 0x58, 0xa1, 0x36, 0xad, 0xe7, 0x35, + 0x30, 0xac, 0xca, 0x4f, 0x48, 0x3a, 0x79, 0x7a, 0xbc, 0x0a, 0xb1, 0x82, + 0xb3, 0x24, 0xfb, 0x61, 0xd1, 0x08, 0xa9, 0x4b, 0xb2, 0xc8, 0xe3, 0xfb, + 0xb9, 0x6a, 0xda, 0xb7, 0x60, 0xd7, 0xf4, 0x68, 0x1d, 0x4f, 0x42, 0xa3, + 0xde, 0x39, 0x4d, 0xf4, 0xae, 0x56, 0xed, 0xe7, 0x63, 0x72, 0xbb, 0x19, + 0x0b, 0x07, 0xa7, 0xc8, 0xee, 0x0a, 0x6d, 0x70, 0x9e, 0x02, 0xfc, 0xe1, + 0xcd, 0xf7, 0xe2, 0xec, 0xc0, 0x34, 0x04, 0xcd, 0x28, 0x34, 0x2f, 0x61, + 0x91, 0x72, 0xfe, 0x9c, 0xe9, 0x85, 0x83, 0xff, 0x8e, 0x4f, 0x12, 0x32, + 0xee, 0xf2, 0x81, 0x83, 0xc3, 0xfe, 0x3b, 0x1b, 0x4c, 0x6f, 0xad, 0x73, + 0x3b, 0xb5, 0xfc, 0xbc, 0x2e, 0xc2, 0x20, 0x05, 0xc5, 0x8e, 0xf1, 0x83, + 0x7d, 0x16, 0x83, 0xb2, 0xc6, 0xf3, 0x4a, 0x26, 0xc1, 0xb2, 0xef, 0xfa, + 0x88, 0x6b, 0x42, 0x38, 0x61, 0x28, 0x5c, 0x97, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff + }; + static unsigned char dhx_g[] = { + 0x02 + }; + static unsigned char dhx_q[] = { + 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xd6, 0xfc, 0x2a, 0x2c, + 0x51, 0x5d, 0xa5, 0x4d, 0x57, 0xee, 0x2b, 0x10, 0x13, 0x9e, 0x9e, 0x78, + 0xec, 0x5c, 0xe2, 0xc1, 0xe7, 0x16, 0x9b, 0x4a, 0xd4, 0xf0, 0x9b, 0x20, + 0x8a, 0x32, 0x19, 0xfd, 0xe6, 0x49, 0xce, 0xe7, 0x12, 0x4d, 0x9f, 0x7c, + 0xbe, 0x97, 0xf1, 0xb1, 0xb1, 0x86, 0x3a, 0xec, 0x7b, 0x40, 0xd9, 0x01, + 0x57, 0x62, 0x30, 0xbd, 0x69, 0xef, 0x8f, 0x6a, 0xea, 0xfe, 0xb2, 0xb0, + 0x92, 0x19, 0xfa, 0x8f, 0xaf, 0x83, 0x37, 0x68, 0x42, 0xb1, 0xb2, 0xaa, + 0x9e, 0xf6, 0x8d, 0x79, 0xda, 0xab, 0x89, 0xaf, 0x3f, 0xab, 0xe4, 0x9a, + 0xcc, 0x27, 0x86, 0x38, 0x70, 0x73, 0x45, 0xbb, 0xf1, 0x53, 0x44, 0xed, + 0x79, 0xf7, 0xf4, 0x39, 0x0e, 0xf8, 0xac, 0x50, 0x9b, 0x56, 0xf3, 0x9a, + 0x98, 0x56, 0x65, 0x27, 0xa4, 0x1d, 0x3c, 0xbd, 0x5e, 0x05, 0x58, 0xc1, + 0x59, 0x92, 0x7d, 0xb0, 0xe8, 0x84, 0x54, 0xa5, 0xd9, 0x64, 0x71, 0xfd, + 0xdc, 0xb5, 0x6d, 0x5b, 0xb0, 0x6b, 0xfa, 0x34, 0x0e, 0xa7, 0xa1, 0x51, + 0xef, 0x1c, 0xa6, 0xfa, 0x57, 0x2b, 0x76, 0xf3, 0xb1, 0xb9, 0x5d, 0x8c, + 0x85, 0x83, 0xd3, 0xe4, 0x77, 0x05, 0x36, 0xb8, 0x4f, 0x01, 0x7e, 0x70, + 0xe6, 0xfb, 0xf1, 0x76, 0x60, 0x1a, 0x02, 0x66, 0x94, 0x1a, 0x17, 0xb0, + 0xc8, 0xb9, 0x7f, 0x4e, 0x74, 0xc2, 0xc1, 0xff, 0xc7, 0x27, 0x89, 0x19, + 0x77, 0x79, 0x40, 0xc1, 0xe1, 0xff, 0x1d, 0x8d, 0xa6, 0x37, 0xd6, 0xb9, + 0x9d, 0xda, 0xfe, 0x5e, 0x17, 0x61, 0x10, 0x02, 0xe2, 0xc7, 0x78, 0xc1, + 0xbe, 0x8b, 0x41, 0xd9, 0x63, 0x79, 0xa5, 0x13, 0x60, 0xd9, 0x77, 0xfd, + 0x44, 0x35, 0xa1, 0x1c, 0x30, 0x94, 0x2e, 0x4b, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff + }; + + return get_dh_from_pg(libctx, "X9.42 DH", + dhx_p, sizeof(dhx_p), + dhx_g, sizeof(dhx_g), + dhx_q, sizeof(dhx_q)); +} + EVP_PKEY *get_dh1024dsa(OSSL_LIB_CTX *libctx) { static unsigned char dh1024_p[] = { diff --git a/test/helpers/predefined_dhparams.h b/test/helpers/predefined_dhparams.h index f0e8709062..2ff6d6e721 100644 --- a/test/helpers/predefined_dhparams.h +++ b/test/helpers/predefined_dhparams.h @@ -12,6 +12,7 @@ #ifndef OPENSSL_NO_DH EVP_PKEY *get_dh512(OSSL_LIB_CTX *libctx); EVP_PKEY *get_dhx512(OSSL_LIB_CTX *libctx); +EVP_PKEY *get_dhx_ffdhe2048(OSSL_LIB_CTX *libctx); EVP_PKEY *get_dh1024dsa(OSSL_LIB_CTX *libct); EVP_PKEY *get_dh2048(OSSL_LIB_CTX *libctx); EVP_PKEY *get_dh4096(OSSL_LIB_CTX *libctx); diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t index cabbe3ecdf..efe56c5665 100644 --- a/test/recipes/80-test_cms.t +++ b/test/recipes/80-test_cms.t @@ -627,10 +627,10 @@ my @smime_cms_param_tests = ( ], [ "enveloped content test streaming S/MIME format, X9.42 DH", - [ "{cmd1}", @prov, "-encrypt", "-in", $smcont, + [ "{cmd1}", @defaultprov, "-encrypt", "-in", $smcont, "-stream", "-out", "{output}.cms", "-recip", catfile($smdir, "smdh.pem"), "-aes128" ], - [ "{cmd2}", @prov, "-decrypt", "-recip", catfile($smdir, "smdh.pem"), + [ "{cmd2}", @defaultprov, "-decrypt", "-recip", catfile($smdir, "smdh.pem"), "-in", "{output}.cms", "-out", "{output}.txt" ], \&final_compare ] diff --git a/test/recipes/80-test_ssl_old.t b/test/recipes/80-test_ssl_old.t index 8c52b637fc..31ed54621b 100644 --- a/test/recipes/80-test_ssl_old.t +++ b/test/recipes/80-test_ssl_old.t @@ -390,6 +390,9 @@ sub testssl { skip "skipping dhe1024dsa test", 1 if ($no_dh); + skip "FIPS 186-4 type DH groups are no longer supported by the FIPS provider", 1 + if $provider eq "fips"; + ok(run(test([@ssltest, "-bio_pair", "-dhe1024dsa", "-v"])), 'test sslv2/sslv3 with 1024bit DHE via BIO pair'); } -- 2.40.1