Add some tests covering edge cases in the ECDSA r <=> x comparison#206
Merged
Add some tests covering edge cases in the ECDSA r <=> x comparison#206
Conversation
botovq
approved these changes
Jan 14, 2026
Contributor
botovq
left a comment
There was a problem hiding this comment.
This looks good to me. I tested this against LibreSSL's ECDSA verifier (which covers all the curves except `secp160* and secp192*) and we agree on the results. That's not too surprising since we're not trying to be smart.
Nit: your commit message has "the generation script to cover all the covers in Wycheproof"
This upstreams some tests from BoringSSL, though I've taken another pass at the generation script to cover all the curves in Wycheproof. (NB: I've only run the test vectors against an implementation for BoringSSL's curves. It is possible I've gotten the obscure curves wrong.) ECDSA verification involves computing some point (x, y) and then checking if x mod n = r. x is only reduced mod p, so this leads to some edge cases right when x does and does not need a reduction. The existing test vectors covered some of these, but add some missing ones, so we catch both sides of the boundary condition. Additionally, there is an optimized variant which leads to another boundary condition. EC points are typically implemented with Jacobian coordinates, (X:Y:Z) => (x, y) = (X/Z², Y/Z³). This avoids expensive inversions in the point addition formulae. Only when extracting the final affine coordinates do you invert a field element, to divide by Z. Naively, ECDSA verification requires one such inversion. However, this inversion can be avoided in ECDSA verification by observing that comparing X/Z² to r is the same as comparing X to r*Z². However, this works mod p instead of mod n and we must deal with this mismatch. In most cases, n < p, where we must check two cases: r == x and r == x - n. 0 <= r < n < p and 0 <= x < p, so the first case is equivalent to checking x = r (mod p), where we can apply our optimization. The second is equivalent to x == r + n, which in turn is equivalent to checking that r + n < p and then x == r + n (mod p), where we can also apply our optimization. This leads to some other edge cases where we don't check the second case when we should, or when we check it when we shouldn't. The latter could happen if we reduce oversized r + n mod p, or forget a carry bit. Add some tests for these, getting as close to the boundary conditions as we can. Tested by simulating some bugs in BoringSSL's implementation of this optimization and ensuring Wycheproof noticed. (A similar optimization works for the less common p < n curves, though it's a bit simpler.)
3b09eb9 to
713daee
Compare
Contributor
Author
Oops, fixed. |
Member
|
Going to merge this now, but happy to do further revisions if anything comes up! |
This file contains hidden or 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
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.
This upstreams some tests from BoringSSL, though I've taken another pass at the generation script to cover all the curves in Wycheproof. (NB: I've only run the test vectors against an implementation for BoringSSL's curves. It is possible I've gotten the obscure curves wrong.)
ECDSA verification involves computing some point (x, y) and then checking if x mod n = r. x is only reduced mod p, so this leads to some edge cases right when x does and does not need a reduction. The existing test vectors covered some of these, but add some missing ones, so we catch both sides of the boundary condition.
Additionally, there is an optimized variant which leads to another boundary condition. EC points are typically implemented with Jacobian coordinates, (X:Y:Z) => (x, y) = (X/Z², Y/Z³). This avoids expensive inversions in the point addition formulae. Only when extracting the final affine coordinates do you invert a field element, to divide by Z.
Naively, ECDSA verification requires one such inversion. However, this inversion can be avoided in ECDSA verification by observing that comparing X/Z² to r is the same as comparing X to r*Z². However, this works mod p instead of mod n and we must deal with this mismatch. In most cases, n < p, where we must check two cases: r == x and r == x - n.
0 <= r < n < p and 0 <= x < p, so the first case is equivalent to checking x = r (mod p), where we can apply our optimization.
The second is equivalent to x == r + n, which in turn is equivalent to checking that r + n < p and then x == r + n (mod p), where we can also apply our optimization.
This leads to some other edge cases where we don't check the second case when we should, or when we check it when we shouldn't. The latter could happen if we reduce oversized r + n mod p, or forget a carry bit. Add some tests for these, getting as close to the boundary conditions as we can. Tested by simulating some bugs in BoringSSL's implementation of this optimization and ensuring Wycheproof noticed.
(A similar optimization works for the less common p < n curves, though it's a bit simpler.)
Test vectors generated with this tool