Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using RSA_blinding_off_temp_for_accp_compatibility #389

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions csrc/bn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,13 @@ jbyteArray bn2jarr(raii_env& env, const BIGNUM* bn)
return jarr;
}

void bn_dup_into(BIGNUM** dst, BIGNUM const* src)
{
BN_free(*dst);
WillChilds-Klein marked this conversation as resolved.
Show resolved Hide resolved
*dst = BN_dup(src);
if (*dst == nullptr) {
throw_openssl("BN_dup failed.");
}
}

} // namespace AmazonCorrettoCryptoProvider
3 changes: 3 additions & 0 deletions csrc/bn.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ class BigNumObj {

inline BigNumObj bn_zero() { return BigNumObj(); }

// Frees the data referenced by *dst, if any, and copies src to *dst.
void bn_dup_into(BIGNUM** dst, BIGNUM const* src);

} // namespace AmazonCorrettoCryptoProvider

#endif
20 changes: 9 additions & 11 deletions csrc/java_evp_keys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -496,21 +496,19 @@ JNIEXPORT jlong JNICALL Java_com_amazon_corretto_crypto_provider_EvpKeyFactory_r
if (privateExponentArr) {
BigNumObj privExp = BigNumObj::fromJavaArray(env, privateExponentArr);

int res = 1;
if (BN_is_zero(pubExp)) {
// RSA blinding can't be performed without |e|.
rsa.set(RSA_new_private_key_no_e(modulus, privExp));
rsa.set(new_private_RSA_key_with_no_e(modulus, privExp));
// new_private_RSA_key_with_no_e does not take the ownership of its arguments
} else {
res = RSA_set0_key(rsa, modulus, pubExp, privExp);
}

if (res != 1) {
throw_openssl(EX_RUNTIME_CRYPTO, "Unable to set RSA values");
if (RSA_set0_key(rsa, modulus, pubExp, privExp) != 1) {
throw_openssl(EX_RUNTIME_CRYPTO, "Unable to set RSA values");
}
// RSA_set0_key takes ownership
modulus.releaseOwnership();
pubExp.releaseOwnership();
privExp.releaseOwnership();
}
// RSA_set0_key takes ownership
modulus.releaseOwnership();
pubExp.releaseOwnership();
privExp.releaseOwnership();
} else {
if (RSA_set0_key(rsa, modulus, pubExp, NULL) != 1) {
throw_openssl(EX_RUNTIME_CRYPTO, "Unable to set RSA values");
Expand Down
39 changes: 19 additions & 20 deletions csrc/keyutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,16 @@ EVP_PKEY* der2EvpPrivateKey(
if (need_rebuild) {
// This key likely only has (n, d) set. Very weird, but it happens in java sometimes.
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));
// No need to copy n or d since new_private_RSA_key_with_no_e does not take ownership.
nulled_rsa.set(new_private_RSA_key_with_no_e(n, d));
if (e != nullptr && !BN_is_zero(e)) {
// Need to copy e since RSA_set0_key takes ownership.
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);
shouldCheckPrivate = false; // We cannot check private keys without CRT parameters
}
Expand Down Expand Up @@ -170,35 +168,36 @@ const EVP_MD* digestFromJstring(raii_env& env, jstring digestName)
return result;
}

RSA* RSA_new_private_key_no_e(BIGNUM* n, BIGNUM* d)
RSA* new_private_RSA_key_with_no_e(BIGNUM const* n, BIGNUM const* d)
{
#ifdef FIPS_BUILD
// AWS-LC-FIPS doesn't have RSA_new_private_key_no_e method yet.
// The following implementation has been copied from AWS-LC:
// https://github.com/aws/aws-lc/blob/v1.30.1/crypto/fipsmodule/rsa/rsa.c#L147
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;

bn_dup_into(&rsa->n, n);
bn_dup_into(&rsa->d, d);

return rsa.take();

#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
RSA* result = RSA_new_private_key_no_e(n, d);
WillChilds-Klein marked this conversation as resolved.
Show resolved Hide resolved

#endif
if (!RSA_set0_key(rsa, n, nullptr, d)) {
throw_openssl("Unable to set RSA key parameters");
if (result == nullptr) {
throw_openssl("RSA_new_private_key_no_e failed.");
}

return rsa.take();
return result;

#endif
}

}
4 changes: 1 addition & 3 deletions csrc/keyutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ class raii_cipher_ctx {

const EVP_MD* digestFromJstring(raii_env& env, jstring digestName);

// The generated RSA structure will own n and d. The generated private key has blinding disabled since for blinding, one
// needs the public exponent.
RSA* RSA_new_private_key_no_e(BIGNUM* n, BIGNUM* d);
RSA* new_private_RSA_key_with_no_e(BIGNUM const* n, BIGNUM const* d);

}

Expand Down
Loading