Validate RSA parameters when importing to OpenSSL
authorKevin Jones <kevin@vcsjones.com>
Wed, 26 Jun 2019 14:03:10 +0000 (10:03 -0400)
committerJeremy Barton <jbarton@microsoft.com>
Wed, 26 Jun 2019 14:03:09 +0000 (07:03 -0700)
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

src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs
src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h
src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c

index 2dcbf42..773b1ea 100644 (file)
@@ -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.
index ade4e08..41655cf 100644 (file)
@@ -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
index 85ba7fc..f764815 100644 (file)
@@ -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);