Skip to content

Commit b9b02b4

Browse files
valentinewallacevalentinewallace
committed
Refuse to send and forward OMs to disconnected peers
We also refuse to connect to peers that don't advertise onion message forwarding support.
1 parent 0b1f476 commit b9b02b4

File tree

5 files changed

+99
-47
lines changed

5 files changed

+99
-47
lines changed

‎lightning/src/ln/features.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,11 @@ mod sealed {
433433
define_feature!(27, ShutdownAnySegwit, [InitContext, NodeContext],
434434
"Feature flags for `opt_shutdown_anysegwit`.", set_shutdown_any_segwit_optional,
435435
set_shutdown_any_segwit_required, supports_shutdown_anysegwit, requires_shutdown_anysegwit);
436+
// We do not yet advertise the onion messages feature bit, but we need to detect when peers
437+
// support it.
438+
define_feature!(39, OnionMessages, [InitContext, NodeContext],
439+
"Feature flags for `option_onion_messages`.", set_onion_messages_optional,
440+
set_onion_messages_required, supports_onion_messages, requires_onion_messages);
436441
define_feature!(45, ChannelType, [InitContext, NodeContext],
437442
"Feature flags for `option_channel_type`.", set_channel_type_optional,
438443
set_channel_type_required, supports_channel_type, requires_channel_type);
@@ -767,7 +772,7 @@ impl<T: sealed::GossipQueries> Features<T> {
767772

768773
impl<T: sealed::InitialRoutingSync> Features<T> {
769774
// We are no longer setting initial_routing_sync now that gossip_queries
770-
// is enabled. This feature is ignored by a peer when gossip_queries has
775+
// is enabled. This feature is ignored by a peer when gossip_queries has
771776
// been negotiated.
772777
#[cfg(test)]
773778
pub(crate) fn clear_initial_routing_sync(&mut self) {

‎lightning/src/ln/msgs.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,12 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider {
949949
pub trait OnionMessageHandler : OnionMessageProvider {
950950
/// Handle an incoming onion_message message from the given peer.
951951
fn handle_onion_message(&self, peer_node_id: &PublicKey, msg: &OnionMessage);
952+
/// Called when a connection is established with a peer. Can be used to track which peers
953+
/// advertise onion message support and are online.
954+
fn peer_connected(&self, their_node_id: &PublicKey, init: &Init);
955+
/// Indicates a connection to the peer failed/an existing connection was lost. Allows handlers to
956+
/// drop and refuse to forward onion messages to this peer.
957+
fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool);
952958
}
953959

954960
mod fuzzy_internal_msgs {

‎lightning/src/ln/peer_handler.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ impl OnionMessageProvider for IgnoringMessageHandler {
8181
}
8282
impl OnionMessageHandler for IgnoringMessageHandler {
8383
fn handle_onion_message(&self, _their_node_id: &PublicKey, _msg: &msgs::OnionMessage) {}
84+
fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) {}
85+
fn peer_disconnected(&self, _their_node_id: &PublicKey, _no_connection_possible: bool) {}
8486
}
8587
impl Deref for IgnoringMessageHandler {
8688
type Target = IgnoringMessageHandler;
@@ -1173,8 +1175,9 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
11731175
}
11741176

11751177
self.message_handler.route_handler.peer_connected(&their_node_id, &msg);
1176-
11771178
self.message_handler.chan_handler.peer_connected(&their_node_id, &msg);
1179+
self.message_handler.onion_message_handler.peer_connected(&their_node_id, &msg);
1180+
11781181
peer_lock.their_features = Some(msg.features);
11791182
return Ok(None);
11801183
} else if peer_lock.their_features.is_none() {
@@ -1729,6 +1732,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
17291732
}
17301733
descriptor.disconnect_socket();
17311734
self.message_handler.chan_handler.peer_disconnected(&node_id, false);
1735+
self.message_handler.onion_message_handler.peer_disconnected(&node_id, false);
17321736
}
17331737
}
17341738
}
@@ -1756,6 +1760,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
17561760
log_pubkey!(node_id), if no_connection_possible { "no " } else { "" });
17571761
self.node_id_to_descriptor.lock().unwrap().remove(&node_id);
17581762
self.message_handler.chan_handler.peer_disconnected(&node_id, no_connection_possible);
1763+
self.message_handler.onion_message_handler.peer_disconnected(&node_id, no_connection_possible);
17591764
}
17601765
}
17611766
};
@@ -1776,6 +1781,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
17761781
log_trace!(self.logger, "Disconnecting peer with id {} due to client request", node_id);
17771782
peers_lock.remove(&descriptor);
17781783
self.message_handler.chan_handler.peer_disconnected(&node_id, no_connection_possible);
1784+
self.message_handler.onion_message_handler.peer_disconnected(&node_id, no_connection_possible);
17791785
descriptor.disconnect_socket();
17801786
}
17811787
}
@@ -1791,6 +1797,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
17911797
if let Some(node_id) = peer.lock().unwrap().their_node_id {
17921798
log_trace!(self.logger, "Disconnecting peer with id {} due to client request to disconnect all peers", node_id);
17931799
self.message_handler.chan_handler.peer_disconnected(&node_id, false);
1800+
self.message_handler.onion_message_handler.peer_disconnected(&node_id, false);
17941801
}
17951802
descriptor.disconnect_socket();
17961803
}
@@ -1881,6 +1888,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
18811888
log_trace!(self.logger, "Disconnecting peer with id {} due to ping timeout", node_id);
18821889
self.node_id_to_descriptor.lock().unwrap().remove(&node_id);
18831890
self.message_handler.chan_handler.peer_disconnected(&node_id, false);
1891+
self.message_handler.onion_message_handler.peer_disconnected(&node_id, false);
18841892
}
18851893
}
18861894
}

‎lightning/src/onion_message/functional_tests.rs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@
1010
//! Onion message testing and test utilities live here.
1111
1212
use chain::keysinterface::{KeysInterface, Recipient};
13-
use ln::msgs::OnionMessageHandler;
13+
use ln::features::InitFeatures;
14+
use ln::msgs::{self, OnionMessageHandler};
1415
use super::{BlindedRoute, Destination, OnionMessenger, SendError};
1516
use util::enforcing_trait_impls::EnforcingSigner;
1617
use util::test_utils;
1718

1819
use bitcoin::network::constants::Network;
19-
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
20+
use bitcoin::secp256k1::{PublicKey, Secp256k1};
2021

2122
use sync::Arc;
2223

@@ -34,26 +35,33 @@ impl MessengerNode {
3435
}
3536

3637
fn create_nodes(num_messengers: u8) -> Vec<MessengerNode> {
37-
let mut res = Vec::new();
38+
let mut nodes = Vec::new();
3839
for i in 0..num_messengers {
3940
let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i)));
4041
let seed = [i as u8; 32];
4142
let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&seed, Network::Testnet));
42-
res.push(MessengerNode {
43+
nodes.push(MessengerNode {
4344
keys_manager: keys_manager.clone(),
4445
messenger: OnionMessenger::new(keys_manager, logger.clone()),
4546
logger,
4647
});
4748
}
48-
res
49+
for idx in 0..num_messengers - 1 {
50+
let i = idx as usize;
51+
let mut features = InitFeatures::known();
52+
features.set_onion_messages_optional();
53+
let init_msg = msgs::Init { features, remote_network_address: None };
54+
nodes[i].messenger.peer_connected(&nodes[i + 1].get_node_pk(), &init_msg.clone());
55+
nodes[i + 1].messenger.peer_connected(&nodes[i].get_node_pk(), &init_msg.clone());
56+
}
57+
nodes
4958
}
5059

5160
fn pass_along_path(path: &Vec<MessengerNode>, expected_path_id: Option<[u8; 32]>) {
5261
let mut prev_node = &path[0];
5362
let num_nodes = path.len();
5463
for (idx, node) in path.into_iter().skip(1).enumerate() {
5564
let events = prev_node.messenger.release_pending_msgs();
56-
assert_eq!(events.len(), 1);
5765
let onion_msg = {
5866
let msgs = events.get(&node.get_node_pk()).unwrap();
5967
assert_eq!(msgs.len(), 1);
@@ -110,12 +118,9 @@ fn three_blinded_hops() {
110118
#[test]
111119
fn too_big_packet_error() {
112120
// Make sure we error as expected if a packet is too big to send.
113-
let nodes = create_nodes(1);
114-
115-
let hop_secret = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
116-
let secp_ctx = Secp256k1::new();
117-
let hop_node_id = PublicKey::from_secret_key(&secp_ctx, &hop_secret);
121+
let nodes = create_nodes(2);
118122

123+
let hop_node_id = nodes[1].get_node_pk();
119124
let hops = [hop_node_id; 400];
120125
let err = nodes[0].messenger.send_onion_message(&hops, Destination::Node(hop_node_id), None).unwrap_err();
121126
assert_eq!(err, SendError::TooBigPacket);

‎lightning/src/onion_message/messenger.rs

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ pub enum SendError {
122122
/// The provided [`Destination`] was an invalid [`BlindedRoute`], due to having fewer than two
123123
/// blinded hops.
124124
TooFewBlindedHops,
125+
/// Our next-hop peer was offline or does not support onion message forwarding.
126+
InvalidFirstHop,
125127
}
126128

127129
impl<Signer: Sign, K: Deref, L: Deref> OnionMessenger<Signer, K, L>
@@ -165,25 +167,28 @@ impl<Signer: Sign, K: Deref, L: Deref> OnionMessenger<Signer, K, L>
165167
.map_err(|e| SendError::Secp256k1(e))?;
166168

167169
let prng_seed = self.keys_manager.get_secure_random_bytes();
168-
let onion_packet = construct_onion_message_packet(
170+
let onion_routing_packet = construct_onion_message_packet(
169171
packet_payloads, packet_keys, prng_seed).map_err(|()| SendError::TooBigPacket)?;
170172

171173
let mut pending_per_peer_msgs = self.pending_messages.lock().unwrap();
172-
let pending_msgs = pending_per_peer_msgs.entry(introduction_node_id).or_insert_with(VecDeque::new);
173-
pending_msgs.push_back(
174-
msgs::OnionMessage {
175-
blinding_point,
176-
onion_routing_packet: onion_packet,
174+
match pending_per_peer_msgs.entry(introduction_node_id) {
175+
hash_map::Entry::Vacant(_) => Err(SendError::InvalidFirstHop),
176+
hash_map::Entry::Occupied(mut e) => {
177+
e.get_mut().push_back(msgs::OnionMessage { blinding_point, onion_routing_packet });
178+
Ok(())
177179
}
178-
);
179-
Ok(())
180+
}
180181
}
181182

182183
#[cfg(test)]
183184
pub(super) fn release_pending_msgs(&self) -> HashMap<PublicKey, VecDeque<msgs::OnionMessage>> {
184185
let mut pending_msgs = self.pending_messages.lock().unwrap();
185186
let mut msgs = HashMap::new();
186-
core::mem::swap(&mut *pending_msgs, &mut msgs);
187+
// We don't want to disconnect the peers by removing them entirely from the original map, so we
188+
// swap the pending message buffers individually.
189+
for (peer_node_id, pending_messages) in &mut *pending_msgs {
190+
msgs.insert(*peer_node_id, core::mem::take(pending_messages));
191+
}
187192
msgs
188193
}
189194
}
@@ -252,32 +257,43 @@ impl<Signer: Sign, K: Deref, L: Deref> OnionMessageHandler for OnionMessenger<Si
252257
};
253258

254259
let mut pending_per_peer_msgs = self.pending_messages.lock().unwrap();
255-
let pending_msgs = pending_per_peer_msgs.entry(next_node_id).or_insert_with(VecDeque::new);
256-
pending_msgs.push_back(
257-
msgs::OnionMessage {
258-
blinding_point: match next_blinding_override {
259-
Some(blinding_point) => blinding_point,
260-
None => {
261-
let blinding_factor = {
262-
let mut sha = Sha256::engine();
263-
sha.input(&msg.blinding_point.serialize()[..]);
264-
sha.input(control_tlvs_ss.as_ref());
265-
Sha256::from_engine(sha).into_inner()
266-
};
267-
let next_blinding_point = msg.blinding_point;
268-
match next_blinding_point.mul_tweak(&self.secp_ctx, &Scalar::from_be_bytes(blinding_factor).unwrap()) {
269-
Ok(bp) => bp,
270-
Err(e) => {
271-
log_trace!(self.logger, "Failed to compute next blinding point: {}", e);
272-
return
273-
}
274-
}
275-
},
276-
},
277-
onion_routing_packet: outgoing_packet,
260+
261+
#[cfg(fuzzing)]
262+
pending_per_peer_msgs.entry(next_node_id).or_insert_with(VecDeque::new);
263+
264+
match pending_per_peer_msgs.entry(next_node_id) {
265+
hash_map::Entry::Vacant(_) => {
266+
log_trace!(self.logger, "Dropping forwarded onion message to disconnected peer {:?}", next_node_id);
267+
return
278268
},
279-
);
280-
log_trace!(self.logger, "Forwarding an onion message to peer {}", next_node_id);
269+
hash_map::Entry::Occupied(mut e) => {
270+
e.get_mut().push_back(
271+
msgs::OnionMessage {
272+
blinding_point: match next_blinding_override {
273+
Some(blinding_point) => blinding_point,
274+
None => {
275+
let blinding_factor = {
276+
let mut sha = Sha256::engine();
277+
sha.input(&msg.blinding_point.serialize()[..]);
278+
sha.input(control_tlvs_ss.as_ref());
279+
Sha256::from_engine(sha).into_inner()
280+
};
281+
let next_blinding_point = msg.blinding_point;
282+
match next_blinding_point.mul_tweak(&self.secp_ctx, &Scalar::from_be_bytes(blinding_factor).unwrap()) {
283+
Ok(bp) => bp,
284+
Err(e) => {
285+
log_trace!(self.logger, "Failed to compute next blinding point: {}", e);
286+
return
287+
}
288+
}
289+
},
290+
},
291+
onion_routing_packet: outgoing_packet,
292+
},
293+
);
294+
log_trace!(self.logger, "Forwarding an onion message to peer {}", next_node_id);
295+
}
296+
};
281297
},
282298
Err(e) => {
283299
log_trace!(self.logger, "Errored decoding onion message packet: {:?}", e);
@@ -287,6 +303,18 @@ impl<Signer: Sign, K: Deref, L: Deref> OnionMessageHandler for OnionMessenger<Si
287303
},
288304
};
289305
}
306+
307+
fn peer_connected(&self, their_node_id: &PublicKey, init: &msgs::Init) {
308+
if init.features.supports_onion_messages() {
309+
let mut peers = self.pending_messages.lock().unwrap();
310+
peers.insert(their_node_id.clone(), VecDeque::new());
311+
}
312+
}
313+
314+
fn peer_disconnected(&self, their_node_id: &PublicKey, _no_connection_possible: bool) {
315+
let mut pending_msgs = self.pending_messages.lock().unwrap();
316+
pending_msgs.remove(their_node_id);
317+
}
290318
}
291319

292320
impl<Signer: Sign, K: Deref, L: Deref> OnionMessageProvider for OnionMessenger<Signer, K, L>

0 commit comments

Comments
 (0)