From 366662e08dc15493dee77d8633f95a37d45eecbf Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 4 Aug 2022 11:05:07 -0400 Subject: [PATCH] Refuse to send and forward OMs to disconnected peers We also refuse to connect to peers that don't advertise onion message forwarding support. However, we don't always enforce this in fuzzing, to maximize coverage. --- lightning/src/ln/features.rs | 7 +++- lightning/src/ln/msgs.rs | 6 ++++ lightning/src/ln/peer_handler.rs | 10 +++++- .../src/onion_message/functional_tests.rs | 27 ++++++++------ lightning/src/onion_message/messenger.rs | 35 ++++++++++++++++++- 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index dbc17a34138..c978c61a3a5 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -433,6 +433,11 @@ mod sealed { define_feature!(27, ShutdownAnySegwit, [InitContext, NodeContext], "Feature flags for `opt_shutdown_anysegwit`.", set_shutdown_any_segwit_optional, set_shutdown_any_segwit_required, supports_shutdown_anysegwit, requires_shutdown_anysegwit); + // We do not yet advertise the onion messages feature bit, but we need to detect when peers + // support it. + define_feature!(39, OnionMessages, [InitContext, NodeContext], + "Feature flags for `option_onion_messages`.", set_onion_messages_optional, + set_onion_messages_required, supports_onion_messages, requires_onion_messages); define_feature!(45, ChannelType, [InitContext, NodeContext], "Feature flags for `option_channel_type`.", set_channel_type_optional, set_channel_type_required, supports_channel_type, requires_channel_type); @@ -767,7 +772,7 @@ impl Features { impl Features { // We are no longer setting initial_routing_sync now that gossip_queries - // is enabled. This feature is ignored by a peer when gossip_queries has + // is enabled. This feature is ignored by a peer when gossip_queries has // been negotiated. #[cfg(test)] pub(crate) fn clear_initial_routing_sync(&mut self) { diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 190907ce26e..a32b17b9f0d 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -949,6 +949,12 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider { pub trait OnionMessageHandler : OnionMessageProvider { /// Handle an incoming onion_message message from the given peer. fn handle_onion_message(&self, peer_node_id: &PublicKey, msg: &OnionMessage); + /// Called when a connection is established with a peer. Can be used to track which peers + /// advertise onion message support and are online. + fn peer_connected(&self, their_node_id: &PublicKey, init: &Init); + /// Indicates a connection to the peer failed/an existing connection was lost. Allows handlers to + /// drop and refuse to forward onion messages to this peer. + fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool); } mod fuzzy_internal_msgs { diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 1258026e17e..f80c8984c1c 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -81,6 +81,8 @@ impl OnionMessageProvider for IgnoringMessageHandler { } impl OnionMessageHandler for IgnoringMessageHandler { fn handle_onion_message(&self, _their_node_id: &PublicKey, _msg: &msgs::OnionMessage) {} + fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) {} + fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {} } impl Deref for IgnoringMessageHandler { type Target = IgnoringMessageHandler; @@ -1173,8 +1175,9 @@ impl Vec { - let mut res = Vec::new(); + let mut nodes = Vec::new(); for i in 0..num_messengers { let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i))); let seed = [i as u8; 32]; let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&seed, Network::Testnet)); - res.push(MessengerNode { + nodes.push(MessengerNode { keys_manager: keys_manager.clone(), messenger: OnionMessenger::new(keys_manager, logger.clone()), logger, }); } - res + for idx in 0..num_messengers - 1 { + let i = idx as usize; + let mut features = InitFeatures::known(); + features.set_onion_messages_optional(); + let init_msg = msgs::Init { features, remote_network_address: None }; + nodes[i].messenger.peer_connected(&nodes[i + 1].get_node_pk(), &init_msg.clone()); + nodes[i + 1].messenger.peer_connected(&nodes[i].get_node_pk(), &init_msg.clone()); + } + nodes } fn pass_along_path(path: &Vec, expected_path_id: Option<[u8; 32]>) { @@ -53,7 +62,6 @@ fn pass_along_path(path: &Vec, expected_path_id: Option<[u8; 32]> let num_nodes = path.len(); for (idx, node) in path.into_iter().skip(1).enumerate() { let events = prev_node.messenger.release_pending_msgs(); - assert_eq!(events.len(), 1); let onion_msg = { let msgs = events.get(&node.get_node_pk()).unwrap(); assert_eq!(msgs.len(), 1); @@ -110,12 +118,9 @@ fn three_blinded_hops() { #[test] fn too_big_packet_error() { // Make sure we error as expected if a packet is too big to send. - let nodes = create_nodes(1); - - let hop_secret = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap(); - let secp_ctx = Secp256k1::new(); - let hop_node_id = PublicKey::from_secret_key(&secp_ctx, &hop_secret); + let nodes = create_nodes(2); + let hop_node_id = nodes[1].get_node_pk(); let hops = [hop_node_id; 400]; let err = nodes[0].messenger.send_onion_message(&hops, Destination::Node(hop_node_id), None).unwrap_err(); assert_eq!(err, SendError::TooBigPacket); diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index d93656c23bc..145ff6ce9f0 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -120,6 +120,8 @@ pub enum SendError { /// The provided [`Destination`] was an invalid [`BlindedRoute`], due to having fewer than two /// blinded hops. TooFewBlindedHops, + /// Our next-hop peer was offline or does not support onion message forwarding. + InvalidFirstHop, } impl OnionMessenger @@ -158,6 +160,9 @@ impl OnionMessenger (introduction_node_id, blinding_point), } }; + if !self.peer_is_connected(&introduction_node_id) { + return Err(SendError::InvalidFirstHop) + } let (packet_payloads, packet_keys) = packet_payloads_and_keys( &self.secp_ctx, intermediate_nodes, destination, reply_path, &blinding_secret) .map_err(|e| SendError::Secp256k1(e))?; @@ -177,11 +182,20 @@ impl OnionMessenger Ok(()) } + fn peer_is_connected(&self, peer_node_id: &PublicKey) -> bool { + self.pending_messages.lock().unwrap().contains_key(peer_node_id) + } + #[cfg(test)] pub(super) fn release_pending_msgs(&self) -> HashMap> { let mut pending_msgs = self.pending_messages.lock().unwrap(); let mut msgs = HashMap::new(); - core::mem::swap(&mut *pending_msgs, &mut msgs); + // We don't want to disconnect the peers by removing them entirely from the original map, so we + // swap the pending message buffers individually. + for (peer_node_id, pending_messages) in &mut *pending_msgs { + msgs.insert(*peer_node_id, VecDeque::new()); + core::mem::swap(pending_messages, msgs.get_mut(&peer_node_id).unwrap()); + } msgs } } @@ -230,6 +244,13 @@ impl OnionMessageHandler for OnionMessenger { + if !self.peer_is_connected(&next_node_id) { + #[cfg(not(fuzzing))] + { + log_trace!(self.logger, "Dropping onion message to disconnected peer {:?}", next_node_id); + return + } + } // TODO: we need to check whether `next_node_id` is our node, in which case this is a dummy // blinded hop and this onion message is destined for us. In this situation, we should keep // unwrapping the onion layers to get to the final payload. Since we don't have the option @@ -285,6 +306,18 @@ impl OnionMessageHandler for OnionMessenger OnionMessageProvider for OnionMessenger