Check unknown features compared to handlers

Each message handler provides which features it supports. A custom
message handler may support unknown features. Therefore, these features
should be checked against instead of the features known by LDK.
Additionally, fail the connection if the peer requires features unknown
to the handler. The peer should already fail the connection in the
latter case.

Author: jkczyz

lightning/src/ln/peer_handler.rs

@@ -1401,10 +1401,17 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 
 		// Need an Init as first message
 		if let wire::Message::Init(msg) = message {
-			if msg.features.requires_unknown_bits() {
-				log_debug!(self.logger, "Peer features required unknown version bits");
+			let our_features = self.init_features(&their_node_id);
+			if msg.features.requires_unknown_bits_from(&our_features) {
+				log_debug!(self.logger, "Peer requires features unknown to us");
 				return Err(PeerHandleError { }.into());
 			}
+
+			if our_features.requires_unknown_bits_from(&msg.features) {
+				log_debug!(self.logger, "We require features unknown to our peer");
+				return Err(PeerHandleError { }.into());
+			}
+
 			if peer_lock.their_features.is_some() {
 				return Err(PeerHandleError { }.into());
 			}
@@ -2276,16 +2283,19 @@ fn is_gossip_msg(type_id: u16) -> bool {
 mod tests {
 	use crate::sign::{NodeSigner, Recipient};
 	use crate::events;
+	use crate::io;
+	use crate::ln::features::{InitFeatures, NodeFeatures};
 	use crate::ln::peer_channel_encryptor::PeerChannelEncryptor;
-	use crate::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler, filter_addresses};
+	use crate::ln::peer_handler::{CustomMessageHandler, PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler, filter_addresses};
 	use crate::ln::{msgs, wire};
-	use crate::ln::msgs::NetAddress;
+	use crate::ln::msgs::{LightningError, NetAddress};
 	use crate::util::test_utils;
 
-	use bitcoin::secp256k1::SecretKey;
+	use bitcoin::secp256k1::{PublicKey, SecretKey};
 
 	use crate::prelude::*;
 	use crate::sync::{Arc, Mutex};
+	use core::convert::Infallible;
 	use core::sync::atomic::{AtomicBool, Ordering};
 
 	#[derive(Clone)]
@@ -2318,19 +2328,74 @@ mod tests {
 	struct PeerManagerCfg {
 		chan_handler: test_utils::TestChannelMessageHandler,
 		routing_handler: test_utils::TestRoutingMessageHandler,
+		custom_handler: TestCustomMessageHandler,
 		logger: test_utils::TestLogger,
 		node_signer: test_utils::TestNodeSigner,
 	}
 
+	struct TestCustomMessageHandler {
+		features: InitFeatures,
+	}
+
+	impl wire::CustomMessageReader for TestCustomMessageHandler {
+		type CustomMessage = Infallible;
+		fn read<R: io::Read>(&self, _: u16, _: &mut R) -> Result<Option<Self::CustomMessage>, msgs::DecodeError> {
+			Ok(None)
+		}
+	}
+
+	impl CustomMessageHandler for TestCustomMessageHandler {
+		fn handle_custom_message(&self, _: Infallible, _: &PublicKey) -> Result<(), LightningError> {
+			unreachable!();
+		}
+
+		fn get_and_clear_pending_msg(&self) -> Vec<(PublicKey, Self::CustomMessage)> { Vec::new() }
+
+		fn provided_node_features(&self) -> NodeFeatures { NodeFeatures::empty() }
+
+		fn provided_init_features(&self, _: &PublicKey) -> InitFeatures {
+			self.features.clone()
+		}
+	}
+
 	fn create_peermgr_cfgs(peer_count: usize) -> Vec<PeerManagerCfg> {
 		let mut cfgs = Vec::new();
 		for i in 0..peer_count {
 			let node_secret = SecretKey::from_slice(&[42 + i as u8; 32]).unwrap();
+			let features = {
+				let mut feature_bits = vec![0u8; 33];
+				feature_bits[32] = 0b00000001;
+				InitFeatures::from_le_bytes(feature_bits)
+			};
+			cfgs.push(
+				PeerManagerCfg{
+					chan_handler: test_utils::TestChannelMessageHandler::new(),
+					logger: test_utils::TestLogger::new(),
+					routing_handler: test_utils::TestRoutingMessageHandler::new(),
+					custom_handler: TestCustomMessageHandler { features },
+					node_signer: test_utils::TestNodeSigner::new(node_secret),
+				}
+			);
+		}
+
+		cfgs
+	}
+
+	fn create_incompatible_peermgr_cfgs(peer_count: usize) -> Vec<PeerManagerCfg> {
+		let mut cfgs = Vec::new();
+		for i in 0..peer_count {
+			let node_secret = SecretKey::from_slice(&[42 + i as u8; 32]).unwrap();
+			let features = {
+				let mut feature_bits = vec![0u8; 33 + i + 1];
+				feature_bits[33 + i] = 0b00000001;
+				InitFeatures::from_le_bytes(feature_bits)
+			};
 			cfgs.push(
 				PeerManagerCfg{
 					chan_handler: test_utils::TestChannelMessageHandler::new(),
 					logger: test_utils::TestLogger::new(),
 					routing_handler: test_utils::TestRoutingMessageHandler::new(),
+					custom_handler: TestCustomMessageHandler { features },
 					node_signer: test_utils::TestNodeSigner::new(node_secret),
 				}
 			);
@@ -2339,13 +2404,13 @@ mod tests {
 		cfgs
 	}
 
-	fn create_network<'a>(peer_count: usize, cfgs: &'a Vec<PeerManagerCfg>) -> Vec<PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler, &'a test_utils::TestRoutingMessageHandler, IgnoringMessageHandler, &'a test_utils::TestLogger, IgnoringMessageHandler, &'a test_utils::TestNodeSigner>> {
+	fn create_network<'a>(peer_count: usize, cfgs: &'a Vec<PeerManagerCfg>) -> Vec<PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler, &'a test_utils::TestRoutingMessageHandler, IgnoringMessageHandler, &'a test_utils::TestLogger, &'a TestCustomMessageHandler, &'a test_utils::TestNodeSigner>> {
 		let mut peers = Vec::new();
 		for i in 0..peer_count {
 			let ephemeral_bytes = [i as u8; 32];
 			let msg_handler = MessageHandler {
 				chan_handler: &cfgs[i].chan_handler, route_handler: &cfgs[i].routing_handler,
-				onion_message_handler: IgnoringMessageHandler {}, custom_message_handler: IgnoringMessageHandler {}
+				onion_message_handler: IgnoringMessageHandler {}, custom_message_handler: &cfgs[i].custom_handler
 			};
 			let peer = PeerManager::new(msg_handler, 0, &ephemeral_bytes, &cfgs[i].logger, &cfgs[i].node_signer);
 			peers.push(peer);
@@ -2354,7 +2419,7 @@ mod tests {
 		peers
 	}
 
-	fn establish_connection<'a>(peer_a: &PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler, &'a test_utils::TestRoutingMessageHandler, IgnoringMessageHandler, &'a test_utils::TestLogger, IgnoringMessageHandler, &'a test_utils::TestNodeSigner>, peer_b: &PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler, &'a test_utils::TestRoutingMessageHandler, IgnoringMessageHandler, &'a test_utils::TestLogger, IgnoringMessageHandler, &'a test_utils::TestNodeSigner>) -> (FileDescriptor, FileDescriptor) {
+	fn establish_connection<'a>(peer_a: &PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler, &'a test_utils::TestRoutingMessageHandler, IgnoringMessageHandler, &'a test_utils::TestLogger, &'a TestCustomMessageHandler, &'a test_utils::TestNodeSigner>, peer_b: &PeerManager<FileDescriptor, &'a test_utils::TestChannelMessageHandler, &'a test_utils::TestRoutingMessageHandler, IgnoringMessageHandler, &'a test_utils::TestLogger, &'a TestCustomMessageHandler, &'a test_utils::TestNodeSigner>) -> (FileDescriptor, FileDescriptor) {
 		let id_a = peer_a.node_signer.get_node_id(Recipient::Node).unwrap();
 		let mut fd_a = FileDescriptor {
 			fd: 1, outbound_data: Arc::new(Mutex::new(Vec::new())),
@@ -2469,6 +2534,42 @@ mod tests {
 		thrd_b.join().unwrap();
 	}
 
+	#[test]
+	fn test_incompatible_peers() {
+		let cfgs = create_peermgr_cfgs(2);
+		let incompatible_cfgs = create_incompatible_peermgr_cfgs(2);
+
+		let peers = create_network(2, &cfgs);
+		let incompatible_peers = create_network(2, &incompatible_cfgs);
+		let peer_pairs = [(&peers[0], &incompatible_peers[0]), (&incompatible_peers[1], &peers[1])];
+		for (peer_a, peer_b) in peer_pairs {
+			let id_a = peer_a.node_signer.get_node_id(Recipient::Node).unwrap();
+			let mut fd_a = FileDescriptor {
+				fd: 1, outbound_data: Arc::new(Mutex::new(Vec::new())),
+				disconnect: Arc::new(AtomicBool::new(false)),
+			};
+			let addr_a = NetAddress::IPv4{addr: [127, 0, 0, 1], port: 1000};
+			let mut fd_b = FileDescriptor {
+				fd: 1, outbound_data: Arc::new(Mutex::new(Vec::new())),
+				disconnect: Arc::new(AtomicBool::new(false)),
+			};
+			let addr_b = NetAddress::IPv4{addr: [127, 0, 0, 1], port: 1001};
+			let initial_data = peer_b.new_outbound_connection(id_a, fd_b.clone(), Some(addr_a.clone())).unwrap();
+			peer_a.new_inbound_connection(fd_a.clone(), Some(addr_b.clone())).unwrap();
+			assert_eq!(peer_a.read_event(&mut fd_a, &initial_data).unwrap(), false);
+			peer_a.process_events();
+
+			let a_data = fd_a.outbound_data.lock().unwrap().split_off(0);
+			assert_eq!(peer_b.read_event(&mut fd_b, &a_data).unwrap(), false);
+
+			peer_b.process_events();
+			let b_data = fd_b.outbound_data.lock().unwrap().split_off(0);
+
+			// Should fail because of unknown required features
+			assert!(peer_a.read_event(&mut fd_a, &b_data).is_err());
+		}
+	}
+
 	#[test]
 	fn test_disconnect_peer() {
 		// Simple test which builds a network of PeerManager, connects and brings them to NoiseState::Finished and