From faff5c2da3d75783a9141ad3cf74011d2d7f9126 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 29 Aug 2018 17:52:26 -0400 Subject: [PATCH 1/1] Fail parsing node/channel announcements with unknown even features This is required for BOLT 7 compliance --- fuzz/fuzz_targets/channel_target.rs | 2 ++ fuzz/fuzz_targets/router_target.rs | 1 + src/ln/msgs.rs | 10 ++++++++++ src/ln/peer_handler.rs | 4 ++++ src/ln/router.rs | 6 +++++- 5 files changed, 22 insertions(+), 1 deletion(-) diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs index ed5fa13b..37a67183 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 452369e5..469db990 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 42ee9d63..072383c1 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 { 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 { 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 7af4c0e2..9d5f58d7 100644 --- a/src/ln/peer_handler.rs +++ b/src/ln/peer_handler.rs @@ -334,6 +334,10 @@ impl PeerManager { 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 1f713f69..5f0cff3c 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(); -- 2.30.2