From: Matt Corallo <git@bluematt.me>
Date: Wed, 29 Aug 2018 21:52:26 +0000 (-0400)
Subject: Fail parsing node/channel announcements with unknown even features
X-Git-Tag: v0.0.12~334^2~1
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=faff5c2da3d75783a9141ad3cf74011d2d7f9126;hp=335012640a809821cd5a0afd35cb1e7463e1002d;p=rust-lightning

Fail parsing node/channel announcements with unknown even features

This is required for BOLT 7 compliance
---

diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs
index ed5fa13b0..37a671836 100644
--- a/fuzz/fuzz_targets/channel_target.rs
+++ b/fuzz/fuzz_targets/channel_target.rs
@@ -124,6 +124,7 @@ pub fn do_test(data: &[u8]) {
 				Ok(msg) => msg,
 				Err(e) => match e {
 					msgs::DecodeError::UnknownRealmByte => return,
+					msgs::DecodeError::UnknownRequiredFeature => return,
 					msgs::DecodeError::BadPublicKey => return,
 					msgs::DecodeError::BadSignature => return,
 					msgs::DecodeError::BadText => return,
@@ -146,6 +147,7 @@ pub fn do_test(data: &[u8]) {
 					Ok(msg) => msg,
 					Err(e) => match e {
 						msgs::DecodeError::UnknownRealmByte => return,
+						msgs::DecodeError::UnknownRequiredFeature => return,
 						msgs::DecodeError::BadPublicKey => return,
 						msgs::DecodeError::BadSignature => return,
 						msgs::DecodeError::BadText => return,
diff --git a/fuzz/fuzz_targets/router_target.rs b/fuzz/fuzz_targets/router_target.rs
index 452369e53..469db990f 100644
--- a/fuzz/fuzz_targets/router_target.rs
+++ b/fuzz/fuzz_targets/router_target.rs
@@ -75,6 +75,7 @@ pub fn do_test(data: &[u8]) {
 				Ok(msg) => msg,
 				Err(e) => match e {
 					msgs::DecodeError::UnknownRealmByte => return,
+					msgs::DecodeError::UnknownRequiredFeature => return,
 					msgs::DecodeError::BadPublicKey => return,
 					msgs::DecodeError::BadSignature => return,
 					msgs::DecodeError::BadText => return,
diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs
index 42ee9d63e..072383c11 100644
--- a/src/ln/msgs.rs
+++ b/src/ln/msgs.rs
@@ -28,6 +28,8 @@ pub trait MsgEncodable {
 pub enum DecodeError {
 	/// Unknown realm byte in an OnionHopData packet
 	UnknownRealmByte,
+	/// Unknown feature mandating we fail to parse message
+	UnknownRequiredFeature,
 	/// Failed to decode a public key (ie it's invalid)
 	BadPublicKey,
 	/// Failed to decode a signature (ie it's invalid)
@@ -510,6 +512,7 @@ impl Error for DecodeError {
 	fn description(&self) -> &str {
 		match *self {
 			DecodeError::UnknownRealmByte => "Unknown realm byte in Onion packet",
+			DecodeError::UnknownRequiredFeature => "Unknown required feature preventing decode",
 			DecodeError::BadPublicKey => "Invalid public key in packet",
 			DecodeError::BadSignature => "Invalid signature in packet",
 			DecodeError::BadText => "Invalid text in packet",
@@ -1197,6 +1200,10 @@ impl MsgEncodable for AnnouncementSignatures {
 impl MsgDecodable for UnsignedNodeAnnouncement {
 	fn decode(v: &[u8]) -> Result<Self, DecodeError> {
 		let features = GlobalFeatures::decode(&v[..])?;
+		if features.requires_unknown_bits() {
+			return Err(DecodeError::UnknownRequiredFeature);
+		}
+
 		if v.len() < features.encoded_len() + 4 + 33 + 3 + 32 + 2 {
 			return Err(DecodeError::ShortRead);
 		}
@@ -1380,6 +1387,9 @@ impl MsgEncodable for NodeAnnouncement {
 impl MsgDecodable for UnsignedChannelAnnouncement {
 	fn decode(v: &[u8]) -> Result<Self, DecodeError> {
 		let features = GlobalFeatures::decode(&v[..])?;
+		if features.requires_unknown_bits() {
+			return Err(DecodeError::UnknownRequiredFeature);
+		}
 		if v.len() < features.encoded_len() + 32 + 8 + 33*4 {
 			return Err(DecodeError::ShortRead);
 		}
diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs
index 7af4c0e29..9d5f58d7f 100644
--- a/src/ln/peer_handler.rs
+++ b/src/ln/peer_handler.rs
@@ -334,6 +334,10 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 										Err(e) => {
 											match e {
 												msgs::DecodeError::UnknownRealmByte => return Err(PeerHandleError{ no_connection_possible: false }),
+												msgs::DecodeError::UnknownRequiredFeature => {
+													log_debug!(self, "Got a channel/node announcement with an known required feature flag, you may want to udpate!");
+													continue;
+												},
 												msgs::DecodeError::BadPublicKey => return Err(PeerHandleError{ no_connection_possible: false }),
 												msgs::DecodeError::BadSignature => return Err(PeerHandleError{ no_connection_possible: false }),
 												msgs::DecodeError::BadText => return Err(PeerHandleError{ no_connection_possible: false }),
diff --git a/src/ln/router.rs b/src/ln/router.rs
index 1f713f69d..5f0cff3cd 100644
--- a/src/ln/router.rs
+++ b/src/ln/router.rs
@@ -172,6 +172,10 @@ impl RoutingMessageHandler for Router {
 		let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
 		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id);
 
+		if msg.contents.features.requires_unknown_bits() {
+			panic!("Unknown-required-features NodeAnnouncements should never deserialize!");
+		}
+
 		let mut network = self.network_map.write().unwrap();
 		match network.nodes.get_mut(&msg.contents.node_id) {
 			None => Err(HandleError{err: "No existing channels for node_announcement", action: Some(ErrorAction::IgnoreError)}),
@@ -201,7 +205,7 @@ impl RoutingMessageHandler for Router {
 		//TODO: Only allow bitcoin chain_hash
 
 		if msg.contents.features.requires_unknown_bits() {
-			return Err(HandleError{err: "Channel announcement required unknown feature flags", action: None});
+			panic!("Unknown-required-features ChannelAnnouncements should never deserialize!");
 		}
 
 		let mut network = self.network_map.write().unwrap();