Check unknown features compared to handlers

Each message handler provides which features it supports. A custom
message handler may support unknown features. Our own handler's unknown
features must therefore be removed before checking if our peer requires
unknown features.

Author: jkczyz

lightning/src/ln/peer_handler.rs

@@ -1346,7 +1346,8 @@ 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() {
+			let additional_features = msg.features.clone() - self.init_features(&their_node_id);
+			if additional_features.requires_unknown_bits() {
 				log_debug!(self.logger, "Peer features required unknown version bits");
 				return Err(PeerHandleError { }.into());
 			}
@@ -2221,16 +2222,19 @@ fn is_gossip_msg(type_id: u16) -> bool {
 mod tests {
 	use crate::chain::keysinterface::{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)]
@@ -2263,19 +2267,51 @@ 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),
 				}
 			);
@@ -2284,19 +2320,42 @@ 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_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; 32 + i + 1];
+				feature_bits[32 + 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),
+				}
+			);
+		}
+
+		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, &'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 {} };
-			let peer = PeerManager::new(msg_handler, 0, &ephemeral_bytes, &cfgs[i].logger, IgnoringMessageHandler {}, &cfgs[i].node_signer);
+			let peer = PeerManager::new(msg_handler, 0, &ephemeral_bytes, &cfgs[i].logger, &cfgs[i].custom_handler, &cfgs[i].node_signer);
 			peers.push(peer);
 		}
 
 		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())),
@@ -2411,6 +2470,39 @@ mod tests {
 		thrd_b.join().unwrap();
 	}
 
+	#[test]
+	fn test_incompatible_peers() {
+		let cfgs = create_incompatible_peermgr_cfgs(2);
+		let peers = create_network(2, &cfgs);
+		let peer_a = &peers[0];
+		let peer_b = &peers[1];
+
+		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