From 107da97cd03803fdd1f61501b05e96260ed19016 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 5 Mar 2020 13:39:53 -0500 Subject: [PATCH] Allow more than one address per type in node_announcement messages lnd has been blatantly ignoring this line in the spec forever, so its somewhat of a lost cause trying to enforce it. --- fuzz/src/router.rs | 1 - lightning/src/ln/msgs.rs | 34 +++++++------------------------- lightning/src/ln/peer_handler.rs | 4 ---- 3 files changed, 7 insertions(+), 32 deletions(-) diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index b0d7b603..b1a766ba 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -124,7 +124,6 @@ pub fn do_test(data: &[u8]) { msgs::DecodeError::UnknownVersion => return, msgs::DecodeError::UnknownRequiredFeature => return, msgs::DecodeError::InvalidValue => return, - msgs::DecodeError::ExtraAddressesPerType => return, msgs::DecodeError::BadLengthDescriptor => return, msgs::DecodeError::ShortRead => panic!("We picked the length..."), msgs::DecodeError::Io(e) => panic!(format!("{}", e)), diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 8d23ac9a..be5be241 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -47,8 +47,6 @@ pub enum DecodeError { InvalidValue, /// Buffer too short ShortRead, - /// node_announcement included more than one address of a given type! - ExtraAddressesPerType, /// A length descriptor in the packet didn't describe the later data correctly BadLengthDescriptor, /// Error from std::io @@ -677,7 +675,6 @@ impl Error for DecodeError { DecodeError::UnknownRequiredFeature => "Unknown required feature preventing decode", DecodeError::InvalidValue => "Nonsense bytes didn't map to the type they were interpreted as", DecodeError::ShortRead => "Packet extended beyond the provided bytes", - DecodeError::ExtraAddressesPerType => "More than one address of a single type", DecodeError::BadLengthDescriptor => "A length descriptor in the packet didn't describe the later data correctly", DecodeError::Io(ref e) => e.description(), } @@ -1209,8 +1206,7 @@ impl Writeable for UnsignedNodeAnnouncement { self.alias.write(w)?; let mut addrs_to_encode = self.addresses.clone(); - addrs_to_encode.sort_unstable_by(|a, b| { a.get_id().cmp(&b.get_id()) }); - addrs_to_encode.dedup_by(|a, b| { a.get_id() == b.get_id() }); + addrs_to_encode.sort_by(|a, b| { a.get_id().cmp(&b.get_id()) }); let mut addr_len = 0; for addr in &addrs_to_encode { addr_len += 1 + addr.len(); @@ -1235,7 +1231,8 @@ impl Readable for UnsignedNodeAnnouncement { let alias: [u8; 32] = Readable::read(r)?; let addr_len: u16 = Readable::read(r)?; - let mut addresses: Vec = Vec::with_capacity(4); + let mut addresses: Vec = Vec::new(); + let mut highest_addr_type = 0; let mut addr_readpos = 0; let mut excess = false; let mut excess_byte = 0; @@ -1243,28 +1240,11 @@ impl Readable for UnsignedNodeAnnouncement { if addr_len <= addr_readpos { break; } match Readable::read(r) { Ok(Ok(addr)) => { - match addr { - NetAddress::IPv4 { .. } => { - if addresses.len() > 0 { - return Err(DecodeError::ExtraAddressesPerType); - } - }, - NetAddress::IPv6 { .. } => { - if addresses.len() > 1 || (addresses.len() == 1 && addresses[0].get_id() != 1) { - return Err(DecodeError::ExtraAddressesPerType); - } - }, - NetAddress::OnionV2 { .. } => { - if addresses.len() > 2 || (addresses.len() > 0 && addresses.last().unwrap().get_id() > 2) { - return Err(DecodeError::ExtraAddressesPerType); - } - }, - NetAddress::OnionV3 { .. } => { - if addresses.len() > 3 || (addresses.len() > 0 && addresses.last().unwrap().get_id() > 3) { - return Err(DecodeError::ExtraAddressesPerType); - } - }, + if addr.get_id() < highest_addr_type { + // Addresses must be sorted in increasing order + return Err(DecodeError::InvalidValue); } + highest_addr_type = addr.get_id(); if addr_len < addr_readpos + 1 + addr.len() { return Err(DecodeError::BadLengthDescriptor); } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 60df3d80..6a909e5f 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -586,10 +586,6 @@ impl PeerManager where log_debug!(self, "Deserialization failed due to shortness of message"); return Err(PeerHandleError { no_connection_possible: false }); } - msgs::DecodeError::ExtraAddressesPerType => { - log_debug!(self, "Error decoding message, ignoring due to lnd spec incompatibility. See https://github.com/lightningnetwork/lnd/issues/1407"); - continue; - } msgs::DecodeError::BadLengthDescriptor => return Err(PeerHandleError { no_connection_possible: false }), msgs::DecodeError::Io(_) => return Err(PeerHandleError { no_connection_possible: false }), } -- 2.30.2