cnct: parse onion for resolvers

With the introduction of additional payload fields for mpp, it becomes
a necessity to have their values available in the on-chain resolution
flow. The incoming contest resolver notifies the invoice registry of the
arrival of a payment and needs to supply all parameters for the registry
to validate the htlc.

contractcourt/chain_arbitrator.go

@@ -151,6 +151,10 @@ type ChainArbitratorConfig struct {
 	// NotifyClosedChannel is a function closure that the ChainArbitrator
 	// will use to notify the ChannelNotifier about a newly closed channel.
 	NotifyClosedChannel func(wire.OutPoint)
+
+	// OnionProcessor is used to decode onion payloads for on-chain
+	// resolution.
+	OnionProcessor OnionProcessor
 }
 
 // ChainArbitrator is a sub-system that oversees the on-chain resolution of all

contractcourt/channel_arbitrator_test.go

@@ -308,6 +308,7 @@ func createTestChannelArbitrator(t *testing.T, log ArbitratorLog) (*chanArbTestC
 			incubateChan <- struct{}{}
 			return nil
 		},
+		OnionProcessor: &mockOnionProcessor{},
 	}
 
 	// We'll use the resolvedChan to synchronize on call to

contractcourt/htlc_incoming_contest_resolver.go

@@ -1,12 +1,14 @@
 package contractcourt
 
 import (
+	"bytes"
 	"encoding/binary"
 	"errors"
 	"io"
 
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/htlcswitch/hop"
 	"github.com/lightningnetwork/lnd/invoices"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
@@ -65,6 +67,22 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 		return nil, nil
 	}
 
+	// First try to parse the payload. If that fails, we can stop resolution
+	// now.
+	payload, err := h.decodePayload()
+	if err != nil {
+		log.Debugf("ChannelArbitrator(%v): cannot decode payload of "+
+			"htlc %v", h.ChanPoint, h.HtlcPoint())
+
+		// If we've locked in an htlc with an invalid payload on our
+		// commitment tx, we don't need to resolve it. The other party
+		// will time it out and get their funds back. This situation can
+		// present itself when we crash before processRemoteAdds in the
+		// link has ran.
+		h.resolved = true
+		return nil, nil
+	}
+
 	// Register for block epochs. After registration, the current height
 	// will be sent on the channel immediately.
 	blockEpochs, err := h.Notifier.RegisterBlockEpochNtfn(nil)
@@ -185,7 +203,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 
 	event, err := h.Registry.NotifyExitHopHtlc(
 		h.htlc.RHash, h.htlc.Amt, h.htlcExpiry, currentHeight,
-		circuitKey, hodlChan, nil,
+		circuitKey, hodlChan, payload,
 	)
 	switch err {
 	case channeldb.ErrInvoiceNotFound:
@@ -344,6 +362,20 @@ func (h *htlcIncomingContestResolver) Supplement(htlc channeldb.HTLC) {
 	h.htlc = htlc
 }
 
+// decodePayload (re)decodes the hop payload of a received htlc.
+func (h *htlcIncomingContestResolver) decodePayload() (*hop.Payload, error) {
+
+	onionReader := bytes.NewReader(h.htlc.OnionBlob)
+	iterator, err := h.OnionProcessor.ReconstructHopIterator(
+		onionReader, h.htlc.RHash[:],
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return iterator.HopPayload()
+}
+
 // A compile time assertion to ensure htlcIncomingContestResolver meets the
 // ContractResolver interface.
 var _ htlcContractResolver = (*htlcIncomingContestResolver)(nil)

contractcourt/htlc_incoming_resolver_test.go

@@ -2,9 +2,12 @@ package contractcourt
 
 import (
 	"bytes"
+	"io"
+	"io/ioutil"
 	"testing"
 
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/htlcswitch/hop"
 	"github.com/lightningnetwork/lnd/invoices"
 	"github.com/lightningnetwork/lnd/lnwallet"
 
@@ -21,6 +24,7 @@ var (
 	testResPreimage   = lntypes.Preimage{1, 2, 3}
 	testResHash       = testResPreimage.Hash()
 	testResCircuitKey = channeldb.CircuitKey{}
+	testOnionBlob     = []byte{4, 5, 6}
 )
 
 // TestHtlcIncomingResolverFwdPreimageKnown tests resolution of a forwarded htlc
@@ -107,6 +111,12 @@ func TestHtlcIncomingResolverExitSettle(t *testing.T) {
 	}
 
 	ctx.waitForResult(true)
+
+	if !bytes.Equal(
+		ctx.onionProcessor.offeredOnionBlob, testOnionBlob,
+	) {
+		t.Fatal("unexpected onion blob")
+	}
 }
 
 // TestHtlcIncomingResolverExitCancel tests resolution of an exit hop htlc for
@@ -168,14 +178,39 @@ func TestHtlcIncomingResolverExitCancelHodl(t *testing.T) {
 	ctx.waitForResult(false)
 }
 
+type mockHopIterator struct {
+	hop.Iterator
+}
+
+func (h *mockHopIterator) HopPayload() (*hop.Payload, error) {
+	return nil, nil
+}
+
+type mockOnionProcessor struct {
+	offeredOnionBlob []byte
+}
+
+func (o *mockOnionProcessor) ReconstructHopIterator(r io.Reader, rHash []byte) (
+	hop.Iterator, error) {
+
+	data, err := ioutil.ReadAll(r)
+	if err != nil {
+		return nil, err
+	}
+	o.offeredOnionBlob = data
+
+	return &mockHopIterator{}, nil
+}
+
 type incomingResolverTestContext struct {
-	registry      *mockRegistry
+	registry       *mockRegistry
-	witnessBeacon *mockWitnessBeacon
+	witnessBeacon  *mockWitnessBeacon
-	resolver      *htlcIncomingContestResolver
+	resolver       *htlcIncomingContestResolver
-	notifier      *mockNotifier
+	notifier       *mockNotifier
-	resolveErr    chan error
+	onionProcessor *mockOnionProcessor
-	nextResolver  ContractResolver
+	resolveErr     chan error
-	t             *testing.T
+	nextResolver   ContractResolver
+	t              *testing.T
 }
 
 func newIncomingResolverTestContext(t *testing.T) *incomingResolverTestContext {
@@ -189,13 +224,16 @@ func newIncomingResolverTestContext(t *testing.T) *incomingResolverTestContext {
 		notifyChan: make(chan notifyExitHopData, 1),
 	}
 
+	onionProcessor := &mockOnionProcessor{}
+
 	checkPointChan := make(chan struct{}, 1)
 
 	chainCfg := ChannelArbitratorConfig{
 		ChainArbitratorConfig: ChainArbitratorConfig{
-			Notifier:   notifier,
+			Notifier:       notifier,
-			PreimageDB: witnessBeacon,
+			PreimageDB:     witnessBeacon,
-			Registry:   registry,
+			Registry:       registry,
+			OnionProcessor: onionProcessor,
 		},
 	}
 
@@ -211,18 +249,20 @@ func newIncomingResolverTestContext(t *testing.T) *incomingResolverTestContext {
 			contractResolverKit: *newContractResolverKit(cfg),
 			htlcResolution:      lnwallet.IncomingHtlcResolution{},
 			htlc: channeldb.HTLC{
-				RHash: testResHash,
+				RHash:     testResHash,
+				OnionBlob: testOnionBlob,
 			},
 		},
 		htlcExpiry: testHtlcExpiry,
 	}
 
 	return &incomingResolverTestContext{
-		registry:      registry,
+		registry:       registry,
-		witnessBeacon: witnessBeacon,
+		witnessBeacon:  witnessBeacon,
-		resolver:      resolver,
+		resolver:       resolver,
-		notifier:      notifier,
+		notifier:       notifier,
-		t:             t,
+		onionProcessor: onionProcessor,
+		t:              t,
 	}
 }
 

contractcourt/htlc_outgoing_contest_resolver_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightningnetwork/lnd/chainntnfs"
+	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
@@ -98,6 +99,8 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext {
 
 	preimageDB := newMockWitnessBeacon()
 
+	onionProcessor := &mockOnionProcessor{}
+
 	chainCfg := ChannelArbitratorConfig{
 		ChainArbitratorConfig: ChainArbitratorConfig{
 			Notifier:   notifier,
@@ -112,6 +115,7 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext {
 				resolutionChan <- msgs[0]
 				return nil
 			},
+			OnionProcessor: onionProcessor,
 		},
 	}
 
@@ -134,6 +138,10 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext {
 		htlcTimeoutResolver: htlcTimeoutResolver{
 			contractResolverKit: *newContractResolverKit(cfg),
 			htlcResolution:      outgoingRes,
+			htlc: channeldb.HTLC{
+				RHash:     testResHash,
+				OnionBlob: testOnionBlob,
+			},
 		},
 	}
 

contractcourt/interfaces.go

@@ -1,7 +1,10 @@
 package contractcourt
 
 import (
+	"io"
+
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/htlcswitch/hop"
 	"github.com/lightningnetwork/lnd/invoices"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -26,3 +29,10 @@ type Registry interface {
 	// HodlUnsubscribeAll unsubscribes from all hodl events.
 	HodlUnsubscribeAll(subscriber chan<- interface{})
 }
+
+// OnionProcessor is an interface used to decode onion blobs.
+type OnionProcessor interface {
+	// ReconstructHopIterator attempts to decode a valid sphinx packet from
+	// the passed io.Reader instance.
+	ReconstructHopIterator(r io.Reader, rHash []byte) (hop.Iterator, error)
+}

htlcswitch/hop/iterator.go

@@ -184,6 +184,30 @@ func (p *OnionProcessor) DecodeHopIterator(r io.Reader, rHash []byte,
 	return makeSphinxHopIterator(onionPkt, sphinxPacket), lnwire.CodeNone
 }
 
+// ReconstructHopIterator attempts to decode a valid sphinx packet from the passed io.Reader
+// instance using the rHash as the associated data when checking the relevant
+// MACs during the decoding process.
+func (p *OnionProcessor) ReconstructHopIterator(r io.Reader, rHash []byte) (
+	Iterator, error) {
+
+	onionPkt := &sphinx.OnionPacket{}
+	if err := onionPkt.Decode(r); err != nil {
+		return nil, err
+	}
+
+	// Attempt to process the Sphinx packet. We include the payment hash of
+	// the HTLC as it's authenticated within the Sphinx packet itself as
+	// associated data in order to thwart attempts a replay attacks. In the
+	// case of a replay, an attacker is *forced* to use the same payment
+	// hash twice, thereby losing their money entirely.
+	sphinxPacket, err := p.router.ReconstructOnionPacket(onionPkt, rHash)
+	if err != nil {
+		return nil, err
+	}
+
+	return makeSphinxHopIterator(onionPkt, sphinxPacket), nil
+}
+
 // DecodeHopIteratorRequest encapsulates all date necessary to process an onion
 // packet, perform sphinx replay detection, and schedule the entry for garbage
 // collection.

server.go

@@ -906,6 +906,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB,
 		Sweeper:             s.sweeper,
 		Registry:            s.invoices,
 		NotifyClosedChannel: s.channelNotifier.NotifyClosedChannelEvent,
+		OnionProcessor:      s.sphinx,
 	}, chanDB)
 
 	s.breachArbiter = newBreachArbiter(&BreachConfig{