Skip to content

feat: add check=>1 validation to new_key_from_parameters()#111

Merged
atoomic merged 1 commit intocpan-authors:mainfrom
atoomic:koan.atoomic/new-key-check-option
Mar 17, 2026
Merged

feat: add check=>1 validation to new_key_from_parameters()#111
atoomic merged 1 commit intocpan-authors:mainfrom
atoomic:koan.atoomic/new-key-check-option

Conversation

@Koan-Bot
Copy link
Copy Markdown
Contributor

What

Adds an optional check => 1 parameter to new_key_from_parameters() that validates key consistency at construction time.

Why

Keys built from raw BIGNUM parameters can silently carry inconsistent values (wrong CRT params, mismatched n/e/d/p/q). Without validation, these broken keys only fail at first use — sign, encrypt, etc. — with cryptic OpenSSL errors far from the construction site.

With check => 1, the caller gets an immediate, clear croak at construction if parameters don't form a valid RSA key.

How

Pure Perl change in RSA.pm — after _new_key_from_parameters() returns the XS object, calls check_key() when check => 1 is passed and the key is private. Public-only keys (n, e only) skip the check since OpenSSL can't validate them.

Testing

New t/check_param.t with 7 tests:

  • Valid full params with check => 1 — succeeds
  • Public-only key with check => 1 — succeeds (check skipped)
  • Bad d with check => 1 — croaks with clear error
  • Default (no check) — backward compatible

All 290 core tests pass on OpenSSL 3.6.1.

🤖 Generated with Claude Code

When check=>1 is passed, check_key() is called after key
reconstruction. Inconsistent parameters (e.g. wrong CRT values,
mismatched n/e/d/p/q) now fail at construction time with a clear
error instead of silently producing a broken key.

The check is skipped for public-only keys since they cannot be
validated.

New test file t/check_param.t with 7 tests covering valid params,
public-only keys, bad parameters, and default (no-check) behavior.
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 fine to me and tests okay here

@atoomic atoomic marked this pull request as ready for review March 17, 2026 22:31
@atoomic atoomic merged commit 2a90399 into cpan-authors:main Mar 17, 2026
28 checks passed
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