Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSA signature verification: Avoid wasteful key re-serialization. #1790

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

briansmith
Copy link
Owner

When we added rsa::PublicKey we changed the ring::signature RSA implementation to construct an rsa::PublicKey and then verify the signature using it. Unfortunately for backward compatibility with old uses of RsaKeyPair, rsa::PublicKey constructor constructs (and allocates) a copy of the ASN.1-serialized public key. This is not acceptable for users who are using ring::signature to verify a single signature. Refactor PublicKey so that it can be bypassed by the ring::signature implementation.

This is a step towards implementing allocation-free RSA signature verification.

When we added `rsa::PublicKey` we changed the `ring::signature` RSA
implementation to construct an `rsa::PublicKey` and then verify the
signature using it. Unfortunately for backward compatibility with old
uses of `RsaKeyPair`, `rsa::PublicKey` constructor constructs (and
allocates) a copy of the ASN.1-serialized public key. This is not
acceptable for users who are using `ring::signature` to verify a
single signature. Refactor `PublicKey` so that it can be bypassed
by the `ring::signature` implementation.

This is a step towards implementing allocation-free RSA signature
verification.
@briansmith briansmith self-assigned this Nov 2, 2023
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #1790 (0af468d) into main (a9b8882) will decrease coverage by 0.01%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1790      +/-   ##
==========================================
- Coverage   96.02%   96.02%   -0.01%     
==========================================
  Files         138      138              
  Lines       20794    20816      +22     
  Branches      226      226              
==========================================
+ Hits        19968    19988      +20     
- Misses        790      792       +2     
  Partials       36       36              
Files Coverage Δ
src/rsa/keypair.rs 96.59% <100.00%> (ø)
src/rsa/public_key.rs 99.09% <100.00%> (+0.22%) ⬆️
src/rsa/public_key_components.rs 46.15% <100.00%> (ø)
src/rsa/verification.rs 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@briansmith briansmith merged commit 8ed4860 into main Nov 3, 2023
137 of 138 checks passed
@briansmith briansmith deleted the b/rsa-inner-key branch November 3, 2023 00:26
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.

1 participant