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

[EC] Unify scalar multiplication of the base point for ec_nistp curves #1720

Closed
wants to merge 37 commits into from

Conversation

dkostic
Copy link
Contributor

@dkostic dkostic commented Jul 24, 2024

Issues:

CryptoAlg-2544

Description of changes:

Added unified scalar multiplication of the base point for curves implemented in ec_nistp.
For this, we implement exactly the same algorithm as the one already implemented for
scalar multiplication of an arbitrary point with one difference -- we use a precomputed
table of multiples of the base point.

Measured on Apple M1:
P-384 scalar_mul_base from 26863 to 28579 ops/s,
P-521 scalar_mul_base from 18051 to 18401 ops/s,

Measured on Intel(R) Xeon(R) Platinum 8488C:
P-384 scalar_mul_base from 22872 to 22962 ops/s,
P-521 scalar_mul_base from 15090 to 14153 ops/s,

Call-outs:

CryptoAlg-2772 captures the issue with the FIPS module size for the static build for Arm. In this PR I temporarily moved the EC precomputed tables outside of the module. Once CryptoAlg-2772 is resolved we'll move them back in.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@dkostic dkostic requested a review from a team as a code owner July 24, 2024 16:32
@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.75%. Comparing base (0cfd3ed) to head (70879b6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1720      +/-   ##
==========================================
- Coverage   78.77%   78.75%   -0.03%     
==========================================
  Files         590      591       +1     
  Lines      101482   101441      -41     
  Branches    14396    14384      -12     
==========================================
- Hits        79945    79892      -53     
- Misses      20902    20914      +12     
  Partials      635      635              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dkostic dkostic marked this pull request as draft September 30, 2024 18:41
@dkostic
Copy link
Contributor Author

dkostic commented Nov 11, 2024

To facilitate easier code review I'm closing this PR in favor of several smaller ones, starting with #1982 and #1983.

@dkostic
Copy link
Contributor Author

dkostic commented Nov 11, 2024

To facilitate easier code review I'm closing this PR in favor of several smaller ones, starting with #1982 and #1983.

@dkostic dkostic closed this Nov 11, 2024
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