Skip to content

Fix ECDSA edge cases and harden new implementation #46

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.

Sign up for GitHub

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

Merged
turbolent merged 3 commits into from
Aug 11, 2026
Merged

Fix ECDSA edge cases and harden new implementation #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50ddda0
fix ECDSA edge cases and harden new implementation
turbolent Aug 10, 2026
b44ebcc
address review comments
turbolent Aug 11, 2026
b03aed1
keep TestECDSASecp256k1DeterministicSigning strictly about signing de…
tarakby Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 59 additions & 30 deletions ecdsa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,26 @@ func bitsToBytes(bits int) int {
return (bits + 7) >> 3
}

func (a *ecdsaContext) checkAlgoAndComputeHash(msg []byte, hasher hash.Hasher) (hash.Hash, error) {
// checkHasherAndComputeHash checks the hasher is valid for ECDSA
// on the receiver curve and returns the hash of the input message.
func (a *ecdsaContext) checkHasherAndComputeHash(msg []byte, hasher hash.Hasher) (hash.Hash, error) {
if hasher == nil {
return nil, errNilHasher
}

// check hasher's size is at least the curve order in bytes
nLen := (a.curveN).BitLen()
if (hasher.Size() << 3) < nLen {
h := hasher.ComputeHash(msg)
// check the computed hash is at least the curve order in bytes.
// All curve orders supported by the package have a bit-length multiple of 8,
// so callers truncate the message hash in bytes
// and the check is done in bytes too.
// The check uses the computed hash length rather than the hasher's declared size,
// so that a hasher implementation computing fewer bytes than it declares
// is rejected instead of panicking in the caller's truncation.
nLen := bitsToBytes((a.curveN).BitLen())
if len(h) < nLen {
Comment thread
turbolent marked this conversation as resolved.
return nil, invalidHasherSizeErrorf(
"hasher's bit-size should be at least %d, got %d", nLen, hasher.Size()<<3)
"hasher's output should be at least %d bytes, got %d bytes", nLen, len(h))
}

h := hasher.ComputeHash(msg)
return h, nil
}

Expand Down Expand Up @@ -106,7 +113,6 @@ func (a *ecdsaContext) signatureFormatCheck(sig Signature) bool {
}

var one = new(big.Int).SetInt64(1)
var two = new(big.Int).SetInt64(2)

// mapToPrivateKey simply maps the input seed to an ECDSA private key
// The private scalar `d` satisfies 0 < d < n.
Expand Down Expand Up @@ -137,14 +143,23 @@ func (a *ecdsaContext) privateKey(d *big.Int) (PrivateKey, error) {
d.FillBytes(dBytes) // dBytes is the big-endian encoding of d padded to the curve order

// build the private key depending on the curve
var sk PrivateKey
var err error
switch a.algo {
case ECDSAP256:
return privateKeyECDSAP256(a, dBytes)
sk, err = privateKeyECDSAP256(a, dBytes)
case ECDSASecp256k1:
return privateKeyECDSASecp256k1(a, dBytes), nil
sk = privateKeyECDSASecp256k1(a, dBytes)
default:
return nil, invalidInputsErrorf("the curve is not supported")
}
if err != nil {
// return an untyped nil,
// otherwise the returned interface is non-nil
// although it holds a nil pointer
return nil, err
}
return sk, nil
}

// generatePrivateKey generates a private key for ECDSA
Expand Down Expand Up @@ -219,14 +234,23 @@ func (a *ecdsaContext) decodePrivateKey(der []byte) (PrivateKey, error) {
// Error Returns:
// - invalidInputsError if the input is not a valid serialization of a public key on the given curve.
func (a *ecdsaContext) rawDecodePublicKey(input []byte) (PublicKey, error) {
var pk PublicKey
var err error
switch a.algo {
case ECDSAP256:
return publicKeyECDSAP256(input)
pk, err = publicKeyECDSAP256(input)
case ECDSASecp256k1:
return publicKeyECDSASecp256k1(a, input)
pk, err = publicKeyECDSASecp256k1(a, input)
default:
return nil, invalidInputsErrorf("curve is not supported")
}
if err != nil {
// return an untyped nil,
// otherwise the returned interface is non-nil
// although it holds a nil pointer
return nil, err
}
return pk, nil
}

func (a *ecdsaContext) decodePublicKey(der []byte) (PublicKey, error) {
Expand All @@ -241,14 +265,23 @@ func (a *ecdsaContext) decodePublicKey(der []byte) (PublicKey, error) {
// - invalidInputsError if the curve isn't supported or the input isn't a valid key serialization
// on the given curve.
func (a *ecdsaContext) decodePublicKeyCompressed(pkBytes []byte) (PublicKey, error) {
var pk PublicKey
var err error
switch a.algo {
case ECDSAP256:
return p256DecodePublicKeyCompressed(pkBytes)
pk, err = p256DecodePublicKeyCompressed(pkBytes)
case ECDSASecp256k1:
return secp256k1DecodePublicKeyCompressed(pkBytes)
pk, err = secp256k1DecodePublicKeyCompressed(pkBytes)
default:
return nil, invalidInputsErrorf("the input curve is not supported")
}
if err != nil {
// return an untyped nil,
// otherwise the returned interface is non-nil
// although it holds a nil pointer
return nil, err
}
return pk, nil
}

// Algorithm returns the algo related to the private key
Expand Down Expand Up @@ -280,6 +313,10 @@ func pubKeyCommonECDSAString(pk PublicKey) string {

// Equals tests the equality of two private keys
func prKeyCommonECDSAEquals(sk, other PrivateKey) bool {
// a nil key is not equal to any key
if other == nil {
return false
}
// check the algorithm
if sk.Algorithm() != other.Algorithm() {
return false
Expand All @@ -300,6 +337,10 @@ func (pk *pubKeyCommonECDSA) Size() int {

// Equals tests the equality of two private keys
func pubKeyCommonECDSAEquals(pk, other PublicKey) bool {
// a nil key is not equal to any key
if other == nil {
return false
}
// check the algorithm
if pk.Algorithm() != other.Algorithm() {
return false
Expand All @@ -313,7 +354,7 @@ func pubKeyCommonECDSAEquals(pk, other PublicKey) bool {
// It assumes the output buffer has at least 2*size byte-length
func padToSizeAndConcat(output []byte, a, b *big.Int, size int) {
a.FillBytes(output[:size])
b.FillBytes(output[size:])
b.FillBytes(output[size : 2*size])
}

// Helper function to read two big integers of "size" bytes each from a concatenated input buffer.
Expand All @@ -333,7 +374,9 @@ func (a *ecdsaContext) isLowS(s *big.Int) bool {
// signatureNormalizeS returns a signature with S normalized to low S.
// (same slice is returned if S is already normalized)
// It assumes len(sig) == 2*nLen where nLen is the byte-length of the curve order.
// This is needed when the underlying signature verification requires S to be in the lower range (to avoid signature malleability). In this package, verification allows high S signatures to be accepted.
// This is needed when the underlying signature verification requires S to be
// in the lower range (to avoid signature malleability).
// In this package, verification allows high S signatures to be accepted.
// The function checks that S is in the range [0, n-1] before normalizing it.
// If S is not in this range, the function returns a false boolean.
// (S and R values will be checked by the go-ethereum verification function - only S check against N is included here, S=0 check is deferred to the signature verification)
Expand Down Expand Up @@ -361,17 +404,3 @@ func (a *ecdsaContext) signatureNormalizeS(sig []byte) ([]byte, bool) {
sComplement.FillBytes(newSig[nLen:]) // write S complement
return newSig, true
}

// Test function only to flip S in a signature. It is used for testing signature malleability
func (a *ecdsaContext) signatureFlipS(sig []byte) []byte {
// read S
nLen := bitsToBytes(a.curveN.BitLen())
s := new(big.Int).SetBytes(sig[nLen:])
// compute N-S
sComplement := new(big.Int).Sub(a.curveN, s)
// write it into a new signature
newSig := make([]byte, len(sig))
copy(newSig, sig[:nLen]) // copy R
sComplement.FillBytes(newSig[nLen:]) // write S complement
return newSig
}
43 changes: 16 additions & 27 deletions ecdsa_p256.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,11 @@ var p256Instance *ecdsaContext
func initECDSAP256() {
curve := elliptic.P256()
n := curve.Params().N
nMinus1 := new(big.Int).Sub(n, one)

p256Instance = &(ecdsaContext{
curveP: curve.Params().P,
curveN: n,
curveNdiv2: new(big.Int).Div(nMinus1, two), // (N-1)/2
curveNdiv2: new(big.Int).Rsh(n, 1), // (N-1)/2, since N is odd
algo: ECDSAP256,
})
}
Expand Down Expand Up @@ -91,7 +90,10 @@ func privateKeyECDSAP256(a *ecdsaContext, dBytes []byte) (*prKeyECDSAP256, error
sk := &prKeyECDSAP256{
prKeyCommonECDSA: &prKeyCommonECDSA{a},
goPrKey: internalSK,
pubKey: nil, // public key is not constructed yet
pubKey: &pubKeyECDSAP256{
pubKeyCommonECDSA: &pubKeyCommonECDSA{p256Instance},
goPubKey: &internalSK.PublicKey,
},
}
return sk, nil
}
Expand All @@ -109,7 +111,7 @@ func privateKeyECDSAP256(a *ecdsaContext, dBytes []byte) (*prKeyECDSAP256, error
// - (nil, error) if an unexpected error occurs
// - (signature, nil) otherwise
func (sk *prKeyECDSAP256) Sign(msg []byte, hasher hash.Hasher) (Signature, error) {
hash, err := sk.checkAlgoAndComputeHash(msg, hasher)
hash, err := sk.checkHasherAndComputeHash(msg, hasher)
if err != nil {
return nil, err
}
Expand All @@ -135,10 +137,12 @@ func publicKeyECDSAP256(XYBytes []byte) (*pubKeyECDSAP256, error) {
// The bytes serialization for non-infinity points is `0x04 || X || Y` and infinity point should be rejected anyway
parsingBytes := append([]byte{ecEncodingUncompressed}, XYBytes...)

// ParseUncompressedPublicKey includes x<p and y<p checks, and on curve checks
// ParseUncompressedPublicKey includes the input length check,
// x<p and y<p checks, and on curve checks
internalPK, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), parsingBytes)
if err != nil {
return nil, invalidInputsErrorf("input point has invalid coordinates or is not on curve: %w", err)
return nil, invalidInputsErrorf("input is not a valid %s key of %d bytes: %w",
ECDSAP256, 2*pLenP256, err)
}
return &pubKeyECDSAP256{
&pubKeyCommonECDSA{p256Instance},
Expand All @@ -153,13 +157,6 @@ func (pk *pubKeyECDSAP256) String() string {

// PublicKey returns the public key associated to the private key
func (sk *prKeyECDSAP256) PublicKey() PublicKey {
// construct the public key once
if sk.pubKey == nil {
sk.pubKey = &pubKeyECDSAP256{
pubKeyCommonECDSA: &pubKeyCommonECDSA{p256Instance},
goPubKey: &sk.goPrKey.PublicKey,
}
}
return sk.pubKey
}

Expand All @@ -177,7 +174,7 @@ func (sk *prKeyECDSAP256) PublicKey() PublicKey {
// - (false, error) if an unexpected error occurs
// - (validity, nil) otherwise
func (pk *pubKeyECDSAP256) Verify(sig Signature, data []byte, alg hash.Hasher) (bool, error) {
h, err := pk.checkAlgoAndComputeHash(data, alg)
h, err := pk.checkHasherAndComputeHash(data, alg)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -266,17 +263,9 @@ func p256DecodePublicKeyCompressed(pkBytes []byte) (*pubKeyECDSAP256, error) {
if x == nil || y == nil {
return nil, invalidInputsErrorf("input %x isn't a compressed serialization of a point on P256", pkBytes)
}
uncompressedPointBytes := make([]byte, 2*pLenP256+1)
uncompressedPointBytes[0] = ecEncodingUncompressed
padToSizeAndConcat(uncompressedPointBytes[1:], x, y, pLenP256)

internalPK, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), uncompressedPointBytes)
if err != nil {
// unexpected error since prior deserialization succeeded
return nil, invalidInputsErrorf("unexpected error: input is not a point on curve P-256: %w", err)
}
return &pubKeyECDSAP256{
&pubKeyCommonECDSA{p256Instance},
internalPK,
}, nil
// serialize the coordinates and delegate to the uncompressed decoding,
// so that both decoding paths construct the key the same way
xyBytes := make([]byte, 2*pLenP256)
padToSizeAndConcat(xyBytes, x, y, pLenP256)
return publicKeyECDSAP256(xyBytes)
}
64 changes: 29 additions & 35 deletions ecdsa_secp256k1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,10 @@ import (

// This implementation is not resistant against side-channel attacks or fault attacks.

// curve parameters for SECG secp256k1 https://www.secg.org/sec2-v2.pdf
// byte lengths of the SECG secp256k1 curve order and prime field https://www.secg.org/sec2-v2.pdf
const (
secp256k1PHex = "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F"
secp256k1NHex = "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141"

nLenSecp256k1 = 32
pLenSecp256k1 = 32

secp256k1Ndiv2Hex = "7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0"
)

const (
Expand All @@ -54,22 +49,15 @@ const (
var secp256k1Instance *ecdsaContext

func initECDSASecp256k1() {
curveP, ok := new(big.Int).SetString(secp256k1PHex, 16)
if !ok {
panic("failed to initialize ECDSA with secp256k1 curve")
}
curveN, ok := new(big.Int).SetString(secp256k1NHex, 16)
if !ok {
panic("failed to initialize ECDSA with secp256k1 curve")
}
curveNdiv2, ok := new(big.Int).SetString(secp256k1Ndiv2Hex, 16)
if !ok {
panic("failed to initialize ECDSA with secp256k1 curve")
}
// the curve parameters are read from the underlying go-ethereum implementation,
// which the package already relies on for the curve arithmetic itself
curve := secp256k1.S256()
n := curve.N

secp256k1Instance = &(ecdsaContext{
curveP: curveP,
curveN: curveN,
curveNdiv2: curveNdiv2,
curveP: curve.P,
curveN: n,
curveNdiv2: new(big.Int).Rsh(n, 1), // (N-1)/2, since N is odd
algo: ECDSASecp256k1,
})
}
Expand Down Expand Up @@ -99,10 +87,23 @@ var _ PublicKey = (*pubKeyECDSASecp256k1)(nil)

// Input scalar d is assumed to satisfy 0 < d < n before calling this function.
func privateKeyECDSASecp256k1(a *ecdsaContext, dBytes []byte) *prKeyECDSASecp256k1 {
x, y := secp256k1.S256().ScalarBaseMult(dBytes)
// `ScalarBaseMult` returns nil coordinates only if the scalar is zero
// or not less than the curve order,
// which all callers of `privateKeyECDSASecp256k1` rule out.
// The check turns an unexpected invariant break into an explicit panic
// instead of a nil dereference inside `secp256k1PkBytes`.
if x == nil || y == nil {
panic("unexpected error: the private key scalar is invalid")
}

sk := &prKeyECDSASecp256k1{
prKeyCommonECDSA: &prKeyCommonECDSA{a},
dBytes: dBytes,
pubKey: nil, // public key is not constructed yet
pubKey: &pubKeyECDSASecp256k1{
pubKeyCommonECDSA: &pubKeyCommonECDSA{secp256k1Instance},
pkBytes: secp256k1PkBytes(x, y),
},
}
return sk
}
Expand All @@ -120,7 +121,7 @@ func privateKeyECDSASecp256k1(a *ecdsaContext, dBytes []byte) *prKeyECDSASecp256
// - (nil, error) if an unexpected error occurs
// - (signature, nil) otherwise
func (sk *prKeyECDSASecp256k1) Sign(msg []byte, hasher hash.Hasher) (Signature, error) {
hash, err := sk.checkAlgoAndComputeHash(msg, hasher)
hash, err := sk.checkHasherAndComputeHash(msg, hasher)
if err != nil {
return nil, err
}
Expand All @@ -131,8 +132,10 @@ func (sk *prKeyECDSASecp256k1) Sign(msg []byte, hasher hash.Hasher) (Signature,
if err != nil {
return nil, fmt.Errorf("failed to sign hash: %w", err)
}
// remove the EC recover byte (last byte)
return signature[:SignatureLenECDSASecp256k1], nil
// remove the EC recover byte (last byte).
// The capacity is clipped so that an append to the returned signature
// cannot reach the recovery byte in the shared backing array.
return signature[:SignatureLenECDSASecp256k1:SignatureLenECDSASecp256k1], nil
}

// String returns the hex string representation of the private key
Expand Down Expand Up @@ -189,15 +192,6 @@ func secp256k1PkBytes(x, y *big.Int) []byte {

// PublicKey returns the public key associated to the private key
func (sk *prKeyECDSASecp256k1) PublicKey() PublicKey {
// construct the public key once
if sk.pubKey == nil {
x, y := secp256k1.S256().ScalarBaseMult(sk.dBytes)

sk.pubKey = &pubKeyECDSASecp256k1{
pubKeyCommonECDSA: &pubKeyCommonECDSA{secp256k1Instance},
pkBytes: secp256k1PkBytes(x, y),
}
}
return sk.pubKey
}

Expand All @@ -215,7 +209,7 @@ func (sk *prKeyECDSASecp256k1) PublicKey() PublicKey {
// - (false, error) if an unexpected error occurs
// - (validity, nil) otherwise
func (pk *pubKeyECDSASecp256k1) Verify(sig Signature, msg []byte, hasher hash.Hasher) (bool, error) {
hash, err := pk.checkAlgoAndComputeHash(msg, hasher)
hash, err := pk.checkHasherAndComputeHash(msg, hasher)
if err != nil {
return false, err
}
Expand Down
Loading
Loading