From 0d7119a8ca2c002aa647841b7e7db52affee9192 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 26 Aug 2019 14:06:57 +0200 Subject: [PATCH] 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 | 4 ++ contractcourt/channel_arbitrator_test.go | 1 + .../htlc_incoming_contest_resolver.go | 34 ++++++++- contractcourt/htlc_incoming_resolver_test.go | 72 ++++++++++++++----- .../htlc_outgoing_contest_resolver_test.go | 8 +++ contractcourt/interfaces.go | 10 +++ htlcswitch/hop/iterator.go | 24 +++++++ server.go | 1 + 8 files changed, 137 insertions(+), 17 deletions(-) diff --git a/contractcourt/chain_arbitrator.go b/contractcourt/chain_arbitrator.go index 098bc5f3..3b25abc7 100644 --- a/contractcourt/chain_arbitrator.go +++ b/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 diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index 3e71c1bb..59902873 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/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 diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index d2bd3f89..2e089724 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/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) diff --git a/contractcourt/htlc_incoming_resolver_test.go b/contractcourt/htlc_incoming_resolver_test.go index 04be1583..ab8ad6ec 100644 --- a/contractcourt/htlc_incoming_resolver_test.go +++ b/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 - witnessBeacon *mockWitnessBeacon - resolver *htlcIncomingContestResolver - notifier *mockNotifier - resolveErr chan error - nextResolver ContractResolver - t *testing.T + registry *mockRegistry + witnessBeacon *mockWitnessBeacon + resolver *htlcIncomingContestResolver + notifier *mockNotifier + onionProcessor *mockOnionProcessor + resolveErr chan error + 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, - PreimageDB: witnessBeacon, - Registry: registry, + Notifier: notifier, + PreimageDB: witnessBeacon, + 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, - witnessBeacon: witnessBeacon, - resolver: resolver, - notifier: notifier, - t: t, + registry: registry, + witnessBeacon: witnessBeacon, + resolver: resolver, + notifier: notifier, + onionProcessor: onionProcessor, + t: t, } } diff --git a/contractcourt/htlc_outgoing_contest_resolver_test.go b/contractcourt/htlc_outgoing_contest_resolver_test.go index 1f7dbed1..3a6c1ea5 100644 --- a/contractcourt/htlc_outgoing_contest_resolver_test.go +++ b/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, + }, }, } diff --git a/contractcourt/interfaces.go b/contractcourt/interfaces.go index c73a23a0..a8847785 100644 --- a/contractcourt/interfaces.go +++ b/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) +} diff --git a/htlcswitch/hop/iterator.go b/htlcswitch/hop/iterator.go index a11ce632..5c8afed2 100644 --- a/htlcswitch/hop/iterator.go +++ b/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. diff --git a/server.go b/server.go index 7047721c..ccdd13ae 100644 --- a/server.go +++ b/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{