Skip to content

Commit a5fc8ff

Browse files
Disable implicit rejection for RSA PKCS#1 (#95216)
Co-authored-by: Kevin Jones <kevin@vcsjones.com>
1 parent 7395a26 commit a5fc8ff

File tree

3 files changed

+58
-10
lines changed

3 files changed

+58
-10
lines changed

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

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -353,19 +353,10 @@ private void RsaCryptRoundtrip(RSAEncryptionPadding paddingMode, bool expectSucc
353353
Assert.Equal(TestData.HelloBytes, output);
354354
}
355355

356-
[ConditionalFact]
356+
[ConditionalFact(nameof(PlatformSupportsEmptyRSAEncryption))]
357357
[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
358358
public void RoundtripEmptyArray()
359359
{
360-
if (OperatingSystem.IsIOS() && !OperatingSystem.IsIOSVersionAtLeast(13, 6))
361-
{
362-
throw new SkipTestException("iOS prior to 13.6 does not reliably support RSA encryption of empty data.");
363-
}
364-
if (OperatingSystem.IsTvOS() && !OperatingSystem.IsTvOSVersionAtLeast(14, 0))
365-
{
366-
throw new SkipTestException("tvOS prior to 14.0 does not reliably support RSA encryption of empty data.");
367-
}
368-
369360
using (RSA rsa = RSAFactory.Create(TestData.RSA2048Params))
370361
{
371362
void RoundtripEmpty(RSAEncryptionPadding paddingMode)
@@ -725,6 +716,26 @@ public void NotSupportedValueMethods()
725716
}
726717
}
727718

719+
[ConditionalTheory]
720+
[InlineData(new byte[] { 1, 2, 3, 4 })]
721+
[InlineData(new byte[0])]
722+
public void Decrypt_Pkcs1_ErrorsForInvalidPadding(byte[] data)
723+
{
724+
if (data.Length == 0 && !PlatformSupportsEmptyRSAEncryption)
725+
{
726+
throw new SkipTestException("Platform does not support RSA encryption of empty data.");
727+
}
728+
729+
using (RSA rsa = RSAFactory.Create(TestData.RSA2048Params))
730+
{
731+
byte[] encrypted = Encrypt(rsa, data, RSAEncryptionPadding.Pkcs1);
732+
encrypted[1] ^= 0xFF;
733+
734+
// PKCS#1, the data, and the key are all deterministic so this should always throw an exception.
735+
Assert.ThrowsAny<CryptographicException>(() => Decrypt(rsa, encrypted, RSAEncryptionPadding.Pkcs1));
736+
}
737+
}
738+
728739
public static IEnumerable<object[]> OaepPaddingModes
729740
{
730741
get
@@ -746,5 +757,23 @@ public static IEnumerable<object[]> OaepPaddingModes
746757
}
747758
}
748759
}
760+
761+
public static bool PlatformSupportsEmptyRSAEncryption
762+
{
763+
get
764+
{
765+
if (OperatingSystem.IsIOS() && !OperatingSystem.IsIOSVersionAtLeast(13, 6))
766+
{
767+
return false;
768+
}
769+
770+
if (OperatingSystem.IsTvOS() && !OperatingSystem.IsTvOSVersionAtLeast(14, 0))
771+
{
772+
return false;
773+
}
774+
775+
return true;
776+
}
777+
}
749778
}
750779
}

src/native/libs/System.Security.Cryptography.Native/opensslshim.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,10 @@ int EVP_DigestFinalXOF(EVP_MD_CTX *ctx, unsigned char *md, size_t len);
296296
REQUIRED_FUNCTION(ERR_peek_error) \
297297
REQUIRED_FUNCTION(ERR_peek_error_line) \
298298
REQUIRED_FUNCTION(ERR_peek_last_error) \
299+
REQUIRED_FUNCTION(ERR_pop_to_mark) \
299300
FALLBACK_FUNCTION(ERR_put_error) \
300301
REQUIRED_FUNCTION(ERR_reason_error_string) \
302+
REQUIRED_FUNCTION(ERR_set_mark) \
301303
LIGHTUP_FUNCTION(ERR_set_debug) \
302304
LIGHTUP_FUNCTION(ERR_set_error) \
303305
REQUIRED_FUNCTION(EVP_aes_128_cbc) \
@@ -353,6 +355,7 @@ int EVP_DigestFinalXOF(EVP_MD_CTX *ctx, unsigned char *md, size_t len);
353355
REQUIRED_FUNCTION(EVP_PKCS82PKEY) \
354356
REQUIRED_FUNCTION(EVP_PKEY2PKCS8) \
355357
REQUIRED_FUNCTION(EVP_PKEY_CTX_ctrl) \
358+
REQUIRED_FUNCTION(EVP_PKEY_CTX_ctrl_str) \
356359
REQUIRED_FUNCTION(EVP_PKEY_CTX_free) \
357360
REQUIRED_FUNCTION(EVP_PKEY_CTX_get0_pkey) \
358361
REQUIRED_FUNCTION(EVP_PKEY_CTX_new) \
@@ -794,8 +797,10 @@ FOR_ALL_OPENSSL_FUNCTIONS
794797
#define ERR_peek_error_line ERR_peek_error_line_ptr
795798
#define ERR_peek_last_error ERR_peek_last_error_ptr
796799
#define ERR_put_error ERR_put_error_ptr
800+
#define ERR_pop_to_mark ERR_pop_to_mark_ptr
797801
#define ERR_reason_error_string ERR_reason_error_string_ptr
798802
#define ERR_set_debug ERR_set_debug_ptr
803+
#define ERR_set_mark ERR_set_mark_ptr
799804
#define ERR_set_error ERR_set_error_ptr
800805
#define EVP_aes_128_cbc EVP_aes_128_cbc_ptr
801806
#define EVP_aes_128_cfb8 EVP_aes_128_cfb8_ptr
@@ -850,6 +855,7 @@ FOR_ALL_OPENSSL_FUNCTIONS
850855
#define EVP_PKCS82PKEY EVP_PKCS82PKEY_ptr
851856
#define EVP_PKEY2PKCS8 EVP_PKEY2PKCS8_ptr
852857
#define EVP_PKEY_CTX_ctrl EVP_PKEY_CTX_ctrl_ptr
858+
#define EVP_PKEY_CTX_ctrl_str EVP_PKEY_CTX_ctrl_str_ptr
853859
#define EVP_PKEY_CTX_free EVP_PKEY_CTX_free_ptr
854860
#define EVP_PKEY_CTX_get0_pkey EVP_PKEY_CTX_get0_pkey_ptr
855861
#define EVP_PKEY_CTX_new EVP_PKEY_CTX_new_ptr

src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,19 @@ static bool ConfigureEncryption(EVP_PKEY_CTX* ctx, RsaPaddingMode padding, const
6767
{
6868
return false;
6969
}
70+
71+
// OpenSSL 3.2 introduced a change where PKCS#1 RSA decryption does not fail for invalid padding.
72+
// If the padding is invalid, the decryption operation returns random data.
73+
// See https://github.com/openssl/openssl/pull/13817 for background.
74+
// Some Linux distributions backported this change to previous versions of OpenSSL.
75+
// Here we do a best-effort to set a flag to revert the behavior to failing if the padding is invalid.
76+
ERR_set_mark();
77+
78+
EVP_PKEY_CTX_ctrl_str(ctx, "rsa_pkcs1_implicit_rejection", "0");
79+
80+
// Undo any changes to the error queue that may have occured while configuring implicit rejection if the
81+
// current version does not support implicit rejection.
82+
ERR_pop_to_mark();
7083
}
7184
else
7285
{

0 commit comments

Comments
 (0)