ecdsa: option to enforce low-S signature (sign+verify)#41
Merged
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds an opt-in ECDSA “low-S” policy that can (a) normalize signatures at signing time and (b) enforce canonical low-S at verification time, across the library’s ECDSA key types. This strengthens signature canonicalization without changing default behavior, and extends the same enforcement option to secp256k1 verification.
Changes:
- Introduces
crypto.WithEcdsaLowSSig()and plumbs it through SigningOpts for opt-in low-S behavior. - Updates P-256/P-384/P-521 signing to optionally normalize
sto low-S for both raw and ASN.1 signatures, and updates verification paths to optionally reject high-S. - Adds a shared ECDSA low-S test suite and updates Go/tooling/dependencies (Go 1.25, updated
golang.org/x/crypto/x/sys, CI matrix).
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Bumps Go version to 1.25.0 and updates golang.org/x/crypto / x/sys requirements. |
| go.sum | Updates module checksums for golang.org/x/crypto and golang.org/x/sys. |
| crypto/options.go | Adds the ecdsaLowS option bit plus WithEcdsaLowSSig() and accessor. |
| crypto/internal/asn1.go | Adds DER ECDSA signature decode helper used for low-S checks on ASN.1 signatures. |
| crypto/p256/private.go | Normalizes s on signing when low-S option is set; ASN.1 signing now encodes via internal helper post-normalization. |
| crypto/p256/public.go | Enforces low-S (opt-in) for both bytes and ASN.1 verify paths, factoring common logic into verify. |
| crypto/p256/key_test.go | Hooks P-256 into the shared low-S test suite. |
| crypto/p384/private.go | Same low-S signing normalization and ASN.1 encoding approach as P-256. |
| crypto/p384/public.go | Same low-S verify enforcement/refactor approach as P-256. |
| crypto/p384/key_test.go | Hooks P-384 into the shared low-S test suite. |
| crypto/p521/private.go | Same low-S signing normalization and ASN.1 encoding approach as P-256. |
| crypto/p521/public.go | Same low-S verify enforcement/refactor approach as P-256. |
| crypto/p521/key_test.go | Hooks P-521 into the shared low-S test suite. |
| crypto/secp256k1/public.go | Adds opt-in low-S enforcement on verification for both bytes and ASN.1 signatures. |
| crypto/secp256k1/key_test.go | Hooks secp256k1 into the shared low-S test suite (providing curve order). |
| crypto/_testsuite/testsuite.go | Adds TestEcdsaLowSSuite covering bytes and ASN.1 sign/verify low-S behavior and high-S rejection when opted in. |
| .github/workflows/gotest.yml | Updates CI Go matrix to 1.25.x and 1.26.x. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dovydas55
approved these changes
Jun 23, 2026
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.
Note
Medium Risk
Changes ECDSA signature acceptance and production when callers opt in; default verify behavior is unchanged, but misusing the new option could break interoperability with peers that emit high-S.
Overview
Adds
WithEcdsaLowSSigso callers can request canonical ECDSA signatures with s ≤ N/2 and matching verification that rejects high-S malleable variants.P-256, P-384, and P-521 sign paths normalize s when the option is set; bytes and ASN.1 verify paths enforce low-S only with the option, while verification without the option still accepts high-S. secp256k1 gains the same optional verify checks (signing already tends toward low-S). A shared
TestEcdsaLowSSuitecovers bytes and DER cases across curves, plusDecodeSignatureFromASN1for DER S checks.CI and module metadata move to Go 1.25.x / 1.26.x and bump
golang.org/x/crypto.Reviewed by Cursor Bugbot for commit 5dffa20. Bugbot is set up for automated code reviews on this repo. Configure here.