From 5af06e7f3b3e568078cdc6c6c5cf7ef345ee398a Mon Sep 17 00:00:00 2001 From: Timothy Legge Date: Sun, 5 Apr 2026 22:54:00 -0300 Subject: [PATCH 1/3] rsa_crypt no longer needs public/private arguement move the max message length check and call it from the correct functions to simplify rsa_crypt --- RSA.xs | 75 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/RSA.xs b/RSA.xs index e2ed373..6035879 100644 --- a/RSA.xs +++ b/RSA.xs @@ -389,56 +389,56 @@ EVP_PKEY* _load_rsa_key(SV* p_keyStringSv, CHECK_OPEN_SSL(rsa); return rsa; } + +static void check_max_message_length(rsaData* p_rsa, STRLEN from_length) { + int size; + int max_len = -1; + const char *pad_name = NULL; + + size = EVP_PKEY_get_size(p_rsa->rsa); + + if (p_rsa->padding == RSA_PKCS1_OAEP_PADDING) { + max_len = size - 42; /* 2 * SHA1_DIGEST_LENGTH + 2 */ + pad_name = "OAEP"; + } else if (p_rsa->padding == RSA_PKCS1_PADDING) { + max_len = size - 11; /* PKCS#1 v1.5 overhead */ + pad_name = "PKCS#1 v1.5"; + } else if (p_rsa->padding == RSA_NO_PADDING) { + max_len = size; + pad_name = "no"; + } + if (max_len >= 0 && from_length > (STRLEN) max_len) { + croak("plaintext too long for key size with %s padding" + " (%d bytes max, got %d)", pad_name, max_len, (int)from_length); + } +} #if OPENSSL_VERSION_NUMBER >= 0x30000000L SV* rsa_crypt(rsaData* p_rsa, SV* p_from, int (*p_crypt)(EVP_PKEY_CTX*, unsigned char*, size_t*, const unsigned char*, size_t), - int (*init_crypt)(EVP_PKEY_CTX*), int public, int is_encrypt) + int (*init_crypt)(EVP_PKEY_CTX*), int is_encrypt) #else SV* rsa_crypt(rsaData* p_rsa, SV* p_from, - int (*p_crypt)(int, const unsigned char*, unsigned char*, RSA*, int), int public, int is_encrypt) + int (*p_crypt)(int, const unsigned char*, unsigned char*, RSA*, int), int is_encrypt) #endif { STRLEN from_length; SIZE_T_INT to_length; - int size; unsigned char* from; UNSIGNED_CHAR *to = NULL; SV* sv; +#if OPENSSL_VERSION_NUMBER < 0x30000000L + int size; +#endif from = (unsigned char*) SvPV(p_from, from_length); - size = EVP_PKEY_get_size(p_rsa->rsa); if(is_encrypt && p_rsa->padding == RSA_PKCS1_PADDING) { croak("PKCS#1 v1.5 padding for encryption is vulnerable to the Marvin attack. " "Use use_pkcs1_oaep_padding() for encryption, or use_pkcs1_padding() with sign()/verify()."); } - /* Pre-validate plaintext length before calling OpenSSL. - Only applies to encryption direction (encrypt, private_encrypt), - not to decryption (decrypt, public_decrypt) where input is ciphertext. */ - if (public == is_encrypt) { - int max_len = -1; - const char *pad_name = NULL; - - if (p_rsa->padding == RSA_PKCS1_OAEP_PADDING) { - max_len = size - 42; /* 2 * SHA1_DIGEST_LENGTH + 2 */ - pad_name = "OAEP"; - } else if (p_rsa->padding == RSA_PKCS1_PADDING) { - max_len = size - 11; /* PKCS#1 v1.5 overhead */ - pad_name = "PKCS#1 v1.5"; - } else if (p_rsa->padding == RSA_NO_PADDING) { - max_len = size; - pad_name = "no"; - } - - if (max_len >= 0 && (int)from_length > max_len) { - croak("plaintext too long for key size with %s padding" - " (%d bytes max, got %d)", pad_name, max_len, (int)from_length); - } - } - #if OPENSSL_VERSION_NUMBER >= 0x30000000L if(p_rsa->padding == RSA_PKCS1_PSS_PADDING) { @@ -476,6 +476,7 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from, CHECK_OPEN_SSL(0); crypt_done: #else + size = EVP_PKEY_get_size(p_rsa->rsa); CHECK_NEW(to, size, UNSIGNED_CHAR); to_length = p_crypt( from_length, from, (unsigned char*) to, p_rsa->rsa, p_rsa->padding); @@ -1134,10 +1135,11 @@ encrypt(p_rsa, p_plaintext) rsaData* p_rsa; SV* p_plaintext; CODE: + check_max_message_length(p_rsa, sv_len(p_plaintext)); #if OPENSSL_VERSION_NUMBER >= 0x30000000L - RETVAL = rsa_crypt(p_rsa, p_plaintext, EVP_PKEY_encrypt, EVP_PKEY_encrypt_init, 1 /* public */, 1 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_plaintext, EVP_PKEY_encrypt, EVP_PKEY_encrypt_init, 1 /* is_encrypt */); #else - RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_public_encrypt, 1 /* public */, 1 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_public_encrypt, 1 /* is_encrypt */); #endif OUTPUT: RETVAL @@ -1152,9 +1154,9 @@ decrypt(p_rsa, p_ciphertext) croak("Public keys cannot decrypt"); } #if OPENSSL_VERSION_NUMBER >= 0x30000000L - RETVAL = rsa_crypt(p_rsa, p_ciphertext, EVP_PKEY_decrypt, EVP_PKEY_decrypt_init, 0 /* private */, 1 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_ciphertext, EVP_PKEY_decrypt, EVP_PKEY_decrypt_init, 1 /* is_encrypt */); #else - RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_private_decrypt, 0 /* private */, 1 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_private_decrypt, 1 /* is_encrypt */); #endif OUTPUT: RETVAL @@ -1168,10 +1170,11 @@ private_encrypt(p_rsa, p_plaintext) { croak("Public keys cannot private_encrypt"); } + check_max_message_length(p_rsa, sv_len(p_plaintext)); #if OPENSSL_VERSION_NUMBER >= 0x30000000L - RETVAL = rsa_crypt(p_rsa, p_plaintext, EVP_PKEY_sign, EVP_PKEY_sign_init, 0 /* private */, 0 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_plaintext, EVP_PKEY_sign, EVP_PKEY_sign_init, 0 /* is_encrypt */); #else - RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_private_encrypt, 0 /* private */, 0 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_private_encrypt, 0 /* is_encrypt */); #endif OUTPUT: RETVAL @@ -1182,9 +1185,9 @@ public_decrypt(p_rsa, p_ciphertext) SV* p_ciphertext; CODE: #if OPENSSL_VERSION_NUMBER >= 0x30000000L - RETVAL = rsa_crypt(p_rsa, p_ciphertext, EVP_PKEY_verify_recover, EVP_PKEY_verify_recover_init, 1 /*public */, 0 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_ciphertext, EVP_PKEY_verify_recover, EVP_PKEY_verify_recover_init, 0 /* is_encrypt */); #else - RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_public_decrypt, 1 /* public */, 0 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_public_decrypt, 0 /* is_encrypt */); #endif OUTPUT: RETVAL From 4384a5510c0de08ba14864c4cdfdef2e5034cab7 Mon Sep 17 00:00:00 2001 From: Timothy Legge Date: Mon, 6 Apr 2026 00:47:17 -0300 Subject: [PATCH 2/3] Also fix the overhead calculaation for OAEP that was hardcoded for SHA1 --- RSA.xs | 4 +++- t/crypto.t | 4 ++-- t/error.t | 3 ++- t/padding.t | 17 ++++++++++++++++- t/rsa.t | 4 +++- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/RSA.xs b/RSA.xs index 6035879..5d5f410 100644 --- a/RSA.xs +++ b/RSA.xs @@ -398,7 +398,9 @@ static void check_max_message_length(rsaData* p_rsa, STRLEN from_length) { size = EVP_PKEY_get_size(p_rsa->rsa); if (p_rsa->padding == RSA_PKCS1_OAEP_PADDING) { - max_len = size - 42; /* 2 * SHA1_DIGEST_LENGTH + 2 */ + int digest_length = 0; + digest_length = get_digest_length(p_rsa->hashMode); /* croak()s on unknown NID */ + max_len = size - (2 * digest_length) - 2; /* OAEP overhead: 2*hLen + 2 */ pad_name = "OAEP"; } else if (p_rsa->padding == RSA_PKCS1_PADDING) { max_len = size - 11; /* PKCS#1 v1.5 overhead */ diff --git a/t/crypto.t b/t/crypto.t index 583031d..05d48de 100644 --- a/t/crypto.t +++ b/t/crypto.t @@ -15,11 +15,11 @@ Crypt::OpenSSL::RSA->import_random_seed(); my $rsa = Crypt::OpenSSL::RSA->generate_key(2048); my $rsa2 = Crypt::OpenSSL::RSA->generate_key(2048); my $key_size = $rsa->size(); # 256 bytes for 2048-bit key - +my $hash_size = 32; # SHA-256 (default hash mode, 32 bytes) # --- OAEP boundary tests --- $rsa->use_pkcs1_oaep_padding(); -my $oaep_max = $key_size - 42; # SHA-1 OAEP overhead +my $oaep_max = $key_size - (2 * $hash_size) - 2; # OAEP overhead: 2*hLen + 2 # Max-length plaintext that fits OAEP { diff --git a/t/error.t b/t/error.t index 2e1e33a..a12f66c 100644 --- a/t/error.t +++ b/t/error.t @@ -86,7 +86,8 @@ ok($@, "decrypt croaks on empty ciphertext"); # --- Plaintext too large for padding mode --- $rsa->use_pkcs1_oaep_padding(); -my $max_oaep = $rsa->size() - 42; +my $hash_size = 32; # SHA-256 (default hash mode, 32 bytes) +my $max_oaep = $rsa->size() - (2 * $hash_size) - 2; # OAEP overhead: 2*hLen + 2 my $too_large = "x" x ($max_oaep + 1); eval { $rsa->encrypt($too_large) }; ok($@, "encrypt croaks when plaintext exceeds OAEP max size"); diff --git a/t/padding.t b/t/padding.t index 595c30b..65d1e2d 100644 --- a/t/padding.t +++ b/t/padding.t @@ -117,6 +117,16 @@ my %padding_methods = ( #'sslv23' => {'sign' => 0, 'encrypt' => 0, 'pad' => 11}, ); +# OAEP overhead is 2*hLen+2 (RFC 8017 §7.1.1); compute per-hash for SHA variants +my %digest_lengths = ( + 'md5' => 20, # OpenSSL seems to require a minimum of 20 + 'sha1' => 20, + 'sha224' => 28, + 'sha256' => 32, + 'sha384' => 48, + 'sha512' => 64, + 'ripemd160' => 20, +); foreach my $padding (keys %padding_methods) { diag $padding; @@ -157,7 +167,12 @@ foreach my $padding (keys %padding_methods) { # Valid encryption methods with padding if ($encrypt) { - _Test_Encrypt_And_Decrypt( $rsa->size() - $pad, $rsa, 0, $padding, $hash ); + my $size = $rsa->size(); + my $max_len = $size - $pad; + if ( $padding eq 'pkcs1_oaep' ) { + $max_len = $size - ($digest_lengths{$hash} * 2) - 2; + } + _Test_Encrypt_And_Decrypt( $max_len, $rsa, 0, $padding, $hash ); } } diff --git a/t/rsa.t b/t/rsa.t index 5436caf..e609c97 100644 --- a/t/rsa.t +++ b/t/rsa.t @@ -80,7 +80,9 @@ _Test_Encrypt_And_Decrypt( $rsa->size(), $rsa, 1 ); $rsa->use_pkcs1_oaep_padding(); # private_encrypt does not work with pkcs1_oaep_padding -_Test_Encrypt_And_Decrypt( $rsa->size() - 42, $rsa, 0 ); +my $hash_size = 32; # SHA-256 (default hash mode, 32 bytes) +my $oaep_max = $rsa->size() - (2 * $hash_size) - 2; # OAEP overhead: 2*hLen + 2 +_Test_Encrypt_And_Decrypt( $oaep_max, $rsa, 0 ); #FIXME - use_sslv23_padding seems to fail on decryption. openssl bug? From dcd159395c4a17c3ff7dccd7de93e324e98054b0 Mon Sep 17 00:00:00 2001 From: Timothy Legge Date: Mon, 6 Apr 2026 11:42:15 -0300 Subject: [PATCH 3/3] Reverse e8f21ba but keep in in hisstory for if we implement alternate digests for OAEP --- RSA.xs | 4 +--- t/crypto.t | 4 ++-- t/error.t | 3 +-- t/padding.t | 17 +---------------- t/rsa.t | 4 +--- 5 files changed, 6 insertions(+), 26 deletions(-) diff --git a/RSA.xs b/RSA.xs index 5d5f410..6035879 100644 --- a/RSA.xs +++ b/RSA.xs @@ -398,9 +398,7 @@ static void check_max_message_length(rsaData* p_rsa, STRLEN from_length) { size = EVP_PKEY_get_size(p_rsa->rsa); if (p_rsa->padding == RSA_PKCS1_OAEP_PADDING) { - int digest_length = 0; - digest_length = get_digest_length(p_rsa->hashMode); /* croak()s on unknown NID */ - max_len = size - (2 * digest_length) - 2; /* OAEP overhead: 2*hLen + 2 */ + max_len = size - 42; /* 2 * SHA1_DIGEST_LENGTH + 2 */ pad_name = "OAEP"; } else if (p_rsa->padding == RSA_PKCS1_PADDING) { max_len = size - 11; /* PKCS#1 v1.5 overhead */ diff --git a/t/crypto.t b/t/crypto.t index 05d48de..583031d 100644 --- a/t/crypto.t +++ b/t/crypto.t @@ -15,11 +15,11 @@ Crypt::OpenSSL::RSA->import_random_seed(); my $rsa = Crypt::OpenSSL::RSA->generate_key(2048); my $rsa2 = Crypt::OpenSSL::RSA->generate_key(2048); my $key_size = $rsa->size(); # 256 bytes for 2048-bit key -my $hash_size = 32; # SHA-256 (default hash mode, 32 bytes) + # --- OAEP boundary tests --- $rsa->use_pkcs1_oaep_padding(); -my $oaep_max = $key_size - (2 * $hash_size) - 2; # OAEP overhead: 2*hLen + 2 +my $oaep_max = $key_size - 42; # SHA-1 OAEP overhead # Max-length plaintext that fits OAEP { diff --git a/t/error.t b/t/error.t index a12f66c..2e1e33a 100644 --- a/t/error.t +++ b/t/error.t @@ -86,8 +86,7 @@ ok($@, "decrypt croaks on empty ciphertext"); # --- Plaintext too large for padding mode --- $rsa->use_pkcs1_oaep_padding(); -my $hash_size = 32; # SHA-256 (default hash mode, 32 bytes) -my $max_oaep = $rsa->size() - (2 * $hash_size) - 2; # OAEP overhead: 2*hLen + 2 +my $max_oaep = $rsa->size() - 42; my $too_large = "x" x ($max_oaep + 1); eval { $rsa->encrypt($too_large) }; ok($@, "encrypt croaks when plaintext exceeds OAEP max size"); diff --git a/t/padding.t b/t/padding.t index 65d1e2d..595c30b 100644 --- a/t/padding.t +++ b/t/padding.t @@ -117,16 +117,6 @@ my %padding_methods = ( #'sslv23' => {'sign' => 0, 'encrypt' => 0, 'pad' => 11}, ); -# OAEP overhead is 2*hLen+2 (RFC 8017 §7.1.1); compute per-hash for SHA variants -my %digest_lengths = ( - 'md5' => 20, # OpenSSL seems to require a minimum of 20 - 'sha1' => 20, - 'sha224' => 28, - 'sha256' => 32, - 'sha384' => 48, - 'sha512' => 64, - 'ripemd160' => 20, -); foreach my $padding (keys %padding_methods) { diag $padding; @@ -167,12 +157,7 @@ foreach my $padding (keys %padding_methods) { # Valid encryption methods with padding if ($encrypt) { - my $size = $rsa->size(); - my $max_len = $size - $pad; - if ( $padding eq 'pkcs1_oaep' ) { - $max_len = $size - ($digest_lengths{$hash} * 2) - 2; - } - _Test_Encrypt_And_Decrypt( $max_len, $rsa, 0, $padding, $hash ); + _Test_Encrypt_And_Decrypt( $rsa->size() - $pad, $rsa, 0, $padding, $hash ); } } diff --git a/t/rsa.t b/t/rsa.t index e609c97..5436caf 100644 --- a/t/rsa.t +++ b/t/rsa.t @@ -80,9 +80,7 @@ _Test_Encrypt_And_Decrypt( $rsa->size(), $rsa, 1 ); $rsa->use_pkcs1_oaep_padding(); # private_encrypt does not work with pkcs1_oaep_padding -my $hash_size = 32; # SHA-256 (default hash mode, 32 bytes) -my $oaep_max = $rsa->size() - (2 * $hash_size) - 2; # OAEP overhead: 2*hLen + 2 -_Test_Encrypt_And_Decrypt( $oaep_max, $rsa, 0 ); +_Test_Encrypt_And_Decrypt( $rsa->size() - 42, $rsa, 0 ); #FIXME - use_sslv23_padding seems to fail on decryption. openssl bug?