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

@@ -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
+}

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)
+		}
+	}
+}