From f86d63144c83107e8bdf0975571031e011e848c1 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 3 Dec 2019 11:38:29 +0200 Subject: [PATCH] multi: Set and check upfront shutdown address on close This commit sets our close addresss to the address specified by option upfront shutdown, if specified, and disconnects from peers that fail to provide their upfront shutdown address for coopertaive closes of channels that were opened with the option set. --- chancloser.go | 70 ++++++++++++++++++++++++++++++++++++++--- chancloser_test.go | 76 +++++++++++++++++++++++++++++++++++++++++++++ lnwallet/channel.go | 12 +++++++ peer.go | 54 ++++++++++++++++++++------------ 4 files changed, 189 insertions(+), 23 deletions(-) create mode 100644 chancloser_test.go diff --git a/chancloser.go b/chancloser.go index 12bd7560..f2b71d19 100644 --- a/chancloser.go +++ b/chancloser.go @@ -1,6 +1,7 @@ package lnd import ( + "bytes" "fmt" "github.com/btcsuite/btcd/txscript" @@ -26,6 +27,12 @@ var ( // ErrInvalidState is returned when the closing state machine receives // a message while it is in an unknown state. ErrInvalidState = fmt.Errorf("invalid state") + + // errUpfrontShutdownScriptMismatch is returned when our peer sends us a + // shutdown message with a script that does not match the upfront shutdown + // script previously set. + errUpfrontShutdownScriptMismatch = fmt.Errorf("peer's shutdown " + + "script does not match upfront shutdown script") ) // closeState represents all the possible states the channel closer state @@ -83,6 +90,9 @@ type chanCloseCfg struct { // forward payments. disableChannel func(wire.OutPoint) error + // disconnect will disconnect from the remote peer in this close. + disconnect func() error + // quit is a channel that should be sent upon in the occasion the state // machine should cease all progress and shutdown. quit chan struct{} @@ -263,6 +273,40 @@ func (c *channelCloser) CloseRequest() *htlcswitch.ChanClose { return c.closeReq } +// 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 { + + // 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 + } + + // If an upfront shutdown script was provided, disconnect from the peer, as + // per BOLT 2, and return an error. + if !bytes.Equal(upfrontScript, peerScript) { + peerLog.Warnf("peer's script: %x does not match upfront "+ + "shutdown script: %x", peerScript, upfrontScript) + + // Disconnect from the peer because they have violated option upfront + // shutdown. + if err := disconnect(); err != nil { + return err + } + + return errUpfrontShutdownScriptMismatch + } + + return nil +} + // ProcessCloseMsg attempts to process the next message in the closing series. // This method will update the state accordingly and return two primary values: // the next set of messages to be sent, and a bool indicating if the fee @@ -283,9 +327,18 @@ func (c *channelCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, b "instead have %v", spew.Sdump(msg)) } - // Next, we'll note the other party's preference for their - // delivery address. We'll use this when we craft the closure - // transaction. + // 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, c.cfg.channel.RemoteUpfrontShutdownScript(), + shutDownMsg.Address, + ); err != nil { + return nil, false, err + } + + // Once we have checked that the other party has not violated option + // upfront shutdown we set their preference for delivery address. We'll + // use this when we craft the closure transaction. c.remoteDeliveryScript = shutDownMsg.Address // We'll generate a shutdown message of our own to send across @@ -333,7 +386,16 @@ func (c *channelCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, b "instead have %v", spew.Sdump(msg)) } - // Now that we know this is a valid shutdown message, we'll + // 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, c.cfg.channel.RemoteUpfrontShutdownScript(), + shutDownMsg.Address, + ); err != nil { + return nil, false, err + } + + // Now that we know this is a valid shutdown message and address, we'll // record their preferred delivery closing script. c.remoteDeliveryScript = shutDownMsg.Address diff --git a/chancloser_test.go b/chancloser_test.go new file mode 100644 index 00000000..f7c481d0 --- /dev/null +++ b/chancloser_test.go @@ -0,0 +1,76 @@ +package lnd + +import ( + "crypto/rand" + "testing" + + "github.com/lightningnetwork/lnd/lnwire" +) + +// 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) + if err != nil { + t.Fatalf("cannot generate random address: %v", err) + } + + 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) { + addr1 := randDeliveryAddress(t) + addr2 := randDeliveryAddress(t) + + tests := []struct { + name string + shutdownScript lnwire.DeliveryAddress + upfrontScript lnwire.DeliveryAddress + expectedErr error + }{ + { + name: "no upfront shutdown set, script ok", + shutdownScript: addr1, + upfrontScript: []byte{}, + expectedErr: nil, + }, + { + name: "upfront shutdown set, script ok", + shutdownScript: addr1, + upfrontScript: addr1, + expectedErr: nil, + }, + { + name: "upfront shutdown set, script not ok", + shutdownScript: addr1, + upfrontScript: addr2, + expectedErr: errUpfrontShutdownScriptMismatch, + }, + { + name: "nil shutdown and empty upfront", + shutdownScript: nil, + upfrontScript: []byte{}, + expectedErr: nil, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + err := maybeMatchScript( + func() error { return nil }, test.upfrontScript, + test.shutdownScript, + ) + + if err != test.expectedErr { + t.Fatalf("Error: %v, expected error: %v", err, test.expectedErr) + } + }) + } +} diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 57f6b370..a958dea8 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -4918,6 +4918,18 @@ func (lc *LightningChannel) ShortChanID() lnwire.ShortChannelID { return lc.channelState.ShortChanID() } +// LocalUpfrontShutdownScript returns the local upfront shutdown script for the +// channel. If it was not set, an empty byte array is returned. +func (lc *LightningChannel) LocalUpfrontShutdownScript() lnwire.DeliveryAddress { + return nil +} + +// RemoteUpfrontShutdownScript returns the remote upfront shutdown script for the +// channel. If it was not set, an empty byte array is returned. +func (lc *LightningChannel) RemoteUpfrontShutdownScript() lnwire.DeliveryAddress { + return nil +} + // genHtlcScript generates the proper P2WSH public key scripts for the HTLC // output modified by two-bits denoting if this is an incoming HTLC, and if the // HTLC is being applied to their commitment transaction or ours. diff --git a/peer.go b/peer.go index a007701b..d95a82ca 100644 --- a/peer.go +++ b/peer.go @@ -2068,13 +2068,18 @@ func (p *peer) fetchActiveChanCloser(chanID lnwire.ChannelID) (*channelCloser, e "channel w/ active htlcs") } - // We'll create a valid closing state machine in order to - // respond to the initiated cooperative channel closure. - deliveryAddr, err := p.genDeliveryScript() - if err != nil { - peerLog.Errorf("unable to gen delivery script: %v", err) - - return nil, fmt.Errorf("close addr unavailable") + // We'll create a valid closing state machine in order to respond to the + // initiated cooperative channel closure. First, we set the delivery + // script that our funds will be paid out to. If an upfront shutdown script + // was set, we will use it. Otherwise, we get a fresh delivery script. + deliveryScript := channel.LocalUpfrontShutdownScript() + if len(deliveryScript) == 0 { + var err error + deliveryScript, err = p.genDeliveryScript() + if err != nil { + peerLog.Errorf("unable to gen delivery script: %v", err) + return nil, fmt.Errorf("close addr unavailable") + } } // In order to begin fee negotiations, we'll first compute our @@ -2099,9 +2104,12 @@ func (p *peer) fetchActiveChanCloser(chanID lnwire.ChannelID) (*channelCloser, e unregisterChannel: p.server.htlcSwitch.RemoveLink, broadcastTx: p.server.cc.wallet.PublishTransaction, disableChannel: p.server.chanStatusMgr.RequestDisable, - quit: p.quit, + disconnect: func() error { + return p.server.DisconnectPeer(p.IdentityKey()) + }, + quit: p.quit, }, - deliveryAddr, + deliveryScript, feePerKw, uint32(startingHeight), nil, @@ -2134,14 +2142,19 @@ func (p *peer) handleLocalCloseReq(req *htlcswitch.ChanClose) { // out this channel on-chain, so we execute the cooperative channel // closure workflow. case htlcswitch.CloseRegular: - // First, we'll fetch a fresh delivery address that we'll use - // to send the funds to in the case of a successful - // negotiation. - deliveryAddr, err := p.genDeliveryScript() - if err != nil { - peerLog.Errorf(err.Error()) - req.Err <- err - return + // First, we'll fetch a delivery script that we'll use to send the + // funds to in the case of a successful negotiation. If an upfront + // shutdown script was set, we will use it. Otherwise, we get a fresh + // delivery script. + deliveryScript := channel.LocalUpfrontShutdownScript() + if len(deliveryScript) == 0 { + var err error + deliveryScript, err = p.genDeliveryScript() + if err != nil { + peerLog.Errorf(err.Error()) + req.Err <- err + return + } } // Next, we'll create a new channel closer state machine to @@ -2159,9 +2172,12 @@ func (p *peer) handleLocalCloseReq(req *htlcswitch.ChanClose) { unregisterChannel: p.server.htlcSwitch.RemoveLink, broadcastTx: p.server.cc.wallet.PublishTransaction, disableChannel: p.server.chanStatusMgr.RequestDisable, - quit: p.quit, + disconnect: func() error { + return p.server.DisconnectPeer(p.IdentityKey()) + }, + quit: p.quit, }, - deliveryAddr, + deliveryScript, req.TargetFeePerKw, uint32(startingHeight), req,