Merge pull request #1715 from TheBlueMatt/2022-09-fix-msg-send

Fix encryption of broadcasted gossip messages

Author: TheBlueMatt

fuzz/src/peer_crypt.rs

@@ -74,7 +74,7 @@ pub fn do_test(data: &[u8]) {
 	};
 	loop {
 		if get_slice!(1)[0] == 0 {
-			crypter.encrypt_message(get_slice!(slice_to_be16(get_slice!(2))));
+			crypter.encrypt_buffer(get_slice!(slice_to_be16(get_slice!(2))));
 		} else {
 			let len = match crypter.decrypt_length_header(get_slice!(16+2)) {
 				Ok(len) => len,

lightning/src/ln/peer_channel_encryptor.rs

@@ -11,6 +11,7 @@ use prelude::*;
 
 use ln::msgs::LightningError;
 use ln::msgs;
+use ln::wire;
 
 use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::hashes::sha256::Hash as Sha256;
@@ -22,6 +23,7 @@ use bitcoin::secp256k1;
 
 use util::chacha20poly1305rfc::ChaCha20Poly1305RFC;
 use util::crypto::hkdf_extract_expand_twice;
+use util::ser::VecWriter;
 use bitcoin::hashes::hex::ToHex;
 
 /// Maximum Lightning message data length according to
@@ -142,6 +144,19 @@ impl PeerChannelEncryptor {
 		res[plaintext.len()..].copy_from_slice(&tag);
 	}
 
+	#[inline]
+	/// Encrypts the message in res[offset..] in-place and pushes a 16-byte tag onto the end of
+	/// res.
+	fn encrypt_in_place_with_ad(res: &mut Vec<u8>, offset: usize, n: u64, key: &[u8; 32], h: &[u8]) {
+		let mut nonce = [0; 12];
+		nonce[4..].copy_from_slice(&n.to_le_bytes()[..]);
+
+		let mut chacha = ChaCha20Poly1305RFC::new(key, &nonce, h);
+		let mut tag = [0; 16];
+		chacha.encrypt_full_message_in_place(&mut res[offset..], &mut tag);
+		res.extend_from_slice(&tag);
+	}
+
 	#[inline]
 	fn decrypt_with_ad(res: &mut[u8], n: u64, key: &[u8; 32], h: &[u8], cyphertext: &[u8]) -> Result<(), LightningError> {
 		let mut nonce = [0; 12];
@@ -372,9 +387,9 @@ impl PeerChannelEncryptor {
 		Ok(self.their_node_id.unwrap().clone())
 	}
 
-	/// Encrypts the given message, returning the encrypted version
+	/// Encrypts the given pre-serialized message, returning the encrypted version.
 	/// panics if msg.len() > 65535 or Noise handshake has not finished.
-	pub fn encrypt_message(&mut self, msg: &[u8]) -> Vec<u8> {
+	pub fn encrypt_buffer(&mut self, msg: &[u8]) -> Vec<u8> {
 		if msg.len() > LN_MAX_MSG_LEN {
 			panic!("Attempted to encrypt message longer than 65535 bytes!");
 		}
@@ -403,6 +418,42 @@ impl PeerChannelEncryptor {
 		res
 	}
 
+	/// Encrypts the given message, returning the encrypted version.
+	/// panics if the length of `message`, once encoded, is greater than 65535 or if the Noise
+	/// handshake has not finished.
+	pub fn encrypt_message<M: wire::Type>(&mut self, message: &M) -> Vec<u8> {
+		// Allocate a buffer with 2KB, fitting most common messages. Reserve the first 16+2 bytes
+		// for the 2-byte message type prefix and its MAC.
+		let mut res = VecWriter(Vec::with_capacity(2048));
+		res.0.resize(16 + 2, 0);
+		wire::write(message, &mut res).expect("In-memory messages must never fail to serialize");
+
+		let msg_len = res.0.len() - 16 - 2;
+		if msg_len > LN_MAX_MSG_LEN {
+			panic!("Attempted to encrypt message longer than 65535 bytes!");
+		}
+
+		match self.noise_state {
+			NoiseState::Finished { ref mut sk, ref mut sn, ref mut sck, rk: _, rn: _, rck: _ } => {
+				if *sn >= 1000 {
+					let (new_sck, new_sk) = hkdf_extract_expand_twice(sck, sk);
+					*sck = new_sck;
+					*sk = new_sk;
+					*sn = 0;
+				}
+
+				Self::encrypt_with_ad(&mut res.0[0..16+2], *sn, sk, &[0; 0], &(msg_len as u16).to_be_bytes());
+				*sn += 1;
+
+				Self::encrypt_in_place_with_ad(&mut res.0, 16+2, *sn, sk, &[0; 0]);
+				*sn += 1;
+			},
+			_ => panic!("Tried to encrypt a message prior to noise handshake completion"),
+		}
+
+		res.0
+	}
+
 	/// Decrypts a message length header from the remote peer.
 	/// panics if noise handshake has not yet finished or msg.len() != 18
 	pub fn decrypt_length_header(&mut self, msg: &[u8]) -> Result<u16, LightningError> {
@@ -682,7 +733,7 @@ mod tests {
 
 		for i in 0..1005 {
 			let msg = [0x68, 0x65, 0x6c, 0x6c, 0x6f];
-			let res = outbound_peer.encrypt_message(&msg);
+			let res = outbound_peer.encrypt_buffer(&msg);
 			assert_eq!(res.len(), 5 + 2*16 + 2);
 
 			let len_header = res[0..2+16].to_vec();
@@ -716,7 +767,7 @@ mod tests {
 	fn max_message_len_encryption() {
 		let mut outbound_peer = get_outbound_peer_for_initiator_test_vectors();
 		let msg = [4u8; LN_MAX_MSG_LEN + 1];
-		outbound_peer.encrypt_message(&msg);
+		outbound_peer.encrypt_buffer(&msg);
 	}
 
 	#[test]

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_buffer(&msg[..]));
 				}
 			}
 			if peer.should_buffer_gossip_backfill() {
@@ -941,22 +943,19 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 
 	/// Append a message to a peer's pending outbound/write buffer
 	fn enqueue_message<M: wire::Type>(&self, peer: &mut Peer, message: &M) {
-		let mut buffer = VecWriter(Vec::with_capacity(2048));
-		wire::write(message, &mut buffer).unwrap(); // crash if the write failed
-
 		if is_gossip_msg(message.type_id()) {
 			log_gossip!(self.logger, "Enqueueing message {:?} to {}", message, log_pubkey!(peer.their_node_id.unwrap()));
 		} else {
 			log_trace!(self.logger, "Enqueueing message {:?} to {}", message, log_pubkey!(peer.their_node_id.unwrap()))
 		}
 		peer.msgs_sent_since_pong += 1;
-		peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&buffer.0[..]));
+		peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(message));
 	}
 
 	/// 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> {
@@ -1438,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) => {
@@ -1461,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) => {
@@ -1481,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"),

lightning/src/util/chacha20poly1305rfc.rs

@@ -74,6 +74,11 @@ mod real_chachapoly {
 			self.mac.raw_result(out_tag);
 		}
 
+		pub fn encrypt_full_message_in_place(&mut self, input_output: &mut [u8], out_tag: &mut [u8]) {
+			self.encrypt_in_place(input_output);
+			self.finish_and_get_tag(out_tag);
+		}
+
 		// Encrypt `input_output` in-place. To finish and calculate the tag, use `finish_and_get_tag`
 		// below.
 		pub(super) fn encrypt_in_place(&mut self, input_output: &mut [u8]) {
@@ -284,6 +289,11 @@ mod fuzzy_chachapoly {
 			self.finished = true;
 		}
 
+		pub fn encrypt_full_message_in_place(&mut self, input_output: &mut [u8], out_tag: &mut [u8]) {
+			self.encrypt_in_place(input_output);
+			self.finish_and_get_tag(out_tag);
+		}
+
 		pub(super) fn encrypt_in_place(&mut self, _input_output: &mut [u8]) {
 			assert!(self.finished == false);
 		}