From 1e67040145eac82ccd3f03cedb4930161dd65139 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 30 Sep 2019 19:50:31 -0700 Subject: [PATCH 1/4] lnwallet: copy commitment transaction in getSignedCommitTx In this commit, we move to make a full deep copy of the commitment transaction in `getSignedCommitTx` to ensure that we don't mutate the commitment on disk, possibly resulting in a "hot commitment". --- lnwallet/channel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 720f5d0d..b29d79d4 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -4946,7 +4946,7 @@ func (lc *LightningChannel) getSignedCommitTx() (*wire.MsgTx, error) { // Fetch the current commitment transaction, along with their signature // for the transaction. localCommit := lc.channelState.LocalCommitment - commitTx := localCommit.CommitTx + commitTx := localCommit.CommitTx.Copy() theirSig := append(localCommit.CommitSig, byte(txscript.SigHashAll)) // With this, we then generate the full witness so the caller can From c199ad30acdcf42219d65a9a4d38d129b03f94a7 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 30 Sep 2019 19:53:58 -0700 Subject: [PATCH 2/4] lnwallet/chanvalidate: create new channel validation package In this commit, we create a new `chanvalidate` package which it to house all logic required for 1st and 3rd party channel verification. 1st party verification occurs when we find a channel in the chain that is allegedly ours, while 3rd party verification will occur when a peer sends us a channel proof of a new channel. In the scope of the recent CVE, we actually fully verified 3rd party channels, but failed to also include those checks in our 1st party verification code. In order to unify this logic, and prevent future issues, in this PR we move to concentrate all validation logic into a single function. Both 1st and 3rd party validation will then use this function. Additionally, having all the logic in a single place makes it easier to audit, and also write tests against. --- lnwallet/chanvalidate/validate.go | 205 +++++++++++++++++ lnwallet/chanvalidate/validate_test.go | 307 +++++++++++++++++++++++++ 2 files changed, 512 insertions(+) create mode 100644 lnwallet/chanvalidate/validate.go create mode 100644 lnwallet/chanvalidate/validate_test.go diff --git a/lnwallet/chanvalidate/validate.go b/lnwallet/chanvalidate/validate.go new file mode 100644 index 00000000..5e02b3e5 --- /dev/null +++ b/lnwallet/chanvalidate/validate.go @@ -0,0 +1,205 @@ +package chanvalidate + +import ( + "bytes" + "fmt" + + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" + "github.com/lightningnetwork/lnd/lnwire" +) + +var ( + // ErrInvalidOutPoint is returned when the ChanLocator is unable to + // find the target outpoint. + ErrInvalidOutPoint = fmt.Errorf("output meant to create channel cannot " + + "be found") + + // ErrWrongPkScript is returned when the alleged funding transaction is + // found to have an incorrect pkSript. + ErrWrongPkScript = fmt.Errorf("wrong pk script") + + // ErrInvalidSize is returned when the alleged funding transaction + // output has the wrong size (channel capacity). + ErrInvalidSize = fmt.Errorf("channel has wrong size") +) + +// ErrScriptValidateError is returned when Script VM validation fails for an +// alleged channel output. +type ErrScriptValidateError struct { + err error +} + +// Error returns a human readable string describing the error. +func (e *ErrScriptValidateError) Error() string { + return fmt.Sprintf("script validation failed: %v", e.err) +} + +// Unwrap returns the underlying wrapped VM execution failure error. +func (e *ErrScriptValidateError) Unwrap() error { + return e.err +} + +// ChanLocator abstracts away obtaining the output that created the channel, as +// well as validating its existence given the funding transaction. We need +// this as there are several ways (outpoint, short chan ID) to identify the +// output of a channel given the funding transaction. +type ChanLocator interface { + // Locate attempts to locate the funding output within the funding + // transaction. It also returns the final out point of the channel + // which uniquely identifies the output which creates the channel. If + // the target output cannot be found, or cannot exist on the funding + // transaction, then an error is to be returned. + Locate(*wire.MsgTx) (*wire.TxOut, *wire.OutPoint, error) +} + +// OutPointChanLocator is an implementation of the ChanLocator that can be used +// when one already knows the expected chan point. +type OutPointChanLocator struct { + // ChanPoint is the expected chan point. + ChanPoint wire.OutPoint +} + +// Locate attempts to locate the funding output within the passed funding +// transaction. +// +// NOTE: Part of the ChanLocator interface. +func (o *OutPointChanLocator) Locate(fundingTx *wire.MsgTx) ( + *wire.TxOut, *wire.OutPoint, error) { + + // If the expected index is greater than the amount of output in the + // transaction, then we'll reject this channel as it's invalid. + if int(o.ChanPoint.Index) >= len(fundingTx.TxOut) { + return nil, nil, ErrInvalidOutPoint + } + + // As an extra sanity check, we'll also ensure the txid hash matches. + fundingHash := fundingTx.TxHash() + if !bytes.Equal(fundingHash[:], o.ChanPoint.Hash[:]) { + return nil, nil, ErrInvalidOutPoint + } + + return fundingTx.TxOut[o.ChanPoint.Index], &o.ChanPoint, nil +} + +// ShortChanIDChanLocator is an implementation of the ChanLocator that can be +// used when one only knows the short channel ID of a channel. This should be +// used in contexts when one is verifying a 3rd party channel. +type ShortChanIDChanLocator struct { + // ID is the short channel ID of the target channel. + ID lnwire.ShortChannelID +} + +// Locate attempts to locate the funding output within the passed funding +// transaction. +// +// NOTE: Part of the ChanLocator interface. +func (s *ShortChanIDChanLocator) Locate(fundingTx *wire.MsgTx) ( + *wire.TxOut, *wire.OutPoint, error) { + + // If the expected index is greater than the amount of output in the + // transaction, then we'll reject this channel as it's invalid. + outputIndex := s.ID.TxPosition + if int(outputIndex) >= len(fundingTx.TxOut) { + return nil, nil, ErrInvalidOutPoint + } + + chanPoint := wire.OutPoint{ + Hash: fundingTx.TxHash(), + Index: uint32(outputIndex), + } + + return fundingTx.TxOut[outputIndex], &chanPoint, nil +} + +// CommitmentContext is optional validation context that can be passed into the +// main Validate for self-owned channel. The information in this context allows +// us to fully verify out initial commitment spend based on the on-chain state +// of the funding output. +type CommitmentContext struct { + // Value is the known size of the channel. + Value btcutil.Amount + + // FullySignedCommitTx is the fully signed commitment transaction. This + // should include a valid witness. + FullySignedCommitTx *wire.MsgTx +} + +// Context is the main validation contxet. For a given channel, all fields but +// the optional CommitCtx should be populated based on existing +// known-to-be-valid parameters. +type Context struct { + // Locator is a concrete implementation of the ChanLocator interface. + Locator ChanLocator + + // MultiSigPkScript is the fully serialized witness script of the + // multi-sig output. This is the final witness program that should be + // found in the funding output. + MultiSigPkScript []byte + + // FundingTx is channel funding transaction as found confirmed in the + // chain. + FundingTx *wire.MsgTx + + // CommitCtx is an optional additional set of validation context + // required to validate a self-owned channel. If present, then fully + // Script VM validation will be performed. + CommitCtx *CommitmentContext +} + +// Validate given the specified context, this function validates that the +// alleged channel is well formed, and spendable (if the optional CommitCtx is +// specified). If this method returns an error, then the alleged channel is +// invalid and should be abandoned immediately. +func Validate(ctx *Context) (*wire.OutPoint, error) { + // First, we'll attempt to locate the target outpoint in the funding + // transaction. If this returns an error, then we know that the + // outpoint doesn't actually exist, so we'll exit early. + fundingOutput, chanPoint, err := ctx.Locator.Locate( + ctx.FundingTx, + ) + if err != nil { + return nil, err + } + + // The scripts should match up exactly, otherwise the channel is + // invalid. + fundingScript := fundingOutput.PkScript + if !bytes.Equal(ctx.MultiSigPkScript, fundingScript) { + return nil, ErrWrongPkScript + } + + // If there's no commitment context, then we're done here as this is a + // 3rd party channel. + if ctx.CommitCtx == nil { + return chanPoint, nil + } + + // Now that we know this is our channel, we'll verify the amount of the + // created output against our expected size of the channel. + fundingValue := fundingOutput.Value + if btcutil.Amount(fundingValue) != ctx.CommitCtx.Value { + return nil, ErrInvalidSize + } + + // If we reach this point, then all other checks have succeeded, so + // we'll now attempt fully Script VM execution to ensure that we're + // able to close the channel using this initial state. + vm, err := txscript.NewEngine( + ctx.MultiSigPkScript, ctx.CommitCtx.FullySignedCommitTx, + 0, txscript.StandardVerifyFlags, nil, nil, fundingValue, + ) + if err != nil { + return nil, err + } + + // Finally, we'll attempt to verify our full spend, if this fails then + // the channel is definitely invalid. + err = vm.Execute() + if err != nil { + return nil, &ErrScriptValidateError{err: err} + } + + return chanPoint, nil +} diff --git a/lnwallet/chanvalidate/validate_test.go b/lnwallet/chanvalidate/validate_test.go new file mode 100644 index 00000000..12bb5c09 --- /dev/null +++ b/lnwallet/chanvalidate/validate_test.go @@ -0,0 +1,307 @@ +package chanvalidate + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/btcec" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwire" +) + +var ( + aliceKey = chainhash.Hash{ + 0xb7, 0x94, 0x38, 0x5f, 0x2d, 0x1e, 0xf7, 0xab, + 0x4d, 0x92, 0x73, 0xd1, 0x90, 0x63, 0x81, 0xb4, + 0x4f, 0x2f, 0x6f, 0x25, 0x18, 0xa3, 0xef, 0xb9, + 0x64, 0x49, 0x18, 0x83, 0x31, 0x98, 0x47, 0x53, + } + bobKey = chainhash.Hash{ + 0xb7, 0x94, 0x38, 0x5f, 0x2d, 0x1e, 0xf7, 0xab, + 0x4d, 0x92, 0x73, 0xd1, 0x90, 0x63, 0x81, 0xb4, + 0x4f, 0x2f, 0x6f, 0x25, 0x98, 0xa3, 0xef, 0xb9, + 0x69, 0x49, 0x18, 0x83, 0x31, 0x98, 0x47, 0x53, + } + + alicePriv, alicePub = btcec.PrivKeyFromBytes(btcec.S256(), aliceKey[:]) + bobPriv, bobPub = btcec.PrivKeyFromBytes(btcec.S256(), bobKey[:]) +) + +// channelTestCtx holds shared context that will be used in all tests cases +// below. +type channelTestCtx struct { + fundingTx *wire.MsgTx + + invalidCommitTx, validCommitTx *wire.MsgTx + + chanPoint wire.OutPoint + cid lnwire.ShortChannelID + + fundingScript []byte +} + +// newChannelTestCtx creates a new channelCtx for use in the validation tests +// below. This creates a fake funding transaction, as well as an invalid and +// valid commitment transaction. +func newChannelTestCtx(chanSize int64) (*channelTestCtx, error) { + multiSigScript, err := input.GenMultiSigScript( + alicePub.SerializeCompressed(), bobPub.SerializeCompressed(), + ) + if err != nil { + return nil, err + } + pkScript, err := input.WitnessScriptHash(multiSigScript) + if err != nil { + return nil, err + } + + fundingOutput := wire.TxOut{ + Value: chanSize, + PkScript: pkScript, + } + + fundingTx := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + {}, + }, + TxOut: []*wire.TxOut{ + &fundingOutput, + { + Value: 9999, + PkScript: bytes.Repeat([]byte{'a'}, 32), + }, + { + Value: 99999, + PkScript: bytes.Repeat([]byte{'b'}, 32), + }, + }, + } + + fundingTxHash := fundingTx.TxHash() + + commitTx := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: wire.OutPoint{ + Hash: fundingTxHash, + Index: 0, + }, + }, + }, + TxOut: []*wire.TxOut{ + &fundingOutput, + }, + } + + sigHashes := txscript.NewTxSigHashes(commitTx) + aliceSig, err := txscript.RawTxInWitnessSignature( + commitTx, sigHashes, 0, chanSize, + multiSigScript, txscript.SigHashAll, alicePriv, + ) + if err != nil { + return nil, err + } + + bobSig, err := txscript.RawTxInWitnessSignature( + commitTx, sigHashes, 0, chanSize, + multiSigScript, txscript.SigHashAll, bobPriv, + ) + if err != nil { + return nil, err + } + + commitTx.TxIn[0].Witness = input.SpendMultiSig( + multiSigScript, alicePub.SerializeCompressed(), aliceSig, + bobPub.SerializeCompressed(), bobSig, + ) + + invalidCommitTx := commitTx.Copy() + invalidCommitTx.TxIn[0].PreviousOutPoint.Index = 2 + + return &channelTestCtx{ + fundingTx: fundingTx, + validCommitTx: commitTx, + invalidCommitTx: invalidCommitTx, + chanPoint: wire.OutPoint{ + Hash: fundingTxHash, + Index: 0, + }, + cid: lnwire.ShortChannelID{ + TxPosition: 0, + }, + fundingScript: pkScript, + }, nil +} + +// TestValidate ensures that the Validate method is able to detect all cases of +// invalid channels, and properly accept invalid channels. +func TestValidate(t *testing.T) { + t.Parallel() + + chanSize := int64(1000000) + channelCtx, err := newChannelTestCtx(chanSize) + if err != nil { + t.Fatalf("unable to make channel context: %v", err) + } + + testCases := []struct { + // expectedErr is the error we expect, this should be nil if + // the channel is valid. + expectedErr error + + // locator is how the Validate method should find the target + // outpoint. + locator ChanLocator + + // chanPoint is the expected final out point. + chanPoint wire.OutPoint + + // chanScript is the funding pkScript. + chanScript []byte + + // fundingTx is the funding transaction to use in the test. + fundingTx *wire.MsgTx + + // commitTx is the commitment transaction to use in the test, + // this is optional. + commitTx *wire.MsgTx + + // expectedValue is the value of the funding transaction we + // should expect. This is only required if commitTx is non-nil. + expectedValue int64 + }{ + // Short chan ID channel locator, unable to find target + // outpoint. + { + expectedErr: ErrInvalidOutPoint, + locator: &ShortChanIDChanLocator{ + ID: lnwire.NewShortChanIDFromInt(9), + }, + fundingTx: &wire.MsgTx{}, + }, + + // Chan point based channel locator, unable to find target + // outpoint. + { + expectedErr: ErrInvalidOutPoint, + locator: &OutPointChanLocator{ + ChanPoint: wire.OutPoint{ + Index: 99, + }, + }, + fundingTx: &wire.MsgTx{}, + }, + + // Invalid pkScript match on mined funding transaction, chan + // point based locator. + { + expectedErr: ErrWrongPkScript, + locator: &OutPointChanLocator{ + ChanPoint: channelCtx.chanPoint, + }, + chanScript: bytes.Repeat([]byte("a"), 32), + fundingTx: channelCtx.fundingTx, + }, + + // Invalid pkScript match on mined funding transaction, short + // chan ID based locator. + { + expectedErr: ErrWrongPkScript, + locator: &ShortChanIDChanLocator{ + ID: channelCtx.cid, + }, + chanScript: bytes.Repeat([]byte("a"), 32), + fundingTx: channelCtx.fundingTx, + }, + + // Invalid amount on funding transaction. + { + expectedErr: ErrInvalidSize, + locator: &OutPointChanLocator{ + ChanPoint: channelCtx.chanPoint, + }, + chanScript: channelCtx.fundingScript, + fundingTx: channelCtx.fundingTx, + expectedValue: 555, + commitTx: channelCtx.validCommitTx, + }, + + // Validation failure on final commitment transaction + { + expectedErr: &ErrScriptValidateError{}, + locator: &OutPointChanLocator{ + ChanPoint: channelCtx.chanPoint, + }, + chanScript: channelCtx.fundingScript, + fundingTx: channelCtx.fundingTx, + expectedValue: chanSize, + commitTx: channelCtx.invalidCommitTx, + }, + + // Fully valid 3rd party verification. + { + expectedErr: nil, + locator: &OutPointChanLocator{ + ChanPoint: channelCtx.chanPoint, + }, + chanScript: channelCtx.fundingScript, + fundingTx: channelCtx.fundingTx, + chanPoint: channelCtx.chanPoint, + }, + + // Fully valid self-channel verification. + { + expectedErr: nil, + locator: &OutPointChanLocator{ + ChanPoint: channelCtx.chanPoint, + }, + chanScript: channelCtx.fundingScript, + fundingTx: channelCtx.fundingTx, + expectedValue: chanSize, + commitTx: channelCtx.validCommitTx, + chanPoint: channelCtx.chanPoint, + }, + } + + for i, testCase := range testCases { + ctx := &Context{ + Locator: testCase.locator, + MultiSigPkScript: testCase.chanScript, + FundingTx: testCase.fundingTx, + } + + if testCase.commitTx != nil { + ctx.CommitCtx = &CommitmentContext{ + Value: btcutil.Amount( + testCase.expectedValue, + ), + FullySignedCommitTx: testCase.commitTx, + } + } + + chanPoint, err := Validate(ctx) + if err != testCase.expectedErr { + _, ok := testCase.expectedErr.(*ErrScriptValidateError) + _, scriptErr := err.(*ErrScriptValidateError) + if ok && scriptErr { + continue + } + + t.Fatalf("test #%v: validation failed: expected %v, "+ + "got %v", i, testCase.expectedErr, err) + } + + if err != nil { + continue + } + + if *chanPoint != testCase.chanPoint { + t.Fatalf("test #%v: wrong outpoint: want %v, got %v", + i, testCase.chanPoint, chanPoint) + } + } +} From 6ebada112f1f44b0cd74de7995e98da571fc0e66 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 30 Sep 2019 19:55:03 -0700 Subject: [PATCH 3/4] routing: update 3d party channel verification to use new chanvalidate package In the process of moving to use the new package, we no longer need to fetch the outpoint directly, and instead only need to pass the funding transaction into the new verification logic. --- routing/router.go | 66 +++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/routing/router.go b/routing/router.go index d1ecefc0..cd048d32 100644 --- a/routing/router.go +++ b/routing/router.go @@ -21,6 +21,7 @@ import ( "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/lnwallet/chanvalidate" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/multimutex" "github.com/lightningnetwork/lnd/routing/chainview" @@ -1174,9 +1175,9 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { // to obtain the full funding outpoint that's encoded within // the channel ID. channelID := lnwire.NewShortChanIDFromInt(msg.ChannelID) - fundingPoint, _, err := r.fetchChanPoint(&channelID) + fundingTx, err := r.fetchFundingTx(&channelID) if err != nil { - return errors.Errorf("unable to fetch chan point for "+ + return errors.Errorf("unable to fetch funding tx for "+ "chan_id=%v: %v", msg.ChannelID, err) } @@ -1189,7 +1190,22 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { if err != nil { return err } - fundingPkScript, err := input.WitnessScriptHash(witnessScript) + pkScript, err := input.WitnessScriptHash(witnessScript) + if err != nil { + return err + } + + // Next we'll validate that this channel is actually well + // formed. If this check fails, then this channel either + // doesn't exist, or isn't the one that was meant to be created + // according to the passed channel proofs. + fundingPoint, err := chanvalidate.Validate(&chanvalidate.Context{ + Locator: &chanvalidate.ShortChanIDChanLocator{ + ID: channelID, + }, + MultiSigPkScript: pkScript, + FundingTx: fundingTx, + }) if err != nil { return err } @@ -1197,6 +1213,10 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { // Now that we have the funding outpoint of the channel, ensure // that it hasn't yet been spent. If so, then this channel has // been closed so we'll ignore it. + fundingPkScript, err := input.WitnessScriptHash(witnessScript) + if err != nil { + return err + } chanUtxo, err := r.cfg.Chain.GetUtxo( fundingPoint, fundingPkScript, channelID.BlockHeight, r.quit, @@ -1207,16 +1227,6 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { msg.ChannelID, fundingPoint, err) } - // By checking the equality of witness pkscripts we checks that - // funding witness script is multisignature lock which contains - // both local and remote public keys which was declared in - // channel edge and also that the announced channel value is - // right. - if !bytes.Equal(fundingPkScript, chanUtxo.PkScript) { - return errors.Errorf("pkScript mismatch: expected %x, "+ - "got %x", fundingPkScript, chanUtxo.PkScript) - } - // TODO(roasbeef): this is a hack, needs to be removed // after commitment fees are dynamic. msg.Capacity = btcutil.Amount(chanUtxo.Value) @@ -1339,25 +1349,24 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { return nil } -// fetchChanPoint retrieves the original outpoint which is encoded within the -// channelID. This method also return the public key script for the target -// transaction. +// fetchFundingTx returns the funding transaction identified by the passed +// short channel ID. // // TODO(roasbeef): replace with call to GetBlockTransaction? (would allow to // later use getblocktxn) -func (r *ChannelRouter) fetchChanPoint( - chanID *lnwire.ShortChannelID) (*wire.OutPoint, *wire.TxOut, error) { +func (r *ChannelRouter) fetchFundingTx( + chanID *lnwire.ShortChannelID) (*wire.MsgTx, error) { // First fetch the block hash by the block number encoded, then use // that hash to fetch the block itself. blockNum := int64(chanID.BlockHeight) blockHash, err := r.cfg.Chain.GetBlockHash(blockNum) if err != nil { - return nil, nil, err + return nil, err } fundingBlock, err := r.cfg.Chain.GetBlock(blockHash) if err != nil { - return nil, nil, err + return nil, err } // As a sanity check, ensure that the advertised transaction index is @@ -1365,21 +1374,12 @@ func (r *ChannelRouter) fetchChanPoint( // block. numTxns := uint32(len(fundingBlock.Transactions)) if chanID.TxIndex > numTxns-1 { - return nil, nil, fmt.Errorf("tx_index=#%v is out of range "+ - "(max_index=%v), network_chan_id=%v\n", chanID.TxIndex, - numTxns-1, spew.Sdump(chanID)) + return nil, fmt.Errorf("tx_index=#%v is out of range "+ + "(max_index=%v), network_chan_id=%v", chanID.TxIndex, + numTxns-1, chanID) } - // Finally once we have the block itself, we seek to the targeted - // transaction index to obtain the funding output and txout. - fundingTx := fundingBlock.Transactions[chanID.TxIndex] - outPoint := &wire.OutPoint{ - Hash: fundingTx.TxHash(), - Index: uint32(chanID.TxPosition), - } - txOut := fundingTx.TxOut[chanID.TxPosition] - - return outPoint, txOut, nil + return fundingBlock.Transactions[chanID.TxIndex], nil } // routingMsg couples a routing related routing topology update to the From 0e13c5ac3f33e754ed0723efaf94638af36337ec Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 30 Sep 2019 19:59:10 -0700 Subject: [PATCH 4/4] lnwallet+funding: expose new ValidateChannel method for 1st party validation In this commit, we use the recently added `chanvalidate` package to verify channels once they have been confirmed in the funding manager. We expose a new method on the `LightningWallet` struct: `ValidateChannels` which calls the new shared 1st party verification code. After the channel is fully confirmed in the funding manager, we'll now use this newly exposed method to handle all validation. As a result, we can remove the existing validation code in the funding manager, and rely on the new code in isolation. --- fundingmanager.go | 72 ++++++++++++++++++------------- lnwallet/chanvalidate/validate.go | 4 +- lnwallet/wallet.go | 56 ++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 32 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index b6108437..99161a14 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -969,7 +969,7 @@ func (f *fundingManager) stateStep(channel *channeldb.OpenChannel, func (f *fundingManager) advancePendingChannelState( channel *channeldb.OpenChannel, pendingChanID [32]byte) error { - shortChanID, err := f.waitForFundingWithTimeout(channel) + confChannel, err := f.waitForFundingWithTimeout(channel) if err == ErrConfirmationTimeout { // We'll get a timeout if the number of blocks mined // since the channel was initiated reaches @@ -1031,9 +1031,9 @@ func (f *fundingManager) advancePendingChannelState( // Success, funding transaction was confirmed. chanID := lnwire.NewChanIDFromOutPoint(&channel.FundingOutpoint) fndgLog.Debugf("ChannelID(%v) is now fully confirmed! "+ - "(shortChanID=%v)", chanID, shortChanID) + "(shortChanID=%v)", chanID, confChannel.shortChanID) - err = f.handleFundingConfirmation(channel, *shortChanID) + err = f.handleFundingConfirmation(channel, confChannel) if err != nil { return fmt.Errorf("unable to handle funding "+ "confirmation for ChannelPoint(%v): %v", @@ -1792,15 +1792,28 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { go f.advanceFundingState(completeChan, pendingChanID, resCtx.updates) } +// confirmedChannel wraps a confirmed funding transaction, as well as the short +// channel ID which identifies that channel into a single struct. We'll use +// this to pass around the final state of a channel after it has been +// confirmed. +type confirmedChannel struct { + // shortChanID expresses where in the block the funding transaction was + // located. + shortChanID lnwire.ShortChannelID + + // fundingTx is the funding transaction that created the channel. + fundingTx *wire.MsgTx +} + // waitForFundingWithTimeout is a wrapper around waitForFundingConfirmation and // waitForTimeout that will return ErrConfirmationTimeout if we are not the // channel initiator and the maxWaitNumBlocksFundingConf has passed from the // funding broadcast height. In case of confirmation, the short channel ID of -// the channel will be returned. +// the channel and the funding transaction will be returned. func (f *fundingManager) waitForFundingWithTimeout( - ch *channeldb.OpenChannel) (*lnwire.ShortChannelID, error) { + ch *channeldb.OpenChannel) (*confirmedChannel, error) { - confChan := make(chan *lnwire.ShortChannelID) + confChan := make(chan *confirmedChannel) timeoutChan := make(chan error, 1) cancelChan := make(chan struct{}) @@ -1816,8 +1829,6 @@ func (f *fundingManager) waitForFundingWithTimeout( } defer close(cancelChan) - var shortChanID *lnwire.ShortChannelID - var ok bool select { case err := <-timeoutChan: if err != nil { @@ -1830,12 +1841,12 @@ func (f *fundingManager) waitForFundingWithTimeout( // startup. return nil, ErrFundingManagerShuttingDown - case shortChanID, ok = <-confChan: + case confirmedChannel, ok := <-confChan: if !ok { return nil, fmt.Errorf("waiting for funding" + "confirmation failed") } - return shortChanID, nil + return confirmedChannel, nil } } @@ -1864,7 +1875,7 @@ func makeFundingScript(channel *channeldb.OpenChannel) ([]byte, error) { // NOTE: This MUST be run as a goroutine. func (f *fundingManager) waitForFundingConfirmation( completeChan *channeldb.OpenChannel, cancelChan <-chan struct{}, - confChan chan<- *lnwire.ShortChannelID) { + confChan chan<- *confirmedChannel) { defer f.wg.Done() defer close(confChan) @@ -1924,22 +1935,8 @@ func (f *fundingManager) waitForFundingConfirmation( } fundingPoint := completeChan.FundingOutpoint - chanID := lnwire.NewChanIDFromOutPoint(&fundingPoint) - if int(fundingPoint.Index) >= len(confDetails.Tx.TxOut) { - fndgLog.Warnf("Funding point index does not exist for "+ - "ChannelPoint(%v)", completeChan.FundingOutpoint) - return - } - - outputAmt := btcutil.Amount(confDetails.Tx.TxOut[fundingPoint.Index].Value) - if outputAmt != completeChan.Capacity { - fndgLog.Warnf("Invalid output value for ChannelPoint(%v)", - completeChan.FundingOutpoint) - return - } - fndgLog.Infof("ChannelPoint(%v) is now active: ChannelID(%x)", - fundingPoint, chanID[:]) + fundingPoint, lnwire.NewChanIDFromOutPoint(&fundingPoint)) // With the block height and the transaction index known, we can // construct the compact chanID which is used on the network to unique @@ -1951,7 +1948,10 @@ func (f *fundingManager) waitForFundingConfirmation( } select { - case confChan <- &shortChanID: + case confChan <- &confirmedChannel{ + shortChanID: shortChanID, + fundingTx: confDetails.Tx, + }: case <-f.quit: return } @@ -2022,7 +2022,7 @@ func (f *fundingManager) waitForTimeout(completeChan *channeldb.OpenChannel, // for this channel. func (f *fundingManager) handleFundingConfirmation( completeChan *channeldb.OpenChannel, - shortChanID lnwire.ShortChannelID) error { + confChannel *confirmedChannel) error { fundingPoint := completeChan.FundingOutpoint chanID := lnwire.NewChanIDFromOutPoint(&fundingPoint) @@ -2030,13 +2030,24 @@ func (f *fundingManager) handleFundingConfirmation( // TODO(roasbeef): ideally persistent state update for chan above // should be abstracted + // Now that that the channel has been fully confirmed, we'll request + // that the wallet fully verify this channel to ensure that it can be + // used. + err := f.cfg.Wallet.ValidateChannel(completeChan, confChannel.fundingTx) + if err != nil { + // TODO(roasbeef): delete chan state? + return fmt.Errorf("unable to validate channel: %v", err) + } + // The funding transaction now being confirmed, we add this channel to // the fundingManager's internal persistent state machine that we use // to track the remaining process of the channel opening. This is // useful to resume the opening process in case of restarts. We set the // opening state before we mark the channel opened in the database, // such that we can receover from one of the db writes failing. - err := f.saveChannelOpeningState(&fundingPoint, markedOpen, &shortChanID) + err = f.saveChannelOpeningState( + &fundingPoint, markedOpen, &confChannel.shortChanID, + ) if err != nil { return fmt.Errorf("error setting channel state to markedOpen: %v", err) @@ -2044,7 +2055,8 @@ func (f *fundingManager) handleFundingConfirmation( // Now that the channel has been fully confirmed and we successfully // saved the opening state, we'll mark it as open within the database. - if err := completeChan.MarkAsOpen(shortChanID); err != nil { + err = completeChan.MarkAsOpen(confChannel.shortChanID) + if err != nil { return fmt.Errorf("error setting channel pending flag to false: "+ "%v", err) } diff --git a/lnwallet/chanvalidate/validate.go b/lnwallet/chanvalidate/validate.go index 5e02b3e5..213767fd 100644 --- a/lnwallet/chanvalidate/validate.go +++ b/lnwallet/chanvalidate/validate.go @@ -143,7 +143,7 @@ type Context struct { FundingTx *wire.MsgTx // CommitCtx is an optional additional set of validation context - // required to validate a self-owned channel. If present, then fully + // required to validate a self-owned channel. If present, then a full // Script VM validation will be performed. CommitCtx *CommitmentContext } @@ -184,7 +184,7 @@ func Validate(ctx *Context) (*wire.OutPoint, error) { } // If we reach this point, then all other checks have succeeded, so - // we'll now attempt fully Script VM execution to ensure that we're + // we'll now attempt a full Script VM execution to ensure that we're // able to close the channel using this initial state. vm, err := txscript.NewEngine( ctx.MultiSigPkScript, ctx.CommitCtx.FullySignedCommitTx, diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index 02321baf..85bf2d60 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -21,6 +21,7 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" + "github.com/lightningnetwork/lnd/lnwallet/chanvalidate" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/shachain" ) @@ -1636,3 +1637,58 @@ func coinSelectSubtractFees(feeRate SatPerKWeight, amt, return selectedUtxos, outputAmt, changeAmt, nil } + +// ValidateChannel will attempt to fully validate a newly mined channel, given +// its funding transaction and existing channel state. If this method returns +// an error, then the mined channel is invalid, and shouldn't be used. +func (l *LightningWallet) ValidateChannel(channelState *channeldb.OpenChannel, + fundingTx *wire.MsgTx) error { + + // First, we'll obtain a fully signed commitment transaction so we can + // pass into it on the chanvalidate package for verification. + channel, err := NewLightningChannel(l.Cfg.Signer, channelState, nil) + if err != nil { + return err + } + signedCommitTx, err := channel.getSignedCommitTx() + if err != nil { + return err + } + + // We'll also need the multi-sig witness script itself so the + // chanvalidate package can check it for correctness against the + // funding transaction, and also commitment validity. + localKey := channelState.LocalChanCfg.MultiSigKey.PubKey + remoteKey := channelState.RemoteChanCfg.MultiSigKey.PubKey + witnessScript, err := input.GenMultiSigScript( + localKey.SerializeCompressed(), + remoteKey.SerializeCompressed(), + ) + if err != nil { + return err + } + pkScript, err := input.WitnessScriptHash(witnessScript) + if err != nil { + return err + } + + // Finally, we'll pass in all the necessary context needed to fully + // validate that this channel is indeed what we expect, and can be + // used. + _, err = chanvalidate.Validate(&chanvalidate.Context{ + Locator: &chanvalidate.OutPointChanLocator{ + ChanPoint: channelState.FundingOutpoint, + }, + MultiSigPkScript: pkScript, + FundingTx: fundingTx, + CommitCtx: &chanvalidate.CommitmentContext{ + Value: channel.Capacity, + FullySignedCommitTx: signedCommitTx, + }, + }) + if err != nil { + return err + } + + return nil +}