NFC: Split internal bigint module into submodules
#1641
Merged
Conversation
This file contains 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
Clarify that `Nonnegative` values are immutable by enforcing this through the module system. Some read-only to-be-refactored-away methods of `Nonnegative` stay in `bigint` to avoid moving them back-and-forth later. `git diff HEAD^1:src/arithmetic/bigint.rs src/arithmetic/nonnegative.rs`
Better encapsulate `PrivateExponent` and enforce its immutability. `git diff HEAD^1:src/arithmetic/bigint.rs src/arithmetic/bigint/private_exponent.rs`
Better encapsulate `Modulus` and `PartialModulus`. `git diff HEAD^1:src/arithmetic/bigint.rs src/arithmetic/bigint/modulus.rs`
Ensure all the functions responsible for maintaining invariants for `Modulus` values are within `Modulus`. Clarify the constraints on the relationship between the moduli in `Modulus::from_elem`.
`git diff HEAD^1:src/arithmetic/bigint.rs src/arithmetic/bigint/n0.rs`
When constructing a `PrivateExponent` we enforce that the exponent is appropriately-sized for its associated modulus; this check is relied on in RSA private key construction for key component consistency checks. However, once the `PrivateExponent` is constructed there is no reason to relate its value to the modulus. Doing so has inhibited us from using some test vectors that are in the BoringSSL test suite. Further this usage blocks encapsulating `BoxedLimbs` into its own submodule.
`git diff HEAD^1:src/arithmetic/bigint.rs src/arithmetic/bigint/boxed_limbs.rs`
briansmith
force-pushed
the
b/bigint-split-1
branch
from
db7f079 to
90e16dd
Compare
Codecov Report
@@ Coverage Diff @@
## main #1641 +/- ##
==========================================
+ Coverage 92.21% 92.22% +0.01%
==========================================
Files 127 132 +5
Lines 18821 18856 +35
Branches 196 196
==========================================
+ Hits 17355 17390 +35
Misses 1430 1430
Partials 36 36
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The primary goal of this change is to make progress towards a better
arithmeticAPI that abstracts over the storage (BoxedLimbs, etc.) to better facilitate doing more arithmetic in Rust instead of C, and to reduce the amount of heap allocation done for RSA operations.The secondary goal is to unblock the merge of the most recent BoringSSL changes into ring. This is an issue only accidentally due to the timing of when I last merged BoringSSL into ring vs. when I made these changes.