Add AsDer and ParsedPublicKey conversion for RSA PublicKeyComponents#1105
Open
Add AsDer and ParsedPublicKey conversion for RSA PublicKeyComponents#1105
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1105 +/- ##
==========================================
- Coverage 95.80% 92.68% -3.12%
==========================================
Files 61 71 +10
Lines 8143 10207 +2064
Branches 0 10207 +10207
==========================================
+ Hits 7801 9460 +1659
- Misses 342 451 +109
- Partials 0 296 +296 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
justsmth
reviewed
Apr 24, 2026
Contributor
justsmth
left a comment
justsmth
left a comment
There was a problem hiding this comment.
Instead of ParsedPublicKey::from_rsa_components, I think a PublicKeyComponents::into_parsed would have a better aesthetic and achieve the same goal.
There are a few more edge cases to cover in testing, but it can wait until the public API is decided upon.
Implements `AsDer<PublicKeyX509Der<'static>>` for `rsa::PublicKeyComponents`, serializing the modulus and exponent into an X.509 `SubjectPublicKeyInfo` structure, and adds `ParsedPublicKey::from_rsa_components` which consumes those components plus an RSA verification algorithm. The new `from_rsa_components` path reuses the existing X.509 parser, so non-RSA algorithms and malformed components are rejected through the same code that already validates DER-encoded inputs. Resolves aws#1055.
justsmth
force-pushed
the
rsa-public-key-components-as-der
branch
from
0d26da8 to
85a60ed
Compare
Contributor
|
I wanted to let you know that I updated your PR based upon the feedback I provided. Please let me know if you have any concerns with these changes. Thanks! |
…nents Replaces ParsedPublicKey::from_rsa_components(algorithm, &components) with RsaPublicKeyComponents::to_parsed_public_key(algorithm). This places the conversion on the source type, symmetric with the existing PublicKeyComponents::verify(params, ...) method, and keeps RSA-specific concerns out of signature.rs. Also tightens the error semantics: - non-RSA algorithms are now rejected up front with KeyRejected::wrong_algorithm() rather than surfacing an incidental DER parse failure - bad components (empty/leading-zero n or e) surface as KeyRejected::inconsistent_components() instead of invalid_encoding() Tests assert the exact KeyRejected variants, add PSS coverage, add a non-ring-io round-trip for AsDer<PublicKeyX509Der>, and pin down that the algorithm's bit-size range is enforced at verify_sig time (matching ParsedPublicKey::new).
justsmth
force-pushed
the
rsa-public-key-components-as-der
branch
from
85a60ed to
63256c6
Compare
The previous implementation went components -> build_rsa (EVP_PKEY) -> marshal RFC 5280 SPKI DER -> parse_rsa_public_key (tries RFC 8017 first, falls back to RFC 5280) -> build another EVP_PKEY. The second EVP_PKEY construction and the failed RFC 8017 parse attempt were both wasted work. Add a crate-internal ParsedPublicKey::from_rsa_evp_pkey constructor that consumes an already-built LcPtr<EVP_PKEY> and marshals the SPKI DER once to populate the bytes field (needed for AsRef<[u8]>). Route to_parsed_public_key through it. Also fully gate test_rsa_public_key_components_as_der_round_trip on ring-io (its only purpose is exercising the ring-io-gated From<&PublicKey> accessor) and drop the outer scaffolding that duplicated rsa_test_public_key_coverage. The default-feature AsDer path remains covered by test_rsa_public_key_components_as_der_round_trip_no_ring_io.
- Use ? instead of map_err for the from_rsa_evp_pkey error path so that a marshalling failure is reported as KeyRejected::unspecified() rather than InconsistentComponents, which would misattribute the cause. - Add test_rsa_public_key_components_as_der_rejects_empty_modulus to explicitly cover the AsDer rejection path for invalid components.
justsmth
force-pushed
the
rsa-public-key-components-as-der
branch
from
0ae248f to
4be1423
Compare
…() outputs Confirms that both construction paths produce byte-for-byte identical canonical DER encodings, guarding against future drift.
justsmth
approved these changes
May 5, 2026
Contributor
|
Clippy CI failure addressed in separate PR: #1112 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues:
Addresses #1055
Description of changes:
Implements
AsDer<PublicKeyX509Der<'static>>forrsa::PublicKeyComponents, which builds anEVP_PKEYfrom the(n, e)components and marshals it as an X.509SubjectPublicKeyInfo. Also addsPublicKeyComponents::to_parsed_public_key, which takes an&'static RsaParametersand returns asignature::ParsedPublicKeybuilt directly from the sameEVP_PKEY, skipping a DER encode/decode round-trip. Placing the conversion on the source type is symmetric with the existingPublicKeyComponents::verify(params, ...)method, keeps RSA-specific code out ofsignature.rs, and narrowsparamstoRsaParametersso non-RSA algorithms are rejected at compile time rather than at runtime. Malformed components (empty or leading-zeron/e) are rejected by the existingbuild_rsa()helper and surface asKeyRejected::inconsistent_components()on both theto_parsed_public_keyandAsDerpaths.Call-outs:
Consistent with
ParsedPublicKey::new,to_parsed_public_keydoes not enforce the algorithm's accepted bit-size range at construction — that check is deferred toverify_sig. This is documented on the new method and pinned down by a test.Testing:
unit tests (PKCS#1 v1.5 and PSS verification; non-RSA algorithm, empty-modulus, and below-range rejection; X.509 round-trip covered both in the default feature configuration and under
ring-io)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.