Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

router+build: update to the latest version of lightning-onion #3027

Merged
merged 9 commits into from May 3, 2019
3 changes: 1 addition & 2 deletions go.mod
Expand Up @@ -5,7 +5,6 @@ require (
github.com/NebulousLabs/fastrand v0.0.0-20180208210444-3cf7173006a0 // indirect
github.com/NebulousLabs/go-upnp v0.0.0-20180202185039-29b680b06c82
github.com/Yawning/aez v0.0.0-20180114000226-4dad034d9db2
github.com/aead/chacha20 v0.0.0-20180709150244-8b13a72661da // indirect
github.com/btcsuite/btcd v0.0.0-20190426011420-63f50db2f70a
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d
Expand All @@ -29,7 +28,7 @@ require (
github.com/juju/version v0.0.0-20180108022336-b64dbd566305 // indirect
github.com/kkdai/bstream v0.0.0-20181106074824-b3251f7901ec
github.com/lightninglabs/neutrino v0.0.0-20190426010803-a655679fe131
github.com/lightningnetwork/lightning-onion v0.0.0-20180605012408-ac4d9da8f1d6
github.com/lightningnetwork/lightning-onion v0.0.0-20190430041606-751fb4dd8b72
github.com/lightningnetwork/lnd/queue v1.0.1
github.com/lightningnetwork/lnd/ticker v1.0.0
github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796
Expand Down
7 changes: 5 additions & 2 deletions go.sum
Expand Up @@ -14,6 +14,7 @@ github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBA
github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4=
github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
github.com/btcsuite/btcd v0.0.0-20180823030728-d81d8877b8f3/go.mod h1:Dmm/EzmjnCiweXmzRIAiUWCInVmPgjkzgv5k4tVyXiQ=
github.com/btcsuite/btcd v0.0.0-20181130015935-7d2daa5bfef2/go.mod h1:Jr9bmNVGZ7TH2Ux1QuP0ec+yGgh0gE9FIlkzQiI5bR0=
github.com/btcsuite/btcd v0.0.0-20190213025234-306aecffea32 h1:qkOC5Gd33k54tobS36cXdAzJbeHaduLtnLQQwNoIi78=
github.com/btcsuite/btcd v0.0.0-20190213025234-306aecffea32/go.mod h1:DrZx5ec/dmnfpw9KyYoQyYo7d0KEvTkk/5M/vbZjAr8=
github.com/btcsuite/btcd v0.0.0-20190426011420-63f50db2f70a h1:wH5Nq2kt+BdVJhVyYZD54lHQpz95mog0Z7r1h638TY4=
Expand Down Expand Up @@ -110,8 +111,8 @@ github.com/lightninglabs/neutrino v0.0.0-20190313035638-e1ad4c33fb18 h1:lxD7RgKY
github.com/lightninglabs/neutrino v0.0.0-20190313035638-e1ad4c33fb18/go.mod h1:v6tz6jbuAubTrRpX8ke2KH9sJxml8KlPQTKgo9mAp1Q=
github.com/lightninglabs/neutrino v0.0.0-20190426010803-a655679fe131 h1:1qKraSAbJFxd2BUHrxFEswNRav749pt4P37Ez8avbAA=
github.com/lightninglabs/neutrino v0.0.0-20190426010803-a655679fe131/go.mod h1:/XWY/6/btfsknUpLPV8vvIZyhod61zYaUJiE8HxsFUs=
github.com/lightningnetwork/lightning-onion v0.0.0-20180605012408-ac4d9da8f1d6 h1:ONLGrYJVQdbtP6CE/ff1KNWZtygRGEh12RzonTiCzPs=
github.com/lightningnetwork/lightning-onion v0.0.0-20180605012408-ac4d9da8f1d6/go.mod h1:8EgEt4a/NUOVQd+3kk6n9aZCJ1Ssj96Pb6lCrci+6oc=
github.com/lightningnetwork/lightning-onion v0.0.0-20190430041606-751fb4dd8b72 h1:KgmypyQfJnEf2vhwboKCtTp4mHxIcLeXPBPWDbPuzFQ=
github.com/lightningnetwork/lightning-onion v0.0.0-20190430041606-751fb4dd8b72/go.mod h1:Sooe/CoCqa85JxqHV+IBR2HW+6t2Cv+36awSmoccswM=
github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796 h1:sjOGyegMIhvgfq5oaue6Td+hxZuf3tDC8lAPrFldqFw=
github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796/go.mod h1:3p7ZTf9V1sNPI5H8P3NkTFF4LuwMdPl2DodF60qAKqY=
github.com/ltcsuite/ltcutil v0.0.0-20181217130922-17f3b04680b6/go.mod h1:8Vg/LTOO0KYa/vlHWJ6XZAevPQThGH5sufO0Hrou/lA=
Expand All @@ -134,6 +135,7 @@ go.etcd.io/bbolt v1.3.0/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.etcd.io/bbolt v1.3.2 h1:Z/90sZLPOeCy2PwprqkFa25PdkusRzaj9P8zm/KNyvk=
go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190103213133-ff983b9c42bc/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67 h1:ng3VDlRp5/DHpSWl02R4rM9I+8M2rhmsuLwAMmkLQWE=
golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand All @@ -156,6 +158,7 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sys v0.0.0-20180821140842-3b58ed4ad339/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190102155601-82a175fd1598/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190209173611-3b5209105503 h1:5SvYFrOM3W8Mexn9/oA44Ji7vhXAZQ9hiP+1Q/DMrWg=
golang.org/x/sys v0.0.0-20190209173611-3b5209105503/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
Expand Down
18 changes: 18 additions & 0 deletions htlcswitch/failure.go
Expand Up @@ -87,6 +87,13 @@ type ErrorEncrypter interface {
// slightly, in that it computes a proper MAC over the error.
EncryptFirstHop(lnwire.FailureMessage) (lnwire.OpaqueReason, error)

// EncryptMalformedError is similar to EncryptFirstHop (it adds the
// MAC), but it accepts an opaque failure reason rather than a failure
// message. This method is used when we receive an
// UpdateFailMalformedHTLC from the remote peer and then need to
// convert that into a proper error from only the raw bytes.
EncryptMalformedError(lnwire.OpaqueReason) lnwire.OpaqueReason

// IntermediateEncrypt wraps an already encrypted opaque reason error
// in an additional layer of onion encryption. This process repeats
// until the error arrives at the source of the payment.
Expand Down Expand Up @@ -153,6 +160,17 @@ func (s *SphinxErrorEncrypter) EncryptFirstHop(failure lnwire.FailureMessage) (l
return s.EncryptError(true, b.Bytes()), nil
}

// EncryptMalformedError is similar to EncryptFirstHop (it adds the MAC), but
// it accepts an opaque failure reason rather than a failure message. This
// method is used when we receive an UpdateFailMalformedHTLC from the remote
// peer and then need to convert that into an proper error from only the raw
// bytes.
//
// NOTE: Part of the ErrorEncrypter interface.
func (s *SphinxErrorEncrypter) EncryptMalformedError(reason lnwire.OpaqueReason) lnwire.OpaqueReason {
return s.EncryptError(true, reason)
}

// IntermediateEncrypt wraps an already encrypted opaque reason error in an
// additional layer of onion encryption. This process repeats until the error
// arrives at the source of the payment. We re-encrypt the message on the
Expand Down
16 changes: 15 additions & 1 deletion htlcswitch/link.go
Expand Up @@ -2369,10 +2369,24 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg,
outgoingHTLCID: pd.ParentIndex,
destRef: pd.DestRef,
htlc: &lnwire.UpdateFailHTLC{
Reason: lnwire.OpaqueReason(pd.FailReason),
Reason: lnwire.OpaqueReason(
pd.FailReason,
),
},
}

// If the failure message lacks an HMAC (but includes
// the 4 bytes for encoding the message and padding
// lengths, then this means that we received it as an
// UpdateFailMalformedHTLC. As a result, we'll signal
// that we need to convert this error within the switch
// to an actual error, by encrypting it as if we were
// the originating hop.
convertedErrorSize := lnwire.FailureMessageLength + 4
if len(pd.FailReason) == convertedErrorSize {
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved
failPacket.convertedError = true
}

// Add the packet to the batch to be forwarded, and
// notify the overflow queue that a spare spot has been
// freed up within the commitment state.
Expand Down
9 changes: 9 additions & 0 deletions htlcswitch/mock.go
Expand Up @@ -368,7 +368,10 @@ func (o *mockObfuscator) EncryptFirstHop(failure lnwire.FailureMessage) (

func (o *mockObfuscator) IntermediateEncrypt(reason lnwire.OpaqueReason) lnwire.OpaqueReason {
return reason
}

func (o *mockObfuscator) EncryptMalformedError(reason lnwire.OpaqueReason) lnwire.OpaqueReason {
return reason
}

// mockDeobfuscator mock implementation of the failure deobfuscator which
Expand Down Expand Up @@ -400,6 +403,8 @@ type mockIteratorDecoder struct {
mu sync.RWMutex

responses map[[32]byte][]DecodeHopIteratorResponse

decodeFail bool
}

func newMockIteratorDecoder() *mockIteratorDecoder {
Expand Down Expand Up @@ -451,6 +456,10 @@ func (p *mockIteratorDecoder) DecodeHopIterators(id []byte,
req.OnionReader, req.RHash, req.IncomingCltv,
)

if p.decodeFail {
failcode = lnwire.CodeTemporaryChannelFailure
}

resp := DecodeHopIteratorResponse{
HopIterator: iterator,
FailCode: failcode,
Expand Down
8 changes: 8 additions & 0 deletions htlcswitch/packet.go
Expand Up @@ -61,6 +61,14 @@ type htlcPacket struct {
// encrypted with any shared secret.
localFailure bool

// convertedError is set to true if this is an HTLC fail that was
// created using an UpdateFailMalformedHTLC from the remote party. If
// this is true, then when forwarding this failure packet, we'll need
// to wrap it as if we were the first hop if it's a multi-hop HTLC. If
// it's a direct HTLC, then we'll decode the error as no encryption has
// taken place.
convertedError bool

// hasSource is set to true if the incomingChanID and incomingHTLCID
// fields of a forwarded fail packet are already set and do not need to
// be looked up in the circuit map.
Expand Down
28 changes: 23 additions & 5 deletions htlcswitch/switch.go
Expand Up @@ -939,7 +939,7 @@ func (s *Switch) parseFailedPayment(payment *pendingPayment, pkt *htlcPacket,
// The payment never cleared the link, so we don't need to
// decrypt the error, simply decode it them report back to the
// user.
case pkt.localFailure:
case pkt.localFailure || pkt.convertedError:
var userErr string
r := bytes.NewReader(htlc.Reason)
failureMsg, err := lnwire.DecodeFailure(r, 0)
Expand Down Expand Up @@ -1155,13 +1155,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
fail, isFail := htlc.(*lnwire.UpdateFailHTLC)
if isFail && !packet.hasSource {
switch {
// No message to encrypt, locally sourced payment.
case circuit.ErrorEncrypter == nil:
// No message to encrypt, locally sourced
// payment.

// If this is a resolution message, then we'll need to
// encrypt it as it's actually internally sourced.
case packet.isResolution:
// If this is a resolution message, then we'll need to encrypt
// it as it's actually internally sourced.
var err error
// TODO(roasbeef): don't need to pass actually?
failure := &lnwire.FailPermanentChannelFailure{}
Expand All @@ -1174,6 +1173,25 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
log.Error(err)
}

// Alternatively, if the remote party send us an
// UpdateFailMalformedHTLC, then we'll need to convert
// this into a proper well formatted onion error as
// there's no HMAC currently.
case packet.convertedError:
log.Infof("Converting malformed HTLC error "+
"for circuit for Circuit(%x: "+
"(%s, %d) <-> (%s, %d))", packet.circuit.PaymentHash,
packet.incomingChanID, packet.incomingHTLCID,
packet.outgoingChanID, packet.outgoingHTLCID)

fail.Reason = circuit.ErrorEncrypter.EncryptMalformedError(
fail.Reason,
)
if err != nil {
err = fmt.Errorf("unable to obfuscate "+
"error: %v", err)
log.Error(err)
}
default:
// Otherwise, it's a forwarded error, so we'll perform a
// wrapper encryption as normal.
Expand Down
73 changes: 73 additions & 0 deletions htlcswitch/switch_test.go
Expand Up @@ -2033,3 +2033,76 @@ func TestMultiHopPaymentForwardingEvents(t *testing.T) {
}
}
}

// TestUpdateFailMalformedHTLCErrorConversion tests that we're able to properly
// convert malformed HTLC errors that originate at the direct link, as well as
// during multi-hop HTLC forwarding.
func TestUpdateFailMalformedHTLCErrorConversion(t *testing.T) {
t.Parallel()

// First, we'll create our traditional three hop network.
channels, cleanUp, _, err := createClusterChannels(
btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5,
)
if err != nil {
t.Fatalf("unable to create channel: %v", err)
}
defer cleanUp()

n := newThreeHopNetwork(
t, channels.aliceToBob, channels.bobToAlice,
channels.bobToCarol, channels.carolToBob, testStartingHeight,
)
if err := n.start(); err != nil {
t.Fatalf("unable to start three hop network: %v", err)
}

assertPaymentFailure := func(t *testing.T) {
// With the decoder modified, we'll now attempt to send a
// payment from Alice to carol.
finalAmt := lnwire.NewMSatFromSatoshis(100000)
htlcAmt, totalTimelock, hops := generateHops(
finalAmt, testStartingHeight, n.firstBobChannelLink,
n.carolChannelLink,
)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = makePayment(
n.aliceServer, n.carolServer, firstHop, hops, finalAmt,
htlcAmt, totalTimelock,
).Wait(30 * time.Second)

// The payment should fail as Carol is unable to decode the
// onion blob sent to her.
if err == nil {
t.Fatalf("unable to send payment: %v", err)
}

fwdingErr := err.(*ForwardingError)
failureMsg := fwdingErr.FailureMessage
if _, ok := failureMsg.(*lnwire.FailTemporaryChannelFailure); !ok {
t.Fatalf("expected temp chan failure instead got: %v",
fwdingErr.FailureMessage)
}
}

t.Run("multi-hop error conversion", func(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer test cases to not share any state, keep them fully independent to prevent one test from forcing the second one to fail and making it hard to find out what is happening.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since they use Fatalf and don't run concurrently, if one of them fails the other one doesn't execute.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean if the first one succeeds, but modifies state in an unexpected way and makes the second one fail.

// Now that we have our network up, we'll modify the hop
// iterator for the Bob <-> Carol channel to fail to decode in
// order to simulate either a replay attack or an issue
// decoding the onion.
n.carolOnionDecoder.decodeFail = true

assertPaymentFailure(t)
})

t.Run("direct channel error conversion", func(t *testing.T) {
// Similar to the above test case, we'll now make the Alice <->
// Bob link always fail to decode an onion. This differs from
// the above test case in that there's no encryption on the
// error at all since Alice will directly receive a
// UpdateFailMalformedHTLC message.
n.bobOnionDecoder.decodeFail = true

assertPaymentFailure(t)
})
}
22 changes: 14 additions & 8 deletions htlcswitch/test_utils.go
Expand Up @@ -589,15 +589,18 @@ func generateRoute(hops ...ForwardingInfo) ([lnwire.OnionPacketSize]byte, error)

// threeHopNetwork is used for managing the created cluster of 3 hops.
type threeHopNetwork struct {
aliceServer *mockServer
aliceChannelLink *channelLink
aliceServer *mockServer
aliceChannelLink *channelLink
aliceOnionDecoder *mockIteratorDecoder

bobServer *mockServer
firstBobChannelLink *channelLink
secondBobChannelLink *channelLink
bobOnionDecoder *mockIteratorDecoder

carolServer *mockServer
carolChannelLink *channelLink
carolServer *mockServer
carolChannelLink *channelLink
carolOnionDecoder *mockIteratorDecoder

hopNetwork
}
Expand Down Expand Up @@ -948,15 +951,18 @@ func newThreeHopNetwork(t testing.TB, aliceChannel, firstBobChannel,
}

return &threeHopNetwork{
aliceServer: aliceServer,
aliceChannelLink: aliceChannelLink.(*channelLink),
aliceServer: aliceServer,
aliceChannelLink: aliceChannelLink.(*channelLink),
aliceOnionDecoder: aliceDecoder,

bobServer: bobServer,
firstBobChannelLink: firstBobChannelLink.(*channelLink),
secondBobChannelLink: secondBobChannelLink.(*channelLink),
bobOnionDecoder: bobDecoder,

carolServer: carolServer,
carolChannelLink: carolChannelLink.(*channelLink),
carolServer: carolServer,
carolChannelLink: carolChannelLink.(*channelLink),
carolOnionDecoder: carolDecoder,

hopNetwork: *hopNetwork,
}
Expand Down
10 changes: 5 additions & 5 deletions lnwire/onion_error.go
Expand Up @@ -26,9 +26,9 @@ type FailureMessage interface {
Error() string
}

// failureMessageLength is the size of the failure message plus the size of
// FailureMessageLength is the size of the failure message plus the size of
// padding. The FailureMessage message should always be EXACTLY this size.
const failureMessageLength = 256
const FailureMessageLength = 256

const (
// FlagBadOnion error flag describes an unparsable, encrypted by
Expand Down Expand Up @@ -1101,7 +1101,7 @@ func DecodeFailure(r io.Reader, pver uint32) (FailureMessage, error) {
if err := ReadElement(r, &failureLength); err != nil {
return nil, fmt.Errorf("unable to read error len: %v", err)
}
if failureLength > failureMessageLength {
if failureLength > FailureMessageLength {
return nil, fmt.Errorf("failure message is too "+
"long: %v", failureLength)
}
Expand Down Expand Up @@ -1170,14 +1170,14 @@ func EncodeFailure(w io.Writer, failure FailureMessage, pver uint32) error {
// The combined size of this message must be below the max allowed
// failure message length.
failureMessage := failureMessageBuffer.Bytes()
if len(failureMessage) > failureMessageLength {
if len(failureMessage) > FailureMessageLength {
return fmt.Errorf("failure message exceed max "+
"available size: %v", len(failureMessage))
}

// Finally, we'll add some padding in order to ensure that all failure
// messages are fixed size.
pad := make([]byte, failureMessageLength-len(failureMessage))
pad := make([]byte, FailureMessageLength-len(failureMessage))

return WriteElements(w,
uint16(len(failureMessage)),
Expand Down