diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index a2e6226c5..1af162f7d 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -43,6 +43,10 @@ var ( // responder. ErrProposalExeceedsMaxFee = fmt.Errorf("latest fee proposal exceeds " + "max fee") + + // ErrInvalidShutdownScript is returned when we receive an address from + // a peer that isn't either a p2wsh or p2tr address. + ErrInvalidShutdownScript = fmt.Errorf("invalid shutdown script") ) // closeState represents all the possible states the channel closer state @@ -153,6 +157,9 @@ type ChanCloseCfg struct { // willing to pay to close the channel. MaxFee chainfee.SatPerKWeight + // ChainParams holds the parameters of the chain that we're active on. + ChainParams *chaincfg.Params + // Quit is a channel that should be sent upon in the occasion the state // machine should cease all progress and shutdown. Quit chan struct{} @@ -359,17 +366,33 @@ func (c *ChanCloser) NegotiationHeight() uint32 { return c.negotiationHeight } -// maybeMatchScript attempts to match the script provided in our peer's -// shutdown message with the upfront shutdown script we have on record. If no -// upfront shutdown script was set, we do not need to enforce option upfront -// shutdown, so the function returns early. If an upfront script is set, we -// check whether it matches the script provided by our peer. If they do not -// match, we use the disconnect function provided to disconnect from the peer. -func maybeMatchScript(disconnect func() error, upfrontScript, - peerScript lnwire.DeliveryAddress) error { +// validateShutdownScript attempts to match and validate the script provided in +// our peer's shutdown message with the upfront shutdown script we have on +// record. For any script specified, we also make sure it matches our +// requirements. If no upfront shutdown script was set, we do not need to +// enforce option upfront shutdown, so the function returns early. If an +// upfront script is set, we check whether it matches the script provided by +// our peer. If they do not match, we use the disconnect function provided to +// disconnect from the peer. +func validateShutdownScript(disconnect func() error, upfrontScript, + peerScript lnwire.DeliveryAddress, netParams *chaincfg.Params) error { - // If no upfront shutdown script was set, return early because we do not - // need to enforce closure to a specific script. + // Either way, we'll make sure that the script passed meets our + // standards. The upfrontScript should have already been checked at an + // earlier stage, but we'll repeat the check here for defense in depth. + if len(upfrontScript) != 0 { + if !lnwallet.ValidateUpfrontShutdown(upfrontScript, netParams) { + return ErrInvalidShutdownScript + } + } + if len(peerScript) != 0 { + if !lnwallet.ValidateUpfrontShutdown(peerScript, netParams) { + return ErrInvalidShutdownScript + } + } + + // If no upfront shutdown script was set, return early because we do + // not need to enforce closure to a specific script. if len(upfrontScript) == 0 { return nil } @@ -435,9 +458,9 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, // If the remote node opened the channel with option upfront shutdown // script, check that the script they provided matches. - if err := maybeMatchScript( + if err := validateShutdownScript( c.cfg.Disconnect, c.cfg.Channel.RemoteUpfrontShutdownScript(), - shutdownMsg.Address, + shutdownMsg.Address, c.cfg.ChainParams, ); err != nil { return nil, false, err } @@ -494,8 +517,10 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, // If the remote node opened the channel with option upfront shutdown // script, check that the script they provided matches. - if err := maybeMatchScript(c.cfg.Disconnect, + if err := validateShutdownScript( + c.cfg.Disconnect, c.cfg.Channel.RemoteUpfrontShutdownScript(), shutdownMsg.Address, + c.cfg.ChainParams, ); err != nil { return nil, false, err } diff --git a/lnwallet/chancloser/chancloser_test.go b/lnwallet/chancloser/chancloser_test.go index f3d2d4bd6..6d194398c 100644 --- a/lnwallet/chancloser/chancloser_test.go +++ b/lnwallet/chancloser/chancloser_test.go @@ -1,12 +1,14 @@ package chancloser import ( - "crypto/rand" + "bytes" "fmt" "testing" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet/chainfee" @@ -14,48 +16,58 @@ import ( "github.com/stretchr/testify/require" ) -// randDeliveryAddress generates a random delivery address for testing. -func randDeliveryAddress(t *testing.T) lnwire.DeliveryAddress { - // Generate an address of maximum length. - da := lnwire.DeliveryAddress(make([]byte, 34)) - - _, err := rand.Read(da) - require.NoError(t, err, "cannot generate random address") - - return da -} - // TestMaybeMatchScript tests that the maybeMatchScript errors appropriately // when an upfront shutdown script is set and the script provided does not // match, and does not error in any other case. func TestMaybeMatchScript(t *testing.T) { t.Parallel() - addr1 := randDeliveryAddress(t) - addr2 := randDeliveryAddress(t) + pubHash := bytes.Repeat([]byte{0x0}, 20) + scriptHash := bytes.Repeat([]byte{0x0}, 32) - tests := []struct { + p2wkh, err := txscript.NewScriptBuilder().AddOp(txscript.OP_0). + AddData(pubHash).Script() + require.NoError(t, err) + + p2wsh, err := txscript.NewScriptBuilder().AddOp(txscript.OP_0). + AddData(scriptHash).Script() + require.NoError(t, err) + + p2tr, err := txscript.NewScriptBuilder().AddOp(txscript.OP_1). + AddData(scriptHash).Script() + require.NoError(t, err) + + p2OtherV1, err := txscript.NewScriptBuilder().AddOp(txscript.OP_1). + AddData(pubHash).Script() + require.NoError(t, err) + + invalidFork, err := txscript.NewScriptBuilder().AddOp(txscript.OP_NOP). + AddData(scriptHash).Script() + require.NoError(t, err) + + type testCase struct { name string shutdownScript lnwire.DeliveryAddress upfrontScript lnwire.DeliveryAddress expectedErr error - }{ + } + tests := []testCase{ { name: "no upfront shutdown set, script ok", - shutdownScript: addr1, + shutdownScript: p2wkh, upfrontScript: []byte{}, expectedErr: nil, }, { name: "upfront shutdown set, script ok", - shutdownScript: addr1, - upfrontScript: addr1, + shutdownScript: p2wkh, + upfrontScript: p2wkh, expectedErr: nil, }, { name: "upfront shutdown set, script not ok", - shutdownScript: addr1, - upfrontScript: addr2, + shutdownScript: p2wkh, + upfrontScript: p2wsh, expectedErr: ErrUpfrontShutdownScriptMismatch, }, { @@ -64,6 +76,40 @@ func TestMaybeMatchScript(t *testing.T) { upfrontScript: []byte{}, expectedErr: nil, }, + { + name: "p2tr is ok", + shutdownScript: p2tr, + }, + { + name: "segwit v1 is ok", + shutdownScript: p2OtherV1, + }, + { + name: "invalid script not allowed", + shutdownScript: invalidFork, + expectedErr: ErrInvalidShutdownScript, + }, + } + + // All future segwit softforks should also be ok. + futureForks := []byte{ + txscript.OP_1, txscript.OP_2, txscript.OP_3, txscript.OP_4, + txscript.OP_5, txscript.OP_6, txscript.OP_7, txscript.OP_8, + txscript.OP_9, txscript.OP_10, txscript.OP_11, txscript.OP_12, + txscript.OP_13, txscript.OP_14, txscript.OP_15, txscript.OP_16, + } + for _, witnessVersion := range futureForks { + p2FutureFork, err := txscript.NewScriptBuilder().AddOp(witnessVersion). + AddData(scriptHash).Script() + require.NoError(t, err) + + opString, err := txscript.DisasmString([]byte{witnessVersion}) + require.NoError(t, err) + + tests = append(tests, testCase{ + name: fmt.Sprintf("witness_version=%v", opString), + shutdownScript: p2FutureFork, + }) } for _, test := range tests { @@ -72,9 +118,9 @@ func TestMaybeMatchScript(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - err := maybeMatchScript( + err := validateShutdownScript( func() error { return nil }, test.upfrontScript, - test.shutdownScript, + test.shutdownScript, &chaincfg.SimNetParams, ) if err != test.expectedErr { diff --git a/peer/brontide.go b/peer/brontide.go index ea6396f08..ee8c7907c 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -2716,7 +2716,8 @@ func (p *Brontide) createChanCloser(channel *lnwallet.LightningChannel, Disconnect: func() error { return p.cfg.DisconnectPeer(p.IdentityKey()) }, - Quit: p.quit, + ChainParams: &p.cfg.Wallet.Cfg.NetParams, + Quit: p.quit, }, deliveryScript, fee,