Avoid accessing RSA struct's internals directly

CMakeLists.txt

@@ -586,6 +586,7 @@ set(COVERAGE_ARGUMENTS
 )
 
 if(FIPS)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DFIPS_BUILD")
     set(TEST_FIPS_PROPERTY "-DFIPS=true")
     # ACCP's default behavior in FIPS mode is to not register a SecureRandom implementation.
     # However, we explicitly register it here to ensure its coverage under test.

build.gradle

@@ -18,7 +18,7 @@ ext.isFips = Boolean.getBoolean('FIPS')
 if (ext.isFips) {
     ext.awsLcGitVersionId = 'AWS-LC-FIPS-2.0.2'
 } else {
-    ext.awsLcGitVersionId = 'v1.17.0'
+    ext.awsLcGitVersionId = '32143aae568a64245f9eae54dcbc49043dbf41e4'
 }
 ext.isLegacyBuild = Boolean.getBoolean('LEGACY_BUILD')
 

csrc/bn.h

@@ -92,6 +92,16 @@ class BigNumObj {
         return result;
     }
 
+    static BigNumObj fromBIGNUM(BIGNUM const* pBN)
+    {
+        BigNumObj result;
+        result.ensure_init();
+        if (!BN_copy(result, pBN)) {
+            throw_openssl("Failed to copy bignum");
+        }
+        return result;
+    }
+
 #ifdef HAVE_CPP11
     BigNumObj(const BigNumObj&) = delete;
     BigNumObj& operator=(const BigNumObj&) = delete;

csrc/java_evp_keys.cpp

@@ -496,11 +496,10 @@ JNIEXPORT jlong JNICALL Java_com_amazon_corretto_crypto_provider_EvpKeyFactory_r
         if (privateExponentArr) {
             BigNumObj privExp = BigNumObj::fromJavaArray(env, privateExponentArr);
 
-            int res;
+            int res = 1;
             if (BN_is_zero(pubExp)) {
-                // RSA blinding can't be performed without |e|; 0 indicates |e|'s absence.
-                rsa->flags |= RSA_FLAG_NO_BLINDING;
-                res = RSA_set0_key(rsa, modulus, NULL, privExp);
+                // RSA blinding can't be performed without |e|.
+                rsa.set(RSA_new_private_key_no_e(modulus, privExp));
             } else {
                 res = RSA_set0_key(rsa, modulus, pubExp, privExp);
             }

csrc/keyutils.cpp

@@ -1,6 +1,8 @@
 // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 #include "keyutils.h"
+#include "auto_free.h"
+#include "bn.h"
 #include <openssl/ec.h>
 #include <openssl/err.h>
 #include <openssl/evp.h>
@@ -50,7 +52,7 @@ EVP_PKEY* der2EvpPrivateKey(
             // If blinding is set and any of the parameters required for blinding
             // are NULL, rebuild to turn blinding off. Otherwise, rebuild if any
             // of the params are 0-valued to NULL them out.
-            if (((rsa->flags & RSA_FLAG_NO_BLINDING) == 0) && (!e || !p || !q)) {
+            if ((RSA_test_flags(rsa, RSA_FLAG_NO_BLINDING) == 0) && (!e || !p || !q)) {
                 need_rebuild = true;
             } else if (e && BN_is_zero(e)) {
                 need_rebuild = true;
@@ -68,22 +70,20 @@ EVP_PKEY* der2EvpPrivateKey(
 
             if (need_rebuild) {
                 // This key likely only has (n, d) set. Very weird, but it happens in java sometimes.
-                RSA* nulled_rsa = RSA_new();
-
-                // Blinding requires |e| and the prime factors |p| and |q|, which we may not have here.
-                nulled_rsa->flags |= RSA_FLAG_NO_BLINDING;
-
-                // |e| might be NULL here, so swap in 0 when calling awslc and
-                // re-NULL it afterwards.
-                if (!RSA_set0_key(nulled_rsa, BN_dup(n), e ? BN_dup(e) : BN_new(), BN_dup(d))) {
-                    throw_openssl(javaExceptionClass, "Unable to set RSA key parameters");
-                }
-                if (BN_is_zero(nulled_rsa->e)) {
-                    BN_free(nulled_rsa->e);
-                    nulled_rsa->e = NULL;
+                RSA_auto nulled_rsa;
+                BigNumObj n_copy = BigNumObj::fromBIGNUM(n);
+                BigNumObj d_copy = BigNumObj::fromBIGNUM(d);
+                nulled_rsa.set(RSA_new_private_key_no_e(n_copy, d_copy));
+                if (e != nullptr && !BN_is_zero(e)) {
+                    BigNumObj e_copy = BigNumObj::fromBIGNUM(e);
+                    if (!RSA_set0_key(nulled_rsa, nullptr, e_copy, nullptr)) {
+                        throw_openssl("Unable to set e for RSA");
+                    }
+                    e_copy.releaseOwnership();
                 }
+                n_copy.releaseOwnership();
+                d_copy.releaseOwnership();
                 EVP_PKEY_set1_RSA(result, nulled_rsa);
-                RSA_free(nulled_rsa); // Decrement reference counter
                 shouldCheckPrivate = false; // We cannot check private keys without CRT parameters
             }
         }
@@ -169,4 +169,36 @@ const EVP_MD* digestFromJstring(raii_env& env, jstring digestName)
 
     return result;
 }
+
+RSA* RSA_new_private_key_no_e(BIGNUM* n, BIGNUM* d)
+{
+    RSA_auto rsa = RSA_auto::from(RSA_new());
+    if (rsa.get() == nullptr) {
+        throw_openssl("RSA_new failed");
+    }
+    // The FIPS builds of AWS-LC do not have an API to disable blinding.
+#ifdef FIPS_BUILD
+
+    // RSA struct is not opaque in FIPS mode.
+    rsa->flags |= RSA_FLAG_NO_BLINDING;
+
+#else
+
+    // RSA_blinding_off_temp_for_accp_compatibility is marked deprecated, so we need to disable the
+    // "deprecated-declarations" check for this invocation.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+    // We need to change this API once AWS-LC provides a method similar to the following:
+    // https://github.com/google/boringssl/blob/master/include/openssl/rsa.h#L630
+    RSA_blinding_off_temp_for_accp_compatibility(rsa);
+#pragma GCC diagnostic pop
+
+#endif
+    if (!RSA_set0_key(rsa, n, nullptr, d)) {
+        throw_openssl("Unable to set RSA key parameters");
+    }
+
+    return rsa.take();
+}
+
 }

csrc/keyutils.h

@@ -139,6 +139,10 @@ class raii_cipher_ctx {
 };
 
 const EVP_MD* digestFromJstring(raii_env& env, jstring digestName);
+
+// The generated RSA structure will own n and d.
+RSA* RSA_new_private_key_no_e(BIGNUM* n, BIGNUM* d);
+
 }
 
 #endif