make ephemeral private key explicit for handshake (todo: remove it from the state machine)

Author: arik-so

lightning/src/ln/peer_handler.rs

@@ -277,8 +277,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_outbound_connection(&self, their_node_id: PublicKey, descriptor: Descriptor) -> Result<Vec<u8>, PeerHandleError> {
-		let mut handshake = PeerHandshake::new(&self.our_node_secret, None);
-		let act_one = handshake.initiate(&self.get_ephemeral_key(), &their_node_id).unwrap();
+		let mut handshake = PeerHandshake::new(&self.our_node_secret, &self.get_ephemeral_key());
+		let act_one = handshake.initiate(&their_node_id).unwrap();
 		let res = Act::One(act_one).serialize();
 
 		let mut peers = self.peers.lock().unwrap();
@@ -313,7 +313,7 @@ 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, None);
+		let handshake = PeerHandshake::new(&self.our_node_secret, &self.get_ephemeral_key());
 
 		let mut peers = self.peers.lock().unwrap();
 		if peers.peers.insert(descriptor, Peer {

lightning/src/ln/peers/conduit.rs

@@ -31,14 +31,8 @@ impl Conduit {
 	}
 
 	pub(super) fn read(&mut self, data: &[u8]) {
-		let mut read_buffer = if let Some(buffer) = self.read_buffer.take() {
-			buffer
-		} else {
-			Vec::new()
-		};
-
+		let mut read_buffer = self.read_buffer.get_or_insert(Vec::new());
 		read_buffer.extend_from_slice(data);
-		self.read_buffer = Some(read_buffer);
 	}
 
 	/// Add newly received data from the peer node to the buffer and decrypt all possible messages
@@ -71,8 +65,6 @@ impl Conduit {
 			}
 		}
 
-		self.read_buffer = Some(read_buffer);
-
 		messages
 	}
 

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

@@ -17,24 +17,18 @@ mod tests;
 pub struct PeerHandshake {
 	state: Option<HandshakeState>,
 	private_key: SecretKey,
+	ephemeral_private_key: SecretKey,
 
-	preset_ephemeral_private_key: Option<SecretKey>,
 	read_buffer: Vec<u8>,
 }
 
 impl PeerHandshake {
-	pub fn new(private_key: &SecretKey, ephemeral_private_key: Option<&SecretKey>) -> Self {
-		let preset_ephemeral_private_key = if let Some(key) = ephemeral_private_key {
-			// deref and clone
-			Some((*key).clone())
-		} else {
-			None
-		};
+	pub fn new(private_key: &SecretKey, ephemeral_private_key: &SecretKey) -> Self {
 
 		let handshake = PeerHandshake {
 			state: Some(HandshakeState::Blank),
 			private_key: (*private_key).clone(),
-			preset_ephemeral_private_key,
+			ephemeral_private_key: (*ephemeral_private_key).clone(),
 			read_buffer: Vec::new(),
 		};
 		handshake
@@ -80,9 +74,7 @@ impl PeerHandshake {
 		match &self.state {
 			Some(HandshakeState::Blank) => {
 				let remote_public_key = remote_public_key.ok_or("Call make_initiator() first")?;
-				let ephemeral_private_key = self.obtain_ephemeral_private_key();
-
-				let act_one = self.initiate(&ephemeral_private_key, &remote_public_key)?;
+				let act_one = self.initiate(&remote_public_key)?;
 				response = act_one.0.to_vec();
 			}
 			Some(HandshakeState::AwaitingActOne(_)) => {
@@ -95,9 +87,7 @@ impl PeerHandshake {
 				act_one_buffer.copy_from_slice(&self.read_buffer[..act_length]);
 				self.read_buffer.drain(..act_length);
 
-				let ephemeral_private_key = self.obtain_ephemeral_private_key();
-
-				let act_two = self.process_act_one(ActOne(act_one_buffer), &ephemeral_private_key)?;
+				let act_two = self.process_act_one(ActOne(act_one_buffer))?;
 				response = act_two.0.to_vec();
 			}
 			Some(HandshakeState::AwaitingActTwo(_)) => {
@@ -147,7 +137,7 @@ impl PeerHandshake {
 		Ok((response, connected_peer, remote_pubkey))
 	}
 
-	pub fn initiate(&mut self, ephemeral_private_key: &SecretKey, remote_public_key: &PublicKey) -> Result<ActOne, String> {
+	pub fn initiate(&mut self, remote_public_key: &PublicKey) -> Result<ActOne, String> {
 		if let Some(HandshakeState::Blank) = &self.state {} else {
 			return Err("incorrect state".to_string());
 		}
@@ -156,7 +146,7 @@ impl PeerHandshake {
 
 		// serialize act one
 		let (act_one, chaining_key, temporary_key) = self.calculate_act_message(
-			ephemeral_private_key,
+			&self.ephemeral_private_key,
 			remote_public_key,
 			chaining_key,
 			&mut hash,
@@ -166,13 +156,13 @@ impl PeerHandshake {
 			hash,
 			chaining_key,
 			temporary_key,
-			ephemeral_private_key: (*ephemeral_private_key).clone(),
+			ephemeral_private_key: (*&self.ephemeral_private_key).clone(),
 		}));
 
 		Ok(ActOne(act_one))
 	}
 
-	pub(crate) fn process_act_one(&mut self, act: ActOne, ephemeral_private_key: &SecretKey) -> Result<ActTwo, String> {
+	pub(crate) fn process_act_one(&mut self, act: ActOne) -> Result<ActTwo, String> {
 		let state = self.state.take();
 		let act_one_expectation = match state {
 			Some(HandshakeState::AwaitingActOne(act_state)) => act_state,
@@ -200,8 +190,10 @@ impl PeerHandshake {
 			&mut hash,
 		)?;
 
+		let ephemeral_private_key = (*&self.ephemeral_private_key).clone();
+
 		let (act_two, chaining_key, temporary_key) = self.calculate_act_message(
-			ephemeral_private_key,
+			&ephemeral_private_key,
 			&remote_ephemeral_public_key,
 			chaining_key,
 			&mut hash,
@@ -211,7 +203,7 @@ impl PeerHandshake {
 			hash,
 			chaining_key,
 			temporary_key,
-			ephemeral_private_key: (*ephemeral_private_key).clone(),
+			ephemeral_private_key,
 			remote_ephemeral_public_key,
 		}));
 
@@ -315,18 +307,6 @@ impl PeerHandshake {
 		Ok((remote_pubkey, connected_peer))
 	}
 
-	fn obtain_ephemeral_private_key(&mut self) -> SecretKey {
-		if let Some(key) = self.preset_ephemeral_private_key.take() {
-			key
-		} else {
-			// generate a random ephemeral private key right here
-			let mut rng = thread_rng();
-			let mut ephemeral_bytes = [0; 32];
-			rng.fill_bytes(&mut ephemeral_bytes);
-			SecretKey::from_slice(&ephemeral_bytes).expect("You broke elliptic curve cryptography")
-		}
-	}
-
 	fn calculate_act_message(&self, local_private_key: &SecretKey, remote_public_key: &PublicKey, chaining_key: [u8; 32], hash: &mut HandshakeHash) -> ([u8; 50], [u8; 32], [u8; 32]) {
 		let local_public_key = Self::private_key_to_public_key(local_private_key);
 

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

@@ -14,13 +14,13 @@ fn test_exchange() {
 	let local_ephemeral_private_key = SecretKey::from_slice(&[0x_12_u8; 32]).unwrap();
 	let remote_ephemeral_private_key = SecretKey::from_slice(&[0x_22_u8; 32]).unwrap();
 
-	let mut local_handshake = PeerHandshake::new(&local_private_key, Some(&local_ephemeral_private_key));
-	let mut remote_handshake = PeerHandshake::new(&remote_private_key, Some(&remote_ephemeral_private_key));
+	let mut local_handshake = PeerHandshake::new(&local_private_key, &local_ephemeral_private_key);
+	let mut remote_handshake = PeerHandshake::new(&remote_private_key, &remote_ephemeral_private_key);
 
 	let remote_public_key = PublicKey::from_secret_key(&curve, &remote_private_key);
 
-	let act_1_message = local_handshake.initiate(&local_ephemeral_private_key, &remote_public_key);
-	let act_2_message = remote_handshake.process_act_one(act_1_message.unwrap(), &remote_ephemeral_private_key);
+	let act_1_message = local_handshake.initiate(&remote_public_key);
+	let act_2_message = remote_handshake.process_act_one(act_1_message.unwrap());
 	let act_3_message = local_handshake.process_act_two(act_2_message.unwrap());
 	remote_handshake.process_act_three(act_3_message.unwrap().0).unwrap();
 }