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

ECDSA verification: Use wNAF-based multiplication for non-nistz256 implementations #1768

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

briansmith
Copy link
Owner

No description provided.

The efficiency of this will be improved in future commits.
Previously we did N doublings for G + N doublings for P = 2N doublings.

Now, we do N doublings.
It won't build without modificatoins, so don't add it to the build yet.
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #1768 (faf67e2) into main (be27e8e) will increase coverage by 0.03%.
Report is 3 commits behind head on main.
The diff coverage is 98.38%.

@@            Coverage Diff             @@
##             main    #1768      +/-   ##
==========================================
+ Coverage   96.00%   96.03%   +0.03%     
==========================================
  Files         138      140       +2     
  Lines       20754    21006     +252     
  Branches      226      231       +5     
==========================================
+ Hits        19924    20174     +250     
- Misses        792      795       +3     
+ Partials       38       37       -1     
Files Coverage Δ
crypto/limbs/limbs.c 95.48% <100.00%> (+0.10%) ⬆️
src/bits.rs 100.00% <100.00%> (ø)
src/ec/suite_b/ops/elem.rs 88.40% <ø> (ø)
src/ec/suite_b/ops/p256.rs 100.00% <100.00%> (ø)
src/ec/suite_b/ops/p384.rs 100.00% <100.00%> (ø)
src/ec/suite_b/ops/vartime.rs 100.00% <100.00%> (ø)
src/limb.rs 99.02% <100.00%> (+0.05%) ⬆️
src/ec/suite_b/ops.rs 98.11% <99.02%> (+0.02%) ⬆️
crypto/fipsmodule/ec/wnaf.c 92.50% <92.50%> (ø)

... and 11 files with indirect coverage changes

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

@briansmith
Copy link
Owner Author

Blocked on adding ECDSA verification benchmarks, #1772.

@vkrasnov
Copy link
Contributor

Benchmarking this on an M1 Pro the p384 verification is 39% faster

ecdsa_p384_verify       time:   [342.53 µs 342.97 µs 343.51 µs]
                        change: [-39.333% -39.041% -38.717%] (p = 0.00 < 0.05)

However does verification performance matters enough to justify all the extra code?

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