Make RSA_generate_multi_prime_key() not segfault if e is NULL.

This is not a big problem for higher level keygen, as these set e
beforehand to a default value. But the logic at the lower level is
incorrect since it was doing a NULL check in one place but then
segfaulting during a later BN_copy().

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

diff --git a/crypto/rsa/rsa_gen.c b/crypto/rsa/rsa_gen.c
index 4a3387f19e5f70b8a9021196c006c51c3b057dc7..4acaa515f77423ceb53c1dacbb8c8af039295873 100644 (file)
--- a/crypto/rsa/rsa_gen.c
+++ b/crypto/rsa/rsa_gen.c
@@ -86,21 +86,22 @@ static int rsa_multiprime_keygen(RSA *rsa, int bits, int primes,
     int ok = -1;
 
     if (bits < RSA_MIN_MODULUS_BITS) {
-        ok = 0;             /* we set our own err */
         ERR_raise(ERR_LIB_RSA, RSA_R_KEY_SIZE_TOO_SMALL);
-        goto err;
+        return 0;
+    }
+    if (e_value == NULL) {
+        ERR_raise(ERR_LIB_RSA, RSA_R_BAD_E_VALUE);
+        return 0;
     }
-
     /* A bad value for e can cause infinite loops */
-    if (e_value != NULL && !ossl_rsa_check_public_exponent(e_value)) {
+    if (!ossl_rsa_check_public_exponent(e_value)) {
         ERR_raise(ERR_LIB_RSA, RSA_R_PUB_EXPONENT_OUT_OF_RANGE);
         return 0;
     }
 
     if (primes < RSA_DEFAULT_PRIME_NUM || primes > ossl_rsa_multip_cap(bits)) {
-        ok = 0;             /* we set our own err */
         ERR_raise(ERR_LIB_RSA, RSA_R_KEY_PRIME_NUM_INVALID);
-        goto err;
+        return 0;
     }
 
     ctx = BN_CTX_new_ex(rsa->libctx);

diff --git a/test/rsa_mp_test.c b/test/rsa_mp_test.c
index 5405df342422fe00d064d2c50e461b42c500002b..81b42a2fdf70d16faffff39de32c3b246063a7e3 100644 (file)
--- a/test/rsa_mp_test.c
+++ b/test/rsa_mp_test.c
@@ -289,8 +289,41 @@ err:
     return ret;
 }
 
+static int test_rsa_mp_gen_bad_input(void)
+{
+    int ret = 0;
+    RSA *rsa = NULL;
+    BIGNUM *ebn = NULL;
+
+    if (!TEST_ptr(rsa = RSA_new()))
+        goto err;
+
+    if (!TEST_ptr(ebn = BN_new()))
+        goto err;
+    if (!TEST_true(BN_set_word(ebn, 65537)))
+        goto err;
+
+    /* Test that a NULL exponent fails and does not segfault */
+    if (!TEST_int_eq(RSA_generate_multi_prime_key(rsa, 1024, 2, NULL, NULL), 0))
+        goto err;
+
+    /* Test invalid bitsize fails */
+    if (!TEST_int_eq(RSA_generate_multi_prime_key(rsa, 500, 2, ebn, NULL), 0))
+        goto err;
+
+    /* Test invalid prime count fails */
+    if (!TEST_int_eq(RSA_generate_multi_prime_key(rsa, 1024, 1, ebn, NULL), 0))
+        goto err;
+    ret = 1;
+err:
+    BN_free(ebn);
+    RSA_free(rsa);
+    return ret;
+}
+
 int setup_tests(void)
 {
+    ADD_TEST(test_rsa_mp_gen_bad_input);
     ADD_ALL_TESTS(test_rsa_mp, 2);
     return 1;
 }