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

bigint: Provide a fallback implementation for bn_mul_mont. #1558

Merged
merged 4 commits into from
Nov 14, 2022

Conversation

briansmith
Copy link
Owner

@briansmith briansmith commented Nov 11, 2022

Add an implementation of bn_mul_mont. Intentionally break the build on big-endian targets so they don't accidentally build/run with incorrect logic. See the individual commit messages for more details. Add mipsel-unknown-linux-gnu to the GitHub Actions test matrix.

Provide an implementation of `bn_mul_mont` that works on all targets that
don't have an assembly language implementation.

Expand `prefixed_export!` to support prefixing functions defined in Rust.
Function definitions don't end with a semicolon so move the semicolon
insertion from `prefixed_item!` to its callers.

Unify the codepaths in `bigint` so that `bn_mul_mont` is always used.
@briansmith briansmith self-assigned this Nov 11, 2022
@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Merging #1558 (81f4e8d) into main (3915942) will increase coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 81f4e8d differs from pull request most recent head 54520a3. Consider uploading reports for the commit 54520a3 to get more accurate results

@@            Coverage Diff             @@
##             main    #1558      +/-   ##
==========================================
+ Coverage   92.24%   92.25%   +0.01%     
==========================================
  Files         127      127              
  Lines       18850    18811      -39     
  Branches      196      196              
==========================================
- Hits        17389    17355      -34     
+ Misses       1424     1421       -3     
+ Partials       37       35       -2     
Impacted Files Coverage Δ
src/arithmetic/bigint.rs 99.23% <ø> (-0.03%) ⬇️
src/lib.rs 29.16% <0.00%> (ø)
crypto/poly1305/poly1305.c 97.20% <0.00%> (+0.46%) ⬆️
src/io/der_writer.rs 100.00% <0.00%> (+2.17%) ⬆️
crypto/cpu_intel.c 69.01% <0.00%> (+4.22%) ⬆️

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

@briansmith
Copy link
Owner Author

Note that one commit in this PR is by @LinusU from PR #1120.

Take MIPSEL out of the GitHub Actions configuration because it fails to
link, and because it makes the build matrix too large.
@ghost
Copy link

ghost commented Nov 12, 2022

Do you know that the machine code this produces executes in constant time?

Can you be sure that future improvements to rustc will maintain those constant time properties?

This isn't something that can be tested for or "fuzzed" away. Throwing random test vectors at a routine will give you back white noise timing plots. The sophisticated attacks involve thinking about what kinds of inputs would create carries in certain bit positions, and possibly even using feedback to make those decisions. The necessary countermeasure is to be correct by construction for anything handling raw key material. Chacha20 makes this easy, and it can be done portably for AES if you accept a performance hit. But RSA is a minefield.

@briansmith briansmith merged commit 450ada2 into main Nov 14, 2022
@briansmith briansmith deleted the b/bn_mul_mont-polyfill branch November 14, 2022 18:55
light4 added a commit to light4/ring that referenced this pull request Jan 10, 2023
@light4 light4 mentioned this pull request Jan 10, 2023
light4 added a commit to light4/ring that referenced this pull request Jan 10, 2023
sameo pushed a commit to sameo/ring that referenced this pull request Feb 24, 2023
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.

2 participants