diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c index e88f14c7af..8db1acac93 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.c +++ b/crypto/fipsmodule/rsa/rsa_impl.c @@ -1214,10 +1214,17 @@ static int RSA_generate_key_ex_maybe_fips(RSA *rsa, int bits, // failure in |BN_GENCB_call| is still fatal. } while (failures < 4 && ERR_GET_LIB(err) == ERR_LIB_RSA && ERR_GET_REASON(err) == RSA_R_TOO_MANY_ITERATIONS); + if (tmp == NULL) { + goto out; + } // Perform PCT test in the case of FIPS - if (tmp == NULL || (check_fips && !RSA_check_fips(tmp))) { - goto out; + if(check_fips && !RSA_check_fips(tmp)) { + RSA_free(tmp); +#if defined(AWSLC_FIPS) + BORINGSSL_FIPS_abort(); +#endif + return ret; } rsa_invalidate_key(rsa); @@ -1241,11 +1248,6 @@ static int RSA_generate_key_ex_maybe_fips(RSA *rsa, int bits, out: RSA_free(tmp); -#if defined(AWSLC_FIPS) - if (ret == 0) { - BORINGSSL_FIPS_abort(); - } -#endif return ret; } diff --git a/crypto/rsa_extra/rsa_test.cc b/crypto/rsa_extra/rsa_test.cc index 0254e2a0df..f624fa8750 100644 --- a/crypto/rsa_extra/rsa_test.cc +++ b/crypto/rsa_extra/rsa_test.cc @@ -695,8 +695,6 @@ TEST(RSATest, BadExponent) { ERR_clear_error(); } -#if !defined(AWSLC_FIPS) - // Attempting to generate an excessively small key should fail. TEST(RSATest, GenerateSmallKey) { bssl::UniquePtr rsa(RSA_new()); @@ -710,25 +708,6 @@ TEST(RSATest, GenerateSmallKey) { ErrorEquals(ERR_get_error(), ERR_LIB_RSA, RSA_R_KEY_SIZE_TOO_SMALL)); } -#else -// AWSLCAndroidTestRunner does not take tests that do |ASSERT_DEATH| very well. -// GTEST issue: https://github.com/google/googletest/issues/1496. -#if !defined(OPENSSL_ANDROID) - -// Attempting to generate an excessively small key should fail. -// In the case of a FIPS build, expect abort() when |RSA_generate_key_ex| fails. -TEST(RSADeathTest, GenerateSmallKeyAndDie) { - bssl::UniquePtr rsa(RSA_new()); - ASSERT_TRUE(rsa); - bssl::UniquePtr e(BN_new()); - ASSERT_TRUE(e); - ASSERT_TRUE(BN_set_word(e.get(), RSA_F4)); - - ASSERT_DEATH_IF_SUPPORTED(RSA_generate_key_ex(rsa.get(), 255, e.get(), nullptr), ""); -} -#endif -#endif - // Attempting to generate an funny RSA key length should round down. TEST(RSATest, RoundKeyLengths) { bssl::UniquePtr e(BN_new()); @@ -1214,70 +1193,28 @@ TEST(RSATest, KeygenFailOnce) { } #else + // AWSLCAndroidTestRunner does not take tests that do |ASSERT_DEATH| very well. // GTEST issue: https://github.com/google/googletest/issues/1496. #if !defined(OPENSSL_ANDROID) -// In the case of a FIPS build, expect abort() when |RSA_generate_key_ex| fails. +// In the case of a FIPS build, expect abort() when PWCTs in +// |RSA_generate_key_fips| fail. TEST(RSADeathTest, KeygenFailAndDie) { - bssl::UniquePtr rsa(RSA_new()); - ASSERT_TRUE(rsa); - - // Cause RSA key generation after a prime has been generated, to test that - // |rsa| is left alone. - BN_GENCB cb; - BN_GENCB_set(&cb, - [](int event, int, BN_GENCB *) -> int { return event != 3; }, - nullptr); - - bssl::UniquePtr e(BN_new()); - ASSERT_TRUE(e); - ASSERT_TRUE(BN_set_word(e.get(), RSA_F4)); - - // Key generation should fail. - ASSERT_DEATH_IF_SUPPORTED(RSA_generate_key_ex(rsa.get(), 2048, e.get(), &cb),""); - - // Failed key generations do not leave garbage in |rsa|. - EXPECT_FALSE(rsa->n); - EXPECT_FALSE(rsa->e); - EXPECT_FALSE(rsa->d); - EXPECT_FALSE(rsa->p); - EXPECT_FALSE(rsa->q); - EXPECT_FALSE(rsa->dmp1); - EXPECT_FALSE(rsa->dmq1); - EXPECT_FALSE(rsa->iqmp); - EXPECT_FALSE(rsa->mont_n); - EXPECT_FALSE(rsa->mont_p); - EXPECT_FALSE(rsa->mont_q); - EXPECT_FALSE(rsa->d_fixed); - EXPECT_FALSE(rsa->dmp1_fixed); - EXPECT_FALSE(rsa->dmq1_fixed); - EXPECT_FALSE(rsa->iqmp_mont); - EXPECT_FALSE(rsa->private_key_frozen); - - // Failed key generations leave the previous contents alone. - EXPECT_TRUE(RSA_generate_key_ex(rsa.get(), 2048, e.get(), nullptr)); - uint8_t *der; - size_t der_len; - ASSERT_TRUE(RSA_private_key_to_bytes(&der, &der_len, rsa.get())); - bssl::UniquePtr delete_der(der); - - ASSERT_DEATH_IF_SUPPORTED(RSA_generate_key_ex(rsa.get(), 2048, e.get(), &cb),""); + const char *const value = getenv("BORINGSSL_FIPS_BREAK_TEST"); + if (!value || strcmp(value, "RSA_PWCT") != 0) { + GTEST_SKIP() << "Skipping BORINGSSL_FIPS_BREAK_TESTS RSA_PWCT Test."; + } - uint8_t *der2; - size_t der2_len; - ASSERT_TRUE(RSA_private_key_to_bytes(&der2, &der2_len, rsa.get())); - bssl::UniquePtr delete_der2(der2); - EXPECT_EQ(Bytes(der, der_len), Bytes(der2, der2_len)); + // Test that all supported key lengths abort when PWCTs fail. + for (const size_t bits : {2048, 3072, 4096}) { + SCOPED_TRACE(bits); + bssl::UniquePtr rsa(RSA_new()); + ASSERT_TRUE(rsa); - // Generating a key over an existing key works, despite any cached state. - EXPECT_TRUE(RSA_generate_key_ex(rsa.get(), 2048, e.get(), nullptr)); - EXPECT_TRUE(RSA_check_key(rsa.get())); - uint8_t *der3; - size_t der3_len; - ASSERT_TRUE(RSA_private_key_to_bytes(&der3, &der3_len, rsa.get())); - bssl::UniquePtr delete_der3(der3); - EXPECT_NE(Bytes(der, der_len), Bytes(der3, der3_len)); + ASSERT_DEATH_IF_SUPPORTED(RSA_generate_key_fips(rsa.get(), bits, nullptr), + ""); + } } #endif diff --git a/tests/ci/run_fips_tests.sh b/tests/ci/run_fips_tests.sh index 143df9183b..f722ee3e09 100755 --- a/tests/ci/run_fips_tests.sh +++ b/tests/ci/run_fips_tests.sh @@ -33,6 +33,10 @@ if static_linux_supported || static_openbsd_supported; then echo "Testing AWS-LC static breakable release build" run_build -DFIPS=1 -DCMAKE_C_FLAGS="-DBORINGSSL_FIPS_BREAK_TESTS" + export BORINGSSL_FIPS_BREAK_TEST="RSA_PWCT" + ${BUILD_ROOT}/crypto/crypto_test --gtest_filter="RSADeathTest.KeygenFailAndDie" + unset BORINGSSL_FIPS_BREAK_TEST + cd $SRC_ROOT MODULE_HASH=$(./util/fipstools/test-break-kat.sh |\ (egrep "Hash of module was:.* ([a-f0-9]*)" || true))