From 41667cc9dde67ce603ea5b2be81751cddaa74a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 21 Mar 2026 00:18:28 -0600 Subject: [PATCH 1/2] feat: add plaintext length pre-validation in rsa_crypt() Replace OpenSSL's cryptic "data too large for key size" error with a clear message that includes the padding type, maximum allowed bytes, and actual input size. Validation runs before any OpenSSL call, giving users actionable feedback. Covers OAEP (size-42), PKCS#1 v1.5 (size-11), and no-padding (size). Only validates encryption-direction calls (encrypt, private_encrypt), not decryption (decrypt, public_decrypt) where input is ciphertext. Adds `public` parameter to pre-3.x rsa_crypt() signature to distinguish encrypt/decrypt direction (already present in 3.x path). 8 new tests in t/crypto.t verify error messages and confirm decrypt is unaffected. Co-Authored-By: Claude Opus 4.6 --- RSA.xs | 34 +++++++++++++++++++++++++++++----- t/crypto.t | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/RSA.xs b/RSA.xs index ed71755..d82fd3b 100644 --- a/RSA.xs +++ b/RSA.xs @@ -343,7 +343,7 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from, #else SV* rsa_crypt(rsaData* p_rsa, SV* p_from, - int (*p_crypt)(int, const unsigned char*, unsigned char*, RSA*, int), int is_encrypt) + int (*p_crypt)(int, const unsigned char*, unsigned char*, RSA*, int), int public, int is_encrypt) #endif { STRLEN from_length; @@ -361,6 +361,30 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from, "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) { @@ -898,7 +922,7 @@ encrypt(p_rsa, 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 */); #else - RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_public_encrypt, 1 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_public_encrypt, 1 /* public */, 1 /* is_encrypt */); #endif OUTPUT: RETVAL @@ -915,7 +939,7 @@ decrypt(p_rsa, p_ciphertext) #if OPENSSL_VERSION_NUMBER >= 0x30000000L RETVAL = rsa_crypt(p_rsa, p_ciphertext, EVP_PKEY_decrypt, EVP_PKEY_decrypt_init, 0 /* private */, 1 /* is_encrypt */); #else - RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_private_decrypt, 1 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_private_decrypt, 0 /* private */, 1 /* is_encrypt */); #endif OUTPUT: RETVAL @@ -932,7 +956,7 @@ private_encrypt(p_rsa, 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 */); #else - RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_private_encrypt, 0 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_plaintext, RSA_private_encrypt, 0 /* private */, 0 /* is_encrypt */); #endif OUTPUT: RETVAL @@ -945,7 +969,7 @@ public_decrypt(p_rsa, p_ciphertext) #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 */); #else - RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_public_decrypt, 0 /* is_encrypt */); + RETVAL = rsa_crypt(p_rsa, p_ciphertext, RSA_public_decrypt, 1 /* public */, 0 /* is_encrypt */); #endif OUTPUT: RETVAL diff --git a/t/crypto.t b/t/crypto.t index ace1bfa..583031d 100644 --- a/t/crypto.t +++ b/t/crypto.t @@ -7,7 +7,7 @@ use Crypt::OpenSSL::RSA; # Tests for encrypt/decrypt error paths, boundary conditions, and edge cases. # These cover gaps not addressed by rsa.t or padding.t. -plan tests => 12; +plan tests => 20; Crypt::OpenSSL::Random::random_seed("OpenSSL needs at least 32 bytes."); Crypt::OpenSSL::RSA->import_random_seed(); @@ -119,3 +119,50 @@ $rsa->use_pkcs1_oaep_padding(); eval { $rsa->encrypt("test") }; ok($@, "PSS padding cannot be used for encryption"); } + +# --- Plaintext length pre-validation error messages --- + +# OAEP: clear message with byte counts +{ + $rsa->use_pkcs1_oaep_padding(); + my $too_long = "x" x ($oaep_max + 1); + eval { $rsa->encrypt($too_long) }; + like($@, qr/plaintext too long for key size with OAEP padding/, + "OAEP oversized plaintext gives clear error message"); + like($@, qr/\Q$oaep_max bytes max\E/, + "OAEP error includes max byte count"); + my $got = $oaep_max + 1; + like($@, qr/got $got/, + "OAEP error includes actual byte count"); +} + +# PKCS#1 v1.5 via private_encrypt: clear message +{ + $rsa->use_pkcs1_padding(); + my $pkcs1_max = $key_size - 11; + my $too_long = "x" x ($pkcs1_max + 1); + eval { $rsa->private_encrypt($too_long) }; + like($@, qr/plaintext too long for key size with PKCS#1 v1\.5 padding/, + "PKCS#1 v1.5 oversized plaintext gives clear error message"); + like($@, qr/\Q$pkcs1_max bytes max\E/, + "PKCS#1 v1.5 error includes max byte count"); +} + +# no-padding: clear message +{ + $rsa->use_no_padding(); + eval { $rsa->encrypt("x" x ($key_size + 1)) }; + like($@, qr/plaintext too long for key size with no padding/, + "no-padding oversized plaintext gives clear error message"); +} + +# Decrypt still works (no false positive from validation) +{ + $rsa->use_pkcs1_oaep_padding(); + my $ct = $rsa->encrypt("validation bypass test"); + my $pt = eval { $rsa->decrypt($ct) }; + ok(!$@, "decrypt not affected by plaintext length validation") + or diag $@; + is($pt, "validation bypass test", + "decrypt returns correct plaintext after validation change"); +} From 658b51e4bcf28803e9ebfb068c38a542da8e3d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 21 Mar 2026 07:21:36 -0600 Subject: [PATCH 2/2] rebase: apply review feedback on #135 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Summary:** - Updated `t/error_queue.t` test 4 regex to also match `plaintext too long` — the new pre-validation error message now fires before OpenSSL's native "data too large" error, so the test pattern needs to accept both forms. This fixes the CI failure reported by @atoomic. --- t/error_queue.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/error_queue.t b/t/error_queue.t index 6cc4ce2..882498f 100644 --- a/t/error_queue.t +++ b/t/error_queue.t @@ -28,5 +28,5 @@ like($second_error, qr/OpenSSL error: \S/, "second error has a meaningful OpenSS # Trigger yet another failure after two eval-caught ones — error queue should be clean eval { $rsa->encrypt("A" x 500) }; my $third_error = $@; -like($third_error, qr/too large|data greater|asym cipher failure/i, +like($third_error, qr/too large|data greater|asym cipher failure|plaintext too long/i, "third error reports actual problem (data too large), not stale from earlier failures");