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

htlcswitch+lnwallet: ReceiveMalformedFailHTLC #7030

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions htlcswitch/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -1842,7 +1842,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// If remote side have been unable to parse the onion blob we
// have sent to it, than we should transform the malformed HTLC
// message to the usual HTLC fail message.
err := l.channel.ReceiveFailHTLC(msg.ID, b.Bytes())
err := l.channel.ReceiveMalformedFailHTLC(msg.ID, b.Bytes())
if err != nil {
l.fail(LinkFailureError{code: ErrInvalidUpdate},
"unable to handle upstream fail HTLC: %v", err)
Expand Down Expand Up @@ -2829,18 +2829,42 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg,

l.log.Debugf("Failed to send %s", pd.Amount)

// 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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to think about this deletion in the context of upgrade scenarios with lingering 'old style' malformed fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Migrating Fail to MalformedFail for the pending updates in the channel db doesn't look very attractive.

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.
switchPackets = append(switchPackets, failPacket)

// A malformed failure message for a previously forwarded HTLC
// has been received. As a result a new slot will be freed up in
// our commitment state, so we'll forward this to the switch so
// the backwards undo can continue.
case lnwallet.MalformedFail:
// If hodl.SettleIncoming is requested, we will not
// forward the FAIL to the switch and will not signal a
// free slot on the commitment transaction.
if l.cfg.HodlMask.Active(hodl.FailIncoming) {
l.log.Warnf(hodl.FailIncoming.Warning())
continue
}

// Fetch the reason the HTLC was canceled so we can
// continue to propagate it. This failure originated
// from another node, so the linkFailure field is not
// set on the packet.
failPacket := &htlcPacket{
outgoingChanID: l.ShortChanID(),
outgoingHTLCID: pd.ParentIndex,
destRef: pd.DestRef,
htlc: &lnwire.UpdateFailHTLC{
Reason: lnwire.OpaqueReason(
pd.FailReason,
),
},
convertedError: true,
}

l.log.Debugf("Failed to send %s", pd.Amount)

// 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
17 changes: 16 additions & 1 deletion htlcswitch/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,16 @@ func (o *mockObfuscator) Reextract(
return nil
}

var fakeHmac = []byte("hmachmachmachmachmachmachmachmac")

func (o *mockObfuscator) EncryptFirstHop(failure lnwire.FailureMessage) (
lnwire.OpaqueReason, error) {

o.failure = failure

var b bytes.Buffer
b.Write(fakeHmac)

if err := lnwire.EncodeFailure(&b, failure, 0); err != nil {
return nil, err
}
Expand All @@ -434,6 +438,11 @@ func (o *mockObfuscator) IntermediateEncrypt(reason lnwire.OpaqueReason) lnwire.
}

func (o *mockObfuscator) EncryptMalformedError(reason lnwire.OpaqueReason) lnwire.OpaqueReason {
var b bytes.Buffer
b.Write(fakeHmac)

b.Write(reason)

return reason
}

Expand All @@ -445,7 +454,13 @@ func newMockDeobfuscator() ErrorDecrypter {
return &mockDeobfuscator{}
}

func (o *mockDeobfuscator) DecryptError(reason lnwire.OpaqueReason) (*ForwardingError, error) {
func (o *mockDeobfuscator) DecryptError(reason lnwire.OpaqueReason) (
*ForwardingError, error) {

if !bytes.Equal(reason[:32], fakeHmac) {
return nil, errors.New("fake decryption error")
}
reason = reason[32:]

r := bytes.NewReader(reason)
failure, err := lnwire.DecodeFailure(r, 0)
Expand Down
27 changes: 27 additions & 0 deletions htlcswitch/switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,33 @@ func (s *Switch) reforwardSettleFails(fwdPkgs []*channeldb.FwdPkg) {
},
}

// 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.
switchPackets = append(switchPackets, failPacket)

// A malformed failure message for a previously forwarded HTLC
// has been received. As a result a new slot will be freed up in
// our commitment state, so we'll forward this to the switch so
// the backwards undo can continue.
case lnwallet.MalformedFail:
// Fetch the reason the HTLC was canceled so
// we can continue to propagate it. This
// failure originated from another node, so
// the linkFailure field is not set on this
// packet.
failPacket := &htlcPacket{
outgoingChanID: fwdPkg.Source,
outgoingHTLCID: pd.ParentIndex,
destRef: pd.DestRef,
htlc: &lnwire.UpdateFailHTLC{
Reason: lnwire.OpaqueReason(
pd.FailReason,
),
},
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
36 changes: 36 additions & 0 deletions lnwallet/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -5635,6 +5635,42 @@ func (lc *LightningChannel) ReceiveFailHTLC(htlcIndex uint64, reason []byte,
return nil
}

func (lc *LightningChannel) ReceiveMalformedFailHTLC(htlcIndex uint64, reason []byte,
) error {

lc.Lock()
defer lc.Unlock()

htlc := lc.localUpdateLog.lookupHtlc(htlcIndex)
if htlc == nil {
return ErrUnknownHtlcIndex{lc.ShortChanID(), htlcIndex}
}

// Now that we know the HTLC exists, we'll ensure that they haven't
// already attempted to fail the HTLC.
if lc.localUpdateLog.htlcHasModification(htlcIndex) {
return ErrHtlcIndexAlreadyFailed(htlcIndex)
}

pd := &PaymentDescriptor{
Amount: htlc.Amount,
RHash: htlc.RHash,
ParentIndex: htlc.HtlcIndex,
LogIndex: lc.remoteUpdateLog.logIndex,
EntryType: MalformedFail,
FailReason: reason,
}

lc.remoteUpdateLog.appendUpdate(pd)

// With the fail added to the remote log, we'll now mark the HTLC as
// modified to prevent ourselves from accidentally attempting a
// duplicate fail.
lc.localUpdateLog.markHtlcModified(htlcIndex)

return nil
}

// ChannelPoint returns the outpoint of the original funding transaction which
// created this active channel. This outpoint is used throughout various
// subsystems to uniquely identify an open channel.
Expand Down