From fcbd616df5b7aa7ecb5dbfa9ab4c0d7bb6ba8c75 Mon Sep 17 00:00:00 2001 From: Pancy Date: Thu, 25 Nov 2021 11:15:00 -0800 Subject: [PATCH] protocols/mdns: Use a random alphanumeric string for peer name (#2311) With https://github.com/libp2p/specs/pull/368 the definition of the _peer name_ changed in the mDNS specification. > peer-name is the case-insensitive unique identifier of the peer, and is less > than 64 characters. > > As the this field doesn't carry any meaning, it is sufficient to ensure the > uniqueness of this identifier. Peers SHOULD generate a random, lower-case > alphanumeric string of least 32 characters in length when booting up their > node. Peers SHOULD NOT use their Peer ID here because a future Peer ID could > exceed the DNS label limit of 63 characters. https://github.com/libp2p/specs/blob/master/discovery/mdns.md This commit adjusts `libp2p-mdns` accordingly. Also see https://github.com/libp2p/go-libp2p/pull/1222 for the corresponding change on the Golang side. Co-authored-by: Max Inden --- CHANGELOG.md | 1 + Cargo.toml | 2 +- protocols/mdns/CHANGELOG.md | 11 +++++ protocols/mdns/Cargo.toml | 2 +- protocols/mdns/src/dns.rs | 87 ++++++++++++++----------------------- protocols/mdns/src/query.rs | 87 +++++++++++++++++++++++++------------ 6 files changed, 106 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3112024..6ad6c1e0 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/Cargo.toml b/Cargo.toml index ad55b734..dfb4c01b 100644 --- a/Cargo.toml +++ b/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 } diff --git a/protocols/mdns/CHANGELOG.md b/protocols/mdns/CHANGELOG.md index 54004fd5..4ea91394 100644 --- a/protocols/mdns/CHANGELOG.md +++ b/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. diff --git a/protocols/mdns/Cargo.toml b/protocols/mdns/Cargo.toml index 4fb7eaf0..44781bc5 100644 --- a/protocols/mdns/Cargo.toml +++ b/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 "] license = "MIT" diff --git a/protocols/mdns/src/dns.rs b/protocols/mdns/src/dns.rs index 9bd1181d..78b8f9e7 100644 --- a/protocols/mdns/src/dns.rs +++ b/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,7 +139,7 @@ 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(); } } @@ -148,13 +147,18 @@ pub fn build_query_response( // 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, 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 { - // 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 { + // 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 diff --git a/protocols/mdns/src/query.rs b/protocols/mdns/src/query.rs index c605298a..12e2748b 100644 --- a/protocols/mdns/src/query.rs +++ b/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 { + let mut my_peer_id: Option = 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); + } + } +}