[mdns] - Support for long mDNS names (Bug #1232) (#1287)

* Dead code -- commenting out with a note referencing future implementation

* Adding "std" feature so that cargo can build in other directories (notably `misc/mdns`, so that I could run these tests)

* Permitting `PeerID` to be built from an `Identity` multihash

* The length limit for DNS labels is 63 characters, as per RFC1035

* Allocates the vector with capacity for the service name plus additional QNAME encoding bytes

* Added support for encoding/decoding peer IDs with an encoded length greater than 63 characters

* Removing "std" from ring features

Co-Authored-By: Pierre Krieger <[email protected]>

* Retaining MAX_INLINE_KEY_LENGTH with comment about future usage

* `segment_peer_id` consumes `peer_id` ... plus an early return for IDs that don't need to be segmented

* Fixing logic

core/src/peer_id.rs

@@ -26,6 +26,8 @@ use std::{convert::TryFrom, fmt, str::FromStr};
 
 /// Public keys with byte-lengths smaller than `MAX_INLINE_KEY_LENGTH` will be
 /// automatically used as the peer id using an identity multihash.
+//
+// Note: see `from_public_key` for how this value will be used in the future.
 const MAX_INLINE_KEY_LENGTH: usize = 42;
 
 /// Identifier of a peer of the network.
@@ -98,7 +100,7 @@ impl PeerId {
     /// returns back the data as an error.
     #[inline]
     pub fn from_multihash(data: multihash::Multihash) -> Result<PeerId, multihash::Multihash> {
-        if data.algorithm() == multihash::Hash::SHA2256 {
+        if data.algorithm() == multihash::Hash::SHA2256 || data.algorithm() == multihash::Hash::Identity {
             Ok(PeerId { multihash: data })
         } else {
             Err(data)

misc/mdns/src/dns.rs

@@ -27,6 +27,9 @@ use libp2p_core::{Multiaddr, PeerId};
 use rand;
 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;
+
 /// Decodes a `<character-string>` (as defined by RFC1035) into a `Vec` of ASCII characters.
 // TODO: better error type?
 pub fn decode_character_string(mut from: &[u8]) -> Result<Cow<'_, [u8]>, ()> {
@@ -117,23 +120,15 @@ pub fn build_query_response(
     // TTL for the answer
     append_u32(&mut out, ttl);
 
-    let peer_id_base58 = peer_id.to_base58();
-
     // Peer Id.
-    let peer_name = format!(
-        "{}.{}",
-        data_encoding::BASE32_DNSCURVE.encode(&peer_id.into_bytes()),
-        str::from_utf8(SERVICE_NAME).expect("SERVICE_NAME is always ASCII")
-    );
-    let mut peer_id_bytes = Vec::with_capacity(64);
-    append_qname(&mut peer_id_bytes, peer_name.as_bytes());
+    let peer_id_bytes = encode_peer_id(&peer_id);
     debug_assert!(peer_id_bytes.len() <= 0xffff);
     append_u16(&mut out, peer_id_bytes.len() as u16);
     out.extend_from_slice(&peer_id_bytes);
 
     // The TXT records for answers.
     for addr in addresses {
-        let txt_to_send = format!("dnsaddr={}/p2p/{}", addr.to_string(), peer_id_base58);
+        let txt_to_send = format!("dnsaddr={}/p2p/{}", addr.to_string(), peer_id.to_base58());
         let mut txt_to_send_bytes = Vec::with_capacity(txt_to_send.len());
         append_character_string(&mut txt_to_send_bytes, txt_to_send.as_bytes())?;
         append_txt_record(&mut out, &peer_id_bytes, ttl, Some(&txt_to_send_bytes[..]))?;
@@ -177,7 +172,7 @@ pub fn build_service_discovery_response(id: u16, ttl: Duration) -> Vec<u8> {
 
     // Service name.
     {
-        let mut name = Vec::new();
+        let mut name = Vec::with_capacity(SERVICE_NAME.len() + 2);
         append_qname(&mut name, SERVICE_NAME);
         append_u16(&mut out, name.len() as u16);
         out.extend_from_slice(&name);
@@ -211,6 +206,40 @@ 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
+}
+
+/// 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.as_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(".");
+
+    // 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());
+
+    peer_id_bytes
+}
+
 /// Appends a `QNAME` (as defined by RFC1035) to the `Vec`.
 ///
 /// # Panic
@@ -223,7 +252,7 @@ fn append_qname(out: &mut Vec<u8>, name: &[u8]) {
     debug_assert!(name.is_ascii());
 
     for element in name.split(|&c| c == b'.') {
-        assert!(element.len() < 256, "Service name has a label too long");
+        assert!(element.len() < 64, "Service name has a label too long");
         assert_ne!(element.len(), 0, "Service name contains zero length label");
         out.push(element.len() as u8);
         for chr in element.iter() {
@@ -367,5 +396,21 @@ mod tests {
         assert!(Packet::parse(&query).is_ok());
     }
 
+    #[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("."));
+    }
+
     // TODO: test limits and errors
 }

misc/mdns/src/service.rs

@@ -411,18 +411,14 @@ impl<'a> MdnsResponse<'a> {
                 _ => return None,
             };
 
-            let peer_name = {
-                let mut iter = record_value.splitn(2, |c| c == '.');
-                let name = match iter.next() {
-                    Some(n) => n.to_owned(),
-                    None => return None,
-                };
-                if iter.next().map(|v| v.as_bytes()) != Some(SERVICE_NAME) {
-                    return None;
-                }
-                name
+            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,
@@ -541,11 +537,10 @@ mod tests {
     use std::{io, time::Duration};
     use tokio::{self, prelude::*};
     use crate::service::{MdnsPacket, MdnsService};
+    use multiaddr::multihash::*;
 
-    #[test]
-    fn discover_ourselves() {
+    fn discover(peer_id: PeerId) {
         let mut service = MdnsService::new().unwrap();
-        let peer_id = PeerId::random();
         let stream = stream::poll_fn(move || -> Poll<Option<()>, io::Error> {
             loop {
                 let packet = match service.poll() {
@@ -575,4 +570,16 @@ mod tests {
                 .for_each(|_| Ok(())),
         );
     }
+
+    #[test]
+    fn discover_normal_peer_id() {
+        discover(PeerId::random())
+    }
+
+    #[test]
+    fn discover_long_peer_id() {
+        let max_value = String::from_utf8(vec![b'f'; 42]).unwrap();
+        let hash = encode(Hash::Identity, max_value.as_ref()).unwrap();
+        discover(PeerId::from_multihash(hash).unwrap())
+    }
 }