From eb7b7bd919b93e6acf00847c19d1a76c09016120 Mon Sep 17 00:00:00 2001 From: Peat Bakke Date: Thu, 31 Oct 2019 02:54:41 -0700 Subject: [PATCH] [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 * 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 | 4 ++- misc/mdns/src/dns.rs | 69 +++++++++++++++++++++++++++++++++------- misc/mdns/src/service.rs | 33 +++++++++++-------- 3 files changed, 80 insertions(+), 26 deletions(-) diff --git a/core/src/peer_id.rs b/core/src/peer_id.rs index d9dc34e1be7..9ebb68299a7 100644 --- a/core/src/peer_id.rs +++ b/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 { - if data.algorithm() == multihash::Hash::SHA2256 { + if data.algorithm() == multihash::Hash::SHA2256 || data.algorithm() == multihash::Hash::Identity { Ok(PeerId { multihash: data }) } else { Err(data) diff --git a/misc/mdns/src/dns.rs b/misc/mdns/src/dns.rs index b65d996e3c7..4ac965843cc 100644 --- a/misc/mdns/src/dns.rs +++ b/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 `` (as defined by RFC1035) into a `Vec` of ASCII characters. // TODO: better error type? pub fn decode_character_string(mut from: &[u8]) -> Result, ()> { @@ -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 { // 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, 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 { + // 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, 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 } diff --git a/misc/mdns/src/service.rs b/misc/mdns/src/service.rs index 1721a656fe6..c2557a4da37 100644 --- a/misc/mdns/src/service.rs +++ b/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, 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()) + } }