Skip to content

Commit

Permalink
Validate RSA parameters when importing to OpenSSL
Browse files Browse the repository at this point in the history 
This imports a key explicitly into the OpenSSL default software RSA implementation to call 
`RSA_check_key` to determine that the key parameters are consistent.

After the key consistency is determined, then it imports into a second new key handle, using the
configured default (which may also be the default software RSA implementation), and returns that one.

Commit migrated from dotnet/corefx@5590a91
  • Loading branch information
@vcsjones @bartonjs
vcsjones authored and bartonjs committed Jun 26, 2019
1 parent 9b65fc7 commit 7fb315d
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 6 deletions.
2 changes: 1 addition & 1 deletion ...ibraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,7 @@ public static void FromInvalidXml()
}

[Fact]
[ActiveIssue(37595, TestPlatforms.AnyUnix)]
[ActiveIssue(37595, TestPlatforms.OSX)]
public static void FromNonsenseXml()
{
// This is DiminishedDPParameters XML, but with a P that is way too long.
Expand Down
11 changes: 11 additions & 0 deletions src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
#undef EVP_MD_CTX_create
#undef EVP_MD_CTX_init
#undef EVP_MD_CTX_destroy
#undef RSA_PKCS1_SSLeay
#undef SSLv23_method
#endif

Expand Down Expand Up @@ -143,6 +144,7 @@ void RSA_get0_crt_params(const RSA* rsa, const BIGNUM** dmp1, const BIGNUM** dmq
void RSA_get0_factors(const RSA* rsa, const BIGNUM** p, const BIGNUM** q);
void RSA_get0_key(const RSA* rsa, const BIGNUM** n, const BIGNUM** e, const BIGNUM** d);
int32_t RSA_meth_get_flags(const RSA_METHOD* meth);
const RSA_METHOD* RSA_PKCS1_OpenSSL(void);
int32_t RSA_set0_crt_params(RSA* rsa, BIGNUM* dmp1, BIGNUM* dmq1, BIGNUM* iqmp);
int32_t RSA_set0_factors(RSA* rsa, BIGNUM* p, BIGNUM* q);
int32_t RSA_set0_key(RSA* rsa, BIGNUM* n, BIGNUM* e, BIGNUM* d);
Expand Down Expand Up @@ -224,6 +226,7 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi
REQUIRED_FUNCTION(BN_bin2bn) \
REQUIRED_FUNCTION(BN_bn2bin) \
REQUIRED_FUNCTION(BN_clear_free) \
REQUIRED_FUNCTION(BN_dup) \
REQUIRED_FUNCTION(BN_free) \
REQUIRED_FUNCTION(BN_new) \
REQUIRED_FUNCTION(BN_num_bits) \
Expand Down Expand Up @@ -421,6 +424,7 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi
REQUIRED_FUNCTION(PKCS7_free) \
REQUIRED_FUNCTION(RAND_bytes) \
REQUIRED_FUNCTION(RAND_poll) \
REQUIRED_FUNCTION(RSA_check_key) \
REQUIRED_FUNCTION(RSA_free) \
REQUIRED_FUNCTION(RSA_generate_key_ex) \
REQUIRED_FUNCTION(RSA_get_method) \
Expand All @@ -429,13 +433,15 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi
FALLBACK_FUNCTION(RSA_get0_key) \
FALLBACK_FUNCTION(RSA_meth_get_flags) \
REQUIRED_FUNCTION(RSA_new) \
RENAMED_FUNCTION(RSA_PKCS1_OpenSSL, RSA_PKCS1_SSLeay) \
REQUIRED_FUNCTION(RSA_private_decrypt) \
REQUIRED_FUNCTION(RSA_private_encrypt) \
REQUIRED_FUNCTION(RSA_public_decrypt) \
REQUIRED_FUNCTION(RSA_public_encrypt) \
FALLBACK_FUNCTION(RSA_set0_crt_params) \
FALLBACK_FUNCTION(RSA_set0_factors) \
FALLBACK_FUNCTION(RSA_set0_key) \
REQUIRED_FUNCTION(RSA_set_method) \
REQUIRED_FUNCTION(RSA_sign) \
REQUIRED_FUNCTION(RSA_size) \
REQUIRED_FUNCTION(RSA_up_ref) \
Expand Down Expand Up @@ -608,6 +614,7 @@ FOR_ALL_OPENSSL_FUNCTIONS
#define BN_bin2bn BN_bin2bn_ptr
#define BN_bn2bin BN_bn2bin_ptr
#define BN_clear_free BN_clear_free_ptr
#define BN_dup BN_dup_ptr
#define BN_free BN_free_ptr
#define BN_new BN_new_ptr
#define BN_num_bits BN_num_bits_ptr
Expand Down Expand Up @@ -805,6 +812,7 @@ FOR_ALL_OPENSSL_FUNCTIONS
#define PKCS7_free PKCS7_free_ptr
#define RAND_bytes RAND_bytes_ptr
#define RAND_poll RAND_poll_ptr
#define RSA_check_key RSA_check_key_ptr
#define RSA_free RSA_free_ptr
#define RSA_generate_key_ex RSA_generate_key_ex_ptr
#define RSA_get0_crt_params RSA_get0_crt_params_ptr
Expand All @@ -813,13 +821,15 @@ FOR_ALL_OPENSSL_FUNCTIONS
#define RSA_get_method RSA_get_method_ptr
#define RSA_meth_get_flags RSA_meth_get_flags_ptr
#define RSA_new RSA_new_ptr
#define RSA_PKCS1_OpenSSL RSA_PKCS1_OpenSSL_ptr
#define RSA_private_decrypt RSA_private_decrypt_ptr
#define RSA_private_encrypt RSA_private_encrypt_ptr
#define RSA_public_decrypt RSA_public_decrypt_ptr
#define RSA_public_encrypt RSA_public_encrypt_ptr
#define RSA_set0_crt_params RSA_set0_crt_params_ptr
#define RSA_set0_factors RSA_set0_factors_ptr
#define RSA_set0_key RSA_set0_key_ptr
#define RSA_set_method RSA_set_method_ptr
#define RSA_sign RSA_sign_ptr
#define RSA_size RSA_size_ptr
#define RSA_up_ref RSA_up_ref_ptr
Expand Down Expand Up @@ -1049,6 +1059,7 @@ FOR_ALL_OPENSSL_FUNCTIONS
// Restore the old names for RENAMED_FUNCTION functions.
#define EVP_MD_CTX_free EVP_MD_CTX_destroy
#define EVP_MD_CTX_new EVP_MD_CTX_create
#define RSA_PKCS1_OpenSSL RSA_PKCS1_SSLeay
#define OPENSSL_sk_free sk_free
#define OPENSSL_sk_new_null sk_new_null
#define OPENSSL_sk_num sk_num
Expand Down
104 changes: 99 additions & 5 deletions src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,90 @@ static BIGNUM* MakeBignum(uint8_t* buffer, int32_t bufferLength)
return NULL;
}

static int32_t ValidatePrivateRsaParameters(BIGNUM* bnN,
BIGNUM* bnE,
BIGNUM* bnD,
BIGNUM* bnP,
BIGNUM* bnQ,
BIGNUM* bnDmp1,
BIGNUM* bnDmq1,
BIGNUM* bnIqmp)
{
if (!bnN || !bnE || !bnD || !bnP || !bnQ ||
!bnDmp1 || !bnDmq1 || !bnIqmp)
{
assert(false);
return 0;
}

// This is shared and should not be freed.
const RSA_METHOD* openssl_method = RSA_PKCS1_OpenSSL();

RSA* rsa = RSA_new();

if (!rsa)
{
return 0;
}

// RSA_check_key only works when it has access to the
// internal values of the RSA*. If the process
// has changed the default ENGINE, the function
// may fail. For purposes of validation, we always
// do it using the openssl methods.
if (!RSA_set_method(rsa, openssl_method))
{
RSA_free(rsa);
return 0;
}

BIGNUM* bnNdup = BN_dup(bnN);
BIGNUM* bnEdup = BN_dup(bnE);
BIGNUM* bnDdup = BN_dup(bnD);

if (!RSA_set0_key(rsa, bnNdup, bnEdup, bnDdup))
{
BN_free(bnNdup);
BN_free(bnEdup);
BN_clear_free(bnDdup);
RSA_free(rsa);
return 0;
}

BIGNUM* bnPdup = BN_dup(bnP);
BIGNUM* bnQdup = BN_dup(bnQ);

if (!RSA_set0_factors(rsa, bnPdup, bnQdup))
{
BN_clear_free(bnPdup);
BN_clear_free(bnQdup);
RSA_free(rsa);
return 0;
}

BIGNUM* bnDmp1dup = BN_dup(bnDmp1);
BIGNUM* bnDmq1dup = BN_dup(bnDmq1);
BIGNUM* bnIqmpdup = BN_dup(bnIqmp);

if (!RSA_set0_crt_params(rsa, bnDmp1dup, bnDmq1dup, bnIqmpdup))
{
BN_clear_free(bnDmp1dup);
BN_clear_free(bnDmq1dup);
BN_clear_free(bnIqmpdup);
RSA_free(rsa);
return 0;
}

if (RSA_check_key(rsa) != 1)
{
RSA_free(rsa);
return 0;
}

RSA_free(rsa);
return 1;
}

int32_t CryptoNative_SetRsaParameters(RSA* rsa,
uint8_t* n,
int32_t nLength,
Expand Down Expand Up @@ -282,18 +366,28 @@ int32_t CryptoNative_SetRsaParameters(RSA* rsa,
{
BIGNUM* bnP = MakeBignum(p, pLength);
BIGNUM* bnQ = MakeBignum(q, qLength);
BIGNUM* bnDmp1 = MakeBignum(dmp1, dmp1Length);
BIGNUM* bnDmq1 = MakeBignum(dmq1, dmq1Length);
BIGNUM* bnIqmp = MakeBignum(iqmp, iqmpLength);

if (!RSA_set0_factors(rsa, bnP, bnQ))
if (!ValidatePrivateRsaParameters(bnN,
bnE,
bnD,
bnP,
bnQ,
bnDmp1,
bnDmq1,
bnIqmp)
|| !RSA_set0_factors(rsa, bnP, bnQ))
{
BN_free(bnP);
BN_free(bnQ);
BN_free(bnDmp1);
BN_free(bnDmq1);
BN_free(bnIqmp);
return 0;
}

BIGNUM* bnDmp1 = MakeBignum(dmp1, dmp1Length);
BIGNUM* bnDmq1 = MakeBignum(dmq1, dmq1Length);
BIGNUM* bnIqmp = MakeBignum(iqmp, iqmpLength);

if (!RSA_set0_crt_params(rsa, bnDmp1, bnDmq1, bnIqmp))
{
BN_free(bnDmp1);
Expand Down

0 comments on commit 7fb315d

Please sign in to comment.