Skip to content

fix: remove OSSL_LIB_CTX per-call overhead in rsa_crypt()#83

Merged
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/remove-ossl-lib-ctx-rsa-crypt
Mar 16, 2026
Merged

fix: remove OSSL_LIB_CTX per-call overhead in rsa_crypt()#83
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/remove-ossl-lib-ctx-rsa-crypt

Conversation

@Koan-Bot
Copy link
Copy Markdown
Contributor

@Koan-Bot Koan-Bot commented Mar 11, 2026

What

Remove unnecessary OSSL_LIB_CTX_new()/OSSL_LIB_CTX_free() calls on every encrypt/decrypt operation on OpenSSL 3.x.

Why

rsa_crypt() created a new library context per call for public key operations, while private key operations already used the default (NULL) context. There's no reason for a custom context here — no custom providers or property queries are involved. The per-call allocation adds overhead (provider initialization) with zero benefit.

How

Replaced the asymmetric public/private code paths with a single EVP_PKEY_CTX_new_from_pkey(NULL, key, NULL) call for both. The NULL first argument uses OpenSSL's default library context, matching what private key operations were already doing.

-9 lines, +1 line.

Testing

Full test suite passes (281 tests) on Perl 5.42.0 / OpenSSL 3.6.1 (macOS).


🤖 Generated with Claude Code


Quality Report

Changes: 1 file changed, 1 insertion(+), 9 deletions(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

@timlegge
Copy link
Copy Markdown
Member

Seems fine but I seem to recall adding this for a reason. I wonder if there was a version issue.

@atoomic atoomic marked this pull request as ready for review March 12, 2026 02:21
@Koan-Bot
Copy link
Copy Markdown
Contributor Author

Good question — I dug into the git history to understand the original intent.

The OSSL_LIB_CTX_new() came in with commit 9a265ae ("Fully support openSSL 3.x API"). Looking at the original code, it seems like the pattern was based on OpenSSL 3.x migration docs that show OSSL_LIB_CTX_new() + EVP_PKEY_CTX_new_from_pkey() as the "proper" way to do EVP operations. The code created a custom context for the public key path but used EVP_PKEY_CTX_new() (with implicit NULL context) for the private key path.

Two things suggest it was scaffolding rather than intentional design:

  1. The propquery variable was uninitializedconst char* propquery; was declared but never assigned, so EVP_PKEY_CTX_new_from_pkey(ossllibctx, key, propquery) was passing garbage as the property query string. Since no custom providers or property queries are actually needed, this didn't matter in practice (OpenSSL ignores invalid propqueries on the default provider), but it shows the OSSL_LIB_CTX wasn't being used for any specific purpose.

  2. Public vs private asymmetry — there's no technical reason to use a custom library context for public key operations but not private key operations. Both go through the same OpenSSL provider under the hood.

OSSL_LIB_CTX_new() is designed for applications that need to load custom providers (PKCS#11, FIPS module isolation, etc.) or set property queries. For a basic RSA encrypt/decrypt, the default context (NULL) is what the OpenSSL docs actually recommend.

The change unifies both paths to EVP_PKEY_CTX_new_from_pkey(NULL, key, NULL) which is the idiomatic OpenSSL 3.x pattern for default-provider operations, and also fixes the uninitialized propquery along the way.

@atoomic atoomic requested a review from timlegge March 12, 2026 17:15
Copy link
Copy Markdown
Member

@timlegge timlegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@atoomic
Copy link
Copy Markdown
Collaborator

atoomic commented Mar 15, 2026

@Koan-Bot rebase

@Koan-Bot
Copy link
Copy Markdown
Contributor Author

Rebase: fix: remove OSSL_LIB_CTX per-call overhead in rsa_crypt()

Branch koan.atoomic/remove-ossl-lib-ctx-rsa-crypt rebased onto main and force-pushed.

Diff: 1 file changed, 1 insertion(+), 9 deletions(-)

Review feedback was analyzed and applied.

Actions

  • Resolved merge conflicts (1 round(s))
  • Rebased koan.atoomic/remove-ossl-lib-ctx-rsa-crypt onto upstream/main
  • Force-pushed koan.atoomic/remove-ossl-lib-ctx-rsa-crypt to origin

Automated by Kōan

@Koan-Bot Koan-Bot force-pushed the koan.atoomic/remove-ossl-lib-ctx-rsa-crypt branch from 8bd1297 to cb047ce Compare March 15, 2026 23:13
Koan-Bot added a commit to Koan-Bot/koan that referenced this pull request Mar 15, 2026
When a PR branch lives on a fork (origin) but targets an upstream repo,
plain `git rebase upstream/main` replays every commit since the fork
diverged — not just the PR's own commits. This caused PRs to balloon
from 1 commit to 50+ after rebase (e.g. cpan-authors/Crypt-OpenSSL-RSA#83).

Fix: use `git rebase --onto target/base fork/base` when the branch was
fetched from a different remote than the rebase target. This limits the
replay to only commits between fork/base and HEAD (the actual PR commits).

Changes:
- _checkout_pr_branch now returns the remote it fetched from
- _rebase_with_conflict_resolution accepts branch_remote and uses --onto
- _rebase_onto_target (claude_step.py) gets the same --onto logic
- pr_review.py passes branch_remote="origin" to _rebase_onto_target

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@timlegge
Copy link
Copy Markdown
Member

@atoomic there seems to be an issue here with the rebase

@timlegge
Copy link
Copy Markdown
Member

@Koan-Bot rebase

On OpenSSL 3.x, rsa_crypt() created a new OSSL_LIB_CTX on every
encrypt/decrypt call, but only used it for the public key path.
Private key operations already used EVP_PKEY_CTX_new() with the
default (NULL) context.

Replace both paths with a single EVP_PKEY_CTX_new_from_pkey(NULL, ...)
call, eliminating the per-call OSSL_LIB_CTX_new()/free() overhead.
The default library context is appropriate here — no custom providers
or property queries are needed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Koan-Bot
Copy link
Copy Markdown
Contributor Author

Rebase: fix: remove OSSL_LIB_CTX per-call overhead in rsa_crypt()

Branch koan.atoomic/remove-ossl-lib-ctx-rsa-crypt rebased onto main and force-pushed.

Diff: 1 file changed, 1 insertion(+), 9 deletions(-)

Review feedback was analyzed and applied.

Actions

  • Rebased koan.atoomic/remove-ossl-lib-ctx-rsa-crypt onto upstream/main
  • Force-pushed koan.atoomic/remove-ossl-lib-ctx-rsa-crypt to origin

Automated by Kōan

@Koan-Bot Koan-Bot force-pushed the koan.atoomic/remove-ossl-lib-ctx-rsa-crypt branch from cb047ce to 85fce04 Compare March 16, 2026 02:09
Copy link
Copy Markdown
Member

@timlegge timlegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atoomic looks good

@atoomic atoomic merged commit fcee50e into cpan-authors:main Mar 16, 2026
27 checks passed
@Koan-Bot
Copy link
Copy Markdown
Contributor Author

Rebase: fix: remove OSSL_LIB_CTX per-call overhead in rsa_crypt()

Branch koan.atoomic/remove-ossl-lib-ctx-rsa-crypt rebased onto main and force-pushed.

Review feedback was analyzed and applied.

Actions

  • Rebased koan.atoomic/remove-ossl-lib-ctx-rsa-crypt onto upstream/main
  • Force-pushed koan.atoomic/remove-ossl-lib-ctx-rsa-crypt to origin

Automated by Kōan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants