fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x) by Koan-Bot · Pull Request #81 · cpan-authors/Crypt-OpenSSL-RSA

Koan-Bot · 2026-02-19T07:23:19Z

Summary

Two issues in the OpenSSL 3.x verify path:

Error-path leaks: If any CHECK_OPEN_SSL fires during EVP_PKEY_CTX or EVP_MD setup, both objects leak because croak() is called immediately
Dead cleanup code: EVP_MD_free(md) and EVP_PKEY_CTX_free(ctx) placed after the switch statement are unreachable — XSRETURN_NO and XSRETURN_YES are longjmps that skip all subsequent code

Fix

Replaced CHECK_OPEN_SSL with THROW()/goto err pattern for proper cleanup during setup
Moved cleanup (EVP_MD_free, EVP_PKEY_CTX_free) to before the switch(verify_result) statement, so resources are freed regardless of which XSRETURN_* path is taken
Separated the OpenSSL 3.x and pre-3.x switch blocks to avoid the shared switch body (which was the root cause of the dead-code issue)

Test plan

All 278 existing tests pass
sign/verify tested in rsa.t with multiple hash algorithms and padding modes

🤖 Generated with Claude Code

timlegge

I would probably try to avoid the switch code duplication.

I have also give thought to re-enabling the oaep. The last time I tested it I think the newer openssl versions seem to protect against the martian attach now. I would wonder whether the changes might affect that

timlegge · 2026-03-12T00:17:36Z

+    EVP_MD_free(md);
+    EVP_PKEY_CTX_free(ctx);
+    switch(verify_result)
+    {


Don't duplicate this

timlegge · 2026-03-12T00:18:15Z

+    verify_done: ;
+    }
 #else
    switch(RSA_verify(p_rsa->hashMode,


use verify_result = here

atoomic · 2026-03-12T02:23:27Z

@Koan-Bot rebase

Koan-Bot · 2026-03-12T03:08:17Z

Rebase: fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x)

Branch koan.atoomic/fix-verify-error-leaks rebased onto main and force-pushed.

Diff: 1 file changed, 30 insertions(+), 19 deletions(-)

Review feedback was analyzed and applied.

Actions

Rebased koan.atoomic/fix-verify-error-leaks onto upstream/main
Applied review feedback
Force-pushed koan.atoomic/fix-verify-error-leaks to origin

Automated by Kōan

atoomic · 2026-03-12T03:11:13Z

@timlegge ready for a second round of review
thanks

timlegge · 2026-03-13T02:07:57Z

    }

    CHECK_OPEN_SSL(digest = get_message_digest(text_SV, p_rsa->hashMode));
+    {


Why is the { here? where is the matching one and why is it needed?

The { opens a block scope (matching } is at line 1112, replacing the old #if/EVP_MD_free/EVP_PKEY_CTX_free/#endif block).

It's needed for C89 compatibility: verify_result must be declared at the top of a block since it's used by both the OpenSSL 3.x and pre-3.x code paths — it has to be declared before the #if/#else fork so the shared switch(verify_result) after #endif can see it.

Without this block, we'd need to either:

Use PREINIT: for all declarations (but error, ctx, md are only meaningful in the 3.x path — polluting PREINIT: with them feels wrong)

Declare verify_result in PREINIT: and drop the block — that would work too, and would be simpler

Happy to go with option 2 if you prefer — move int verify_result; to PREINIT: and remove the {/} wrapper entirely.

Declare verify_result in PREINIT:

Koan-Bot · 2026-03-14T07:34:31Z

@timlegge — Regarding your review comment about re-enabling OAEP and the Marvin attack:

The switch duplication is fixed in the rebase (see inline reply). As for the PKCS#1 v1.5 / Marvin attack question — that's addressed separately in PR #92.

The key insight: the Marvin attack (CVE-2024-2467) only affects PKCS#1 v1.5 decryption (the padding oracle in RSA_private_decrypt). It does NOT affect signatures — RSA_sign()/RSA_verify() with PKCS#1 v1.5 are not vulnerable because signatures don't involve padding-dependent branching on secret data.

PR #92 re-enables PKCS#1 v1.5 for sign/verify while keeping it disabled for encrypt/decrypt (where the Marvin attack is relevant). That's orthogonal to this PR, which is purely about resource cleanup.

Re: modern OpenSSL protecting against Marvin on the decryption side — OpenSSL 3.2+ added implicit Marvin mitigation (OSSL_PKEY_PARAM_IMPLICIT_REJECTION), but relying on that for a library that supports OpenSSL 3.0.x feels premature. PR #92 takes the conservative approach: safe for signatures, still blocked for encryption.

timlegge · 2026-03-14T12:00:41Z

The PR to re-enable will need some testing against the Marvin tool kit to be sure.

Koan-Bot · 2026-03-14T20:01:52Z

Rebase: fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x)

Branch koan.atoomic/fix-verify-error-leaks rebased onto main and force-pushed.

Diff: 1 file changed, 30 insertions(+), 19 deletions(-)

Review feedback was analyzed and applied.

Actions

Rebased koan.atoomic/fix-verify-error-leaks onto upstream/main
Applied review feedback
Force-pushed koan.atoomic/fix-verify-error-leaks to origin

Automated by Kōan

timlegge · 2026-03-15T00:09:29Z

@Koan-Bot rebase

Koan-Bot · 2026-03-15T00:19:03Z

❌ Permission denied. Only users with write access can trigger bot commands.

timlegge · 2026-03-15T21:42:42Z

@atoomic this looks fine but does need it be rebased

atoomic · 2026-03-15T21:56:44Z

@Koan-Bot rebase

atoomic · 2026-03-15T21:57:16Z

@timlegge I've added you to the list of authorized users, just need a reboot ( in a few min ) before it takes action

… 3.x) Two issues in the OpenSSL 3.x verify path: 1. Error-path leaks: CHECK_OPEN_SSL during setup croaks immediately, leaking the EVP_PKEY_CTX and EVP_MD objects. 2. Dead cleanup code: EVP_MD_free and EVP_PKEY_CTX_free after the switch block were unreachable because XSRETURN_NO/XSRETURN_YES are longjmps that skip all subsequent code. Fix: use THROW/goto err pattern for setup errors, and free resources before the switch statement on the success path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Koan-Bot · 2026-03-15T23:16:11Z

Rebase: fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x)

Branch koan.atoomic/fix-verify-error-leaks rebased onto main and force-pushed.

Diff: 1 file changed, 28 insertions(+), 31 deletions(-)

Review feedback was analyzed and applied.

Actions

Resolved merge conflicts (1 round(s))
Rebased koan.atoomic/fix-verify-error-leaks onto upstream/main
Force-pushed koan.atoomic/fix-verify-error-leaks to origin

Automated by Kōan

timlegge

lgtm

timlegge · 2026-03-14T16:52:42Z

    }

    CHECK_OPEN_SSL(digest = get_message_digest(text_SV, p_rsa->hashMode));
+    {


Declare verify_result in PREINIT:

timlegge · 2026-03-15T23:41:32Z

@atoomic let's merge and see if any of the other PRs need to be rebased

atoomic marked this pull request as ready for review February 20, 2026 02:28

atoomic requested a review from timlegge February 25, 2026 05:58

timlegge requested changes Mar 12, 2026

View reviewed changes

Koan-Bot added a commit to atoomic/Crypt-OpenSSL-RSA that referenced this pull request Mar 12, 2026

rebase: apply review feedback on cpan-authors#81

e9d7ab4

Koan-Bot force-pushed the koan.atoomic/fix-verify-error-leaks branch from 83c386f to e9d7ab4 Compare March 12, 2026 03:08

atoomic requested a review from timlegge March 12, 2026 03:10

timlegge reviewed Mar 13, 2026

View reviewed changes

Koan-Bot mentioned this pull request Mar 14, 2026

fix: multiple OpenSSL 3.x memory leaks #75

Merged

3 tasks

Koan-Bot added a commit to atoomic/Crypt-OpenSSL-RSA that referenced this pull request Mar 14, 2026

rebase: apply review feedback on cpan-authors#81

e07436e

Koan-Bot added a commit to atoomic/Crypt-OpenSSL-RSA that referenced this pull request Mar 14, 2026

rebase: apply review feedback on cpan-authors#81

0c95b9a

Koan-Bot force-pushed the koan.atoomic/fix-verify-error-leaks branch from e9d7ab4 to 0c95b9a Compare March 14, 2026 20:01

Koan-Bot and others added 3 commits March 15, 2026 17:15

rebase: apply review feedback on cpan-authors#81

2584807

rebase: apply review feedback on cpan-authors#81

e69c9b0

Koan-Bot force-pushed the koan.atoomic/fix-verify-error-leaks branch from 0c95b9a to e69c9b0 Compare March 15, 2026 23:16

timlegge approved these changes Mar 15, 2026

View reviewed changes

atoomic merged commit 0b4f820 into cpan-authors:main Mar 16, 2026
27 checks passed

This was referenced Mar 16, 2026

fix: generate_key() error-path resource leaks on OpenSSL 3.x #108

Merged

fix: get_public_key_string() error-path leaks on 3.x #114

Merged

Koan-Bot added a commit to atoomic/Crypt-OpenSSL-RSA that referenced this pull request Mar 16, 2026

rebase: apply review feedback on cpan-authors#81

0a0c0ce

Koan-Bot added a commit to atoomic/Crypt-OpenSSL-RSA that referenced this pull request Mar 16, 2026

rebase: apply review feedback on cpan-authors#81

4010285

Koan-Bot mentioned this pull request Mar 16, 2026

fix: free EVP_MD and EVP_PKEY_CTX before switch in verify() #97

Closed

Conversation

Koan-Bot commented Feb 19, 2026

Summary

Fix

Test plan

Uh oh!

timlegge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

timlegge Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

timlegge Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

atoomic commented Mar 12, 2026

Uh oh!

Koan-Bot commented Mar 12, 2026

Rebase: fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x)

Actions

Uh oh!

atoomic commented Mar 12, 2026

Uh oh!

timlegge Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Koan-Bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

timlegge Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

Koan-Bot commented Mar 14, 2026

Uh oh!

timlegge commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Koan-Bot commented Mar 14, 2026

Rebase: fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x)

Actions

Uh oh!

timlegge commented Mar 15, 2026

Uh oh!

Koan-Bot commented Mar 15, 2026

Uh oh!

timlegge commented Mar 15, 2026

Uh oh!

atoomic commented Mar 15, 2026

Uh oh!

atoomic commented Mar 15, 2026

Uh oh!

Koan-Bot commented Mar 15, 2026

Rebase: fix: verify() resource leaks and dead cleanup code (OpenSSL 3.x)

Actions

Uh oh!

timlegge left a comment

Choose a reason for hiding this comment

Uh oh!

timlegge Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

timlegge commented Mar 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

timlegge commented Mar 14, 2026 •

edited

Loading