From 9f4f0e94f5fe2c10a915c625e41d42a508dede93 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 9 May 2022 14:09:45 +0200 Subject: [PATCH] multi: avoid direct use of dcrec/secp256k1 library Because the original dcrec secp256k1 library that is used for the Schnorr signature primitives uses different hash algorithms than the btcd secp256k1 library. Therefore pulling in the wrong library can lead to weird and unexpected errors. We try to make it harder to make the mistake by not using the library directly in lnd in the first place. Note that it is still indirectly needed by the btcd secp256k1 library, therefore the module dependency is still expected to be there, just moved to the indirect section. --- go.mod | 2 +- input/script_utils.go | 6 +++--- input/taproot.go | 16 +++++++++++++--- lnwallet/rpcwallet/rpcwallet.go | 3 +-- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 7773e4eb8..623d70b6e 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,6 @@ require ( github.com/btcsuite/btcwallet/wtxmgr v1.5.0 github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f github.com/davecgh/go-spew v1.1.1 - github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 github.com/dvyukov/go-fuzz v0.0.0-20210602112143-b1f3d6f4ef4e github.com/go-errors/errors v1.0.1 github.com/golang/protobuf v1.5.2 @@ -76,6 +75,7 @@ require ( github.com/coreos/go-systemd/v22 v22.3.2 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect github.com/decred/dcrd/crypto/blake256 v1.0.0 // indirect + github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 // indirect github.com/decred/dcrd/lru v1.0.0 // indirect github.com/dsnet/compress v0.0.1 // indirect github.com/dustin/go-humanize v1.0.0 // indirect diff --git a/input/script_utils.go b/input/script_utils.go index 8302a972a..e12b57c43 100644 --- a/input/script_utils.go +++ b/input/script_utils.go @@ -9,7 +9,6 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" - secp "github.com/decred/dcrd/dcrec/secp256k1/v4" "golang.org/x/crypto/ripemd160" ) @@ -1399,7 +1398,7 @@ func TweakPubKeyWithTweak(pubKey *btcec.PublicKey, tweakJacobian btcec.JacobianPoint resultJacobian btcec.JacobianPoint ) - tweakKey := secp.PrivKeyFromBytes(tweakBytes) + tweakKey, _ := btcec.PrivKeyFromBytes(tweakBytes) btcec.ScalarBaseMultNonConst(&tweakKey.Key, &tweakJacobian) pubKey.AsJacobian(&pubKeyJacobian) @@ -1549,5 +1548,6 @@ func DeriveRevocationPrivKey(revokeBasePriv *btcec.PrivateKey, // the key-ring and also to used as a tweak to derive new public+private keys // for the state. func ComputeCommitmentPoint(commitSecret []byte) *btcec.PublicKey { - return secp.PrivKeyFromBytes(commitSecret).PubKey() + _, pubKey := btcec.PrivKeyFromBytes(commitSecret) + return pubKey } diff --git a/input/taproot.go b/input/taproot.go index 255fba427..0319228ba 100644 --- a/input/taproot.go +++ b/input/taproot.go @@ -7,7 +7,17 @@ import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/waddrmgr" - secp "github.com/decred/dcrd/dcrec/secp256k1/v4" +) + +const ( + // PubKeyFormatCompressedOdd is the identifier prefix byte for a public + // key whose Y coordinate is odd when serialized in the compressed + // format per section 2.3.4 of + // [SEC1](https://secg.org/sec1-v2.pdf#subsubsection.2.3.4). + // This is copied from the github.com/decred/dcrd/dcrec/secp256k1/v4 to + // avoid needing to directly reference (and by accident pull in + // incompatible crypto primitives) the package. + PubKeyFormatCompressedOdd byte = 0x03 ) // NewTxSigHashesV0Only returns a new txscript.TxSigHashes instance that will @@ -56,7 +66,7 @@ func TapscriptFullTree(internalKey *btcec.PublicKey, tapKey := txscript.ComputeTaprootOutputKey(internalKey, rootHash[:]) var outputKeyYIsOdd bool - if tapKey.SerializeCompressed()[0] == secp.PubKeyFormatCompressedOdd { + if tapKey.SerializeCompressed()[0] == PubKeyFormatCompressedOdd { outputKeyYIsOdd = true } @@ -85,7 +95,7 @@ func TapscriptPartialReveal(internalKey *btcec.PublicKey, rootHash := controlBlock.RootHash(revealedLeaf.Script) tapKey := txscript.ComputeTaprootOutputKey(internalKey, rootHash) - if tapKey.SerializeCompressed()[0] == secp.PubKeyFormatCompressedOdd { + if tapKey.SerializeCompressed()[0] == PubKeyFormatCompressedOdd { controlBlock.OutputKeyYIsOdd = true } diff --git a/lnwallet/rpcwallet/rpcwallet.go b/lnwallet/rpcwallet/rpcwallet.go index b2c468705..de47a0759 100644 --- a/lnwallet/rpcwallet/rpcwallet.go +++ b/lnwallet/rpcwallet/rpcwallet.go @@ -22,7 +22,6 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/waddrmgr" basewallet "github.com/btcsuite/btcwallet/wallet" - secp "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lncfg" @@ -1057,7 +1056,7 @@ func (r *RPCKeyRing) remoteSign(tx *wire.MsgTx, signDesc *input.SignDescriptor, // of this RPC), we can get by with faking certain information // that we don't have. fakeInternalKey, _ := btcec.ParsePubKey(d.PubKey) - fakeKeyIsOdd := d.PubKey[0] == secp.PubKeyFormatCompressedOdd + fakeKeyIsOdd := d.PubKey[0] == input.PubKeyFormatCompressedOdd controlBlock := txscript.ControlBlock{ InternalKey: fakeInternalKey, OutputKeyYIsOdd: fakeKeyIsOdd,