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

Avoid an inversion during ECDSA/SM2/etc signature verification #4211

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

randombit
Copy link
Owner

Instead of converting the multiscalar product to affine and extracting the x coordinate for comparison, project the verification value. This saves an inversion and improves ECDSA verification by 4-9%

@coveralls
Copy link

coveralls commented Jul 14, 2024

Coverage Status

coverage: 91.709% (-0.006%) from 91.715%
when pulling ef6efa3 on jack/project-r
into 5fd660b on master.

@randombit randombit added this to the Botan 3.6.0 milestone Jul 14, 2024
@randombit randombit requested a review from reneme July 14, 2024 12:40
src/lib/pubkey/ec_group/ec_group.h Outdated Show resolved Hide resolved
src/lib/pubkey/ec_group/ec_inner_bn.cpp Outdated Show resolved Hide resolved
…ality check

@gmaxwell pointed out in a really great comment on #1479 that you
don't need to actually perform a projective->affine conversion in
ECDSA verification, since instead you can project the r value.

However in the current setup that's not possible since the function is
defining as returning the value and then the comparison happens in the
pubkey code. Instead have the expected value be passed down and all
that comes back is a boolean accept or reject. This allows the
project-r optimization.

This also avoids some back and forth with the various type wrappers,
which is a small win on its own.
Given a projective coordinate, we previously performed an inversion
to extract the affine x. But instead we can project v using the
same value of z as the projective point.

This improves ECDSA verification performance by between 4% and 9%,
depending on the curve.
@randombit randombit merged commit 55007af into master Jul 15, 2024
39 checks passed
@randombit randombit deleted the jack/project-r branch July 15, 2024 08:34
randombit added a commit that referenced this pull request Jul 15, 2024
This is the same trick as #4211 applied in the pcurves implementation.

Improves ECDSA verification by 6-12% depending on the curve.
randombit added a commit that referenced this pull request Jul 15, 2024
This is the same trick as #4211 applied in the pcurves implementation.

Improves ECDSA verification by 6-12% depending on the curve.
randombit added a commit that referenced this pull request Jul 15, 2024
This is the same trick as #4211 applied in the pcurves implementation.

Improves ECDSA verification by 6-12% depending on the curve.
randombit added a commit that referenced this pull request Jul 17, 2024
The trick in #4211 doesn't work correctly for curves with cofactors.
Since such curves are weird and deprecated, just force to affine
for such curves, rather than complicating the fast path.

Fixes #4219
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.

3 participants