htlcswitch: split shared secret extraction and encrypter instantiation

Preparation for the instantiation of an attributable error encrypter. This gets rid
of the sphinx encrypter instantiation in OnionProcessor. This would
otherwise be problematic when an attributable encrypter would need to be
created there without having access to the error structure.

Author: joostjager

htlcswitch/circuit.go

@@ -199,7 +199,7 @@ func (c *PaymentCircuit) Decode(r io.Reader) error {
 
 	case hop.EncrypterTypeSphinx:
 		// Sphinx encrypter was used as this is a forwarded HTLC.
-		c.ErrorEncrypter = hop.NewSphinxErrorEncrypter()
+		c.ErrorEncrypter = hop.NewSphinxErrorEncrypterUninitialized()
 
 	case hop.EncrypterTypeMock:
 		// Test encrypter.

htlcswitch/circuit_map.go

@@ -210,9 +210,9 @@ type CircuitMapConfig struct {
 	FetchClosedChannels func(
 		pendingOnly bool) ([]*channeldb.ChannelCloseSummary, error)
 
-	// ExtractErrorEncrypter derives the shared secret used to encrypt
+	// ExtractSharedSecret derives the shared secret used to encrypt
 	// errors from the obfuscator's ephemeral public key.
-	ExtractErrorEncrypter hop.ErrorEncrypterExtracter
+	ExtractSharedSecret hop.SharedSecretGenerator
 
 	// CheckResolutionMsg checks whether a given resolution message exists
 	// for the passed CircuitKey.
@@ -633,7 +633,7 @@ func (cm *circuitMap) decodeCircuit(v []byte) (*PaymentCircuit, error) {
 	// Otherwise, we need to reextract the encrypter, so that the shared
 	// secret is rederived from what was decoded.
 	err := circuit.ErrorEncrypter.Reextract(
-		cm.cfg.ExtractErrorEncrypter,
+		cm.cfg.ExtractSharedSecret,
 	)
 	if err != nil {
 		return nil, err

htlcswitch/circuit_test.go

@@ -66,16 +66,16 @@ func initTestExtracter() {
 	onionProcessor := newOnionProcessor(nil)
 	defer onionProcessor.Stop()
 
-	obfuscator, _ := onionProcessor.ExtractErrorEncrypter(
+	sharedSecret, failCode := onionProcessor.ExtractSharedSecret(
 		testEphemeralKey,
 	)
-
-	sphinxExtracter, ok := obfuscator.(*hop.SphinxErrorEncrypter)
-	if !ok {
-		panic("did not extract sphinx error encrypter")
+	if failCode != lnwire.CodeNone {
+		panic("did not extract shared secret")
 	}
 
-	testExtracter = sphinxExtracter
+	testExtracter = hop.NewSphinxErrorEncrypter(
+		testEphemeralKey, sharedSecret,
+	)
 
 	// We also set this error extracter on startup, otherwise it will be nil
 	// at compile-time.
@@ -107,10 +107,10 @@ func newCircuitMap(t *testing.T, resMsg bool) (*htlcswitch.CircuitMapConfig,
 
 	db := makeCircuitDB(t, "")
 	circuitMapCfg := &htlcswitch.CircuitMapConfig{
-		DB:                    db,
-		FetchAllOpenChannels:  db.ChannelStateDB().FetchAllOpenChannels,
-		FetchClosedChannels:   db.ChannelStateDB().FetchClosedChannels,
-		ExtractErrorEncrypter: onionProcessor.ExtractErrorEncrypter,
+		DB:                   db,
+		FetchAllOpenChannels: db.ChannelStateDB().FetchAllOpenChannels,
+		FetchClosedChannels:  db.ChannelStateDB().FetchClosedChannels,
+		ExtractSharedSecret:  onionProcessor.ExtractSharedSecret,
 	}
 
 	if resMsg {
@@ -217,7 +217,7 @@ func TestHalfCircuitSerialization(t *testing.T) {
 		// encrypters, this will be a NOP.
 		if circuit2.ErrorEncrypter != nil {
 			err := circuit2.ErrorEncrypter.Reextract(
-				onionProcessor.ExtractErrorEncrypter,
+				onionProcessor.ExtractSharedSecret,
 			)
 			if err != nil {
 				t.Fatalf("unable to reextract sphinx error "+
@@ -646,11 +646,11 @@ func restartCircuitMap(t *testing.T, cfg *htlcswitch.CircuitMapConfig) (
 	// Reinitialize circuit map with same db path.
 	db := makeCircuitDB(t, dbPath)
 	cfg2 := &htlcswitch.CircuitMapConfig{
-		DB:                    db,
-		FetchAllOpenChannels:  db.ChannelStateDB().FetchAllOpenChannels,
-		FetchClosedChannels:   db.ChannelStateDB().FetchClosedChannels,
-		ExtractErrorEncrypter: cfg.ExtractErrorEncrypter,
-		CheckResolutionMsg:    cfg.CheckResolutionMsg,
+		DB:                   db,
+		FetchAllOpenChannels: db.ChannelStateDB().FetchAllOpenChannels,
+		FetchClosedChannels:  db.ChannelStateDB().FetchClosedChannels,
+		ExtractSharedSecret:  cfg.ExtractSharedSecret,
+		CheckResolutionMsg:   cfg.CheckResolutionMsg,
 	}
 	cm2, err := htlcswitch.NewCircuitMap(cfg2)
 	require.NoError(t, err, "unable to recreate persistent circuit map")

htlcswitch/hop/error_encryptor.go

@@ -27,9 +27,9 @@ const (
 	EncrypterTypeMock = 2
 )
 
-// ErrorEncrypterExtracter defines a function signature that extracts an
-// ErrorEncrypter from an sphinx OnionPacket.
-type ErrorEncrypterExtracter func(*btcec.PublicKey) (ErrorEncrypter,
+// SharedSecretGenerator defines a function signature that extracts a shared
+// secret from an sphinx OnionPacket.
+type SharedSecretGenerator func(*btcec.PublicKey) (sphinx.Hash256,
 	lnwire.FailCode)
 
 // ErrorEncrypter is an interface that is used to encrypt HTLC related errors
@@ -66,12 +66,13 @@ type ErrorEncrypter interface {
 	// given io.Reader.
 	Decode(io.Reader) error
 
-	// Reextract rederives the encrypter using the extracter, performing an
-	// ECDH with the sphinx router's key and the ephemeral public key.
+	// Reextract rederives the encrypter using the shared secret generator,
+	// performing an ECDH with the sphinx router's key and the ephemeral
+	// public key.
 	//
 	// NOTE: This should be called shortly after Decode to properly
 	// reinitialize the error encrypter.
-	Reextract(ErrorEncrypterExtracter) error
+	Reextract(SharedSecretGenerator) error
 }
 
 // SphinxErrorEncrypter is a concrete implementation of both the ErrorEncrypter
@@ -84,20 +85,42 @@ type SphinxErrorEncrypter struct {
 	EphemeralKey *btcec.PublicKey
 }
 
-// NewSphinxErrorEncrypter initializes a blank sphinx error encrypter, that
-// should be used to deserialize an encoded SphinxErrorEncrypter. Since the
-// actual encrypter is not stored in plaintext while at rest, reconstructing the
-// error encrypter requires:
+// NewSphinxErrorEncrypterUninitialized initializes a blank sphinx error
+// encrypter, that should be used to deserialize an encoded
+// SphinxErrorEncrypter. Since the actual encrypter is not stored in plaintext
+// while at rest, reconstructing the error encrypter requires:
 //  1. Decode: to deserialize the ephemeral public key.
 //  2. Reextract: to "unlock" the actual error encrypter using an active
 //     OnionProcessor.
-func NewSphinxErrorEncrypter() *SphinxErrorEncrypter {
+func NewSphinxErrorEncrypterUninitialized() *SphinxErrorEncrypter {
 	return &SphinxErrorEncrypter{
 		OnionErrorEncrypter: nil,
 		EphemeralKey:        &btcec.PublicKey{},
 	}
 }
 
+// NewSphinxErrorEncrypter creates a new instance of a SphinxErrorEncrypter,
+// initialized with the provided shared secret. To deserialize an encoded
+// SphinxErrorEncrypter, use the NewSphinxErrorEncrypterUninitialized
+// constructor.
+func NewSphinxErrorEncrypter(ephemeralKey *btcec.PublicKey,
+	sharedSecret sphinx.Hash256) *SphinxErrorEncrypter {
+
+	encrypter := &SphinxErrorEncrypter{
+		EphemeralKey: ephemeralKey,
+	}
+
+	encrypter.initialize(sharedSecret)
+
+	return encrypter
+}
+
+func (s *SphinxErrorEncrypter) initialize(sharedSecret sphinx.Hash256) {
+	s.OnionErrorEncrypter = sphinx.NewOnionErrorEncrypter(
+		sharedSecret,
+	)
+}
+
 // EncryptFirstHop transforms a concrete failure message into an encrypted
 // opaque failure reason. This method will be used at the source that the error
 // occurs. It differs from BackwardObfuscate slightly, in that it computes a
@@ -177,9 +200,9 @@ func (s *SphinxErrorEncrypter) Decode(r io.Reader) error {
 // This intended to be used shortly after Decode, to fully initialize a
 // SphinxErrorEncrypter.
 func (s *SphinxErrorEncrypter) Reextract(
-	extract ErrorEncrypterExtracter) error {
+	extract SharedSecretGenerator) error {
 
-	obfuscator, failcode := extract(s.EphemeralKey)
+	sharedSecret, failcode := extract(s.EphemeralKey)
 	if failcode != lnwire.CodeNone {
 		// This should never happen, since we already validated that
 		// this obfuscator can be extracted when it was received in the
@@ -188,16 +211,9 @@ func (s *SphinxErrorEncrypter) Reextract(
 			"obfuscator, got failcode: %d", failcode)
 	}
 
-	sphinxEncrypter, ok := obfuscator.(*SphinxErrorEncrypter)
-	if !ok {
-		return fmt.Errorf("incorrect onion error extracter")
-	}
-
-	// Copy the freshly extracted encrypter.
-	s.OnionErrorEncrypter = sphinxEncrypter.OnionErrorEncrypter
+	s.initialize(sharedSecret)
 
 	return nil
-
 }
 
 // A compile time check to ensure SphinxErrorEncrypter implements the

htlcswitch/hop/iterator.go

@@ -29,10 +29,11 @@ type Iterator interface {
 	// into the passed io.Writer.
 	EncodeNextHop(w io.Writer) error
 
-	// ExtractErrorEncrypter returns the ErrorEncrypter needed for this hop,
-	// along with a failure code to signal if the decoding was successful.
-	ExtractErrorEncrypter(ErrorEncrypterExtracter) (ErrorEncrypter,
-		lnwire.FailCode)
+	// ExtractEncrypterParams extracts the ephemeral key and shared secret
+	// from the onion packet and returns them to the caller along with a
+	// failure code to signal if the decoding was successful.
+	ExtractEncrypterParams(SharedSecretGenerator) (
+		*btcec.PublicKey, sphinx.Hash256, lnwire.FailCode)
 }
 
 // sphinxHopIterator is the Sphinx implementation of hop iterator which uses
@@ -100,16 +101,21 @@ func (r *sphinxHopIterator) HopPayload() (*Payload, error) {
 	}
 }
 
-// ExtractErrorEncrypter decodes and returns the ErrorEncrypter for this hop,
-// along with a failure code to signal if the decoding was successful. The
-// ErrorEncrypter is used to encrypt errors back to the sender in the event that
-// a payment fails.
+// ExtractEncrypterParams extracts the ephemeral key and shared secret from the
+// onion packet and returns them to the caller along with a failure code to
+// signal if the decoding was successful.
 //
 // NOTE: Part of the HopIterator interface.
-func (r *sphinxHopIterator) ExtractErrorEncrypter(
-	extracter ErrorEncrypterExtracter) (ErrorEncrypter, lnwire.FailCode) {
+func (r *sphinxHopIterator) ExtractEncrypterParams(
+	extracter SharedSecretGenerator) (*btcec.PublicKey, sphinx.Hash256,
+	lnwire.FailCode) {
 
-	return extracter(r.ogPacket.EphemeralKey)
+	sharedSecret, failCode := extracter(r.ogPacket.EphemeralKey)
+	if failCode != lnwire.CodeNone {
+		return nil, sphinx.Hash256{}, failCode
+	}
+
+	return r.ogPacket.EphemeralKey, sharedSecret, lnwire.CodeNone
 }
 
 // OnionProcessor is responsible for keeping all sphinx dependent parts inside
@@ -375,33 +381,26 @@ func (p *OnionProcessor) DecodeHopIterators(id []byte,
 	return resps, nil
 }
 
-// ExtractErrorEncrypter takes an io.Reader which should contain the onion
-// packet as original received by a forwarding node and creates an
-// ErrorEncrypter instance using the derived shared secret. In the case that en
-// error occurs, a lnwire failure code detailing the parsing failure will be
-// returned.
-func (p *OnionProcessor) ExtractErrorEncrypter(ephemeralKey *btcec.PublicKey) (
-	ErrorEncrypter, lnwire.FailCode) {
+// ExtractSharedSecret takes an ephemeral session key as original received by a
+// forwarding node and generates the shared secret. In the case that en error
+// occurs, a lnwire failure code detailing the parsing failure will be returned.
+func (p *OnionProcessor) ExtractSharedSecret(ephemeralKey *btcec.PublicKey) (
+	sphinx.Hash256, lnwire.FailCode) {
 
 	sharedSecret, err := p.router.GenerateSharedSecret(ephemeralKey)
 	if err != nil {
 		switch err {
 		case sphinx.ErrInvalidOnionVersion:
-			return nil, lnwire.CodeInvalidOnionVersion
+			return sphinx.Hash256{}, lnwire.CodeInvalidOnionVersion
 		case sphinx.ErrInvalidOnionHMAC:
-			return nil, lnwire.CodeInvalidOnionHmac
+			return sphinx.Hash256{}, lnwire.CodeInvalidOnionHmac
 		case sphinx.ErrInvalidOnionKey:
-			return nil, lnwire.CodeInvalidOnionKey
+			return sphinx.Hash256{}, lnwire.CodeInvalidOnionKey
 		default:
 			log.Errorf("unable to process onion packet: %v", err)
-			return nil, lnwire.CodeInvalidOnionKey
+			return sphinx.Hash256{}, lnwire.CodeInvalidOnionKey
 		}
 	}
 
-	onionObfuscator := sphinx.NewOnionErrorEncrypter(sharedSecret)
-
-	return &SphinxErrorEncrypter{
-		OnionErrorEncrypter: onionObfuscator,
-		EphemeralKey:        ephemeralKey,
-	}, lnwire.CodeNone
+	return sharedSecret, lnwire.CodeNone
 }

htlcswitch/link.go

@@ -10,11 +10,13 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btclog"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/go-errors/errors"
+	sphinx "github.com/lightningnetwork/lightning-onion"
 	"github.com/lightningnetwork/lnd/build"
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/channeldb/models"
@@ -153,9 +155,14 @@ type ChannelLinkConfig struct {
 	DecodeHopIterators func([]byte, []hop.DecodeHopIteratorRequest) (
 		[]hop.DecodeHopIteratorResponse, error)
 
-	// ExtractErrorEncrypter function is responsible for decoding HTLC
-	// Sphinx onion blob, and creating onion failure obfuscator.
-	ExtractErrorEncrypter hop.ErrorEncrypterExtracter
+	// ExtractSharedSecret function is responsible for decoding HTLC
+	// Sphinx onion blob, and deriving the shared secret.
+	ExtractSharedSecret hop.SharedSecretGenerator
+
+	// CreateErrorEncrypter instantiates an error encrypter based on the
+	// provided encryption parameters.
+	CreateErrorEncrypter func(*btcec.PublicKey,
+		sphinx.Hash256) hop.ErrorEncrypter
 
 	// FetchLastChannelUpdate retrieves the latest routing policy for a
 	// target channel. This channel will typically be the outgoing channel
@@ -3006,9 +3013,8 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 
 		// Retrieve onion obfuscator from onion blob in order to
 		// produce initial obfuscation of the onion failureCode.
-		obfuscator, failureCode := chanIterator.ExtractErrorEncrypter(
-			l.cfg.ExtractErrorEncrypter,
-		)
+		ephemeralKey, sharedSecret, failureCode := chanIterator.
+			ExtractEncrypterParams(l.cfg.ExtractSharedSecret)
 		if failureCode != lnwire.CodeNone {
 			// If we're unable to process the onion blob than we
 			// should send the malformed htlc error to payment
@@ -3022,6 +3028,12 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 			continue
 		}
 
+		// Instantiate an error encrypter based on the extracted
+		// encryption parameters.
+		obfuscator := l.cfg.CreateErrorEncrypter(
+			ephemeralKey, sharedSecret,
+		)
+
 		heightNow := l.cfg.BestHeight()
 
 		pld, err := chanIterator.HopPayload()

htlcswitch/link_test.go

@@ -1576,9 +1576,10 @@ func TestChannelLinkMultiHopDecodeError(t *testing.T) {
 	t.Cleanup(n.stop)
 
 	// Replace decode function with another which throws an error.
-	n.carolChannelLink.cfg.ExtractErrorEncrypter = func(
-		*btcec.PublicKey) (hop.ErrorEncrypter, lnwire.FailCode) {
-		return nil, lnwire.CodeInvalidOnionVersion
+	n.carolChannelLink.cfg.ExtractSharedSecret = func(
+		*btcec.PublicKey) (sphinx.Hash256, lnwire.FailCode) {
+
+		return sphinx.Hash256{}, lnwire.CodeInvalidOnionVersion
 	}
 
 	carolBandwidthBefore := n.carolChannelLink.Bandwidth()
@@ -1962,9 +1963,15 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount)
 			return aliceSwitch.ForwardPackets(linkQuit, packets...)
 		},
 		DecodeHopIterators: decoder.DecodeHopIterators,
-		ExtractErrorEncrypter: func(*btcec.PublicKey) (
-			hop.ErrorEncrypter, lnwire.FailCode) {
-			return obfuscator, lnwire.CodeNone
+		ExtractSharedSecret: func(*btcec.PublicKey) (
+			sphinx.Hash256, lnwire.FailCode) {
+
+			return sphinx.Hash256{}, lnwire.CodeNone
+		},
+		CreateErrorEncrypter: func(*btcec.PublicKey,
+			sphinx.Hash256) hop.ErrorEncrypter {
+
+			return obfuscator
 		},
 		FetchLastChannelUpdate: mockGetChanUpdateMessage,
 		PreimageCache:          pCache,
@@ -4413,10 +4420,15 @@ func (h *persistentLinkHarness) restartLink(
 			return aliceSwitch.ForwardPackets(linkQuit, packets...)
 		},
 		DecodeHopIterators: decoder.DecodeHopIterators,
-		ExtractErrorEncrypter: func(*btcec.PublicKey) (
-			hop.ErrorEncrypter, lnwire.FailCode) {
+		ExtractSharedSecret: func(*btcec.PublicKey) (
+			sphinx.Hash256, lnwire.FailCode) {
+
+			return sphinx.Hash256{}, lnwire.CodeNone
+		},
+		CreateErrorEncrypter: func(*btcec.PublicKey,
+			sphinx.Hash256) hop.ErrorEncrypter {
 
-			return obfuscator, lnwire.CodeNone
+			return obfuscator
 		},
 		FetchLastChannelUpdate: mockGetChanUpdateMessage,
 		PreimageCache:          pCache,