dsa_check: Perform simple parameter check if seed is not available

Added primality check on p and q in the ossl_ffc_params_simple_validate().
Checking for p and q sizes in the default provider is made more
lenient.
Added two testcases for invalid parameters.

Fixes #13950

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/14148)

14 files changed, 140 insertions, 41 deletions

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 <openssl/err.h>
+#include <openssl/bn.h>
+#include <openssl/dsaerr.h>
+#include <openssl/dherr.h>
 #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-----