DH_check_pub_key() should not fail when setting result code

The semantics of ossl_ffc_validate_public_key() and
ossl_ffc_validate_public_key_partial() needs to be changed
to not return error on non-fatal problems.

Fixes #22287

Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22291)

(cherry picked from commit eaee1765a49c6a8ba728e3e2d18bb67bff8aaa55)

3 files changed, 11 insertions, 14 deletions

diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c
index f4173e2137..7ba2beae7f 100644
--- a/crypto/dh/dh_check.c
+++ b/crypto/dh/dh_check.c
@@ -259,7 +259,8 @@ int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *ret)
  */
 int ossl_dh_check_pub_key_partial(const DH *dh, const BIGNUM *pub_key, int *ret)
 {
-    return ossl_ffc_validate_public_key_partial(&dh->params, pub_key, ret);
+    return ossl_ffc_validate_public_key_partial(&dh->params, pub_key, ret)
+           && *ret == 0;
 }
 
 int ossl_dh_check_priv_key(const DH *dh, const BIGNUM *priv_key, int *ret)
diff --git a/crypto/dsa/dsa_check.c b/crypto/dsa/dsa_check.c
index 7ee914a477..ec3534d35c 100644
--- a/crypto/dsa/dsa_check.c
+++ b/crypto/dsa/dsa_check.c
@@ -39,7 +39,8 @@ int ossl_dsa_check_params(const DSA *dsa, int checktype, int *ret)
  */
 int ossl_dsa_check_pub_key(const DSA *dsa, const BIGNUM *pub_key, int *ret)
 {
-    return ossl_ffc_validate_public_key(&dsa->params, pub_key, ret);
+    return ossl_ffc_validate_public_key(&dsa->params, pub_key, ret)
+           && *ret == 0;
 }
 
 /*
@@ -49,7 +50,8 @@ int ossl_dsa_check_pub_key(const DSA *dsa, const BIGNUM *pub_key, int *ret)
  */
 int ossl_dsa_check_pub_key_partial(const DSA *dsa, const BIGNUM *pub_key, int *ret)
 {
-    return ossl_ffc_validate_public_key_partial(&dsa->params, pub_key, ret);
+    return ossl_ffc_validate_public_key_partial(&dsa->params, pub_key, ret)
+           && *ret == 0;
 }
 
 int ossl_dsa_check_priv_key(const DSA *dsa, const BIGNUM *priv_key, int *ret)
diff --git a/crypto/ffc/ffc_key_validate.c b/crypto/ffc/ffc_key_validate.c
index 342789621d..a4a2a58e9a 100644
--- a/crypto/ffc/ffc_key_validate.c
+++ b/crypto/ffc/ffc_key_validate.c
@@ -26,7 +26,7 @@ int ossl_ffc_validate_public_key_partial(const FFC_PARAMS *params,
     *ret = 0;
     if (params == NULL || pub_key == NULL || params->p == NULL) {
         *ret = FFC_ERROR_PASSED_NULL_PARAM;
-        return 0;
+        return 1;
     }
 
     ctx = BN_CTX_new_ex(NULL);
@@ -39,18 +39,14 @@ int ossl_ffc_validate_public_key_partial(const FFC_PARAMS *params,
     if (tmp == NULL
         || !BN_set_word(tmp, 1))
         goto err;
-    if (BN_cmp(pub_key, tmp) <= 0) {
+    if (BN_cmp(pub_key, tmp) <= 0)
         *ret |= FFC_ERROR_PUBKEY_TOO_SMALL;
-        goto err;
-    }
     /* Step(1): Verify pub_key <=  p-2 */
     if (BN_copy(tmp, params->p) == NULL
         || !BN_sub_word(tmp, 1))
         goto err;
-    if (BN_cmp(pub_key, tmp) >= 0) {
+    if (BN_cmp(pub_key, tmp) >= 0)
         *ret |= FFC_ERROR_PUBKEY_TOO_LARGE;
-        goto err;
-    }
     ok = 1;
  err:
     if (ctx != NULL) {
@@ -73,7 +69,7 @@ int ossl_ffc_validate_public_key(const FFC_PARAMS *params,
     if (!ossl_ffc_validate_public_key_partial(params, pub_key, ret))
         return 0;
 
-    if (params->q != NULL) {
+    if (*ret == 0 && params->q != NULL) {
         ctx = BN_CTX_new_ex(NULL);
         if (ctx == NULL)
             goto err;
@@ -84,10 +80,8 @@ int ossl_ffc_validate_public_key(const FFC_PARAMS *params,
         if (tmp == NULL
             || !BN_mod_exp(tmp, pub_key, params->q, params->p, ctx))
             goto err;
-        if (!BN_is_one(tmp)) {
+        if (!BN_is_one(tmp))
             *ret |= FFC_ERROR_PUBKEY_INVALID;
-            goto err;
-        }
     }
 
     ok = 1;