From b7db24e271ac86d20002cf37ea5400cb1790e5e8 Mon Sep 17 00:00:00 2001
From: Toddr Bot <toddbot@rinaldo.us>
Date: Wed, 1 Jul 2026 22:49:13 +0000
Subject: [PATCH] Fix error queue leak and EVP_PKEY_fromdata selection on
 OpenSSL 3.x

_detect_private_key() calls EVP_PKEY_get_bn_param() to check for the
private exponent d.  On public keys this fails and pushes errors onto
the OpenSSL error queue, but never clears them.  These stale errors
can leak into subsequent croakSsl() calls from unrelated operations,
potentially producing misleading error messages.

Add ERR_clear_error() after the check to keep the queue clean.

Also fix _new_key_from_parameters() to pass EVP_PKEY_PUBLIC_KEY (not
EVP_PKEY_KEYPAIR) when constructing a public-only key from n and e
without d.  This matches the actual key type and avoids unnecessary
error queue pollution from EVP_PKEY_fromdata.
---
 RSA.xs          |  8 +++++++-
 t/error_queue.t | 22 +++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/RSA.xs b/RSA.xs
index 238bebb..352c819 100644
--- a/RSA.xs
+++ b/RSA.xs
@@ -160,8 +160,13 @@ static int _detect_private_key(EVP_PKEY* p_rsa)
     EVP_PKEY_get_bn_param(p_rsa, OSSL_PKEY_PARAM_RSA_D, &d);
     if (d) {
         BN_clear_free(d);
+        ERR_clear_error();
         return 1;
     }
+    /* EVP_PKEY_get_bn_param pushes errors when the param is absent
+       (e.g. public key has no d).  Clear them so they don't leak
+       into the next croakSsl() call from an unrelated operation. */
+    ERR_clear_error();
     return 0;
 #else
     const BIGNUM* d = NULL;
@@ -1057,7 +1062,8 @@ _new_key_from_parameters(proto, n, e, d, p, q)
         params = OSSL_PARAM_BLD_to_param(params_build);
         THROW(params != NULL);
 
-        int status = EVP_PKEY_fromdata(pctx, &rsa, EVP_PKEY_KEYPAIR, params);
+        int selection = (d != NULL) ? EVP_PKEY_KEYPAIR : EVP_PKEY_PUBLIC_KEY;
+        int status = EVP_PKEY_fromdata(pctx, &rsa, selection, params);
         OSSL_PARAM_BLD_free(params_build);
         OSSL_PARAM_free(params);
         params_build = NULL;
diff --git a/t/error_queue.t b/t/error_queue.t
index 882498f..00bf9e8 100644
--- a/t/error_queue.t
+++ b/t/error_queue.t
@@ -4,7 +4,7 @@ use Test::More;
 
 use Crypt::OpenSSL::RSA;
 
-plan tests => 4;
+plan tests => 7;
 
 # Test that eval-caught OpenSSL failures don't pollute subsequent error messages.
 # Bug: croakSsl() used ERR_get_error() once (oldest error) instead of draining
@@ -30,3 +30,23 @@ eval { $rsa->encrypt("A" x 500) };
 my $third_error = $@;
 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");
+
+# Test that public key construction from parameters doesn't pollute
+# the error queue.  On OpenSSL 3.x, _detect_private_key() calls
+# EVP_PKEY_get_bn_param() for d which fails on public keys and may
+# push errors.  Verify subsequent failures report their own error.
+{
+    my ($n, $e) = $rsa->get_key_parameters();
+    my $pub = Crypt::OpenSSL::RSA->new_key_from_parameters($n, $e);
+    ok(!$pub->is_private(), "public key from parameters is not private");
+
+    eval { $pub->decrypt("garbage ciphertext") };
+    like($@, qr/Public keys cannot decrypt/,
+        "error after public key construction is about the actual problem");
+
+    # Load garbage PEM after the public key — this exercises croakSsl().
+    # If _detect_private_key left stale errors, the message could be
+    # misleading.  Verify it mentions the actual PEM/ASN1 problem.
+    eval { Crypt::OpenSSL::RSA->new_private_key("-----BEGIN RSA PRIVATE KEY-----\nZw==\n-----END RSA PRIVATE KEY-----\n") };
+    ok($@, "garbage PEM after public key construction still croaks correctly");
+}