protocols/mdns: Use a random alphanumeric string for peer name

CHANGELOG.md

@@ -46,6 +46,7 @@
 
 - Update individual crates.
     - `libp2p-swarm-derive`
+    - `libp2p-mdns` (breaking compatibility with previous versions)
 
 ## Version 0.41.0 [2021-11-16]
 

Cargo.toml

@@ -99,7 +99,7 @@ smallvec = "1.6.1"
 [target.'cfg(not(any(target_os = "emscripten", target_os = "wasi", target_os = "unknown")))'.dependencies]
 libp2p-deflate = { version = "0.30.0", path = "transports/deflate", optional = true }
 libp2p-dns = { version = "0.30.0", path = "transports/dns", optional = true, default-features = false }
-libp2p-mdns = { version = "0.33.0", path = "protocols/mdns", optional = true }
+libp2p-mdns = { version = "0.34.0", path = "protocols/mdns", optional = true }
 libp2p-tcp = { version = "0.30.0", path = "transports/tcp", default-features = false, optional = true }
 libp2p-websocket = { version = "0.32.0", path = "transports/websocket", optional = true }
 

protocols/mdns/CHANGELOG.md

@@ -1,3 +1,14 @@
+# 0.34.0 [unreleased]
+
+- Use a random alphanumeric string instead of the local peer ID for mDNS peer
+  name (see [PR 2311]).
+
+  Note that previous versions of `libp2p-mdns` expect the peer name to be a
+  valid peer ID. Thus they will be unable to discover nodes running this new
+  version of `libp2p-mdns`.
+
+[PR 2311]: https://github.com/libp2p/rust-libp2p/pull/2311/
+
 # 0.33.0 [2021-11-16]
 
 - Update dependencies.

protocols/mdns/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "libp2p-mdns"
 edition = "2018"
-version = "0.33.0"
+version = "0.34.0"
 description = "Implementation of the libp2p mDNS discovery method"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"

protocols/mdns/src/dns.rs

@@ -22,11 +22,10 @@
 
 use crate::{META_QUERY_SERVICE, SERVICE_NAME};
 use libp2p_core::{Multiaddr, PeerId};
+use rand::distributions::Alphanumeric;
+use rand::{thread_rng, Rng};
 use std::{borrow::Cow, cmp, error, fmt, str, time::Duration};
 
-/// Maximum size of a DNS label as per RFC1035.
-const MAX_LABEL_LENGTH: usize = 63;
-
 /// DNS TXT records can have up to 255 characters as a single string value.
 ///
 /// Current values are usually around 170-190 bytes long, varying primarily
@@ -116,8 +115,8 @@ pub fn build_query_response(
     // Add a limit to 2^16-1 addresses, as the protocol limits to this number.
     let addresses = addresses.take(65535);
 
-    let peer_id_bytes = encode_peer_id(&peer_id);
-    debug_assert!(peer_id_bytes.len() <= 0xffff);
+    let peer_name_bytes = generate_peer_name();
+    debug_assert!(peer_name_bytes.len() <= 0xffff);
 
     // The accumulated response packets.
     let mut packets = Vec::new();
@@ -130,7 +129,7 @@ pub fn build_query_response(
     for addr in addresses {
         let txt_to_send = format!("dnsaddr={}/p2p/{}", addr.to_string(), peer_id.to_base58());
         let mut txt_record = Vec::with_capacity(txt_to_send.len());
-        match append_txt_record(&mut txt_record, &peer_id_bytes, ttl, &txt_to_send) {
+        match append_txt_record(&mut txt_record, &peer_name_bytes, ttl, &txt_to_send) {
             Ok(()) => {
                 records.push(txt_record);
             }
@@ -140,21 +139,26 @@ pub fn build_query_response(
         }
 
         if records.len() == MAX_RECORDS_PER_PACKET {
-            packets.push(query_response_packet(id, &peer_id_bytes, &records, ttl));
+            packets.push(query_response_packet(id, &peer_name_bytes, &records, ttl));
             records.clear();
         }
     }
 
     // If there are still unpacked records, i.e. if the number of records is not
     // a multiple of `MAX_RECORDS_PER_PACKET`, create a final packet.
     if !records.is_empty() {
-        packets.push(query_response_packet(id, &peer_id_bytes, &records, ttl));
+        packets.push(query_response_packet(id, &peer_name_bytes, &records, ttl));
     }
 
     // If no packets have been built at all, because `addresses` is empty,
     // construct an empty response packet.
     if packets.is_empty() {
-        packets.push(query_response_packet(id, &peer_id_bytes, &Vec::new(), ttl));
+        packets.push(query_response_packet(
+            id,
+            &peer_name_bytes,
+            &Vec::new(),
+            ttl,
+        ));
     }
 
     packets
@@ -260,40 +264,26 @@ fn append_u16(out: &mut Vec<u8>, value: u16) {
     out.push((value & 0xff) as u8);
 }
 
-/// If a peer ID is longer than 63 characters, split it into segments to
-/// be compatible with RFC 1035.
-fn segment_peer_id(peer_id: String) -> String {
-    // Guard for the most common case
-    if peer_id.len() <= MAX_LABEL_LENGTH {
-        return peer_id;
-    }
-
-    // This will only perform one allocation except in extreme circumstances.
-    let mut out = String::with_capacity(peer_id.len() + 8);
-
-    for (idx, chr) in peer_id.chars().enumerate() {
-        if idx > 0 && idx % MAX_LABEL_LENGTH == 0 {
-            out.push('.');
-        }
-        out.push(chr);
-    }
-    out
+/// Generates and returns a random alphanumeric string of `length` size.
+fn random_string(length: usize) -> String {
+    thread_rng()
+        .sample_iter(&Alphanumeric)
+        .take(length)
+        .map(char::from)
+        .collect()
 }
 
-/// Combines and encodes a `PeerId` and service name for a DNS query.
-fn encode_peer_id(peer_id: &PeerId) -> Vec<u8> {
-    // DNS-safe encoding for the Peer ID
-    let raw_peer_id = data_encoding::BASE32_DNSCURVE.encode(&peer_id.to_bytes());
-    // ensure we don't have any labels over 63 bytes long
-    let encoded_peer_id = segment_peer_id(raw_peer_id);
-    let service_name = str::from_utf8(SERVICE_NAME).expect("SERVICE_NAME is always ASCII");
-    let peer_name = [&encoded_peer_id, service_name].join(".");
+/// Generates a random peer name as bytes for a DNS query.
+fn generate_peer_name() -> Vec<u8> {
+    // Use a variable-length random string for mDNS peer name.
+    // See https://github.com/libp2p/rust-libp2p/pull/2311/
+    let peer_name = random_string(32 + thread_rng().gen_range(0..32));
 
     // allocate with a little extra padding for QNAME encoding
-    let mut peer_id_bytes = Vec::with_capacity(peer_name.len() + 32);
-    append_qname(&mut peer_id_bytes, peer_name.as_bytes());
+    let mut peer_name_bytes = Vec::with_capacity(peer_name.len() + 32);
+    append_qname(&mut peer_name_bytes, peer_name.as_bytes());
 
-    peer_id_bytes
+    peer_name_bytes
 }
 
 /// Appends a `QNAME` (as defined by RFC1035) to the `Vec`.
@@ -438,22 +428,11 @@ mod tests {
     }
 
     #[test]
-    fn test_segment_peer_id() {
-        let str_32 = String::from_utf8(vec![b'x'; 32]).unwrap();
-        let str_63 = String::from_utf8(vec![b'x'; 63]).unwrap();
-        let str_64 = String::from_utf8(vec![b'x'; 64]).unwrap();
-        let str_126 = String::from_utf8(vec![b'x'; 126]).unwrap();
-        let str_127 = String::from_utf8(vec![b'x'; 127]).unwrap();
-
-        assert_eq!(segment_peer_id(str_32.clone()), str_32);
-        assert_eq!(segment_peer_id(str_63.clone()), str_63);
-
-        assert_eq!(segment_peer_id(str_64), [&str_63, "x"].join("."));
-        assert_eq!(
-            segment_peer_id(str_126),
-            [&str_63, str_63.as_str()].join(".")
-        );
-        assert_eq!(segment_peer_id(str_127), [&str_63, &str_63, "x"].join("."));
+    fn test_random_string() {
+        let varsize = thread_rng().gen_range(0..32);
+        let size = 32 + varsize;
+        let name = random_string(size);
+        assert_eq!(name.len(), size);
     }
 
     // TODO: test limits and errors

protocols/mdns/src/query.rs

@@ -159,23 +159,7 @@ impl MdnsResponse {
                     _ => return None,
                 };
 
-                let mut peer_name = match record_value.rsplitn(4, |c| c == '.').last() {
-                    Some(n) => n.to_owned(),
-                    None => return None,
-                };
-
-                // if we have a segmented name, remove the '.'
-                peer_name.retain(|c| c != '.');
-
-                let peer_id = match data_encoding::BASE32_DNSCURVE.decode(peer_name.as_bytes()) {
-                    Ok(bytes) => match PeerId::from_bytes(&bytes) {
-                        Ok(id) => id,
-                        Err(_) => return None,
-                    },
-                    Err(_) => return None,
-                };
-
-                Some(MdnsPeer::new(&packet, record_value, peer_id, record.ttl))
+                MdnsPeer::new(&packet, record_value, record.ttl)
             })
             .collect();
 
@@ -215,12 +199,8 @@ pub struct MdnsPeer {
 
 impl MdnsPeer {
     /// Creates a new `MdnsPeer` based on the provided `Packet`.
-    pub fn new(
-        packet: &Packet<'_>,
-        record_value: String,
-        my_peer_id: PeerId,
-        ttl: u32,
-    ) -> MdnsPeer {
+    pub fn new(packet: &Packet<'_>, record_value: String, ttl: u32) -> Option<MdnsPeer> {
+        let mut my_peer_id: Option<PeerId> = None;
         let addrs = packet
             .additional
             .iter()
@@ -256,8 +236,12 @@ impl MdnsPeer {
                 match addr.pop() {
                     Some(Protocol::P2p(peer_id)) => {
                         if let Ok(peer_id) = PeerId::try_from(peer_id) {
-                            if peer_id != my_peer_id {
-                                return None;
+                            if let Some(pid) = &my_peer_id {
+                                if peer_id != *pid {
+                                    return None;
+                                }
+                            } else {
+                                my_peer_id.replace(peer_id);
                             }
                         } else {
                             return None;
@@ -269,11 +253,11 @@ impl MdnsPeer {
             })
             .collect();
 
-        MdnsPeer {
+        my_peer_id.map(|peer_id| MdnsPeer {
             addrs,
-            peer_id: my_peer_id,
+            peer_id,
             ttl,
-        }
+        })
     }
 
     /// Returns the id of the peer.
@@ -303,3 +287,50 @@ impl fmt::Debug for MdnsPeer {
             .finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    use super::*;
+    use crate::dns::build_query_response;
+
+    #[test]
+    fn test_create_mdns_peer() {
+        let ttl = 300;
+        let peer_id = PeerId::random();
+
+        let mut addr1: Multiaddr = "/ip4/1.2.3.4/tcp/5000".parse().expect("bad multiaddress");
+        let mut addr2: Multiaddr = "/ip6/::1/udp/10000".parse().expect("bad multiaddress");
+        addr1.push(Protocol::P2p(peer_id.clone().into()));
+        addr2.push(Protocol::P2p(peer_id.clone().into()));
+
+        let packets = build_query_response(
+            0xf8f8,
+            peer_id,
+            vec![addr1, addr2].into_iter(),
+            Duration::from_secs(60),
+        );
+
+        for bytes in packets {
+            let packet = Packet::parse(&bytes).expect("unable to parse packet");
+            let record_value = packet
+                .answers
+                .iter()
+                .filter_map(|record| {
+                    if record.name.to_string().as_bytes() != SERVICE_NAME {
+                        return None;
+                    }
+                    let record_value = match record.data {
+                        RData::PTR(record) => record.0.to_string(),
+                        _ => return None,
+                    };
+                    return Some(record_value);
+                })
+                .next()
+                .expect("empty record value");
+
+            let peer = MdnsPeer::new(&packet, record_value, ttl).expect("fail to create peer");
+            assert_eq!(peer.peer_id, peer_id);
+        }
+    }
+}