fix: prevent resource leaks in rsa_crypt() on OpenSSL 3.x

[toddr-bot]

What

Free OSSL_LIB_CTX, EVP_PKEY_CTX, and the output buffer on all error paths in rsa_crypt() before croaking.

Why

On OpenSSL 3.x, rsa_crypt() allocates three resources (to buffer, ossllibctx, ctx) then runs five CHECK_OPEN_SSL checks. Each CHECK_OPEN_SSL expands to croakSsl() which does a Perl croak (longjmp) — skipping the cleanup at lines 356-357. Every failed encrypt/decrypt operation leaks memory.

The PSS padding early-exit croak at line 334 also leaked the to buffer allocated just above it.

How

Replaced five individual CHECK_OPEN_SSL calls with a single short-circuit conditional. On failure, all three resources are freed before calling croakSsl(). EVP_PKEY_CTX_free(NULL) and OSSL_LIB_CTX_free(NULL) are documented no-ops, so cleanup is safe regardless of which step fails.

Also added Safefree(to) before the PSS padding croak.

Testing

• Build and full test suite (318 tests) pass on OpenSSL 1.1.1k
• The 3.x code path is #if-guarded and not compiled on this system; the fix is structural and follows the same cleanup pattern used elsewhere in _new_key_from_parameters() (the err: label at line 686)
• Verified EVP_PKEY_CTX_free and OSSL_LIB_CTX_free are NULL-safe per OpenSSL docs

🤖 Generated with Claude Code

---

Quality Report

Changes: 1 file changed, 14 insertions(+), 12 deletions(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

[atoomic]

@toddr-bot rebase

[Koan-Bot]

PR Review — fix: prevent resource leaks in rsa_crypt() on OpenSSL 3.x

This PR fixes real bugs (resource leaks in rsa_crypt() error paths and the PSS padding early-exit leak), but all of these issues have already been addressed on main by merged PRs #79 (THROW/goto err pattern) and #83 (OSSL_LIB_CTX removal). The branch is stale and needs rebasing as @atoomic requested. After rebase, this PR will likely have an empty diff and can be closed.

---

🔴 Blocking

1. PR is stale — OSSL_LIB_CTX already removed on main (RSA.xs, L338)
This PR still allocates OSSL_LIB_CTX *ossllibctx = OSSL_LIB_CTX_new() and passes it to EVP_PKEY_CTX_new_from_pkey(ossllibctx, ...). However, PR #83 (fcee50e) was already merged into main and removed the per-call OSSL_LIB_CTX allocation entirely, passing NULL instead. Additionally, PR #79 (467635a) already converted the CHECK_OPEN_SSL calls to the THROW/goto err cleanup pattern that this PR is trying to achieve with the short-circuit conditional.

After rebasing onto current main, this PR will either have no meaningful diff (the leaks are already fixed) or will conflict heavily. The rebase requested by @atoomic is the right next step.

OSSL_LIB_CTX *ossllibctx = OSSL_LIB_CTX_new();

🟡 Important

1. Short-circuit chain loses per-step error granularity (RSA.xs, L344-353)
Collapsing five separate checks into a single if (!ctx || init_crypt(ctx) != 1 || ... || ...) means that when any step fails, croakSsl reports the line number of the combined conditional rather than the specific failing operation. The existing code on main (from PR #79) uses individual THROW() macros which preserve per-step line numbers via __LINE__. This is a minor debuggability regression compared to the approach already merged.

This is moot given the staleness issue above, but worth noting for the pattern.

if (!ctx
        || init_crypt(ctx) != 1
        || EVP_PKEY_CTX_set_rsa_padding(ctx, ...) <= 0
        || p_crypt(ctx, NULL, &to_length, from, from_length) != 1
        || p_crypt(ctx, to, &to_length, from, from_length) != 1)

🟢 Suggestions

1. PSS Safefree fix is valid but already merged (RSA.xs, L333-336)
The Safefree(to) before the PSS padding croak is a correct fix — the original code did leak the to buffer. However, this fix was already included in PR #79 which is merged on main. No action needed.

if(p_rsa->padding == RSA_PKCS1_PSS_PADDING) {
        Safefree(to);
        croak("PKCS#1 v2.1 RSA-PSS...");
    }

---

Checklist

• No resource leaks on error paths
• NULL-safe cleanup calls
• Branch up-to-date with main — critical quietened critic to -3, added test #1
• Error diagnostics preserved — warning quietened critic to -3, added test #1

---

Summary

This PR fixes real bugs (resource leaks in rsa_crypt() error paths and the PSS padding early-exit leak), but all of these issues have already been addressed on main by merged PRs #79 (THROW/goto err pattern) and #83 (OSSL_LIB_CTX removal). The branch is stale and needs rebasing as @atoomic requested. After rebase, this PR will likely have an empty diff and can be closed.

---

Automated review by Kōan

[Koan-Bot]

@atoomic: @toddr-bot rebase

Agreed — this PR needs a rebase. Both PR #79 (467635a, merged) and PR #83 (fcee50e, merged) have already landed on main and together they address the same resource leaks this PR targets:

• PR fix: rsa_crypt() resource leaks on error paths (OpenSSL 3.x) #79 converted the CHECK_OPEN_SSL calls to the THROW/goto err cleanup pattern with proper Safefree(to) and EVP_PKEY_CTX_free(ctx) on all error paths.
• PR fix: remove OSSL_LIB_CTX per-call overhead in rsa_crypt() #83 removed the OSSL_LIB_CTX per-call allocation entirely, eliminating that leak category.

After rebasing, this PR will likely conflict on the entire #if OPENSSL_VERSION_NUMBER >= 0x30000000L block in rsa_crypt(). If all changes are already covered, it may be simplest to close this PR rather than resolve the conflicts.

[toddr-bot]

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

[toddr-bot]

Rebase: fix: prevent resource leaks in rsa_crypt() on OpenSSL 3.x

Branch koan.toddr.bot/fix-rsa-crypt-resource-leaks rebased onto main and force-pushed.

Review feedback was analyzed and applied.

Actions

• Resolved merge conflicts (1 round(s))
• Rebased koan.toddr.bot/fix-rsa-crypt-resource-leaks onto upstream/main
• Force-pushed koan.toddr.bot/fix-rsa-crypt-resource-leaks to origin

---

Automated by Kōan