From: Matt Corallo <git@bluematt.me>
Date: Mon, 12 Sep 2022 15:16:41 +0000 (+0000)
Subject: Fix encryption of broadcasted gossip messages
X-Git-Tag: v0.0.111~2^2~1
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=8ec92f5b6bb2b204708252427b1fd6835f358c58;p=rust-lightning

Fix encryption of broadcasted gossip messages

In 47e818f198abafba01b9ad278582886f9007dac2, forwarding broadcasted
gossip messages was split into a separate per-peer message buffer.
However, both it and the original regular-message queue are
encrypted immediately when the messages are enqueued. Because the
lightning P2P encryption algorithm is order-dependent, this causes
messages to fail their MAC checks as the messages from the two
queues may not be sent to peers in the order in which they were
encrypted.

The fix is to simply queue broadcast gossip messages unencrypted,
encrypting them when we add them to the regular outbound buffer.
---

diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 838927cb2..9b7490c30 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -367,8 +367,10 @@ struct Peer {
 
 	pending_outbound_buffer: LinkedList<Vec<u8>>,
 	pending_outbound_buffer_first_msg_offset: usize,
-	// Queue gossip broadcasts separately from `pending_outbound_buffer` so we can easily prioritize
-	// channel messages over them.
+	/// Queue gossip broadcasts separately from `pending_outbound_buffer` so we can easily
+	/// prioritize channel messages over them.
+	///
+	/// Note that these messages are *not* encrypted/MAC'd, and are only serialized.
 	gossip_broadcast_buffer: LinkedList<Vec<u8>>,
 	awaiting_write_event: bool,
 
@@ -822,7 +824,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 			}
 			if peer.should_buffer_gossip_broadcast() {
 				if let Some(msg) = peer.gossip_broadcast_buffer.pop_front() {
-					peer.pending_outbound_buffer.push_back(msg);
+					peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&msg[..]));
 				}
 			}
 			if peer.should_buffer_gossip_backfill() {
@@ -954,9 +956,9 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 	}
 
 	/// Append a message to a peer's pending outbound/write gossip broadcast buffer
-	fn enqueue_encoded_gossip_broadcast(&self, peer: &mut Peer, encoded_message: &Vec<u8>) {
+	fn enqueue_encoded_gossip_broadcast(&self, peer: &mut Peer, encoded_message: Vec<u8>) {
 		peer.msgs_sent_since_pong += 1;
-		peer.gossip_broadcast_buffer.push_back(peer.channel_encryptor.encrypt_message(&encoded_message[..]));
+		peer.gossip_broadcast_buffer.push_back(encoded_message);
 	}
 
 	fn do_read_event(&self, peer_descriptor: &mut Descriptor, data: &[u8]) -> Result<bool, PeerHandleError> {
@@ -1435,7 +1437,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 					if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
 						continue;
 					}
-					self.enqueue_encoded_gossip_broadcast(&mut *peer, &encoded_msg);
+					self.enqueue_encoded_gossip_broadcast(&mut *peer, encoded_msg.clone());
 				}
 			},
 			wire::Message::NodeAnnouncement(ref msg) => {
@@ -1458,7 +1460,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 					if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
 						continue;
 					}
-					self.enqueue_encoded_gossip_broadcast(&mut *peer, &encoded_msg);
+					self.enqueue_encoded_gossip_broadcast(&mut *peer, encoded_msg.clone());
 				}
 			},
 			wire::Message::ChannelUpdate(ref msg) => {
@@ -1478,7 +1480,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 					if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
 						continue;
 					}
-					self.enqueue_encoded_gossip_broadcast(&mut *peer, &encoded_msg);
+					self.enqueue_encoded_gossip_broadcast(&mut *peer, encoded_msg.clone());
 				}
 			},
 			_ => debug_assert!(false, "We shouldn't attempt to forward anything but gossip messages"),