make linter complain less about docs

fix last lint doc issue?

make the &Some matches explicit for Rust 1.22

appease Rust 1.22 some more with ampersandery

appease Rust 1.22 by using byte_utils for endianness functionality

appease Rust 1.22 by using byte_utils and ref in match arms

experimenting some more with ref matching for Rust 1.22

might Rust 1.22 finally work? Rearranged a lot of borrowing locations and macro behavior and definition locations

fix bug that was kindly caught by fuzz tests

fix fuzz test

improve error messaging

the different rust versions are a balancing act, and I can't juggle

Author: arik-so

lightning/src/ln/peer_handler.rs

@@ -114,7 +114,7 @@ enum PeerState {
 
 impl PeerState {
 	fn is_ready_for_encryption(&self) -> bool {
-		if let PeerState::Connected(_) = self {
+		if let &PeerState::Connected(_) = self {
 			true
 		} else {
 			false
@@ -285,7 +285,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
 		if peers.peers.insert(descriptor, Peer {
 			encryptor: PeerState::Handshake(handshake),
 			outbound: true,
-			their_node_id: None,
+			their_node_id: Some(their_node_id.clone()),
 			their_features: None,
 
 			pending_outbound_buffer: LinkedList::new(),
@@ -313,7 +313,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
 	/// Panics if descriptor is duplicative with some other descriptor which has not yet has a
 	/// disconnect_event.
 	pub fn new_inbound_connection(&self, descriptor: Descriptor) -> Result<(), PeerHandleError> {
-		let handshake = PeerHandshake::new(&self.our_node_secret, &self.get_ephemeral_key());
+		let mut handshake = PeerHandshake::new(&self.our_node_secret, &self.get_ephemeral_key());
+		handshake.make_inbound();
 
 		let mut peers = self.peers.lock().unwrap();
 		if peers.peers.insert(descriptor, Peer {
@@ -465,78 +466,83 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
 					peer.pending_read_buffer.extend_from_slice(&data);
 					while peer.pending_read_buffer.len() > 0 {
 
-						macro_rules! encode_and_send_msg {
-							($msg: expr) => {
-								{
-									log_trace!(self, "Encoding and sending message of type {} to {}", $msg.type_id(), log_pubkey!(peer.their_node_id.unwrap()));
-									if let PeerState::Connected(ref mut conduit) = peer.encryptor {
-										peer.pending_outbound_buffer.push_back(conduit.encrypt(&encode_msg!(&$msg)[..]));
-									}
-									peers.peers_needing_send.insert(peer_descriptor.clone());
-								}
+						let mut conduit_option = None;
+						let mut remote_pubkey_option = None;
+
+						if let &mut PeerState::Handshake(ref mut handshake) = &mut peer.encryptor {
+							let (response, conduit, remote_pubkey) = handshake.process_act(&peer.pending_read_buffer, None).unwrap();
+							peer.pending_read_buffer.drain(..); // we read it all
+
+							if let Some(key) = remote_pubkey {
+								remote_pubkey_option = Some(key);
 							}
-						}
 
-						macro_rules! try_potential_handleerror {
-							($thing: expr) => {
-								match $thing {
-									Ok(x) => x,
-									Err(e) => {
-										match e.action {
-											msgs::ErrorAction::DisconnectPeer { msg: _ } => {
-												//TODO: Try to push msg
-												log_trace!(self, "Got Err handling message, disconnecting peer because {}", e.err);
-												return Err(PeerHandleError{ no_connection_possible: false });
-											},
-											msgs::ErrorAction::IgnoreError => {
-												log_trace!(self, "Got Err handling message, ignoring because {}", e.err);
-												continue;
-											},
-											msgs::ErrorAction::SendErrorMessage { msg } => {
-												log_trace!(self, "Got Err handling message, sending Error message because {}", e.err);
-												encode_and_send_msg!(msg);
-												continue;
-											},
-										}
-									}
-								};
+							peer.pending_outbound_buffer.push_back(response);
+							if let Some(conduit) = conduit {
+								conduit_option = Some(conduit);
 							}
 						}
 
-						macro_rules! insert_node_id {
-							() => {
-								match peers.node_id_to_descriptor.entry(peer.their_node_id.unwrap()) {
-									hash_map::Entry::Occupied(_) => {
-										log_trace!(self, "Got second connection with {}, closing", log_pubkey!(peer.their_node_id.unwrap()));
-										peer.their_node_id = None; // Unset so that we don't generate a peer_disconnected event
-										return Err(PeerHandleError{ no_connection_possible: false })
-									},
-									hash_map::Entry::Vacant(entry) => {
-										log_trace!(self, "Finished noise handshake for connection with {}", log_pubkey!(peer.their_node_id.unwrap()));
-										entry.insert(peer_descriptor.clone())
-									},
-								};
-							}
+						if let Some(key) = remote_pubkey_option {
+							peer.their_node_id = Some(key);
 						}
 
-						if let PeerState::Handshake(ref mut handshake) = &mut peer.encryptor {
-							let (response, conduit, remote_pubkey) = handshake.process_act(&peer.pending_read_buffer, None).unwrap();
-							peer.pending_read_buffer.drain(..); // we read it all
+						if let Some(conduit) = conduit_option {
+							// Rust 1.22 does not allow assignment in a borrowed context, even if mutable
+							peer.encryptor = PeerState::Connected(conduit);
+						}
 
-							peer.pending_outbound_buffer.push_back(response);
-							if let Some(conduit) = conduit {
-								peer.encryptor = PeerState::Connected(conduit);
+						if let &mut PeerState::Connected(ref mut conduit) = &mut peer.encryptor {
+
+							macro_rules! encode_and_send_msg {
+								($msg: expr) => {
+									{
+										log_trace!(self, "Encoding and sending message of type {} to {}", $msg.type_id(), log_pubkey!(peer.their_node_id.unwrap()));
+										// we are in a context where conduit is known
+										peer.pending_outbound_buffer.push_back(conduit.encrypt(&encode_msg!(&$msg)[..]));
+										peers.peers_needing_send.insert(peer_descriptor.clone());
+									}
+								}
+							}
+
+							macro_rules! try_potential_handleerror {
+								($thing: expr) => {
+									match $thing {
+										Ok(x) => x,
+										Err(e) => {
+											match e.action {
+												msgs::ErrorAction::DisconnectPeer { msg: _ } => {
+													//TODO: Try to push msg
+													log_trace!(self, "Got Err handling message, disconnecting peer because {}", e.err);
+													return Err(PeerHandleError{ no_connection_possible: false });
+												},
+												msgs::ErrorAction::IgnoreError => {
+													log_trace!(self, "Got Err handling message, ignoring because {}", e.err);
+													continue;
+												},
+												msgs::ErrorAction::SendErrorMessage { msg } => {
+													log_trace!(self, "Got Err handling message, sending Error message because {}", e.err);
+													encode_and_send_msg!(msg);
+													continue;
+												},
+											}
+										}
+									};
+								}
 							}
-						}
 
-						if let PeerState::Connected(ref mut conduit) = &mut peer.encryptor {
-							let mut messages = conduit.decrypt_message_stream(Some(&peer.pending_read_buffer));
+							let mut next_message_result = conduit.decrypt(&peer.pending_read_buffer);
 
-							if messages.len() != 1 {
-								break; // the length should initially be one, though there will be a possibility of decrypting multiple messages at once in the future
+							let offset = next_message_result.1;
+							if offset == 0 {
+								// nothing got read
+								break;
+							}else{
+								peer.pending_read_buffer.drain(0..offset);
 							}
 
-							let msg_data = messages.remove(1);
+							// something got read, so we definitely have a message
+							let msg_data = next_message_result.0.unwrap();
 
 							let mut reader = ::std::io::Cursor::new(&msg_data[..]);
 							let message_result = wire::read(&mut reader);

lightning/src/ln/peers/chacha.rs

@@ -1,8 +1,9 @@
+use util::byte_utils;
 use util::chacha20poly1305rfc::ChaCha20Poly1305RFC;
 
 pub fn encrypt(key: &[u8], nonce: u64, associated_data: &[u8], plaintext: &[u8]) -> Vec<u8> {
 	let mut nonce_bytes = [0; 12];
-	nonce_bytes[4..].copy_from_slice(&nonce.to_le_bytes());
+	nonce_bytes[4..].copy_from_slice(&byte_utils::le64_to_array(nonce));
 
 	let mut chacha = ChaCha20Poly1305RFC::new(key, &nonce_bytes, associated_data);
 	let mut ciphertext = vec![0u8; plaintext.len()];
@@ -17,11 +18,15 @@ pub fn encrypt(key: &[u8], nonce: u64, associated_data: &[u8], plaintext: &[u8])
 
 pub fn decrypt(key: &[u8], nonce: u64, associated_data: &[u8], tagged_ciphertext: &[u8]) -> Result<Vec<u8>, String> {
 	let mut nonce_bytes = [0; 12];
-	nonce_bytes[4..].copy_from_slice(&nonce.to_le_bytes());
+	nonce_bytes[4..].copy_from_slice(&byte_utils::le64_to_array(nonce));
 
 	let length = tagged_ciphertext.len();
-	let ciphertext = &tagged_ciphertext[0..length - 16];
-	let authentication_tag = &tagged_ciphertext[length - 16..length];
+	if length < 16 {
+		return Err("ciphertext cannot be shorter than tag length of 16 bytes".to_string());
+	}
+	let end_index = length - 16;
+	let ciphertext = &tagged_ciphertext[0..end_index];
+	let authentication_tag = &tagged_ciphertext[end_index..length];
 
 	let mut chacha = ChaCha20Poly1305RFC::new(key, &nonce_bytes, associated_data);
 	let mut plaintext = vec![0u8; length - 16];

lightning/src/ln/peers/conduit.rs

@@ -1,6 +1,11 @@
+//! Handles all over the wire message encryption and decryption upon handshake completion.
+
 use ln::peers::{chacha, hkdf};
+use util::byte_utils;
 
 /// Returned after a successful handshake to encrypt and decrypt communication with peer nodes
+/// Automatically handles key rotation.
+/// For decryption, it is recommended to call `decrypt_message_stream` for automatic buffering.
 pub struct Conduit {
 	pub(crate) sending_key: [u8; 32],
 	pub(crate) receiving_key: [u8; 32],
@@ -15,9 +20,10 @@ pub struct Conduit {
 }
 
 impl Conduit {
+	/// Encrypt data to be sent to peer
 	pub fn encrypt(&mut self, buffer: &[u8]) -> Vec<u8> {
 		let length = buffer.len() as u16;
-		let length_bytes = length.to_be_bytes();
+		let length_bytes = byte_utils::be16_to_array(length);
 
 		let encrypted_length = chacha::encrypt(&self.sending_key, self.sending_nonce as u64, &[0; 0], &length_bytes);
 		self.increment_sending_nonce();
@@ -69,7 +75,7 @@ impl Conduit {
 	}
 
 	/// Decrypt a single message. Buffer is an undelimited amount of bytes
-	fn decrypt(&mut self, buffer: &[u8]) -> (Option<Vec<u8>>, usize) { // the response slice should have the same lifetime as the argument. It's the slice data is read from
+	pub(crate) fn decrypt(&mut self, buffer: &[u8]) -> (Option<Vec<u8>>, usize) { // the response slice should have the same lifetime as the argument. It's the slice data is read from
 		if buffer.len() < 18 {
 			return (None, 0);
 		}
@@ -78,9 +84,9 @@ impl Conduit {
 		let length_vec = chacha::decrypt(&self.receiving_key, self.receiving_nonce as u64, &[0; 0], encrypted_length).unwrap();
 		let mut length_bytes = [0u8; 2];
 		length_bytes.copy_from_slice(length_vec.as_slice());
-		let message_length = u16::from_be_bytes(length_bytes) as usize;
+		let message_length = byte_utils::slice_to_be16(&length_bytes) as usize;
 
-		let message_end_index = message_length + 18; // todo: abort if too short
+		let message_end_index = message_length + 18 + 16; // todo: abort if too short
 		if buffer.len() < message_end_index {
 			return (None, 0);
 		}
@@ -121,6 +127,7 @@ impl Conduit {
 
 #[cfg(test)]
 mod tests {
+	use hex;
 	use ln::peers::conduit::Conduit;
 
 	#[test]
@@ -147,6 +154,16 @@ mod tests {
 			read_buffer: None,
 		};
 
+		let mut remote_peer = Conduit {
+			sending_key: receiving_key,
+			receiving_key: sending_key,
+			sending_chaining_key: chaining_key,
+			receiving_chaining_key: chaining_key,
+			sending_nonce: 0,
+			receiving_nonce: 0,
+			read_buffer: None,
+		};
+
 		let message = hex::decode("68656c6c6f").unwrap();
 		let mut encrypted_messages: Vec<Vec<u8>> = Vec::new();
 
@@ -161,5 +178,13 @@ mod tests {
 		assert_eq!(encrypted_messages[501], hex::decode("1b186c57d44eb6de4c057c49940d79bb838a145cb528d6e8fd26dbe50a60ca2c104b56b60e45bd").unwrap());
 		assert_eq!(encrypted_messages[1000], hex::decode("4a2f3cc3b5e78ddb83dcb426d9863d9d9a723b0337c89dd0b005d89f8d3c05c52b76b29b740f09").unwrap());
 		assert_eq!(encrypted_messages[1001], hex::decode("2ecd8c8a5629d0d02ab457a0fdd0f7b90a192cd46be5ecb6ca570bfc5e268338b1a16cf4ef2d36").unwrap());
+
+		for _ in 0..1002 {
+			let encrypted_message = encrypted_messages.remove(0);
+			let mut decrypted_messages = remote_peer.decrypt_message_stream(Some(&encrypted_message));
+			assert_eq!(decrypted_messages.len(), 1);
+			let decrypted_message = decrypted_messages.remove(0);
+			assert_eq!(decrypted_message, hex::decode("68656c6c6f").unwrap());
+		}
 	}
 }

lightning/src/ln/peers/handshake/acts.rs

@@ -1,31 +1,36 @@
+/// Wrapper for the first act message
 pub struct ActOne(
 	pub(super) [u8; 50]
 );
 
+/// Wrapper for the second act message
 pub struct ActTwo(
 	pub(super) [u8; 50]
 );
 
+/// Wrapper for the third act message
 pub struct ActThree(
 	pub(super) [u8; 66]
 );
 
+/// Wrapper for any act message
 pub enum Act {
 	One(ActOne),
 	Two(ActTwo),
 	Three(ActThree),
 }
 
 impl Act {
+	/// Convert any act into a byte vector
 	pub fn serialize(&self) -> Vec<u8> {
 		match self {
-			Act::One(act) => {
+			&Act::One(ref act) => {
 				act.0.to_vec()
 			}
-			Act::Two(act) => {
+			&Act::Two(ref act) => {
 				act.0.to_vec()
 			}
-			Act::Three(act) => {
+			&Act::Three(ref act) => {
 				act.0.to_vec()
 			}
 		}