Skip to content

Commit

Permalink
btcec/schnorr: use private key copy for BIP-340 signatures
Browse files Browse the repository at this point in the history
This is a fix similar to #1905.
We'll always make a copy of the key in the local scope before passing it
around elsewhere. Depending on the parity of the public key, the private
key itself might need to be negated.

A similar test is added here that fails without the patch to the
signature.go file.
  • Loading branch information
Roasbeef committed Nov 9, 2022
1 parent 14bb56f commit a8244f5
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 5 deletions.
12 changes: 7 additions & 5 deletions btcec/schnorr/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ func NewSignature(r *btcec.FieldVal, s *btcec.ModNScalar) *Signature {
// Serialize returns the Schnorr signature in the more strict format.
//
// The signatures are encoded as
// sig[0:32] x coordinate of the point R, encoded as a big-endian uint256
// sig[32:64] s, encoded also as big-endian uint256
//
// sig[0:32] x coordinate of the point R, encoded as a big-endian uint256
// sig[32:64] s, encoded also as big-endian uint256
func (sig Signature) Serialize() []byte {
// Total length of returned signature is the length of r and s.
var b [SignatureSize]byte
Expand Down Expand Up @@ -441,7 +442,8 @@ func Sign(privKey *btcec.PrivateKey, hash []byte,
// Step 1.
//
// d' = int(d)
privKeyScalar := &privKey.Key
var privKeyScalar btcec.ModNScalar
privKeyScalar.Set(&privKey.Key)

// Step 2.
//
Expand Down Expand Up @@ -512,7 +514,7 @@ func Sign(privKey *btcec.PrivateKey, hash []byte,
return nil, signatureError(ecdsa_schnorr.ErrSchnorrHashValue, str)
}

sig, err := schnorrSign(privKeyScalar, &kPrime, pub, hash, opts)
sig, err := schnorrSign(&privKeyScalar, &kPrime, pub, hash, opts)
kPrime.Zero()
if err != nil {
return nil, err
Expand All @@ -535,7 +537,7 @@ func Sign(privKey *btcec.PrivateKey, hash []byte,
)

// Steps 10-15.
sig, err := schnorrSign(privKeyScalar, k, pub, hash, opts)
sig, err := schnorrSign(&privKeyScalar, k, pub, hash, opts)
k.Zero()
if err != nil {
// Try again with a new nonce.
Expand Down
37 changes: 37 additions & 0 deletions btcec/schnorr/signature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import (
"errors"
"strings"
"testing"
"testing/quick"

"github.com/btcsuite/btcd/btcec/v2"
"github.com/davecgh/go-spew/spew"
secp_ecdsa "github.com/decred/dcrd/dcrec/secp256k1/v4"
ecdsa_schnorr "github.com/decred/dcrd/dcrec/secp256k1/v4/schnorr"
)
Expand Down Expand Up @@ -254,3 +256,38 @@ func TestSchnorrVerify(t *testing.T) {
}
}
}

// TestSchnorrSignNoMutate tests that generating a schnorr signature doesn't
// modify/mutate the underlying private key.
func TestSchnorrSignNoMutate(t *testing.T) {
t.Parallel()

// Assert that given a random private key and message, we can generate
// a signature from that w/o modifying the underlying private key.
f := func(privBytes, msg [32]byte) bool {
privBytesCopy := privBytes
privKey, _ := btcec.PrivKeyFromBytes(privBytesCopy[:])

// Generate a signature for private key with our message.
_, err := Sign(privKey, msg[:])
if err != nil {
t.Logf("unable to gen sig: %v", err)
return false
}

// We should be able to re-derive the private key from raw
// bytes and have that match up again.
privKeyCopy, _ := btcec.PrivKeyFromBytes(privBytes[:])
if *privKey != *privKeyCopy {
t.Logf("private doesn't match: expected %v, got %v",
spew.Sdump(privKeyCopy), spew.Sdump(privKey))
return false
}

return true
}

if err := quick.Check(f, nil); err != nil {
t.Fatalf("private key modified: %v", err)
}
}

0 comments on commit a8244f5

Please sign in to comment.