From 7efc653c43851dcbc3ec043baded029c7d31ab9f Mon Sep 17 00:00:00 2001 From: slontis Date: Wed, 11 Jan 2023 14:32:07 +1000 Subject: [PATCH] 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 Reviewed-by: Paul Dale Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/20025) --- crypto/rsa/rsa_gen.c | 13 +++++++------ test/rsa_mp_test.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/crypto/rsa/rsa_gen.c b/crypto/rsa/rsa_gen.c index 4a3387f19e..4acaa515f7 100644 --- 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 5405df3424..81b42a2fdf 100644 --- 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; }