fix: reject non-RSA keys on OpenSSL 3.x

[toddr-bot]

Summary

• Validates that loaded keys are actually RSA after PEM parsing on OpenSSL 3.x
• Rejects EC, DSA, and other non-RSA keys with a clear error message
• Adds test coverage for non-RSA key rejection

Why

On OpenSSL 3.x, the compatibility macros remap PEM_read_bio_RSAPrivateKey to the generic PEM_read_bio_PrivateKey, which accepts any key type. This means loading an EC key as RSA silently "succeeds" — but is_private() returns false (because EVP_PKEY_get_bn_param for RSA's d fails), and crypto operations produce confusing errors like "Public keys cannot sign messages" instead of telling the user the key isn't RSA.

Pre-3.x used RSA-specific loaders that rejected non-RSA keys at parse time.

How

Added EVP_PKEY_get_base_id() != EVP_PKEY_RSA check in _load_rsa_key() after successful PEM loading on 3.x. This is the single loading function used by all three key loaders (new_private_key, _new_public_key_pkcs1, _new_public_key_x509), so one check covers all entry points.

Testing

• Verified EC private/public keys are rejected with "not an RSA key"
• Verified RSA keys in all formats (private, PKCS1 public, X509 public) still load correctly
• Full test suite passes: 511 tests (507 existing + 4 new)
• Built and tested on OpenSSL 3.5.1

🤖 Generated with Claude Code

---

Quality Report

Changes: 2 files changed, 30 insertions(+), 1 deletion(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

[atoomic]

@Koan-Bot review

[Koan-Bot]

PR Review — fix: reject non-RSA keys on OpenSSL 3.x

Clean, well-targeted fix. The C code correctly validates key type after PEM loading on OpenSSL 3.x, frees the key before croaking to avoid leaks, and is placed in the single loading function that covers all entry points. The test coverage is appropriate with proper skip guards. The only non-trivial concern is shell interpolation of PEM content in backticks (warning), plus a documentation note about RSA-PSS key handling. None of these are blocking — the PR is merge-ready.

---

🟡 Important

1. Shell interpolation of Perl variable in backticks (`t/format.t`, L170)

Interpolating $ec_pem directly into a shell command via backticks is fragile. While PEM base64 content won't contain shell metacharacters in practice, this pattern is brittle (e.g., $ in a hypothetical future test string would be interpreted by the shell, and echo behavior varies across platforms for certain escape sequences).

Safer alternative using a pipe open:

my $ec_pub;
{
    open my $fh, '|-', 'openssl pkey -pubout 2>/dev/null' or skip ...;
    # or use IPC::Open2 / pipe
}

Or simplest fix — write $ec_pem to a temp file and use -in:

use File::Temp qw(tempfile);
my ($fh, $tmpfile) = tempfile(UNLINK => 1);
print $fh $ec_pem;
close $fh;
my $ec_pub = `openssl pkey -in $tmpfile -pubout 2>/dev/null`;

This avoids shell interpretation entirely and works consistently across platforms.

my $ec_pub = `echo "$ec_pem" | openssl pkey -pubout 2>/dev/null`;

🟢 Suggestions

1. Consider adding RSA-PSS key type to rejection tests (`t/format.t`, L158-160)

On OpenSSL 3.x, EVP_PKEY_get_base_id() returns EVP_PKEY_RSA_PSS (not EVP_PKEY_RSA) for RSA-PSS keys. If a user loads an RSA-PSS key, it will also be rejected by this check. This may be intentional, but it would be worth either:

1. Adding a test for RSA-PSS key rejection to document this behavior, or
2. Adding a comment in RSA.xs noting that RSA-PSS keys are intentionally excluded.

Generate with: openssl genpkey -algorithm RSA-PSS -pkeyopt rsa_keygen_bits:2048

2. Consider whether RSA-PSS keys should be accepted (`RSA.xs`, L335-340)

EVP_PKEY_get_base_id() returns EVP_PKEY_RSA_PSS (6) for RSA-PSS keys, which is distinct from EVP_PKEY_RSA (6). This means RSA-PSS keys will be rejected with "not an RSA key" even though they are RSA keys (just with restricted parameters). This is probably fine for this module's scope, but worth a brief comment documenting the intentional exclusion, e.g.:

/* Also rejects RSA-PSS keys (EVP_PKEY_RSA_PSS) — this module
   only supports traditional RSA (EVP_PKEY_RSA). */

if (EVP_PKEY_get_base_id(rsa) != EVP_PKEY_RSA) {

---

Checklist

• No hardcoded secrets
• Resource cleanup on error paths
• Shell injection in test commands — warning quietened critic to -3, added test #1
• Test plan count matches actual tests
• Test isolation (no shared state)
• Edge case coverage

---

Summary

Clean, well-targeted fix. The C code correctly validates key type after PEM loading on OpenSSL 3.x, frees the key before croaking to avoid leaks, and is placed in the single loading function that covers all entry points. The test coverage is appropriate with proper skip guards. The only non-trivial concern is shell interpolation of PEM content in backticks (warning), plus a documentation note about RSA-PSS key handling. None of these are blocking — the PR is merge-ready.

---

Automated review by Kōan

[timlegge]

@toddr-bot fix the potential Shell interpolation of Perl variable in backticks as recommended

[atoomic]

@toddr-bot rebase

[toddr-bot]

Rebase: fix: reject non-RSA keys on OpenSSL 3.x

Branch koan.toddr.bot/fix-key-type-validation rebased onto main and force-pushed.

Diff: 2 files changed, 63 insertions(+), 1 deletion(-)

Review feedback was analyzed and applied.

Changes

action previously used echo "$ec_pem" | openssl pkey -pubout which interpolates PEM content through the shell via backticks. Replaced with File::Temp — writes PEM to a temp file and passes -in $tmpfile to openssl. This avoids shell interpretation entirely and is portable across platforms. Per reviewer @Koan-Bot's warning about shell interpolation fragility.

• Added RSA-PSS exclusion comment (RSA.xs): Added comment documenting that EVP_PKEY_RSA_PSS keys are intentionally rejected since this module only supports traditional EVP_PKEY_RSA. Per reviewer @Koan-Bot's suggestion.
• Added RSA-PSS rejection tests (t/format.t): Added 4 new tests verifying RSA-PSS private and public keys are rejected with appropriate error messages, with skip guards for environments where RSA-PSS key generation isn't available. Uses the same temp file pattern for public key extraction. Per reviewer @Koan-Bot's suggestion to document RSA-PSS behavior via tests. Test plan updated from 43 to 47.

Actions

• Already-solved check: negative (confidence=high, reasoning=None of the recent commits on main address non-RSA key type validation in _load_rsa_key() for OpenSS)
• Rebased koan.toddr.bot/fix-key-type-validation onto upstream/main
• Applied review feedback
• Force-pushed koan.toddr.bot/fix-key-type-validation to origin
• CI check enqueued (async)

CI

CI will be checked asynchronously.

---

Automated by Kōan

[timlegge]

Looks fine. We should probably log an issue about RSA-PSS not being supported though

[timlegge]

@toddr-bot rebase from main

[toddr-bot]

Rebase: fix: reject non-RSA keys on OpenSSL 3.x

Branch koan.toddr.bot/fix-key-type-validation rebased onto main and force-pushed.

Diff: 2 files changed, 69 insertions(+), 1 deletion(-)

Review feedback was analyzed and applied.

Actions

• Already-solved check: negative (confidence=high, reasoning=None of the recent commits on main address non-RSA key type validation in _load_rsa_key(); the close)
• Resolved merge conflicts (2 round(s))
• Rebased koan.toddr.bot/fix-key-type-validation onto upstream/main
• Force-pushed koan.toddr.bot/fix-key-type-validation to origin
• CI check enqueued (async)

CI

CI will be checked asynchronously.

---

Automated by Kōan

[timlegge]

looks fine