Skip to content

Fix encryption of broadcasted gossip messages #1715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 12, 2022
Merged

Fix encryption of broadcasted gossip messages #1715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ec92f5
Fix encryption of broadcasted gossip messages
TheBlueMatt Sep 12, 2022
45ec1db
Encrypt+MAC most P2P messages in-place
TheBlueMatt Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fuzz/src/peer_crypt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
59 changes: 55 additions & 4 deletions lightning/src/ln/peer_channel_encryptor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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!");
}
Expand Down Expand Up @@ -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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we consolidate this with encrypt_buffer if we made gossip_broadcast_buffer contain messages instead of encoded messages?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but I'm not sure that's better - many messages have multiple heap-allocated structs, and we'd have to clone all of that. The nice thing about storing the encoded message is we just need one heap-allocated Vec per peer, rather than potentially many.

// 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> {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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]
Expand Down
23 changes: 11 additions & 12 deletions lightning/src/ln/peer_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -1435,7 +1434,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) => {
Expand All @@ -1458,7 +1457,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) => {
Expand All @@ -1478,7 +1477,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"),
Expand Down
10 changes: 10 additions & 0 deletions lightning/src/util/chacha20poly1305rfc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -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);
}
Expand Down