Allow more than one address per type in node_announcement messages
authorMatt Corallo <git@bluematt.me>
Thu, 5 Mar 2020 18:39:53 +0000 (13:39 -0500)
committerMatt Corallo <git@bluematt.me>
Thu, 5 Mar 2020 23:42:49 +0000 (18:42 -0500)
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
lightning/src/ln/msgs.rs
lightning/src/ln/peer_handler.rs

index b0d7b6031486039e98f974065102b1010a5d0677..b1a766ba79149b5640a080c55a625eb338424a03 100644 (file)
@@ -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)),
index 8d23ac9a8e575436f2234014edd91ad53485e8da..be5be241ec1b60343d3bd54aec0e35a49b309612 100644 (file)
@@ -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<NetAddress> = Vec::with_capacity(4);
+               let mut addresses: Vec<NetAddress> = 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);
                                        }
index 60df3d80196698234146400260411ef5f4a70b2e..6a909e5f94f93115066e95a0c9e41e8f44cd7271 100644 (file)
@@ -586,10 +586,6 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> 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 }),
                                                                                                }