From 1db38bd16a7ca8fd2edb39bba9d6199afab6b14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Thu, 19 Mar 2026 00:17:24 -0600 Subject: [PATCH 1/8] feat: support DER-encoded public keys in new_public_key() new_public_key() now accepts DER-encoded public keys (raw binary ASN.1) in both PKCS#1 (RSAPublicKey) and X.509 (SubjectPublicKeyInfo) formats, auto-detecting which format is provided. Previously, passing a DER key produced an unhelpful "unrecognized key format" error. Now DER keys are loaded directly, and error messages for other invalid inputs are more specific about what formats are expected. XS: add _new_public_key_x509_der (d2i_PUBKEY_bio on 3.x, d2i_RSA_PUBKEY_bio on pre-3.x) and _new_public_key_pkcs1_der (OSSL_DECODER on 3.x, d2i_RSAPublicKey_bio on pre-3.x). Co-Authored-By: Claude Opus 4.6 --- RSA.pm | 27 ++++++++++++++--- RSA.xs | 63 +++++++++++++++++++++++++++++++++++++++ t/der.t | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 t/der.t diff --git a/RSA.pm b/RSA.pm index ed0e00f..1cb5d31 100644 --- a/RSA.pm +++ b/RSA.pm @@ -25,8 +25,21 @@ sub new_public_key { elsif ( $p_key_string =~ /^-----BEGIN PUBLIC KEY-----/ ) { return $proto->_new_public_key_x509($p_key_string); } + elsif ( $p_key_string =~ /^-----/ ) { + croak "unrecognized key format: PEM header not recognized as RSA public key. " + . "Expected '-----BEGIN RSA PUBLIC KEY-----' (PKCS#1) or " + . "'-----BEGIN PUBLIC KEY-----' (X.509)"; + } + elsif ( length($p_key_string) > 0 && substr($p_key_string, 0, 1) eq "\x30" ) { + # ASN.1 SEQUENCE tag detected — likely DER-encoded key. + # Try X.509 SubjectPublicKeyInfo first (most common), then PKCS#1 RSAPublicKey. + my $rsa = eval { $proto->_new_public_key_x509_der($p_key_string) }; + return $rsa if $rsa; + return $proto->_new_public_key_pkcs1_der($p_key_string); + } else { - croak "unrecognized key format"; + croak "unrecognized key format: expected PEM-encoded key (starting with '-----BEGIN') " + . "or DER-encoded key (binary ASN.1 data)"; } } @@ -125,9 +138,15 @@ this (never documented) behavior is no longer the case. =item new_public_key Create a new C object by loading a public key in -from a string containing Base64/DER-encoding of either the PKCS1 or -X.509 representation of the key. The string should include the -C<-----BEGIN...-----> and C<-----END...-----> lines. +from a string containing either PEM or DER encoding of the PKCS#1 or +X.509 representation of the key. + +For PEM keys, the string should include the C<-----BEGIN...-----> and +C<-----END...-----> lines. Both C (PKCS#1) and +C (X.509/SubjectPublicKeyInfo) formats are supported. + +DER-encoded keys (raw binary ASN.1) are also accepted and the format +(PKCS#1 vs X.509) is auto-detected. The padding is set to PKCS1_OAEP, but can be changed with the C methods. diff --git a/RSA.xs b/RSA.xs index 10b4a4d..4b89840 100644 --- a/RSA.xs +++ b/RSA.xs @@ -25,6 +25,7 @@ #include #include #include +#include #endif #if OPENSSL_VERSION_NUMBER >= 0x30000000L @@ -506,6 +507,68 @@ _new_public_key_x509(proto, key_string_SV) OUTPUT: RETVAL +SV* +_new_public_key_x509_der(proto, key_string_SV) + SV* proto; + SV* key_string_SV; + PREINIT: + STRLEN keyStringLength; + char* keyString; + EVP_PKEY* pkey; + BIO* bio; + CODE: + keyString = SvPV(key_string_SV, keyStringLength); + CHECK_OPEN_SSL(bio = BIO_new_mem_buf(keyString, keyStringLength)); +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + pkey = d2i_PUBKEY_bio(bio, NULL); +#else + pkey = d2i_RSA_PUBKEY_bio(bio, NULL); +#endif + BIO_free(bio); + CHECK_OPEN_SSL(pkey); + RETVAL = make_rsa_obj(proto, pkey); + OUTPUT: + RETVAL + +SV* +_new_public_key_pkcs1_der(proto, key_string_SV) + SV* proto; + SV* key_string_SV; + PREINIT: + STRLEN keyStringLength; + char* keyString; + EVP_PKEY* pkey; + BIO* bio; +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + OSSL_DECODER_CTX* dctx; +#endif + CODE: + keyString = SvPV(key_string_SV, keyStringLength); + CHECK_OPEN_SSL(bio = BIO_new_mem_buf(keyString, keyStringLength)); +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + pkey = NULL; + dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", "type-specific", + "RSA", OSSL_KEYMGMT_SELECT_PUBLIC_KEY, + NULL, NULL); + if (!dctx) { + BIO_free(bio); + croakSsl(__FILE__, __LINE__); + } + if (!OSSL_DECODER_from_bio(dctx, bio)) { + OSSL_DECODER_CTX_free(dctx); + BIO_free(bio); + croakSsl(__FILE__, __LINE__); + } + OSSL_DECODER_CTX_free(dctx); +#else + pkey = d2i_RSAPublicKey_bio(bio, NULL); +#endif + BIO_free(bio); + CHECK_OPEN_SSL(pkey); + RETVAL = make_rsa_obj(proto, pkey); + OUTPUT: + RETVAL + void DESTROY(p_rsa) rsaData* p_rsa; diff --git a/t/der.t b/t/der.t new file mode 100644 index 0000000..b0ccd33 --- /dev/null +++ b/t/der.t @@ -0,0 +1,92 @@ +use strict; +use warnings; +use Test::More; +use MIME::Base64; +use Crypt::OpenSSL::RSA; + +BEGIN { plan tests => 14 } + +# --- Generate a key pair for testing --- + +my $rsa = Crypt::OpenSSL::RSA->generate_key(2048); + +# --- Extract PEM public keys --- + +my $pkcs1_pem = $rsa->get_public_key_string(); # PKCS#1 (BEGIN RSA PUBLIC KEY) +my $x509_pem = $rsa->get_public_key_x509_string(); # X.509 (BEGIN PUBLIC KEY) + +# --- Convert PEM to DER by stripping headers and base64-decoding --- + +sub pem_to_der { + my ($pem) = @_; + $pem =~ s/-----BEGIN [^-]+-----//; + $pem =~ s/-----END [^-]+-----//; + $pem =~ s/\s+//g; + return decode_base64($pem); +} + +my $pkcs1_der = pem_to_der($pkcs1_pem); +my $x509_der = pem_to_der($x509_pem); + +# Sanity check: DER data starts with ASN.1 SEQUENCE tag +is( ord(substr($pkcs1_der, 0, 1)), 0x30, "PKCS#1 DER starts with SEQUENCE tag" ); +is( ord(substr($x509_der, 0, 1)), 0x30, "X.509 DER starts with SEQUENCE tag" ); + +# --- Load DER keys via new_public_key --- + +my ($pub_from_x509_der, $pub_from_pkcs1_der); + +ok( $pub_from_x509_der = Crypt::OpenSSL::RSA->new_public_key($x509_der), + "new_public_key loads X.509 DER key" ); + +ok( $pub_from_pkcs1_der = Crypt::OpenSSL::RSA->new_public_key($pkcs1_der), + "new_public_key loads PKCS#1 DER key" ); + +# --- Verify round-trip: DER-loaded keys produce the same PEM output --- + +is( $pub_from_x509_der->get_public_key_x509_string(), $x509_pem, + "X.509 DER key exports to same X.509 PEM" ); + +is( $pub_from_x509_der->get_public_key_string(), $pkcs1_pem, + "X.509 DER key exports to same PKCS#1 PEM" ); + +is( $pub_from_pkcs1_der->get_public_key_x509_string(), $x509_pem, + "PKCS#1 DER key exports to same X.509 PEM" ); + +is( $pub_from_pkcs1_der->get_public_key_string(), $pkcs1_pem, + "PKCS#1 DER key exports to same PKCS#1 PEM" ); + +# --- Verify DER-loaded keys can actually verify signatures --- + +$rsa->use_sha256_hash(); +my $plaintext = "Hello, DER world!"; +my $sig = $rsa->sign($plaintext); + +$pub_from_x509_der->use_sha256_hash(); +ok( $pub_from_x509_der->verify($plaintext, $sig), + "X.509 DER-loaded key verifies signature" ); + +$pub_from_pkcs1_der->use_sha256_hash(); +ok( $pub_from_pkcs1_der->verify($plaintext, $sig), + "PKCS#1 DER-loaded key verifies signature" ); + +# --- Error cases --- + +# DER-like data that isn't a valid key +eval { Crypt::OpenSSL::RSA->new_public_key("\x30\x00") }; +ok( $@, "new_public_key croaks on truncated DER data" ); + +# Completely bogus binary data (not starting with 0x30) +eval { Crypt::OpenSSL::RSA->new_public_key("\x01\x02\x03\x04") }; +like( $@, qr/unrecognized key format/, + "new_public_key gives helpful error on random binary data" ); + +# Empty string +eval { Crypt::OpenSSL::RSA->new_public_key("") }; +like( $@, qr/unrecognized key format/, + "new_public_key gives helpful error on empty string" ); + +# PEM header for wrong type +eval { Crypt::OpenSSL::RSA->new_public_key("-----BEGIN CERTIFICATE-----\nfoo\n-----END CERTIFICATE-----\n") }; +like( $@, qr/unrecognized key format/, + "new_public_key gives helpful error on certificate PEM" ); From 6f038a159ba27b03fe2fa7b63c320afeb0fbb9fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 21 Mar 2026 00:27:39 -0600 Subject: [PATCH 2/8] rebase: apply review feedback on #126 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **Added DER-encoded private key support in `new_private_key()`** per @timlegge's request to support importing private keys from DER format, matching the existing DER support added for public keys. - **RSA.xs**: Renamed XS `new_private_key` to `_new_private_key_pem` (internal), added `_new_private_key_der` XS function using `OSSL_DECODER` on OpenSSL 3.x and `d2i_PrivateKey_bio` on pre-3.x. - **RSA.pm**: Added Perl-level `new_private_key` wrapper that detects PEM (starts with `-----`) vs DER (ASN.1 SEQUENCE tag `\x30`) and routes to the appropriate XS function. Passphrase parameter is passed through for PEM keys. - **RSA.pm POD**: Updated `new_private_key` documentation to describe DER support and clarify passphrase is PEM-only. - **t/der.t**: Added 8 new tests (14 → 22 total): DER private key loading, round-trip verification, signing with DER-loaded private key, error cases for invalid DER private key data, and regression test that PEM private keys still work through the new wrapper. --- RSA.pm | 34 +++++++++++++++++++++++++++------- RSA.xs | 41 ++++++++++++++++++++++++++++++++++++++++- t/der.t | 39 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/RSA.pm b/RSA.pm index 1cb5d31..53939cb 100644 --- a/RSA.pm +++ b/RSA.pm @@ -43,6 +43,21 @@ sub new_public_key { } } +sub new_private_key { + my ( $proto, $p_key_string, @rest ) = @_; + if ( $p_key_string =~ /^-----/ ) { + return $proto->_new_private_key_pem($p_key_string, @rest); + } + elsif ( length($p_key_string) > 0 && substr($p_key_string, 0, 1) eq "\x30" ) { + # ASN.1 SEQUENCE tag detected — likely DER-encoded private key. + return $proto->_new_private_key_der($p_key_string); + } + else { + croak "unrecognized key format: expected PEM-encoded key (starting with '-----BEGIN') " + . "or DER-encoded key (binary ASN.1 data)"; + } +} + sub new_key_from_parameters { my ( $proto, $n, $e, $d, $p, $q, %opts ) = @_; my $rsa = $proto->_new_key_from_parameters( map { $_ ? $_->pointer_copy() : 0 } $n, $e, $d, $p, $q ); @@ -157,18 +172,23 @@ C or C prior to signing operations. =item new_private_key Create a new C object by loading a private key in -from an string containing the Base64/DER encoding of the PKCS1 -representation of the key. The string should include the -C<-----BEGIN...-----> and C<-----END...-----> lines. The padding is set to -PKCS1_OAEP, but can be changed with C. +from a string containing either PEM or DER encoding of the key. + +For PEM keys, the string should include the C<-----BEGIN...-----> and +C<-----END...-----> lines. The padding is set to PKCS1_OAEP, but can +be changed with C. -An optional parameter can be passed for passphase protected private key: +DER-encoded keys (raw binary ASN.1) are also accepted. + +An optional parameter can be passed for passphrase-protected PEM private +keys: =over -=item passphase +=item passphrase -The passphase which protects the private key. +The passphrase which protects the private key. Note: passphrase +protection is only supported for PEM-encoded keys. =back diff --git a/RSA.xs b/RSA.xs index 4b89840..05389d0 100644 --- a/RSA.xs +++ b/RSA.xs @@ -477,7 +477,7 @@ BOOT: #endif SV* -new_private_key(proto, key_string_SV, passphase_SV=&PL_sv_undef) +_new_private_key_pem(proto, key_string_SV, passphase_SV=&PL_sv_undef) SV* proto; SV* key_string_SV; SV* passphase_SV; @@ -569,6 +569,45 @@ _new_public_key_pkcs1_der(proto, key_string_SV) OUTPUT: RETVAL +SV* +_new_private_key_der(proto, key_string_SV) + SV* proto; + SV* key_string_SV; + PREINIT: + STRLEN keyStringLength; + char* keyString; + EVP_PKEY* pkey; + BIO* bio; +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + OSSL_DECODER_CTX* dctx; +#endif + CODE: + keyString = SvPV(key_string_SV, keyStringLength); + CHECK_OPEN_SSL(bio = BIO_new_mem_buf(keyString, keyStringLength)); +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + pkey = NULL; + dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", NULL, + "RSA", OSSL_KEYMGMT_SELECT_ALL, + NULL, NULL); + if (!dctx) { + BIO_free(bio); + croakSsl(__FILE__, __LINE__); + } + if (!OSSL_DECODER_from_bio(dctx, bio)) { + OSSL_DECODER_CTX_free(dctx); + BIO_free(bio); + croakSsl(__FILE__, __LINE__); + } + OSSL_DECODER_CTX_free(dctx); +#else + pkey = d2i_PrivateKey_bio(bio, NULL); +#endif + BIO_free(bio); + CHECK_OPEN_SSL(pkey); + RETVAL = make_rsa_obj(proto, pkey); + OUTPUT: + RETVAL + void DESTROY(p_rsa) rsaData* p_rsa; diff --git a/t/der.t b/t/der.t index b0ccd33..1841954 100644 --- a/t/der.t +++ b/t/der.t @@ -4,7 +4,7 @@ use Test::More; use MIME::Base64; use Crypt::OpenSSL::RSA; -BEGIN { plan tests => 14 } +BEGIN { plan tests => 22 } # --- Generate a key pair for testing --- @@ -70,6 +70,43 @@ $pub_from_pkcs1_der->use_sha256_hash(); ok( $pub_from_pkcs1_der->verify($plaintext, $sig), "PKCS#1 DER-loaded key verifies signature" ); +# --- Private key DER support --- + +my $priv_pem = $rsa->get_private_key_string(); +my $priv_der = pem_to_der($priv_pem); + +is( ord(substr($priv_der, 0, 1)), 0x30, "Private key DER starts with SEQUENCE tag" ); + +my $priv_from_der; +ok( $priv_from_der = Crypt::OpenSSL::RSA->new_private_key($priv_der), + "new_private_key loads DER-encoded private key" ); + +ok( $priv_from_der->is_private(), + "DER-loaded private key is recognized as private" ); + +is( $priv_from_der->get_public_key_x509_string(), $x509_pem, + "DER-loaded private key exports same public key" ); + +# Verify DER-loaded private key can sign and original public key can verify +$priv_from_der->use_sha256_hash(); +my $sig2 = $priv_from_der->sign($plaintext); +ok( $pub_from_x509_der->verify($plaintext, $sig2), + "signature from DER-loaded private key verifies" ); + +# Error: DER-like data for private key +eval { Crypt::OpenSSL::RSA->new_private_key("\x30\x00") }; +ok( $@, "new_private_key croaks on truncated DER data" ); + +# Error: bogus binary data for private key +eval { Crypt::OpenSSL::RSA->new_private_key("\x01\x02\x03\x04") }; +like( $@, qr/unrecognized key format/, + "new_private_key gives helpful error on random binary data" ); + +# PEM private keys still work through the wrapper +my $priv_from_pem; +ok( $priv_from_pem = Crypt::OpenSSL::RSA->new_private_key($priv_pem), + "new_private_key still loads PEM-encoded private key" ); + # --- Error cases --- # DER-like data that isn't a valid key From 714c89e177598169d32bad3aaa92dfaf897630dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 21 Mar 2026 00:34:53 -0600 Subject: [PATCH 3/8] fix: resolve CI failures on #126 (attempt 1) --- RSA.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RSA.xs b/RSA.xs index 05389d0..306bb9b 100644 --- a/RSA.xs +++ b/RSA.xs @@ -600,7 +600,7 @@ _new_private_key_der(proto, key_string_SV) } OSSL_DECODER_CTX_free(dctx); #else - pkey = d2i_PrivateKey_bio(bio, NULL); + pkey = d2i_RSAPrivateKey_bio(bio, NULL); #endif BIO_free(bio); CHECK_OPEN_SSL(pkey); From 205a3f6001850321a4e2cc8f6909a964d4280bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Fri, 3 Apr 2026 17:44:32 -0600 Subject: [PATCH 4/8] rebase: apply review feedback on #126 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Good — the test already expects `new_private_key(undef)` to croak. With the guard added, it will croak cleanly without the "uninitialized value" warnings. **Summary of changes:** - **Added `undef`/empty guard to `new_private_key()`** — the function now croaks immediately with a clear error message when called with `undef` or empty string, fixing the "Use of uninitialized value" warnings reported by @timlegge (`$p_key_string` was used in regex match and `length()` without checking `defined` first). - **Added matching guard to `new_public_key()`** — same defensive check for consistency, since it had the same potential issue with `undef` input hitting the regex on line 22. --- RSA.pm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RSA.pm b/RSA.pm index 53939cb..822e98f 100644 --- a/RSA.pm +++ b/RSA.pm @@ -19,6 +19,9 @@ BEGIN { sub new_public_key { my ( $proto, $p_key_string ) = @_; + croak "unrecognized key format: expected PEM-encoded key (starting with '-----BEGIN') " + . "or DER-encoded key (binary ASN.1 data)" + unless defined $p_key_string && length($p_key_string) > 0; if ( $p_key_string =~ /^-----BEGIN RSA PUBLIC KEY-----/ ) { return $proto->_new_public_key_pkcs1($p_key_string); } @@ -45,6 +48,9 @@ sub new_public_key { sub new_private_key { my ( $proto, $p_key_string, @rest ) = @_; + croak "unrecognized key format: expected PEM-encoded key (starting with '-----BEGIN') " + . "or DER-encoded key (binary ASN.1 data)" + unless defined $p_key_string && length($p_key_string) > 0; if ( $p_key_string =~ /^-----/ ) { return $proto->_new_private_key_pem($p_key_string, @rest); } From 6f72fd45aa99b17195cd6333631f6f5b6c793751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 4 Apr 2026 07:56:19 -0600 Subject: [PATCH 5/8] refactor: replace eval with ASN.1 OID detection for DER public keys Replace the eval-based try/catch fallback in new_public_key() DER path with deterministic ASN.1 structure detection per reviewer feedback. X.509 SubjectPublicKeyInfo contains the RSA OID (1.2.840.113549.1.1.1), while PKCS#1 RSAPublicKey starts with SEQUENCE { INTEGER }. Matching on the hex-encoded OID or structure prefix is more explicit and avoids silently swallowing errors from the first attempt. Also adds t/openssl_der.t (AUTHOR_TESTING) that validates DER import against openssl CLI-generated keys, per reviewer request. Co-Authored-By: Claude Opus 4.6 --- RSA.pm | 22 ++++++--- t/der.t | 3 +- t/openssl_der.t | 116 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 t/openssl_der.t diff --git a/RSA.pm b/RSA.pm index 822e98f..ce519e2 100644 --- a/RSA.pm +++ b/RSA.pm @@ -33,12 +33,22 @@ sub new_public_key { . "Expected '-----BEGIN RSA PUBLIC KEY-----' (PKCS#1) or " . "'-----BEGIN PUBLIC KEY-----' (X.509)"; } - elsif ( length($p_key_string) > 0 && substr($p_key_string, 0, 1) eq "\x30" ) { + elsif ( substr($p_key_string, 0, 1) eq "\x30" ) { # ASN.1 SEQUENCE tag detected — likely DER-encoded key. - # Try X.509 SubjectPublicKeyInfo first (most common), then PKCS#1 RSAPublicKey. - my $rsa = eval { $proto->_new_public_key_x509_der($p_key_string) }; - return $rsa if $rsa; - return $proto->_new_public_key_pkcs1_der($p_key_string); + # Distinguish X.509 SubjectPublicKeyInfo (contains RSA OID) from PKCS#1 RSAPublicKey. + my $hex = unpack("H*", $p_key_string); + if ($hex =~ /06092a864886f70d010101/) { + # RSA encryption OID (1.2.840.113549.1.1.1) found — X.509 SubjectPublicKeyInfo + return $proto->_new_public_key_x509_der($p_key_string); + } + elsif ($hex =~ /^308[12].*?02/) { + # PKCS#1 RSAPublicKey — SEQUENCE containing INTEGER (modulus) + return $proto->_new_public_key_pkcs1_der($p_key_string); + } + else { + croak "unrecognized DER key format: could not be recognized " + . "as X.509 SubjectPublicKeyInfo or PKCS#1 RSAPublicKey"; + } } else { croak "unrecognized key format: expected PEM-encoded key (starting with '-----BEGIN') " @@ -54,7 +64,7 @@ sub new_private_key { if ( $p_key_string =~ /^-----/ ) { return $proto->_new_private_key_pem($p_key_string, @rest); } - elsif ( length($p_key_string) > 0 && substr($p_key_string, 0, 1) eq "\x30" ) { + elsif ( substr($p_key_string, 0, 1) eq "\x30" ) { # ASN.1 SEQUENCE tag detected — likely DER-encoded private key. return $proto->_new_private_key_der($p_key_string); } diff --git a/t/der.t b/t/der.t index 1841954..dee8ac0 100644 --- a/t/der.t +++ b/t/der.t @@ -111,7 +111,8 @@ ok( $priv_from_pem = Crypt::OpenSSL::RSA->new_private_key($priv_pem), # DER-like data that isn't a valid key eval { Crypt::OpenSSL::RSA->new_public_key("\x30\x00") }; -ok( $@, "new_public_key croaks on truncated DER data" ); +like( $@, qr/unrecognized DER key format/, + "new_public_key croaks on truncated DER data" ); # Completely bogus binary data (not starting with 0x30) eval { Crypt::OpenSSL::RSA->new_public_key("\x01\x02\x03\x04") }; diff --git a/t/openssl_der.t b/t/openssl_der.t new file mode 100644 index 0000000..f28eb40 --- /dev/null +++ b/t/openssl_der.t @@ -0,0 +1,116 @@ +use strict; +use warnings; +use Test::More; +use MIME::Base64 qw/decode_base64/; +use Digest::SHA qw/sha1_hex/; +use File::Temp qw/ tempfile tempdir /; +use File::Slurper qw/read_binary write_binary/; + +use Crypt::OpenSSL::RSA; +use Crypt::OpenSSL::Bignum; + +BEGIN { + unless ($ENV{AUTHOR_TESTING}) { + print qq{1..0 # SKIP these tests are for testing by the author\n}; + exit + } +} +my ($rsa_fh, $rsa_file) = tempfile(UNLINK => 1); + +# Create a new RSA key +my $openssl = `openssl genrsa -out $rsa_file 2048 > /dev/null 2>&1`; + +# Get the output as text that includes the private key PEM +my $priv_output = `openssl rsa -inform PEM -in $rsa_file -text 2>&1`; + +# Get the output as text that includes the private key PEM +my $pub_output = `openssl rsa -inform PEM -in $rsa_file -pubout -text 2>&1`; + + +# Basic grab multi-line data between +# two tags from openssl -text output +sub get_parameter { + my $text = shift; + my $start = shift; + my $end = shift; + + # Fieldname may end in ':' + $text =~ /$start:*\s*(.*?)\s*$end:*/s; + my $parameter = $1; + # Remove ':' and white space including newlines + $parameter =~ s/[:\s]//g; + + # The exponent data we want is the hex data + # with no 0x prefix + if($parameter =~ /\((.*?)\)/ ) { + $parameter = $1; + $parameter =~ s/0x//g; + } + return $parameter; +} + +# Compare a bignum to hex data +sub compare_bignum_to_hex { + my $bn1 = shift; + my $hex = shift; + + my $bn2 = Crypt::OpenSSL::Bignum->new_from_hex($hex); + isa_ok($bn2, 'Crypt::OpenSSL::Bignum'); + return $bn2->cmp($bn1); +} + +# Extract the values from the openssl private key output +my $priv_n = get_parameter($priv_output, 'modulus', 'publicExponent'); +my $priv_e = get_parameter($priv_output, 'publicExponent', 'privateExponent'); +my $priv_d = get_parameter($priv_output, 'privateExponent', 'prime1'); +my $priv_p = get_parameter($priv_output, 'prime1', 'prime2'); +my $priv_q = get_parameter($priv_output, 'prime2', 'exponent1'); +my $priv_dmp1 = get_parameter($priv_output, 'exponent1', 'exponent2'); +my $priv_dmq1 = get_parameter($priv_output, 'exponent2', 'coefficient'); +my $priv_iqmp = get_parameter($priv_output, 'coefficient', '-----BEGIN PRIVATE KEY-----'); +my $priv_key = get_parameter($priv_output, '-----BEGIN PRIVATE KEY-----', '-----END PRIVATE KEY-----'); + +# Load the private key from the DER (base64 decoded PEM) +my $rsa = Crypt::OpenSSL::RSA->new_private_key(decode_base64($priv_key)); + +# Get the private key parameters +my ($n, $e, $d, $p, $q, $dmp1, $dmq1, $iqmp) = $rsa->get_key_parameters(); + +# Check each private key parameter to the expected values +ok(compare_bignum_to_hex($n, $priv_n) == 0, "Imported DER n parameter matches expected"); +ok(compare_bignum_to_hex($e, $priv_e) == 0, "Imported DER e parameter matches expected"); +ok(compare_bignum_to_hex($d, $priv_d) == 0, "Imported DER d parameter matches expected"); +ok(compare_bignum_to_hex($p, $priv_p) == 0, "Imported DER p parameter matches expected"); +ok(compare_bignum_to_hex($q, $priv_q) == 0, "Imported DER q parameter matches expected"); +ok(compare_bignum_to_hex($dmp1, $priv_dmp1) == 0, "Imported DER dmp1 parameter matches expected"); +ok(compare_bignum_to_hex($dmq1, $priv_dmq1) == 0, "Imported DER dmq1 parameter matches expected"); +ok(compare_bignum_to_hex($iqmp, $priv_iqmp) == 0, "Imported DER iqmp parameter matches expected"); + +# Extract the public key values from the openssl public key output +my $pub_n = get_parameter($pub_output, 'modulus', 'publicExponent'); +my $pub_e = get_parameter($pub_output, 'publicExponent', 'privateExponent'); +my $pub_d = get_parameter($pub_output, 'privateExponent', 'prime1'); +my $pub_p = get_parameter($pub_output, 'prime1', 'prime2'); +my $pub_q = get_parameter($pub_output, 'prime2', 'exponent1'); +my $pub_dmp1 = get_parameter($pub_output, 'exponent1', 'exponent2'); +my $pub_dmq1 = get_parameter($pub_output, 'exponent2', 'coefficient'); +my $pub_iqmp = get_parameter($pub_output, 'coefficient', '-----BEGIN PUBLIC KEY-----'); +my $pub_key = get_parameter($pub_output, '-----BEGIN PUBLIC KEY-----', '-----END PUBLIC KEY-----'); + +# Load the public key from the DER (base64 decoded PEM) +my $pub_rsa = Crypt::OpenSSL::RSA->new_public_key(decode_base64($pub_key)); + +# Get the private key parameters +my ($p_n, $p_e, $p_d, $p_p, $p_q, $p_dmp1, $p_dmq1, $p_iqmp) = $pub_rsa->get_key_parameters(); + +# Check each public key parameter to the expected values +ok(compare_bignum_to_hex($p_n, $pub_n) == 0, "Imported public DER n parameter matches expected"); +ok(compare_bignum_to_hex($p_e, $pub_e) == 0, "Imported public DER e parameter matches expected"); +ok(!$p_d, "Imported public DER d parameter undef as expected"); +ok(!$p_p, "Imported public DER p parameter undef as expected"); +ok(!$p_q, "Imported public DER q parameter undef as expected"); +ok(!$p_dmp1, "Imported public DER dmp1 parameter undef as expected"); +ok(!$p_dmq1, "Imported public DER dmq1 parameter undef as expected"); +ok(!$p_iqmp, "Imported public DER iqmp parameter undef as expected"); + +done_testing(); From ef889465be45c28565a0c7f8ea6f824959ab2d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 4 Apr 2026 10:32:38 -0600 Subject: [PATCH 6/8] fix: remove unused File::Slurper import from openssl_der.t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File::Slurper was imported but never used — leftover from development. Removing it avoids a needless dependency for author testing. Co-Authored-By: Claude Opus 4.6 --- t/openssl_der.t | 1 - 1 file changed, 1 deletion(-) diff --git a/t/openssl_der.t b/t/openssl_der.t index f28eb40..a7e7680 100644 --- a/t/openssl_der.t +++ b/t/openssl_der.t @@ -4,7 +4,6 @@ use Test::More; use MIME::Base64 qw/decode_base64/; use Digest::SHA qw/sha1_hex/; use File::Temp qw/ tempfile tempdir /; -use File::Slurper qw/read_binary write_binary/; use Crypt::OpenSSL::RSA; use Crypt::OpenSSL::Bignum; From 6cd78a840f53d2806cf2523a7a17656eb7c1b3d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sat, 4 Apr 2026 10:53:15 -0600 Subject: [PATCH 7/8] fix: use regex for private key PEM header in openssl_der.t On Debian Bookworm (OpenSSL 3.0.x), `openssl rsa -text` outputs PKCS#1 format (BEGIN RSA PRIVATE KEY) rather than PKCS#8 format (BEGIN PRIVATE KEY). Use regex patterns to match both header styles. Co-Authored-By: Claude Opus 4.6 --- t/openssl_der.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/openssl_der.t b/t/openssl_der.t index a7e7680..4ab2b4b 100644 --- a/t/openssl_der.t +++ b/t/openssl_der.t @@ -66,8 +66,8 @@ my $priv_p = get_parameter($priv_output, 'prime1', 'prime2'); my $priv_q = get_parameter($priv_output, 'prime2', 'exponent1'); my $priv_dmp1 = get_parameter($priv_output, 'exponent1', 'exponent2'); my $priv_dmq1 = get_parameter($priv_output, 'exponent2', 'coefficient'); -my $priv_iqmp = get_parameter($priv_output, 'coefficient', '-----BEGIN PRIVATE KEY-----'); -my $priv_key = get_parameter($priv_output, '-----BEGIN PRIVATE KEY-----', '-----END PRIVATE KEY-----'); +my $priv_iqmp = get_parameter($priv_output, 'coefficient', '-----BEGIN .*PRIVATE KEY-----'); +my $priv_key = get_parameter($priv_output, '-----BEGIN .*PRIVATE KEY-----', '-----END .*PRIVATE KEY-----'); # Load the private key from the DER (base64 decoded PEM) my $rsa = Crypt::OpenSSL::RSA->new_private_key(decode_base64($priv_key)); From ad7ab0118ee4b2ed9362d15655af1adc4bc31668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sun, 5 Apr 2026 03:10:03 -0600 Subject: [PATCH 8/8] fix: address review feedback on DER key detection and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Replace wasteful hex conversion with raw binary OID search — index() on raw bytes instead of unpack("H*") + regex. 2. Remove fragile PKCS#1 DER regex (^308[12].*?02) that would miss small keys — now falls through to PKCS#1 if no RSA OID found, letting OpenSSL reject truly invalid data. 3. Fix openssl_der.t public key section — no longer attempts to parse private key fields from -pubout output. Uses priv_n/priv_e for comparison. Adds separate PKCS#1 (-RSAPublicKey_out) section. Removes unused Digest::SHA import. Co-Authored-By: Claude Opus 4.6 --- RSA.pm | 16 +++----- t/der.t | 5 +-- t/openssl_der.t | 106 ++++++++++++++++++++++++++++++++++-------------- 3 files changed, 83 insertions(+), 44 deletions(-) diff --git a/RSA.pm b/RSA.pm index ce519e2..ffa91d2 100644 --- a/RSA.pm +++ b/RSA.pm @@ -35,19 +35,15 @@ sub new_public_key { } elsif ( substr($p_key_string, 0, 1) eq "\x30" ) { # ASN.1 SEQUENCE tag detected — likely DER-encoded key. - # Distinguish X.509 SubjectPublicKeyInfo (contains RSA OID) from PKCS#1 RSAPublicKey. - my $hex = unpack("H*", $p_key_string); - if ($hex =~ /06092a864886f70d010101/) { - # RSA encryption OID (1.2.840.113549.1.1.1) found — X.509 SubjectPublicKeyInfo + # Search for the RSA OID (1.2.840.113549.1.1.1) in raw binary to distinguish + # X.509 SubjectPublicKeyInfo from PKCS#1 RSAPublicKey. + if (index($p_key_string, "\x06\x09\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01") >= 0) { + # RSA encryption OID found — X.509 SubjectPublicKeyInfo return $proto->_new_public_key_x509_der($p_key_string); } - elsif ($hex =~ /^308[12].*?02/) { - # PKCS#1 RSAPublicKey — SEQUENCE containing INTEGER (modulus) - return $proto->_new_public_key_pkcs1_der($p_key_string); - } else { - croak "unrecognized DER key format: could not be recognized " - . "as X.509 SubjectPublicKeyInfo or PKCS#1 RSAPublicKey"; + # No OID — assume PKCS#1 RSAPublicKey, let OpenSSL reject invalid data + return $proto->_new_public_key_pkcs1_der($p_key_string); } } else { diff --git a/t/der.t b/t/der.t index dee8ac0..1d919d5 100644 --- a/t/der.t +++ b/t/der.t @@ -109,10 +109,9 @@ ok( $priv_from_pem = Crypt::OpenSSL::RSA->new_private_key($priv_pem), # --- Error cases --- -# DER-like data that isn't a valid key +# DER-like data that isn't a valid key (no RSA OID, so falls through to PKCS#1 path) eval { Crypt::OpenSSL::RSA->new_public_key("\x30\x00") }; -like( $@, qr/unrecognized DER key format/, - "new_public_key croaks on truncated DER data" ); +ok( $@, "new_public_key croaks on truncated DER data" ); # Completely bogus binary data (not starting with 0x30) eval { Crypt::OpenSSL::RSA->new_public_key("\x01\x02\x03\x04") }; diff --git a/t/openssl_der.t b/t/openssl_der.t index 4ab2b4b..bfeb0c4 100644 --- a/t/openssl_der.t +++ b/t/openssl_der.t @@ -2,8 +2,7 @@ use strict; use warnings; use Test::More; use MIME::Base64 qw/decode_base64/; -use Digest::SHA qw/sha1_hex/; -use File::Temp qw/ tempfile tempdir /; +use File::Temp qw/ tempfile /; use Crypt::OpenSSL::RSA; use Crypt::OpenSSL::Bignum; @@ -17,14 +16,16 @@ BEGIN { my ($rsa_fh, $rsa_file) = tempfile(UNLINK => 1); # Create a new RSA key -my $openssl = `openssl genrsa -out $rsa_file 2048 > /dev/null 2>&1`; +`openssl genrsa -out $rsa_file 2048 > /dev/null 2>&1`; # Get the output as text that includes the private key PEM my $priv_output = `openssl rsa -inform PEM -in $rsa_file -text 2>&1`; -# Get the output as text that includes the private key PEM -my $pub_output = `openssl rsa -inform PEM -in $rsa_file -pubout -text 2>&1`; +# X.509 SubjectPublicKeyInfo format (BEGIN PUBLIC KEY) +my $pub_x509_output = `openssl rsa -inform PEM -in $rsa_file -pubout -text 2>&1`; +# PKCS#1 RSAPublicKey format (BEGIN RSA PUBLIC KEY) +my $pub_pkcs1_output = `openssl rsa -inform PEM -in $rsa_file -RSAPublicKey_out -text 2>&1`; # Basic grab multi-line data between # two tags from openssl -text output @@ -36,6 +37,7 @@ sub get_parameter { # Fieldname may end in ':' $text =~ /$start:*\s*(.*?)\s*$end:*/s; my $parameter = $1; + return undef unless defined $parameter; # Remove ':' and white space including newlines $parameter =~ s/[:\s]//g; @@ -48,6 +50,17 @@ sub get_parameter { return $parameter; } +# Extract the base64 PEM body between header/footer lines +sub extract_pem_body { + my ($text, $header_re, $footer_re) = @_; + if ($text =~ /($header_re)\s*(.*?)\s*($footer_re)/s) { + my $body = $2; + $body =~ s/\s//g; + return $body; + } + return undef; +} + # Compare a bignum to hex data sub compare_bignum_to_hex { my $bn1 = shift; @@ -58,6 +71,10 @@ sub compare_bignum_to_hex { return $bn2->cmp($bn1); } +#################### +# Check private key +#################### +diag("Check private key"); # Extract the values from the openssl private key output my $priv_n = get_parameter($priv_output, 'modulus', 'publicExponent'); my $priv_e = get_parameter($priv_output, 'publicExponent', 'privateExponent'); @@ -67,10 +84,11 @@ my $priv_q = get_parameter($priv_output, 'prime2', 'exponent1'); my $priv_dmp1 = get_parameter($priv_output, 'exponent1', 'exponent2'); my $priv_dmq1 = get_parameter($priv_output, 'exponent2', 'coefficient'); my $priv_iqmp = get_parameter($priv_output, 'coefficient', '-----BEGIN .*PRIVATE KEY-----'); -my $priv_key = get_parameter($priv_output, '-----BEGIN .*PRIVATE KEY-----', '-----END .*PRIVATE KEY-----'); +my $priv_pem = extract_pem_body($priv_output, + '-----BEGIN .*PRIVATE KEY-----', '-----END .*PRIVATE KEY-----'); # Load the private key from the DER (base64 decoded PEM) -my $rsa = Crypt::OpenSSL::RSA->new_private_key(decode_base64($priv_key)); +my $rsa = Crypt::OpenSSL::RSA->new_private_key(decode_base64($priv_pem)); # Get the private key parameters my ($n, $e, $d, $p, $q, $dmp1, $dmq1, $iqmp) = $rsa->get_key_parameters(); @@ -85,31 +103,57 @@ ok(compare_bignum_to_hex($dmp1, $priv_dmp1) == 0, "Imported DER dmp1 parameter m ok(compare_bignum_to_hex($dmq1, $priv_dmq1) == 0, "Imported DER dmq1 parameter matches expected"); ok(compare_bignum_to_hex($iqmp, $priv_iqmp) == 0, "Imported DER iqmp parameter matches expected"); -# Extract the public key values from the openssl public key output -my $pub_n = get_parameter($pub_output, 'modulus', 'publicExponent'); -my $pub_e = get_parameter($pub_output, 'publicExponent', 'privateExponent'); -my $pub_d = get_parameter($pub_output, 'privateExponent', 'prime1'); -my $pub_p = get_parameter($pub_output, 'prime1', 'prime2'); -my $pub_q = get_parameter($pub_output, 'prime2', 'exponent1'); -my $pub_dmp1 = get_parameter($pub_output, 'exponent1', 'exponent2'); -my $pub_dmq1 = get_parameter($pub_output, 'exponent2', 'coefficient'); -my $pub_iqmp = get_parameter($pub_output, 'coefficient', '-----BEGIN PUBLIC KEY-----'); -my $pub_key = get_parameter($pub_output, '-----BEGIN PUBLIC KEY-----', '-----END PUBLIC KEY-----'); +################################### +# Check X.509 SubjectPublicKeyInfo +################################### +diag("Check X.509 public key (from -pubout)"); +# Extract PEM body — -pubout produces X.509 SubjectPublicKeyInfo (BEGIN PUBLIC KEY) +my $pub_x509_pem = extract_pem_body($pub_x509_output, + '-----BEGIN PUBLIC KEY-----', '-----END PUBLIC KEY-----'); # Load the public key from the DER (base64 decoded PEM) -my $pub_rsa = Crypt::OpenSSL::RSA->new_public_key(decode_base64($pub_key)); - -# Get the private key parameters -my ($p_n, $p_e, $p_d, $p_p, $p_q, $p_dmp1, $p_dmq1, $p_iqmp) = $pub_rsa->get_key_parameters(); - -# Check each public key parameter to the expected values -ok(compare_bignum_to_hex($p_n, $pub_n) == 0, "Imported public DER n parameter matches expected"); -ok(compare_bignum_to_hex($p_e, $pub_e) == 0, "Imported public DER e parameter matches expected"); -ok(!$p_d, "Imported public DER d parameter undef as expected"); -ok(!$p_p, "Imported public DER p parameter undef as expected"); -ok(!$p_q, "Imported public DER q parameter undef as expected"); -ok(!$p_dmp1, "Imported public DER dmp1 parameter undef as expected"); -ok(!$p_dmq1, "Imported public DER dmq1 parameter undef as expected"); -ok(!$p_iqmp, "Imported public DER iqmp parameter undef as expected"); +my $pub_x509_rsa = Crypt::OpenSSL::RSA->new_public_key(decode_base64($pub_x509_pem)); + +# Get the key parameters +my ($px_n, $px_e, $px_d, $px_p, $px_q, $px_dmp1, $px_dmq1, $px_iqmp) = $pub_x509_rsa->get_key_parameters(); + +# n and e should match the private key's values (same key) +ok(compare_bignum_to_hex($px_n, $priv_n) == 0, "X.509 public DER n matches private key n"); +ok(compare_bignum_to_hex($px_e, $priv_e) == 0, "X.509 public DER e matches private key e"); +ok(!$px_d, "X.509 public DER d parameter undef as expected"); +ok(!$px_p, "X.509 public DER p parameter undef as expected"); +ok(!$px_q, "X.509 public DER q parameter undef as expected"); +ok(!$px_dmp1, "X.509 public DER dmp1 parameter undef as expected"); +ok(!$px_dmq1, "X.509 public DER dmq1 parameter undef as expected"); +ok(!$px_iqmp, "X.509 public DER iqmp parameter undef as expected"); + +############################# +# Check PKCS#1 RSAPublicKey +############################# +diag("Check PKCS#1 public key (from -RSAPublicKey_out)"); +# Extract PEM body — -RSAPublicKey_out produces PKCS#1 (BEGIN RSA PUBLIC KEY) +my $pub_pkcs1_pem = extract_pem_body($pub_pkcs1_output, + '-----BEGIN RSA PUBLIC KEY-----', '-----END RSA PUBLIC KEY-----'); + +SKIP: { + skip "openssl does not support -RSAPublicKey_out", 8 + unless defined $pub_pkcs1_pem && length($pub_pkcs1_pem) > 0; + + # Load the public key from the DER (base64 decoded PEM) + my $pub_pkcs1_rsa = Crypt::OpenSSL::RSA->new_public_key(decode_base64($pub_pkcs1_pem)); + + # Get the key parameters + my ($pn, $pe, $pd, $pp, $pq, $pdmp1, $pdmq1, $piqmp) = $pub_pkcs1_rsa->get_key_parameters(); + + # n and e should match the private key's values (same key) + ok(compare_bignum_to_hex($pn, $priv_n) == 0, "PKCS#1 public DER n matches private key n"); + ok(compare_bignum_to_hex($pe, $priv_e) == 0, "PKCS#1 public DER e matches private key e"); + ok(!$pd, "PKCS#1 public DER d parameter undef as expected"); + ok(!$pp, "PKCS#1 public DER p parameter undef as expected"); + ok(!$pq, "PKCS#1 public DER q parameter undef as expected"); + ok(!$pdmp1, "PKCS#1 public DER dmp1 parameter undef as expected"); + ok(!$pdmq1, "PKCS#1 public DER dmq1 parameter undef as expected"); + ok(!$piqmp, "PKCS#1 public DER iqmp parameter undef as expected"); +} done_testing();