diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c index be940456cd..f8cbbd593b 100644 --- a/crypto/dh/dh_key.c +++ b/crypto/dh/dh_key.c @@ -328,7 +328,7 @@ static int generate_key(DH *dh) { /* Do a partial check for invalid p, q, g */ if (!ossl_ffc_params_simple_validate(dh->libctx, &dh->params, - FFC_PARAM_TYPE_DH)) + FFC_PARAM_TYPE_DH, NULL)) goto err; /* * For FFC FIPS 186-4 keygen diff --git a/crypto/dsa/dsa_check.c b/crypto/dsa/dsa_check.c index 9a1b129df8..7f56a785ab 100644 --- a/crypto/dsa/dsa_check.c +++ b/crypto/dsa/dsa_check.c @@ -19,14 +19,19 @@ #include "dsa_local.h" #include "crypto/dsa.h" -int dsa_check_params(const DSA *dsa, int *ret) +int dsa_check_params(const DSA *dsa, int checktype, int *ret) { - /* - * (2b) FFC domain params conform to FIPS-186-4 explicit domain param - * validity tests. - */ - return ossl_ffc_params_FIPS186_4_validate(dsa->libctx, &dsa->params, - FFC_PARAM_TYPE_DSA, ret, NULL); + if (checktype == OSSL_KEYMGMT_VALIDATE_QUICK_CHECK) + return ossl_ffc_params_simple_validate(dsa->libctx, &dsa->params, + FFC_PARAM_TYPE_DSA, ret); + else + /* + * Do full FFC domain params validation according to FIPS-186-4 + * - always in FIPS_MODULE + * - only if possible (i.e., seed is set) in default provider + */ + return ossl_ffc_params_full_validate(dsa->libctx, &dsa->params, + FFC_PARAM_TYPE_DSA, ret); } /* diff --git a/crypto/dsa/dsa_err.c b/crypto/dsa/dsa_err.c index 99fc0e80fb..6481e2dc58 100644 --- a/crypto/dsa/dsa_err.c +++ b/crypto/dsa/dsa_err.c @@ -32,6 +32,7 @@ static const ERR_STRING_DATA DSA_str_reasons[] = { {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_NO_PARAMETERS_SET), "no parameters set"}, {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_PARAMETER_ENCODING_ERROR), "parameter encoding error"}, + {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_P_NOT_PRIME), "p not prime"}, {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_Q_NOT_PRIME), "q not prime"}, {ERR_PACK(ERR_LIB_DSA, 0, DSA_R_SEED_LEN_SMALL), "seed_len is less than the length of q"}, diff --git a/crypto/dsa/dsa_key.c b/crypto/dsa/dsa_key.c index 899663353f..8646d01957 100644 --- a/crypto/dsa/dsa_key.c +++ b/crypto/dsa/dsa_key.c @@ -77,7 +77,7 @@ static int dsa_keygen(DSA *dsa, int pairwise_test) /* Do a partial check for invalid p, q, g */ if (!ossl_ffc_params_simple_validate(dsa->libctx, &dsa->params, - FFC_PARAM_TYPE_DSA)) + FFC_PARAM_TYPE_DSA, NULL)) goto err; /* diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 002a7a0f10..530e3217e4 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -502,6 +502,7 @@ DSA_R_MISSING_PRIVATE_KEY:111:missing private key DSA_R_MODULUS_TOO_LARGE:103:modulus too large DSA_R_NO_PARAMETERS_SET:107:no parameters set DSA_R_PARAMETER_ENCODING_ERROR:105:parameter encoding error +DSA_R_P_NOT_PRIME:115:p not prime DSA_R_Q_NOT_PRIME:113:q not prime DSA_R_SEED_LEN_SMALL:110:seed_len is less than the length of q DSO_R_CTRL_FAILED:100:control command failed diff --git a/crypto/ffc/ffc_params_generate.c b/crypto/ffc/ffc_params_generate.c index 9285f93c05..2e50c2b801 100644 --- a/crypto/ffc/ffc_params_generate.c +++ b/crypto/ffc/ffc_params_generate.c @@ -77,12 +77,12 @@ static int ffc_validate_LN(size_t L, size_t N, int type, int verify) ERR_raise(ERR_LIB_DH, DH_R_BAD_FFC_PARAMETERS); # endif } else if (type == FFC_PARAM_TYPE_DSA) { - if (L == 1024 && N == 160) - return 80; - if (L == 2048 && (N == 224 || N == 256)) - return 112; - if (L == 3072 && N == 256) + if (L >= 3072 && N >= 256) return 128; + if (L >= 2048 && N >= 224) + return 112; + if (L >= 1024 && N >= 160) + return 80; # ifndef OPENSSL_NO_DSA ERR_raise(ERR_LIB_DSA, DSA_R_BAD_FFC_PARAMETERS); # endif diff --git a/crypto/ffc/ffc_params_validate.c b/crypto/ffc/ffc_params_validate.c index 22983d62ef..a2bfe22da2 100644 --- a/crypto/ffc/ffc_params_validate.c +++ b/crypto/ffc/ffc_params_validate.c @@ -13,6 +13,10 @@ * It calls the same functions as the generation as the code is very similar. */ +#include +#include +#include +#include #include "internal/ffc.h" /* FIPS186-4 A.2.2 Unverifiable partial validation of Generator g */ @@ -88,30 +92,92 @@ int ossl_ffc_params_FIPS186_2_validate(OSSL_LIB_CTX *libctx, * extra parameters such as the digest and seed, which may not be available for * this test. */ -int ossl_ffc_params_simple_validate(OSSL_LIB_CTX *libctx, FFC_PARAMS *params, - int type) +int ossl_ffc_params_simple_validate(OSSL_LIB_CTX *libctx, const FFC_PARAMS *params, + int paramstype, int *res) { - int ret, res = 0; - int save_gindex; - unsigned int save_flags; + int ret; + int tmpres = 0; + FFC_PARAMS tmpparams = {0}; if (params == NULL) return 0; - save_flags = params->flags; - save_gindex = params->gindex; - params->flags = FFC_PARAM_FLAG_VALIDATE_G; - params->gindex = FFC_UNVERIFIABLE_GINDEX; + if (res == NULL) + res = &tmpres; + + if (!ossl_ffc_params_copy(&tmpparams, params)) + return 0; + + tmpparams.flags = FFC_PARAM_FLAG_VALIDATE_G; + tmpparams.gindex = FFC_UNVERIFIABLE_GINDEX; #ifndef FIPS_MODULE - if (save_flags & FFC_PARAM_FLAG_VALIDATE_LEGACY) - ret = ossl_ffc_params_FIPS186_2_validate(libctx, params, type, &res, - NULL); + if (params->flags & FFC_PARAM_FLAG_VALIDATE_LEGACY) + ret = ossl_ffc_params_FIPS186_2_validate(libctx, &tmpparams, paramstype, + res, NULL); else #endif - ret = ossl_ffc_params_FIPS186_4_validate(libctx, params, type, &res, - NULL); - params->flags = save_flags; - params->gindex = save_gindex; + ret = ossl_ffc_params_FIPS186_4_validate(libctx, &tmpparams, paramstype, + res, NULL); +#ifndef OPENSSL_NO_DH + if (ret == FFC_PARAM_RET_STATUS_FAILED + && (*res & FFC_ERROR_NOT_SUITABLE_GENERATOR) != 0) { + ERR_raise(ERR_LIB_DH, DH_R_NOT_SUITABLE_GENERATOR); + } +#endif + + ossl_ffc_params_cleanup(&tmpparams); + return ret != FFC_PARAM_RET_STATUS_FAILED; } + +/* + * If possible (or always in FIPS_MODULE) do full FIPS 186-4 validation. + * Otherwise do simple check but in addition also check the primality of the + * p and q. + */ +int ossl_ffc_params_full_validate(OSSL_LIB_CTX *libctx, const FFC_PARAMS *params, + int paramstype, int *res) +{ + int tmpres = 0; + + if (params == NULL) + return 0; + + if (res == NULL) + res = &tmpres; + +#ifdef FIPS_MODULE + return ossl_ffc_params_FIPS186_4_validate(libctx, params, paramstype, + res, NULL); +#else + if (params->seed != NULL) { + return ossl_ffc_params_FIPS186_4_validate(libctx, params, paramstype, + res, NULL); + } else { + int ret = 0; + + ret = ossl_ffc_params_simple_validate(libctx, params, paramstype, res); + if (ret) { + BN_CTX *ctx; + + if ((ctx = BN_CTX_new_ex(libctx)) == NULL) + return 0; + if (BN_check_prime(params->q, ctx, NULL) != 1) { +# ifndef OPENSSL_NO_DSA + ERR_raise(ERR_LIB_DSA, DSA_R_Q_NOT_PRIME); +# endif + ret = 0; + } + if (ret && BN_check_prime(params->p, ctx, NULL) != 1) { +# ifndef OPENSSL_NO_DSA + ERR_raise(ERR_LIB_DSA, DSA_R_P_NOT_PRIME); +# endif + ret = 0; + } + BN_CTX_free(ctx); + } + return ret; + } +#endif +} diff --git a/include/crypto/dsa.h b/include/crypto/dsa.h index 8d282ab188..3da5696795 100644 --- a/include/crypto/dsa.h +++ b/include/crypto/dsa.h @@ -33,7 +33,7 @@ int dsa_key_fromdata(DSA *dsa, const OSSL_PARAM params[]); int dsa_generate_public_key(BN_CTX *ctx, const DSA *dsa, const BIGNUM *priv_key, BIGNUM *pub_key); -int dsa_check_params(const DSA *dsa, int *ret); +int dsa_check_params(const DSA *dsa, int checktype, int *ret); int dsa_check_pub_key(const DSA *dsa, const BIGNUM *pub_key, int *ret); int dsa_check_pub_key_partial(const DSA *dsa, const BIGNUM *pub_key, int *ret); int dsa_check_priv_key(const DSA *dsa, const BIGNUM *priv_key, int *ret); diff --git a/include/internal/ffc.h b/include/internal/ffc.h index 7653b6e2fa..4cffc720a6 100644 --- a/include/internal/ffc.h +++ b/include/internal/ffc.h @@ -162,8 +162,12 @@ int ossl_ffc_params_FIPS186_2_gen_verify(OSSL_LIB_CTX *libctx, size_t L, size_t N, int *res, BN_GENCB *cb); -int ossl_ffc_params_simple_validate(OSSL_LIB_CTX *libctx, FFC_PARAMS *params, - int type); +int ossl_ffc_params_simple_validate(OSSL_LIB_CTX *libctx, + const FFC_PARAMS *params, + int paramstype, int *res); +int ossl_ffc_params_full_validate(OSSL_LIB_CTX *libctx, + const FFC_PARAMS *params, + int paramstype, int *res); int ossl_ffc_params_FIPS186_4_validate(OSSL_LIB_CTX *libctx, const FFC_PARAMS *params, int type, int *res, BN_GENCB *cb); diff --git a/include/openssl/dsaerr.h b/include/openssl/dsaerr.h index 49dabbf575..669cd6c87f 100644 --- a/include/openssl/dsaerr.h +++ b/include/openssl/dsaerr.h @@ -35,6 +35,7 @@ # define DSA_R_MODULUS_TOO_LARGE 103 # define DSA_R_NO_PARAMETERS_SET 107 # define DSA_R_PARAMETER_ENCODING_ERROR 105 +# define DSA_R_P_NOT_PRIME 115 # define DSA_R_Q_NOT_PRIME 113 # define DSA_R_SEED_LEN_SMALL 110 diff --git a/providers/implementations/keymgmt/dsa_kmgmt.c b/providers/implementations/keymgmt/dsa_kmgmt.c index 28e8409aa2..467f75bb55 100644 --- a/providers/implementations/keymgmt/dsa_kmgmt.c +++ b/providers/implementations/keymgmt/dsa_kmgmt.c @@ -309,11 +309,11 @@ static const OSSL_PARAM *dsa_gettable_params(void *provctx) return dsa_params; } -static int dsa_validate_domparams(const DSA *dsa) +static int dsa_validate_domparams(const DSA *dsa, int checktype) { int status = 0; - return dsa_check_params(dsa, &status); + return dsa_check_params(dsa, checktype, &status); } static int dsa_validate_public(const DSA *dsa) @@ -350,7 +350,7 @@ static int dsa_validate(const void *keydata, int selection, int checktype) ok = 1; if ((selection & OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS) != 0) - ok = ok && dsa_validate_domparams(dsa); + ok = ok && dsa_validate_domparams(dsa, checktype); if ((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) ok = ok && dsa_validate_public(dsa); diff --git a/test/recipes/15-test_dsaparam.t b/test/recipes/15-test_dsaparam.t index c0c73841bd..c34d8ec9cd 100644 --- a/test/recipes/15-test_dsaparam.t +++ b/test/recipes/15-test_dsaparam.t @@ -64,15 +64,15 @@ plan skip_all => "DSA isn't supported in this build" if disabled("dsa"); my @valid = glob(data_file("valid", "*.pem")); -#my @invalid = glob(data_file("invalid", "*.pem")); +my @invalid = glob(data_file("invalid", "*.pem")); -my $num_tests = scalar @valid; +my $num_tests = scalar @valid + scalar @invalid; plan tests => $num_tests; foreach (@valid) { ok(run(app([qw{openssl pkeyparam -noout -check -in}, $_]))); } -#foreach (@invalid) { -# ok(!run(app([qw{openssl pkeyparam -noout -check -in}, $_]))); -#} +foreach (@invalid) { + ok(!run(app([qw{openssl pkeyparam -noout -check -in}, $_]))); +} diff --git a/test/recipes/15-test_dsaparam_data/invalid/p2048_q256_bad_q.pem b/test/recipes/15-test_dsaparam_data/invalid/p2048_q256_bad_q.pem new file mode 100644 index 0000000000..6f7d98ddfb --- /dev/null +++ b/test/recipes/15-test_dsaparam_data/invalid/p2048_q256_bad_q.pem @@ -0,0 +1,14 @@ +-----BEGIN DSA PARAMETERS----- +MIICLAKCAQEAwmWp4Y+deYlczoQUPiioJt6Qxthrk6L1CAVpGH2uRRlHfTl41WUX +JHIJyCMBgRDtVVQdyAQ7AZ+CxOl1wpazvGJddyQVynhmIFsaUwHF2fYIT00MvBRL +9VA5PQqUmX2Tnog5ezu35CTsEqlBTOYcGqkQ7ctNVjfvjYCkwzvTxJS/Qsvki+dA +fE7NDWe+9r5QjSGEFZtH45alIM4qUnBS1mcN2Az5+S8JxPiivY5Jkt0pXoRQnvCM +In4bHjM8ZOVmLxFCIrpB0dNgKDg+2zEjRjmL7B4aZRcO7wDyrtDPc7jiYPH/rlt/ +wU1o/Y1fnzN9+R3f0AMeWR44bqf5Ol9jVQIhAKDEbXZJcYLbvkUYWBr8TKsVu2hc +H57M3PwkTsq+v2/dAoIBADKkGYUe9qsp4mqxkBKaEdpcjmjfLrvtE+3ikipPPGHh +tbAX7NwZc9WCyhniKYskEbJBWsuAZJXDgIRNaSpCVLK7dd9fx8ZnIKJESO6Htv1z +JfSIST57xW8L6m78Lq2kxpr5dVcm7I4pelTfL5jscTURm/1Ua+2skI9YlZU/Vgux +Wrr30H8bp4fUgWjcgPJbeirSY7xVI8FKrQaES0s4NRFbgGMFUrEGddBF0bgaGkwd +mFEpcXAEQDTJV7SPJp3rbjFug3CF4Atw3RmkV2T/sHAbplyr9YsQDmAQDhPsaWjQ +eSsoRUq0aQ4aa2V4X/gSzSj9It3Q4ngQwkGGOPJEo44= +-----END DSA PARAMETERS----- diff --git a/test/recipes/15-test_dsaparam_data/invalid/p768_q160_too_small.pem b/test/recipes/15-test_dsaparam_data/invalid/p768_q160_too_small.pem new file mode 100644 index 0000000000..c717c917a1 --- /dev/null +++ b/test/recipes/15-test_dsaparam_data/invalid/p768_q160_too_small.pem @@ -0,0 +1,7 @@ +-----BEGIN DSA PARAMETERS----- +MIHcAmEA702xO4DjQl4WxLId1FR8Q0tZ+FQDEqyhzfYheBnLra8Uaf3gLp7V0g52 +aQqDTeY1TK76ZmNo/SvESDcYTHjlgKphYCKLRxAhvuyGfX1RRPWa80BrM76wYtJD +mwB9KSBnAhUArp9BUvskZ9/K8Bzo0MVejsHC6AkCYEugdq5OD0HjCrxt3hFMD3sJ +ZQ7VAZa+Fnu9SJNjCeMYLEww4/A6fktqokDWITjSQpdJAAxwc+r8OlDRwBb0q7jT +w1IDvvbF/xGex5VzHHBZmQU1G1jH+Lq3h7dQ6d4l+g== +-----END DSA PARAMETERS-----