refactor: Make Conduit a simple wrapper

The public interface was in a half-state with some callers referencing
the Decryptor directly for message iteration and others using the
public interface for the encryption path. This patch moves everything to
using the Encryptor and Decryptor directly.

This is motivated by feedback from 494, that recommended the objects
are split up but still have common functions for the key rotation.

Author: julianknutsen

fuzz/src/peer_crypt.rs

@@ -137,7 +137,7 @@ fn do_conduit_tests(generator: &mut FuzzGen, initiator_conduit: &mut Conduit, re
 
 		// randomly choose sender of message
 		let receiver_unencrypted_msg = if generator.generate_bool()? {
-			let encrypted_msg = initiator_conduit.encrypt(sender_unencrypted_msg);
+			let encrypted_msg = initiator_conduit.encryptor.encrypt(sender_unencrypted_msg);
 			if let Ok(_) = responder_conduit.decryptor.read(&encrypted_msg) {
 				if let Some(msg) = responder_conduit.decryptor.next() {
 					msg
@@ -150,7 +150,7 @@ fn do_conduit_tests(generator: &mut FuzzGen, initiator_conduit: &mut Conduit, re
 				return Ok(());
 			}
 		} else {
-			let encrypted_msg = responder_conduit.encrypt(sender_unencrypted_msg);
+			let encrypted_msg = responder_conduit.encryptor.encrypt(sender_unencrypted_msg);
 			if let Ok(_) = initiator_conduit.decryptor.read(&encrypted_msg) {
 				if let Some(msg) = initiator_conduit.decryptor.next() {
 					msg

lightning/src/ln/peers/conduit.rs

@@ -19,18 +19,12 @@ const KEY_ROTATION_INDEX: u32 = 1000;
 /// Returned after a successful handshake to encrypt and decrypt communication with peer nodes.
 /// It should not normally be manually instantiated.
 /// Automatically handles key rotation.
-/// For decryption, it is recommended to call `decrypt_message_stream` for automatic buffering.
 pub struct Conduit {
-	pub(super) encryptor: Encryptor,
-
-	#[cfg(feature = "fuzztarget")]
+	pub encryptor: Encryptor,
 	pub decryptor: Decryptor,
-	#[cfg(not(feature = "fuzztarget"))]
-	pub(super) decryptor: Decryptor,
-
 }
 
-pub(super) struct Encryptor {
+pub struct Encryptor {
 	sending_key: SymmetricKey,
 	sending_chaining_key: SymmetricKey,
 	sending_nonce: u32,
@@ -74,15 +68,6 @@ impl Conduit {
 		}
 	}
 
-	/// Encrypt data to be sent to peer
-	pub fn encrypt(&mut self, buffer: &[u8]) -> Vec<u8> {
-		self.encryptor.encrypt(buffer)
-	}
-
-	pub(super) fn read(&mut self, data: &[u8]) -> Result<(), String>{
-		self.decryptor.read(data)
-	}
-
 	fn increment_nonce(nonce: &mut u32, chaining_key: &mut SymmetricKey, key: &mut SymmetricKey) {
 		*nonce += 1;
 		if *nonce == KEY_ROTATION_INDEX {
@@ -99,7 +84,7 @@ impl Conduit {
 }
 
 impl Encryptor {
-	pub(super) fn encrypt(&mut self, buffer: &[u8]) -> Vec<u8> {
+	pub fn encrypt(&mut self, buffer: &[u8]) -> Vec<u8> {
 		if buffer.len() > LN_MAX_MSG_LEN {
 			panic!("Attempted to encrypt message longer than 65535 bytes!");
 		}
@@ -250,7 +235,7 @@ mod tests {
 		let (mut connected_peer, mut remote_peer) = setup_peers();
 
 		let message: Vec<u8> = vec![];
-		let encrypted_message = connected_peer.encrypt(&message);
+		let encrypted_message = connected_peer.encryptor.encrypt(&message);
 		assert_eq!(encrypted_message.len(), 2 + 16 + 16);
 
 		remote_peer.decryptor.read(&encrypted_message[..]).unwrap();
@@ -266,7 +251,7 @@ mod tests {
 		let (mut connected_peer, mut remote_peer) = setup_peers();
 
 		let message: Vec<u8> = vec![1];
-		let encrypted_message = connected_peer.encrypt(&message);
+		let encrypted_message = connected_peer.encryptor.encrypt(&message);
 
 		remote_peer.decryptor.read(&encrypted_message[..1]).unwrap();
 		assert!(remote_peer.decryptor.next().is_none());
@@ -283,7 +268,7 @@ mod tests {
 		let (mut connected_peer, mut remote_peer) = setup_peers();
 
 		let message: Vec<u8> = vec![1];
-		let encrypted_message = connected_peer.encrypt(&message);
+		let encrypted_message = connected_peer.encryptor.encrypt(&message);
 
 		remote_peer.decryptor.read(&encrypted_message[..20]).unwrap();
 		assert!(remote_peer.decryptor.next().is_none());
@@ -299,11 +284,11 @@ mod tests {
 		let (mut connected_peer, _remote_peer) = setup_peers();
 		let message = hex::decode("68656c6c6f").unwrap();
 
-		let encrypted_message = connected_peer.encrypt(&message);
+		let encrypted_message = connected_peer.encryptor.encrypt(&message);
 		assert_eq!(encrypted_message, hex::decode("cf2b30ddf0cf3f80e7c35a6e6730b59fe802473180f396d88a8fb0db8cbcf25d2f214cf9ea1d95").unwrap());
 
 		// the second time the same message is encrypted, the ciphertext should be different
-		let encrypted_message = connected_peer.encrypt(&message);
+		let encrypted_message = connected_peer.encryptor.encrypt(&message);
 		assert_eq!(encrypted_message, hex::decode("72887022101f0b6753e0c7de21657d35a4cb2a1f5cde2650528bbc8f837d0f0d7ad833b1a256a1").unwrap());
 	}
 
@@ -316,7 +301,7 @@ mod tests {
 		let mut encrypted_messages: Vec<Vec<u8>> = Vec::new();
 
 		for _ in 0..1002 {
-			let encrypted_message = connected_peer.encrypt(&message);
+			let encrypted_message = connected_peer.encryptor.encrypt(&message);
 			encrypted_messages.push(encrypted_message);
 		}
 
@@ -334,7 +319,7 @@ mod tests {
 		let mut encrypted_messages: Vec<Vec<u8>> = Vec::new();
 
 		for _ in 0..1002 {
-			let encrypted_message = connected_peer.encrypt(&message);
+			let encrypted_message = connected_peer.encryptor.encrypt(&message);
 			encrypted_messages.push(encrypted_message);
 		}
 
@@ -343,15 +328,15 @@ mod tests {
 			let mut current_encrypted_message = encrypted_messages.remove(0);
 			let next_encrypted_message = encrypted_messages.remove(0);
 			current_encrypted_message.extend_from_slice(&next_encrypted_message);
-			remote_peer.read(&current_encrypted_message[..]).unwrap();
+			remote_peer.decryptor.read(&current_encrypted_message[..]).unwrap();
 
 			let decrypted_message = remote_peer.decryptor.next().unwrap();
 			assert_eq!(decrypted_message, message);
 		}
 
 		for _ in 0..501 {
 			// decrypt messages directly from buffer without adding to it
-			remote_peer.read(&[]).unwrap();
+			remote_peer.decryptor.read(&[]).unwrap();
 			let decrypted_message = remote_peer.decryptor.next().unwrap();
 			assert_eq!(decrypted_message, message);
 		}
@@ -361,10 +346,10 @@ mod tests {
 	#[test]
 	fn decryption_failure_errors() {
 		let (mut connected_peer, mut remote_peer) = setup_peers();
-		let encrypted = remote_peer.encrypt(&[1]);
+		let encrypted = remote_peer.encryptor.encrypt(&[1]);
 
 		connected_peer.decryptor.receiving_key = [0; 32];
-		assert_eq!(connected_peer.read(&encrypted), Err("invalid hmac".to_string()));
+		assert_eq!(connected_peer.decryptor.read(&encrypted), Err("invalid hmac".to_string()));
 	}
 
 	// Test next()::None
@@ -379,8 +364,8 @@ mod tests {
 	#[test]
 	fn decryptor_iterator_one_item_valid() {
 		let (mut connected_peer, mut remote_peer) = setup_peers();
-		let encrypted = remote_peer.encrypt(&[1]);
-		connected_peer.read(&encrypted).unwrap();
+		let encrypted = remote_peer.encryptor.encrypt(&[1]);
+		connected_peer.decryptor.read(&encrypted).unwrap();
 
 		assert_eq!(connected_peer.decryptor.next(), Some(vec![1]));
 		assert_eq!(connected_peer.decryptor.next(), None);
@@ -397,7 +382,7 @@ mod tests {
 	fn max_message_len_encryption() {
 		let (mut connected_peer, _) = setup_peers();
 		let msg = [4u8; LN_MAX_MSG_LEN + 1];
-		let _should_panic = connected_peer.encrypt(&msg);
+		let _should_panic = connected_peer.encryptor.encrypt(&msg);
 	}
 
 	#[test]
@@ -407,6 +392,6 @@ mod tests {
 
 		// MSG should not exceed LN_MAX_MSG_LEN + 16
 		let msg = [4u8; LN_MAX_MSG_LEN + 17];
-		connected_peer.read(&msg).unwrap();
+		connected_peer.decryptor.read(&msg).unwrap();
 	}
 }

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

@@ -391,7 +391,7 @@ impl IHandshakeState for ResponderAwaitingActThreeState {
 
 		// Any remaining data in the read buffer would be encrypted, so transfer ownership
 		// to the Conduit for future use.
-		conduit.read(&input[bytes_read..])?;
+		conduit.decryptor.read(&input[bytes_read..])?;
 
 		Ok((
 			None,

lightning/src/ln/peers/transport.rs

@@ -91,7 +91,7 @@ impl<PeerHandshakeImpl: IPeerHandshake> ITransport for Transport<PeerHandshakeIm
 				}
 			}
 			Some(ref mut conduit) => {
-				conduit.read(input)?;
+				conduit.decryptor.read(input)?;
 				Ok(false) // newly connected
 			}
 		}
@@ -160,7 +160,7 @@ impl<PeerHandshakeImpl: IPeerHandshake> MessageQueuer for Transport<PeerHandshak
 
 				let mut buffer = VecWriter(Vec::new());
 				wire::write(message, &mut buffer).unwrap();
-				output_buffer.push_back(conduit.encrypt(&buffer.0));
+				output_buffer.push_back(conduit.encryptor.encrypt(&buffer.0));
 			}
 		}
 	}