From ea943a9816c29987b3377e84ff61dacca302af5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sun, 5 Apr 2026 18:09:31 -0600 Subject: [PATCH] fix: re-enable PKCS#1 v1.5 padding for signature operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PKCS#1 v1.5 is safe for signing and raw RSA operations but vulnerable to the Marvin attack (CVE-2024-2467) only in decryption padding oracles. v0.35 disabled it for all operations indiscriminately, breaking RSASSA-PKCS1-v1.5 usage (RS256 JWT signing) across 9+ CPAN modules. Restore PKCS#1 v1.5 support for sign/verify and private_encrypt/public_decrypt while keeping it blocked for encrypt/decrypt operations. Default sign/verify to PKCS#1 v1.5 padding (matching pre-3.x behavior) and add OAEP→PKCS#1 v1.5 fallback for raw signing operations where OAEP is invalid. All 324 tests pass (+46 new tests covering v1.5 signing and fallback behavior). --- RSA.xs | 37 +++++++++++++++++++++++++++++++------ t/padding.t | 41 ++++++++++++++++++++++++++++++++++++++++- t/private_crypt.t | 19 ++++++++++++++----- 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/RSA.xs b/RSA.xs index fc98463..24af57a 100644 --- a/RSA.xs +++ b/RSA.xs @@ -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; @@ -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."); } @@ -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); @@ -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) { diff --git a/t/padding.t b/t/padding.t index 595c30b..5c8f6e5 100644 --- a/t/padding.t +++ b/t/padding.t @@ -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 { @@ -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 diff --git a/t/private_crypt.t b/t/private_crypt.t index 70444a5..b845489 100644 --- a/t/private_crypt.t +++ b/t/private_crypt.t @@ -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(); @@ -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 ---