From 8ec92f5b6bb2b204708252427b1fd6835f358c58 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 12 Sep 2022 15:16:41 +0000 Subject: [PATCH 1/2] 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. --- lightning/src/ln/peer_handler.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 838927cb236..9b7490c3049 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>, 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>, awaiting_write_event: bool, @@ -822,7 +824,7 @@ impl) { + fn enqueue_encoded_gossip_broadcast(&self, peer: &mut Peer, encoded_message: Vec) { 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 { @@ -1435,7 +1437,7 @@ impl { @@ -1458,7 +1460,7 @@ impl { @@ -1478,7 +1480,7 @@ impl debug_assert!(false, "We shouldn't attempt to forward anything but gossip messages"), From 45ec1db2e04cc1c8b142dbeccf85d9c62dbe8da5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 12 Sep 2022 15:20:37 +0000 Subject: [PATCH 2/2] Encrypt+MAC most P2P messages in-place For non-gossip-broadcast messages, our current flow is to first serialize the message into a `Vec`, and then allocate a new `Vec` into which we write the encrypted+MAC'd message and header. This is somewhat wasteful, and its rather simple to instead allocate only one buffer and encrypt the message in-place. --- fuzz/src/peer_crypt.rs | 2 +- lightning/src/ln/peer_channel_encryptor.rs | 59 ++++++++++++++++++++-- lightning/src/ln/peer_handler.rs | 7 +-- lightning/src/util/chacha20poly1305rfc.rs | 10 ++++ 4 files changed, 68 insertions(+), 10 deletions(-) diff --git a/fuzz/src/peer_crypt.rs b/fuzz/src/peer_crypt.rs index de0443ebd5b..0cb429cff73 100644 --- a/fuzz/src/peer_crypt.rs +++ b/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, diff --git a/lightning/src/ln/peer_channel_encryptor.rs b/lightning/src/ln/peer_channel_encryptor.rs index 29fa84505fc..fe47f5b4e9f 100644 --- a/lightning/src/ln/peer_channel_encryptor.rs +++ b/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, 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 { + pub fn encrypt_buffer(&mut self, msg: &[u8]) -> Vec { 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(&mut self, message: &M) -> Vec { + // 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 { @@ -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] diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 9b7490c3049..a54f1c6d62d 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -824,7 +824,7 @@ impl(&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 diff --git a/lightning/src/util/chacha20poly1305rfc.rs b/lightning/src/util/chacha20poly1305rfc.rs index 1dbd91e65e0..5fddb57eb36 100644 --- a/lightning/src/util/chacha20poly1305rfc.rs +++ b/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); }