From c1bcaffeea5ef7dc51a231c8260e600f25d8bb7c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 8 Nov 2022 17:43:07 -0800 Subject: [PATCH] btcec/schnorr: use private key copy for BIP-340 signatures This is a fix similar to https://github.com/btcsuite/btcd/pull/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. --- btcec/schnorr/signature.go | 12 ++++++----- btcec/schnorr/signature_test.go | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/btcec/schnorr/signature.go b/btcec/schnorr/signature.go index f4532c7d091..8f58d52b4f7 100644 --- a/btcec/schnorr/signature.go +++ b/btcec/schnorr/signature.go @@ -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 @@ -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. // @@ -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 @@ -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. diff --git a/btcec/schnorr/signature_test.go b/btcec/schnorr/signature_test.go index b99614ff6cb..a2a956525c3 100644 --- a/btcec/schnorr/signature_test.go +++ b/btcec/schnorr/signature_test.go @@ -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" ) @@ -254,3 +256,37 @@ 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 { + privKey, _ := btcec.PrivKeyFromBytes(privBytes[:]) + + // 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) + } +}