From 1fa6d5463e84261744f8538b5024ec115651a69e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sun, 5 Apr 2026 22:21:00 -0600 Subject: [PATCH] fix: extract check_max_message_length() and use get_digest_length() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract plaintext length validation from rsa_crypt() into a dedicated static check_max_message_length() function. Call it only from encrypt() and private_encrypt() — not from decrypt/public_decrypt where the input is already-sized ciphertext. Use get_digest_length(p_rsa->hashMode) for OAEP max-length formula (RFC 8017 §7.1.1: mLen ≤ k − 2·hLen − 2) instead of the hardcoded 42 that assumed SHA-1. Works on all OpenSSL versions without allocation. Remove the now-redundant `public` parameter from rsa_crypt(). Use sv_len() instead of SvCUR() at call sites to ensure the SV is stringified. Use STRLEN for from_length parameter type. Update tests to compute OAEP overhead from actual hash size. Co-Authored-By: Claude --- RSA.xs | 87 +++++++++++++++++++++++++++++------------------------ t/crypto.t | 3 +- t/error.t | 3 +- t/padding.t | 17 ++++++++++- t/rsa.t | 4 ++- 5 files changed, 71 insertions(+), 43 deletions(-) diff --git a/RSA.xs b/RSA.xs index 61b3cb6..61c3b73 100644 --- a/RSA.xs +++ b/RSA.xs @@ -401,56 +401,60 @@ EVP_PKEY* _load_rsa_key(SV* p_keyStringSv, #endif return rsa; } + +/* Pre-validate plaintext length before passing to OpenSSL. + Call this only for encryption operations (encrypt, private_encrypt), + not for decryption where the input is already-sized ciphertext. */ +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) { + int digest_length = get_digest_length(p_rsa->hashMode); + 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 */ + 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 > (size_t)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; 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) { @@ -488,9 +492,12 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from, CHECK_OPEN_SSL(0); crypt_done: #else - CHECK_NEW(to, size, UNSIGNED_CHAR); - to_length = p_crypt( - from_length, from, (unsigned char*) to, p_rsa->rsa, p_rsa->padding); + { + int 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); + } #endif if (to_length < 0) { @@ -1146,10 +1153,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 @@ -1164,9 +1172,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 @@ -1180,10 +1188,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 @@ -1194,9 +1203,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 diff --git a/t/crypto.t b/t/crypto.t index 583031d..45e1329 100644 --- a/t/crypto.t +++ b/t/crypto.t @@ -15,11 +15,12 @@ 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..97b853f 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' => 16, + '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' && $hash =~ /sha/ ) { + $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..d62fd30 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; +_Test_Encrypt_And_Decrypt( $oaep_max, $rsa, 0 ); #FIXME - use_sslv23_padding seems to fail on decryption. openssl bug?