Avoid setting gen_type to -1 in dsa_gen_set_params

gh_gen_type_common_set_params looks up a dsa contexts gen_type using
name2id, but if it returns error, we inadvertently set gctx->gen_type to
-1, which is an invalid value, which may lead to improper behavior in
future calls, in the event that said future calls preform an operation
of the form;
if (gen_type == <VALID VALUE>) {
        do_stuff
else {
        do_other_stuff
}

Technically it is not correct to continue with the operations on the
gen context after failed parameters setting but this makes it more
predictable.

Fix it by assigning the result of a lookup to a stack variable, and only
update gctx->gen_value if the lookup returns a non-failing value

In leiu of testing this specific case, also add an ossl_assert in dsa_gen
to validate the gen_val input prior to continuing, should other code
points attempt to do the same thing

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22991)

2 files changed, 29 insertions, 2 deletions

diff --git a/providers/implementations/keymgmt/dh_kmgmt.c b/providers/implementations/keymgmt/dh_kmgmt.c
index 183738b61b..82c3093b12 100644
--- a/providers/implementations/keymgmt/dh_kmgmt.c
+++ b/providers/implementations/keymgmt/dh_kmgmt.c
@@ -710,7 +710,12 @@ static void *dh_gen(void *genctx, OSSL_CALLBACK *osslcb, void *cbarg)
     if (gctx->group_nid != NID_undef)
         gctx->gen_type = DH_PARAMGEN_TYPE_GROUP;
 
-    /* Bounds check on context gen_type */
+    /*
+     * Do a bounds check on context gen_type. Must be in range:
+     * DH_PARAMGEN_TYPE_GENERATOR <= gen_type <= DH_PARAMGEN_TYPE_GROUP
+     * Noted here as this needs to be adjusted if a new group type is
+     * added.
+     */
     if (!ossl_assert((gctx->gen_type >= DH_PARAMGEN_TYPE_GENERATOR)
                     && (gctx->gen_type <= DH_PARAMGEN_TYPE_GROUP))) {
         ERR_raise_data(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR,
diff --git a/providers/implementations/keymgmt/dsa_kmgmt.c b/providers/implementations/keymgmt/dsa_kmgmt.c
index a89d20822b..3fdc78432b 100644
--- a/providers/implementations/keymgmt/dsa_kmgmt.c
+++ b/providers/implementations/keymgmt/dsa_kmgmt.c
@@ -462,6 +462,7 @@ static int dsa_gen_set_params(void *genctx, const OSSL_PARAM params[])
 {
     struct dsa_gen_ctx *gctx = genctx;
     const OSSL_PARAM *p;
+    int gen_type = -1;
 
     if (gctx == NULL)
         return 0;
@@ -472,10 +473,18 @@ static int dsa_gen_set_params(void *genctx, const OSSL_PARAM params[])
     p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_FFC_TYPE);
     if (p != NULL) {
         if (p->data_type != OSSL_PARAM_UTF8_STRING
-            || ((gctx->gen_type = dsa_gen_type_name2id(p->data)) == -1)) {
+            || ((gen_type = dsa_gen_type_name2id(p->data)) == -1)) {
             ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT);
             return 0;
         }
+
+        /*
+         * Ony assign context gen_type if it was set by dsa_gen_type_name2id
+         * must be in range:
+         * DSA_PARAMGEN_TYPE_FIPS_186_4 <= gen_type <= DSA_PARAMGEN_TYPE_FIPS_DEFAULT
+         */
+        if (gen_type != -1)
+            gctx->gen_type = gen_type;
     }
     p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_FFC_GINDEX);
     if (p != NULL
@@ -568,6 +577,19 @@ static void *dsa_gen(void *genctx, OSSL_CALLBACK *osslcb, void *cbarg)
         gctx->gen_type = (gctx->pbits >= 2048 ? DSA_PARAMGEN_TYPE_FIPS_186_4 :
                                                 DSA_PARAMGEN_TYPE_FIPS_186_2);
 
+    /*
+     * Do a bounds check on context gen_type. Must be in range:
+     * DSA_PARAMGEN_TYPE_FIPS_186_4 <= gen_type <= DSA_PARAMGEN_TYPE_FIPS_DEFAULT
+     * Noted here as this needs to be adjusted if a new type is
+     * added.
+     */
+    if (!ossl_assert((gctx->gen_type >= DSA_PARAMGEN_TYPE_FIPS_186_4)
+                    && (gctx->gen_type <= DSA_PARAMGEN_TYPE_FIPS_DEFAULT))) {
+        ERR_raise_data(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR,
+                       "gen_type set to unsupported value %d", gctx->gen_type);
+        return NULL;
+    }
+
     gctx->cb = osslcb;
     gctx->cbarg = cbarg;
     gencb = BN_GENCB_new();