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: Support RSA key pairs where q < p without converting to p > q. #1802

Merged
merged 1 commit into from
Nov 11, 2023

Conversation

briansmith
Copy link
Owner

Previously we swapped p and q and calcualted a new qInv if p < q so that we could avoid doing a redunction during the CRT computation. Instead, just do the reduction during CRT as it's cheap. This notably reduces the number of operations we need in bigint, and it eliminates the need for the Prime modulus marker type.

Now there are more things that can go wrong during CRT. First, we may wrongly forget to reduce m_2 mod p; before this wasn't necessary since every element of q was an element of p. Next, we may wrongly use the the value of m_2 mod p instead of m_2 later; before we could do this since previously m_2 mod p == m_2 since m_2 < q < p. Add tests for these cases.

Rewrite the tests for elem_reduced_once given its new constraints.

Previously we swapped p and q and calcualted a new qInv if p < q so
that we could avoid doing a redunction during the CRT computation.
Instead, just do the reduction during CRT as it's cheap. This
notably reduces the number of operations we need in `bigint`, and
it eliminates the need for the `Prime` modulus marker type.

Now there are more things that can go wrong during CRT. First, we
may wrongly forget to reduce m_2 mod p; before this wasn't necessary
since every element of q was an element of p. Next, we may wrongly
use the the value of m_2 mod p instead of m_2 later; before we could
do this since previously m_2 mod p == m_2 since m_2 < q < p. Add
tests for these cases.

Rewrite the tests for `elem_reduced_once` given its new constraints.
@briansmith briansmith self-assigned this Nov 11, 2023
Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Merging #1802 (cd491c9) into main (23975ff) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1802      +/-   ##
==========================================
+ Coverage   96.01%   96.03%   +0.02%     
==========================================
  Files         137      137              
  Lines       20755    20747       -8     
  Branches      226      226              
==========================================
- Hits        19927    19924       -3     
+ Misses        794      789       -5     
  Partials       34       34              
Files Coverage Δ
src/arithmetic/bigint.rs 99.16% <100.00%> (-0.01%) ⬇️
src/arithmetic/bigint/modulus.rs 99.18% <ø> (-0.04%) ⬇️
src/arithmetic/bigint/private_exponent.rs 100.00% <ø> (ø)
src/arithmetic/nonnegative.rs 100.00% <ø> (ø)
src/rsa/keypair.rs 97.34% <100.00%> (+0.20%) ⬆️

... and 2 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@briansmith briansmith merged commit cfa3737 into main Nov 11, 2023
138 checks passed
@briansmith briansmith deleted the b/p_less_than_q branch November 11, 2023 01:10
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