Skip to content

Commit 2460c46

Browse files
Invoice server: treat forwarded invreqs as OM forwards
Previously, when a static invoice server forwarded an invoice request to an often-offline recipient, they would treat the outbound message like any other outbound onion message initiated by their own node. That means they would buffer the onion message internally in the onion messenger and generate a ConnectionNeeded event if the next-hop node was offline. Buffering the onion message in this case poses a DoS risk for the invoice server node, since they do not control the quantity of invoice requests they receive on behalf of often-offline recipients. Instead, we should treat these forwarded invoice requests like any other onion message that needs to be forwarded -- if the next-hop node is offline, either drop the message or generate an OnionMessageIntercepted event for it (pushing the DoS management onto the handler of the interception event).
1 parent 332b3e5 commit 2460c46

File tree

5 files changed

+76
-7
lines changed

5 files changed

+76
-7
lines changed

lightning/src/events/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,6 +1617,9 @@ pub enum Event {
16171617
/// `OnionMessenger` was initialized with
16181618
/// [`OnionMessenger::new_with_offline_peer_interception`], see its docs.
16191619
///
1620+
/// The offline peer should be awoken if possible on receipt of this event, such as via the LSPS5
1621+
/// protocol.
1622+
///
16201623
/// # Failure Behavior and Persistence
16211624
/// This event will eventually be replayed after failures-to-handle (i.e., the event handler
16221625
/// returning `Err(ReplayEvent ())`), but won't be persisted across restarts.
@@ -1661,6 +1664,14 @@ pub enum Event {
16611664
/// recipient is online to provide a new invoice. This path should be persisted and
16621665
/// later provided to [`ChannelManager::respond_to_static_invoice_request`].
16631666
///
1667+
/// This path's [`BlindedMessagePath::introduction_node`] MUST be set to our node or one of our
1668+
/// peers. This is because, for DoS protection, invoice requests forwarded over this path are
1669+
/// treated by our node like any other onion message forward and will not generate
1670+
/// [`Event::ConnectionNeeded`] if the first hop in the path is not our peer.
1671+
///
1672+
/// If the next-hop peer in the path is offline, if configured to do so we will generate an
1673+
/// [`Event::OnionMessageIntercepted`] for the invoice request.
1674+
///
16641675
/// [`ChannelManager::respond_to_static_invoice_request`]: crate::ln::channelmanager::ChannelManager::respond_to_static_invoice_request
16651676
invoice_request_path: BlindedMessagePath,
16661677
/// Useful for the recipient to replace a specific invoice stored by us as the static invoice

lightning/src/ln/async_payments_tests.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2887,6 +2887,21 @@ fn async_payment_e2e() {
28872887
.into_iter()
28882888
.find_map(|ev| {
28892889
if let Event::OnionMessageIntercepted { message, .. } = ev {
2890+
// At least one of the intercepted onion messages will be an invoice request that the
2891+
// invoice server is attempting to forward to the recipient, ignore that as we're testing
2892+
// the static invoice flow
2893+
let peeled_onion = recipient.onion_messenger.peel_onion_message(&message).unwrap();
2894+
if matches!(
2895+
peeled_onion,
2896+
PeeledOnion::Offers(OffersMessage::InvoiceRequest { .. }, _, _)
2897+
) {
2898+
return None;
2899+
}
2900+
2901+
assert!(matches!(
2902+
peeled_onion,
2903+
PeeledOnion::AsyncPayments(AsyncPaymentsMessage::HeldHtlcAvailable(_), _, _)
2904+
));
28902905
Some(message)
28912906
} else {
28922907
None

lightning/src/offers/flow.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,9 +1169,9 @@ where
11691169
) {
11701170
let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap();
11711171
let message = OffersMessage::InvoiceRequest(invoice_request);
1172-
let instructions = MessageSendInstructions::WithSpecifiedReplyPath {
1172+
let instructions = MessageSendInstructions::ForwardedMessage {
11731173
destination: Destination::BlindedPath(destination),
1174-
reply_path: reply_path.into_blinded_path(),
1174+
reply_path: Some(reply_path.into_blinded_path()),
11751175
};
11761176
pending_offers_messages.push((message, instructions));
11771177
}

lightning/src/onion_message/async_payments.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,11 @@ pub struct ServeStaticInvoice {
169169
/// [`Bolt12Invoice`] if the recipient is online at the time. Use this path to forward the
170170
/// [`InvoiceRequest`] to the async recipient.
171171
///
172+
/// This path's [`BlindedMessagePath::introduction_node`] MUST be set to the static invoice server
173+
/// node or one of its peers. This is because, for DoS protection, invoice requests forwarded over
174+
/// this path are treated by the server node like any other onion message forward and the server
175+
/// will not directly connect to the introduction node if they are not already peers.
176+
///
172177
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
173178
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
174179
pub forward_invoice_request_path: BlindedMessagePath,

lightning/src/onion_message/messenger.rs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,21 @@ pub enum MessageSendInstructions {
490490
/// The instructions provided by the [`Responder`].
491491
instructions: ResponseInstruction,
492492
},
493+
/// Indicates that this onion message did not originate from our node and is being forwarded
494+
/// through us from another node on the network to the destination.
495+
///
496+
/// We separate out this case because forwarded onion messages are treated differently from
497+
/// outbound onion messages initiated by our node. Outbounds are buffered internally, whereas, for
498+
/// DoS protection, forwards should never be buffered internally and instead will either be
499+
/// dropped or generate an [`Event::OnionMessageIntercepted`] if the next-hop node is
500+
/// disconnected.
501+
ForwardedMessage {
502+
/// The destination where we need to send the forwarded onion message.
503+
destination: Destination,
504+
/// The reply path which should be included in the message, that terminates at the original
505+
/// sender of this forwarded message.
506+
reply_path: Option<BlindedMessagePath>,
507+
},
493508
}
494509

495510
/// A trait defining behavior for routing an [`OnionMessage`].
@@ -1467,6 +1482,7 @@ where
14671482
fn send_onion_message_internal<T: OnionMessageContents>(
14681483
&self, contents: T, instructions: MessageSendInstructions, log_suffix: fmt::Arguments,
14691484
) -> Result<SendSuccess, SendError> {
1485+
let is_forward = matches!(instructions, MessageSendInstructions::ForwardedMessage { .. });
14701486
let (destination, reply_path) = match instructions {
14711487
MessageSendInstructions::WithSpecifiedReplyPath { destination, reply_path } => {
14721488
(destination, Some(reply_path))
@@ -1490,12 +1506,24 @@ where
14901506
| MessageSendInstructions::ForReply {
14911507
instructions: ResponseInstruction { destination, context: None },
14921508
} => (destination, None),
1509+
MessageSendInstructions::ForwardedMessage { destination, reply_path } => {
1510+
(destination, reply_path)
1511+
},
14931512
};
14941513

1495-
let path = self.find_path(destination).map_err(|e| {
1496-
log_trace!(self.logger, "Failed to find path {}", log_suffix);
1497-
e
1498-
})?;
1514+
let path = if is_forward {
1515+
// If this onion message is being treated as a forward, we shouldn't pathfind to the next hop.
1516+
OnionMessagePath {
1517+
intermediate_nodes: Vec::new(),
1518+
first_node_addresses: None,
1519+
destination,
1520+
}
1521+
} else {
1522+
self.find_path(destination).map_err(|e| {
1523+
log_trace!(self.logger, "Failed to find path {}", log_suffix);
1524+
e
1525+
})?
1526+
};
14991527
let first_hop = path.intermediate_nodes.get(0).map(|p| *p);
15001528
let logger = WithContext::from(&self.logger, first_hop, None, None);
15011529

@@ -1514,7 +1542,17 @@ where
15141542
e
15151543
})?;
15161544

1517-
let result = self.enqueue_outbound_onion_message(onion_message, first_node_id, addresses);
1545+
let result = if is_forward {
1546+
self.enqueue_forwarded_onion_message(
1547+
NextMessageHop::NodeId(first_node_id),
1548+
onion_message,
1549+
log_suffix,
1550+
)
1551+
.map(|()| SendSuccess::Buffered)
1552+
} else {
1553+
self.enqueue_outbound_onion_message(onion_message, first_node_id, addresses)
1554+
};
1555+
15181556
match result.as_ref() {
15191557
Err(SendError::GetNodeIdFailed) => {
15201558
log_warn!(logger, "Unable to retrieve node id {}", log_suffix);

0 commit comments

Comments
 (0)