Skip to content

fix: re-enable PKCS#1 v1.5 for signatures (fixes #61) #92

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Koan-Bot wants to merge 1 commit into from
+85 −12
Closed

fix: re-enable PKCS#1 v1.5 for signatures (fixes #61) #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions RSA.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,19 @@ EVP_MD *get_md_bynid(int hash_method)
static int
setup_pss_sign_ctx(EVP_PKEY_CTX *ctx, int padding, int hash_nid, EVP_MD **md_out)
{
int effective_pad = padding;
int effective_pad;
EVP_MD *md = NULL;

if (padding != RSA_NO_PADDING && padding != RSA_PKCS1_PADDING)
if (padding == RSA_PKCS1_PSS_PADDING) {
effective_pad = RSA_PKCS1_PSS_PADDING;
} else if (padding == RSA_NO_PADDING) {
effective_pad = RSA_NO_PADDING;
} else {
/* Default: RSASSA-PKCS1-v1.5, matching pre-3.x RSA_sign() behavior.
* PKCS#1 v1.5 signatures are not vulnerable to the Marvin attack
* (CVE-2024-2467), which only affects decryption padding oracles. */
effective_pad = RSA_PKCS1_PADDING;
}

if (EVP_PKEY_CTX_set_rsa_padding(ctx, effective_pad) <= 0)
return 0;
Expand Down Expand Up @@ -453,7 +461,7 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from,

#if OPENSSL_VERSION_NUMBER >= 0x30000000L

if(p_rsa->padding == RSA_PKCS1_PSS_PADDING) {
if(is_encrypt && p_rsa->padding == RSA_PKCS1_PSS_PADDING) {
croak("PKCS#1 v2.1 RSA-PSS cannot be used for encryption operations call \"use_pkcs1_oaep_padding\" instead.");
}

Expand All @@ -470,8 +478,18 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from,
values here are RSA_NO_PADDING and RSA_PKCS1_OAEP_PADDING (for
encrypt/decrypt) or RSA_PKCS1_PADDING (for private_encrypt/public_decrypt). */
crypt_pad = p_rsa->padding;
if (is_encrypt && p_rsa->padding != RSA_NO_PADDING) {
crypt_pad = RSA_PKCS1_OAEP_PADDING;
if (is_encrypt) {
/* Encryption: force OAEP for all non-NO_PADDING modes */
if (p_rsa->padding != RSA_NO_PADDING) {
crypt_pad = RSA_PKCS1_OAEP_PADDING;
}
} else {
/* private_encrypt/public_decrypt: use the actual padding.
* PKCS#1 v1.5 is valid here (RSASSA-PKCS1-v1.5 type 1 padding).
* OAEP is not valid for signing — fall back to PKCS#1 v1.5. */
if (p_rsa->padding == RSA_PKCS1_OAEP_PADDING) {
crypt_pad = RSA_PKCS1_PADDING;
}
}
THROW(EVP_PKEY_CTX_set_rsa_padding(ctx, crypt_pad) > 0);
THROW(p_crypt(ctx, NULL, &to_length, from, from_length) == 1);
Expand All @@ -490,8 +508,15 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from,
#else
size = EVP_PKEY_get_size(p_rsa->rsa);
CHECK_NEW(to, size, UNSIGNED_CHAR);
{
int crypt_pad = p_rsa->padding;
if (!is_encrypt && crypt_pad == RSA_PKCS1_OAEP_PADDING) {
/* OAEP is not valid for private_encrypt/public_decrypt — fall back to PKCS#1 v1.5 */
crypt_pad = RSA_PKCS1_PADDING;
}
to_length = p_crypt(
from_length, from, (unsigned char*) to, p_rsa->rsa, p_rsa->padding);
from_length, from, (unsigned char*) to, p_rsa->rsa, crypt_pad);
}
#endif
if (to_length < 0)
{
Expand Down
41 changes: 40 additions & 1 deletion t/padding.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use Crypt::OpenSSL::Guess qw(openssl_version);
my ($major, $minor, $patch) = openssl_version;

BEGIN {
plan tests => 124 + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ? 4 * 5 : 0 );
plan tests => 131 + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ? 4 * 5 : 0 );
}

sub _Test_Encrypt_And_Decrypt {
Expand Down Expand Up @@ -163,4 +163,43 @@ foreach my $padding (keys %padding_methods) {
}
}

# PKCS#1 v1.5 padding is allowed for signing but not for encryption (Marvin attack)
SKIP: {
skip "PKCS#1 v1.5 encryption croak test requires OpenSSL 3.x", 2 unless $major ge '3.0';
$rsa->use_pkcs1_padding;
eval { $rsa->encrypt("test") };
like($@, qr/Marvin attack/, "PKCS#1 v1.5 encrypt croaks with Marvin attack warning");

eval { $rsa->decrypt("test" x 64) };
like($@, qr/Marvin attack/, "PKCS#1 v1.5 decrypt croaks with Marvin attack warning");

# Restore default padding
$rsa->use_pkcs1_oaep_padding;
}

# PKCS#1 v1.5 private_encrypt/public_decrypt (raw RSA signing, not affected by Marvin)
{
my $rsa_sign = Crypt::OpenSSL::RSA->generate_key(2048);
my $rsa_verify = Crypt::OpenSSL::RSA->new_public_key($rsa_sign->get_public_key_string());

# With explicit PKCS#1 v1.5 padding
$rsa_sign->use_pkcs1_padding;
$rsa_verify->use_pkcs1_padding;
my $msg = "PKCS1 v1.5 private_encrypt test";
my $ct = eval { $rsa_sign->private_encrypt($msg) };
ok(!$@, "private_encrypt with PKCS#1 v1.5 succeeds");
my $pt = eval { $rsa_verify->public_decrypt($ct) };
ok(!$@, "public_decrypt with PKCS#1 v1.5 succeeds");
is($pt, $msg, "private_encrypt/public_decrypt PKCS#1 v1.5 round-trips");

# With default OAEP padding (should fall back for signing ops)
my $rsa_sign2 = Crypt::OpenSSL::RSA->generate_key(2048);
my $rsa_verify2 = Crypt::OpenSSL::RSA->new_public_key($rsa_sign2->get_public_key_string());
$ct = eval { $rsa_sign2->private_encrypt($msg) };
ok(!$@, "private_encrypt with default padding succeeds");
$pt = eval { $rsa_verify2->public_decrypt($ct) };
ok(!$@, "public_decrypt with default padding succeeds");
is($pt, $msg, "private_encrypt/public_decrypt default padding round-trips");
}

# Try
19 changes: 14 additions & 5 deletions t/private_crypt.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use Crypt::OpenSSL::RSA;
# pre-3.x: RSA_private_encrypt / RSA_public_decrypt
# 3.x: EVP_PKEY_sign / EVP_PKEY_verify_recover

plan tests => 16;
plan tests => 17;

Crypt::OpenSSL::Random::random_seed("OpenSSL needs at least 32 bytes.");
Crypt::OpenSSL::RSA->import_random_seed();
Expand Down Expand Up @@ -136,13 +136,22 @@ my $rsa_pub = Crypt::OpenSSL::RSA->new_public_key($rsa->get_public_key_string())
ok($@, "PSS padding cannot be used with private_encrypt");
}

# --- OAEP padding rejected for private_encrypt ---
# OAEP is an encryption scheme, invalid for sign-type operations.
# --- OAEP padding falls back to PKCS#1 v1.5 for private_encrypt ---
# OAEP is an encryption scheme; for sign-type operations it falls back
# to PKCS#1 v1.5 (matching the default padding behavior).

{
$rsa->use_pkcs1_oaep_padding();
eval { $rsa->private_encrypt("oaep test") };
ok($@, "OAEP padding cannot be used with private_encrypt");
$rsa_pub->use_pkcs1_oaep_padding();
my $msg = "oaep fallback test";
my $ct = eval { $rsa->private_encrypt($msg) };
ok(!$@, "private_encrypt with OAEP falls back to PKCS#1 v1.5")
or diag $@;
SKIP: {
skip "private_encrypt failed", 1 if $@;
my $pt = eval { $rsa_pub->public_decrypt($ct) };
is($pt, $msg, "OAEP fallback round-trips via public_decrypt");
}
}

# --- Public key cannot private_encrypt ---
Expand Down
Loading