From 649e080da6e433801eb2a8c04cc20c3c5a523dad Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 30 Apr 2019 18:20:23 -0700 Subject: [PATCH] htlcswitch: for UpdateFailMalformedHTLC packets mark fail as needing conversion In this commit, we start the first phase of fixing an existing bug within the switch. As is, we don't properly convert `UpdateFailMalformedHTLC` to regular `UpdateFailHTLC` messages that are fully encrypted. When we receive a `UpdateFailMalformedHTLC` today, we'll convert it into a regular fail message by simply encoding the failure message raw. This failure message doesn't have a MAC yet, so if we sent it backwards, then the destination wouldn't be able to decrypt it. We can recognize this type of failure as it'll be the same size as the raw failure message max size, but it has 4 extra bytes for the encoding. When we come across this message, we'll mark is as needing conversion so the switch can take care of it. --- htlcswitch/link.go | 16 +++++++++++++++- htlcswitch/packet.go | 8 ++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 79a37f00..f84c4b63 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -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 { + 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. diff --git a/htlcswitch/packet.go b/htlcswitch/packet.go index 41e364a9..363a71dd 100644 --- a/htlcswitch/packet.go +++ b/htlcswitch/packet.go @@ -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.