From 076687f5f9e7e1fce24f33f498b4e03c4150108e Mon Sep 17 00:00:00 2001

From: Omair Majid <omajid@redhat.com>

Date: Fri, 2 Feb 2024 12:09:52 -0500

Subject: [PATCH] Revert "Disable implicit rejection for RSA PKCS#1 (#95218)"

This reverts commit e3500b8e8ad18e8bf067dc5250863b64bb8f0de0.

To quote Clemens Lang:

> [Disabling implcit rejection] re-enables a Bleichenbacher timing oracle

> attack against PKCS#1v1.5 decryption. See

> https://people.redhat.com/~hkario/marvin/ for details and

> https://github.com/dotnet/runtime/pull/95157#issuecomment-1842784399 for a

> comment by the researcher who published the vulnerability and proposed the

> change in OpenSSL.

For more details, see:

https://github.com/dotnet/runtime/pull/95216#issuecomment-1842799314

---

 .../RSA/EncryptDecrypt.cs                     | 49 ++++---------------

 .../opensslshim.h                             |  6 ---

 .../pal_evp_pkey_rsa.c                        | 13 -----

 3 files changed, 10 insertions(+), 58 deletions(-)

diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.cs

index 55a044d62a6..e72d42e87d2 100644

--- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.cs

+++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.cs

@@ -338,10 +338,19 @@ private void RsaCryptRoundtrip(RSAEncryptionPadding paddingMode, bool expectSucc

             Assert.Equal(TestData.HelloBytes, output);

         }

-        [ConditionalFact(nameof(PlatformSupportsEmptyRSAEncryption))]

+        [ConditionalFact]

         [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]

         public void RoundtripEmptyArray()

         {

+            if (OperatingSystem.IsIOS() && !OperatingSystem.IsIOSVersionAtLeast(13, 6))

+            {

+                throw new SkipTestException("iOS prior to 13.6 does not reliably support RSA encryption of empty data.");

+            }

+            if (OperatingSystem.IsTvOS() && !OperatingSystem.IsTvOSVersionAtLeast(14, 0))

+            {

+                throw new SkipTestException("tvOS prior to 14.0 does not reliably support RSA encryption of empty data.");

+            }

+

             using (RSA rsa = RSAFactory.Create(TestData.RSA2048Params))

             {

                 void RoundtripEmpty(RSAEncryptionPadding paddingMode)

@@ -692,26 +701,6 @@ public void NotSupportedValueMethods()

             }

         }

-        [ConditionalTheory]

-        [InlineData(new byte[] { 1, 2, 3, 4 })]

-        [InlineData(new byte[0])]

-        public void Decrypt_Pkcs1_ErrorsForInvalidPadding(byte[] data)

-        {

-            if (data.Length == 0 && !PlatformSupportsEmptyRSAEncryption)

-            {

-                throw new SkipTestException("Platform does not support RSA encryption of empty data.");

-            }

-

-            using (RSA rsa = RSAFactory.Create(TestData.RSA2048Params))

-            {

-                byte[] encrypted = Encrypt(rsa, data, RSAEncryptionPadding.Pkcs1);

-                encrypted[1] ^= 0xFF;

-

-                // PKCS#1, the data, and the key are all deterministic so this should always throw an exception.

-                Assert.ThrowsAny<CryptographicException>(() => Decrypt(rsa, encrypted, RSAEncryptionPadding.Pkcs1));

-            }

-        }

-

         public static IEnumerable<object[]> OaepPaddingModes

         {

             get

@@ -726,23 +715,5 @@ public static IEnumerable<object[]> OaepPaddingModes

                 }

             }

         }

-

-        public static bool PlatformSupportsEmptyRSAEncryption

-        {

-            get

-            {

-                if (OperatingSystem.IsIOS() && !OperatingSystem.IsIOSVersionAtLeast(13, 6))

-                {

-                    return false;

-                }

-

-                if (OperatingSystem.IsTvOS() && !OperatingSystem.IsTvOSVersionAtLeast(14, 0))

-                {

-                    return false;

-                }

-

-                return true;

-            }

-        }

     }

 }

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 050df1193ff..dad18ebd9a1 100644

--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h

+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h

@@ -272,10 +272,8 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void);

     REQUIRED_FUNCTION(ERR_peek_error) \

     REQUIRED_FUNCTION(ERR_peek_error_line) \

     REQUIRED_FUNCTION(ERR_peek_last_error) \

-    REQUIRED_FUNCTION(ERR_pop_to_mark) \

     FALLBACK_FUNCTION(ERR_put_error) \

     REQUIRED_FUNCTION(ERR_reason_error_string) \

-    REQUIRED_FUNCTION(ERR_set_mark) \

     LIGHTUP_FUNCTION(ERR_set_debug) \

     LIGHTUP_FUNCTION(ERR_set_error) \

     REQUIRED_FUNCTION(EVP_aes_128_cbc) \

@@ -330,7 +328,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void);

     REQUIRED_FUNCTION(EVP_PKCS82PKEY) \

     REQUIRED_FUNCTION(EVP_PKEY2PKCS8) \

     REQUIRED_FUNCTION(EVP_PKEY_CTX_ctrl) \

-    REQUIRED_FUNCTION(EVP_PKEY_CTX_ctrl_str) \

     REQUIRED_FUNCTION(EVP_PKEY_CTX_free) \

     REQUIRED_FUNCTION(EVP_PKEY_CTX_get0_pkey) \

     REQUIRED_FUNCTION(EVP_PKEY_CTX_new) \

@@ -728,10 +725,8 @@ FOR_ALL_OPENSSL_FUNCTIONS

 #define ERR_peek_error_line ERR_peek_error_line_ptr

 #define ERR_peek_last_error ERR_peek_last_error_ptr

 #define ERR_put_error ERR_put_error_ptr

-#define ERR_pop_to_mark ERR_pop_to_mark_ptr

 #define ERR_reason_error_string ERR_reason_error_string_ptr

 #define ERR_set_debug ERR_set_debug_ptr

-#define ERR_set_mark ERR_set_mark_ptr

 #define ERR_set_error ERR_set_error_ptr

 #define EVP_aes_128_cbc EVP_aes_128_cbc_ptr

 #define EVP_aes_128_cfb8 EVP_aes_128_cfb8_ptr

@@ -785,7 +780,6 @@ FOR_ALL_OPENSSL_FUNCTIONS

 #define EVP_PKCS82PKEY EVP_PKCS82PKEY_ptr

 #define EVP_PKEY2PKCS8 EVP_PKEY2PKCS8_ptr

 #define EVP_PKEY_CTX_ctrl EVP_PKEY_CTX_ctrl_ptr

-#define EVP_PKEY_CTX_ctrl_str EVP_PKEY_CTX_ctrl_str_ptr

 #define EVP_PKEY_CTX_free EVP_PKEY_CTX_free_ptr

 #define EVP_PKEY_CTX_get0_pkey EVP_PKEY_CTX_get0_pkey_ptr

 #define EVP_PKEY_CTX_new EVP_PKEY_CTX_new_ptr

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c

index c3e491a868f..36924abb505 100644

--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c

+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c

@@ -63,19 +63,6 @@ static bool ConfigureEncryption(EVP_PKEY_CTX* ctx, RsaPaddingMode padding, const

         {

             return false;

         }

-

-        // OpenSSL 3.2 introduced a change where PKCS#1 RSA decryption does not fail for invalid padding.

-        // If the padding is invalid, the decryption operation returns random data.

-        // See https://github.com/openssl/openssl/pull/13817 for background.

-        // Some Linux distributions backported this change to previous versions of OpenSSL.

-        // Here we do a best-effort to set a flag to revert the behavior to failing if the padding is invalid.

-        ERR_set_mark();

-

-        EVP_PKEY_CTX_ctrl_str(ctx, "rsa_pkcs1_implicit_rejection", "0");

-

-        // Undo any changes to the error queue that may have occured while configuring implicit rejection if the

-        // current version does not support implicit rejection.

-        ERR_pop_to_mark();

     }

     else

     {

-- 

2.43.0