Skip to content

Commit

Permalink
Revert "Disable implicit rejection for RSA PKCS#1 (dotnet#95218)"
Browse files Browse the repository at this point in the history
This reverts commit e3500b8.

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
> dotnet#95157 (comment) for a
> comment by the researcher who published the vulnerability and proposed the
> change in OpenSSL.

For more details, see:
dotnet#95216 (comment)
  • Loading branch information
omajid committed Feb 2, 2024
1 parent a43d69f commit 076687f
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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) \
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit 076687f

Please sign in to comment.