htlcswitch: use evaluateDustThreshold in SendHTLC, handlePacketForward

This commit makes SendHTLC (we are the source) evaluate the dust
threshold of the outgoing channel against the default threshold of
500K satoshis. If the threshold is exceeded by adding this HTLC, we
fail backwards. It also makes handlePacketForward (we are forwarding)
evaluate the dust threshold of the incoming channel and the outgoing
channel and fails backwards if either channel's dust sum exceeds
the default threshold. This helps mitigate the dust griefing attack.

Author: Crypt-iQ

htlcswitch/switch.go

@@ -71,6 +71,15 @@ var (
 	// ErrLocalAddFailed signals that the ADD htlc for a local payment
 	// failed to be processed.
 	ErrLocalAddFailed = errors.New("local add HTLC failed")
+
+	// errDustThresholdExceeded is only surfaced to callers of SendHTLC and
+	// signals that sending the HTLC would exceed the outgoing link's dust
+	// threshold.
+	errDustThresholdExceeded = errors.New("dust threshold exceeded")
+
+	// defaultDustThreshold is the default threshold after which we'll fail
+	// payments if they are dust. This is currently set to 500k sats.
+	defaultDustThreshold = lnwire.MilliSatoshi(500_000_000)
 )
 
 // plexPacket encapsulates switch packet and adds error channel to receive
@@ -469,6 +478,51 @@ func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, attemptID uint64,
 		htlc:           htlc,
 	}
 
+	// Attempt to fetch the target link before creating a circuit so that
+	// we don't leave dangling circuits. The getLocalLink method does not
+	// require the circuit variable to be set on the *htlcPacket.
+	link, linkErr := s.getLocalLink(packet, htlc)
+	if linkErr != nil {
+		// Notify the htlc notifier of a link failure on our outgoing
+		// link. Incoming timelock/amount values are not set because
+		// they are not present for local sends.
+		s.cfg.HtlcNotifier.NotifyLinkFailEvent(
+			newHtlcKey(packet),
+			HtlcInfo{
+				OutgoingTimeLock: htlc.Expiry,
+				OutgoingAmt:      htlc.Amount,
+			},
+			HtlcEventTypeSend,
+			linkErr,
+			false,
+		)
+
+		return linkErr
+	}
+
+	// Evaluate whether this HTLC would increase our exposure to dust. If
+	// it does, don't send it out and instead return an error.
+	if s.evaluateDustThreshold(link, htlc.Amount, false) {
+		// Notify the htlc notifier of a link failure on our outgoing
+		// link. We use the FailTemporaryChannelFailure in place of a
+		// more descriptive error message.
+		linkErr := NewLinkError(
+			&lnwire.FailTemporaryChannelFailure{},
+		)
+		s.cfg.HtlcNotifier.NotifyLinkFailEvent(
+			newHtlcKey(packet),
+			HtlcInfo{
+				OutgoingTimeLock: htlc.Expiry,
+				OutgoingAmt:      htlc.Amount,
+			},
+			HtlcEventTypeSend,
+			linkErr,
+			false,
+		)
+
+		return errDustThresholdExceeded
+	}
+
 	circuit := newPaymentCircuit(&htlc.PaymentHash, packet)
 	actions, err := s.circuits.CommitCircuits(circuit)
 	if err != nil {
@@ -488,27 +542,6 @@ func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, attemptID uint64,
 	// Send packet to link.
 	packet.circuit = circuit
 
-	// User has created the htlc update therefore we should find the
-	// appropriate channel link and send the payment over this link.
-	link, linkErr := s.getLocalLink(packet, htlc)
-	if linkErr != nil {
-		// Notify the htlc notifier of a link failure on our
-		// outgoing link. Incoming timelock/amount values are
-		// not set because they are not present for local sends.
-		s.cfg.HtlcNotifier.NotifyLinkFailEvent(
-			newHtlcKey(packet),
-			HtlcInfo{
-				OutgoingTimeLock: htlc.Expiry,
-				OutgoingAmt:      htlc.Amount,
-			},
-			HtlcEventTypeSend,
-			linkErr,
-			false,
-		)
-
-		return linkErr
-	}
-
 	return link.handleLocalAddPacket(packet)
 }
 
@@ -1098,6 +1131,52 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 		// what the best channel is.
 		destination := destinations[rand.Intn(len(destinations))]
 
+		// Retrieve the incoming link by its ShortChannelID. Note that
+		// the incomingChanID is never set to hop.Source here.
+		s.indexMtx.RLock()
+		incomingLink, err := s.getLinkByShortID(packet.incomingChanID)
+		if err != nil {
+			s.indexMtx.RUnlock()
+
+			// If we couldn't find the incoming link, we can't
+			// evaluate the incoming's exposure to dust, so we just
+			// fail the HTLC back.
+			linkErr := NewLinkError(
+				&lnwire.FailTemporaryChannelFailure{},
+			)
+
+			return s.failAddPacket(packet, linkErr)
+		}
+		s.indexMtx.RUnlock()
+
+		// Evaluate whether this HTLC would increase our exposure to
+		// dust on the incoming link. If it does, fail it backwards.
+		if s.evaluateDustThreshold(
+			incomingLink, packet.incomingAmount, true,
+		) {
+			// The incoming dust exceeds the threshold, so we fail
+			// the add back.
+			linkErr := NewLinkError(
+				&lnwire.FailTemporaryChannelFailure{},
+			)
+
+			return s.failAddPacket(packet, linkErr)
+		}
+
+		// Also evaluate whether this HTLC would increase our exposure
+		// to dust on the destination link. If it does, fail it back.
+		if s.evaluateDustThreshold(
+			destination, packet.amount, false,
+		) {
+			// The outgoing dust exceeds the threshold, so we fail
+			// the add back.
+			linkErr := NewLinkError(
+				&lnwire.FailTemporaryChannelFailure{},
+			)
+
+			return s.failAddPacket(packet, linkErr)
+		}
+
 		// Send the packet to the destination channel link which
 		// manages the channel.
 		packet.outgoingChanID = destination.ShortChanID()
@@ -2279,3 +2358,69 @@ func (s *Switch) FlushForwardingEvents() error {
 func (s *Switch) BestHeight() uint32 {
 	return atomic.LoadUint32(&s.bestHeight)
 }
+
+// evaluateDustThreshold takes in a ChannelLink, HTLC amount, and a boolean to
+// determine whether the default dust threshold has been exceeded. This
+// heuristic is only used for HTLC's that are below either side's dust limits.
+// The sum of the commitment's dust with the mailbox's dust with the amount is
+// checked against the default threshold. If incoming is true, then the amount
+// is not included in the sum as it was already included in the commitment's
+// dust. A boolean is returned telling the caller whether the HTLC should be
+// failed back.
+func (s *Switch) evaluateDustThreshold(link ChannelLink,
+	amount lnwire.MilliSatoshi, incoming bool) bool {
+
+	// Retrieve the link's local and remote dust limits.
+	localLimit, remoteLimit := link.getDustLimits()
+
+	// Now check whether this HTLC amount is dust on either side's
+	// commitment transaction.
+	isLocalDust := amount < localLimit
+	isRemoteDust := amount < remoteLimit
+	if isLocalDust || isRemoteDust {
+		// Fetch the dust sums currently in the mailbox for this link.
+		cid := link.ChanID()
+		sid := link.ShortChanID()
+		mailbox := s.mailOrchestrator.GetOrCreateMailBox(cid, sid)
+		localMailDust, remoteMailDust := mailbox.DustPackets()
+
+		// If the htlc is dust on the local commitment, we'll obtain
+		// the dust sum for it.
+		if isLocalDust {
+			localSum := link.getDustSum(false)
+			localSum += localMailDust
+
+			// Optionally include the HTLC amount only for outgoing
+			// channels.
+			if !incoming {
+				localSum += amount
+			}
+
+			// Finally check against the defined dust threshold.
+			if localSum > defaultDustThreshold {
+				return true
+			}
+		}
+
+		// Also check if the htlc is dust on the remote commitment, if
+		// we've reached this point.
+		if isRemoteDust {
+			remoteSum := link.getDustSum(true)
+			remoteSum += remoteMailDust
+
+			// Optionally include the HTLC amount only for outgoing
+			// channels.
+			if !incoming {
+				remoteSum += amount
+			}
+
+			// Finally check against the defined dust threshold.
+			if remoteSum > defaultDustThreshold {
+				return true
+			}
+		}
+	}
+
+	// If we reached this point, this HTLC is fine to forward.
+	return false
+}