From 7fb315d819255317205295211f9d7dd763684564 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 26 Jun 2019 10:03:10 -0400 Subject: [PATCH] Validate RSA parameters when importing to OpenSSL 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 https://github.com/dotnet/corefx/commit/5590a9172a65be8cd6f93fdd76f547e63006c837 --- .../AlgorithmImplementations/RSA/RSAXml.cs | 2 +- .../opensslshim.h | 11 +++ .../System.Security.Cryptography.Native/pal_rsa.c | 104 ++++++++++++++++++++- 3 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs index 2dcbf42..773b1ea 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs @@ -1340,7 +1340,7 @@ zM= } [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. diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index ade4e08..41655cf 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -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 @@ -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); @@ -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) \ @@ -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) \ @@ -429,6 +433,7 @@ 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) \ @@ -436,6 +441,7 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi 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) \ @@ -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 @@ -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 @@ -813,6 +821,7 @@ 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 @@ -820,6 +829,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #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 @@ -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 diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c index 85ba7fc..f764815 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c @@ -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, @@ -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); -- 2.7.4