From 117ec27bbbbbe9ce7e246aa16e0f6f35ca45790b Mon Sep 17 00:00:00 2001 From: Han Xu Date: Sun, 26 Jul 2026 12:34:35 -0700 Subject: [PATCH 1/4] feat: limit generated packet size per RFC 6762 section 17 Generated packets were capped only by MAX_MSG_ABSOLUTE (8972 bytes), well past a typical 1500-byte MTU. Such a packet is fragmented by the IP layer, which section 17 forbids for a multi-record message: "A Multicast DNS packet larger than the interface MTU, which is sent using fragments, MUST NOT contain more than one resource record." Fragments are also widely dropped, as "many simple devices do not reassemble fragmented IP datagrams". Outgoing messages are now limited to MAX_MSG_MTU_SAFE (1452 bytes = 1500 - 40 IPv6 header - 8 UDP header), with ServiceDaemon::set_max_packet_size() to raise it on links known to carry bigger frames. Lowering the limit made overflow routine, so the encoder had to stop discarding what did not fit. to_packets() previously dropped any question, answer, or authority that overflowed, and only spilled additionals, for queries alone. It is now a fill-and-flush loop over all four sections: an item that does not fit is retried in a fresh packet, so nothing is lost. A single record too big for an otherwise empty packet is sent alone in an oversized packet, as section 17 both permits and requires. The TC bit is now set on every packet but the last of a multi-packet query, per section 7.2, and never on a response, per section 18.5: "In multicast response messages, the TC bit MUST be zero on transmission." The previous TC path was unreachable, since known answers go to the answer section while only the additionals loop split. Also on the receive side: - MAX_MSG_ABSOLUTE is now 8952, computed with the IPv6 header (9000 - 40 - 8) rather than the IPv4 one. Section 17's 9000-byte cap includes the IP and UDP headers, so 8972 let an IPv6 packet reach 9008 bytes. This also makes the outgoing send guards compliant over IPv6. - An incoming datagram over the limit is dropped instead of parsed. The socket layer truncates it, and a truncated message does not fail cleanly: names run into whatever bytes follow, yielding bogus records or confusing parse errors. The receive buffer is one byte larger than the limit so that an over-sized datagram can be told apart from a legal one exactly at it. One deliberate behavior change: a response now spills its additional records into another packet rather than dropping them, per RFC 6763 section 12. Closes #468 --- src/dns_cache.rs | 2 + src/dns_parser.rs | 552 +++++++++++++++++++++++++++++++++++------- src/lib.rs | 2 +- src/service_daemon.rs | 376 +++++++++++++++++++++++++--- src/service_info.rs | 17 ++ 5 files changed, 824 insertions(+), 125 deletions(-) diff --git a/src/dns_cache.rs b/src/dns_cache.rs index 9124750..d673040 100644 --- a/src/dns_cache.rs +++ b/src/dns_cache.rs @@ -871,6 +871,8 @@ mod tests { name: name.to_string(), index, addrs: HashSet::new(), + max_packet_size_v4: crate::MAX_PKT_DEFAULT, + max_packet_size_v6: crate::MAX_PKT_DEFAULT, } } diff --git a/src/dns_parser.rs b/src/dns_parser.rs index b2bf53e..94420fa 100644 --- a/src/dns_parser.rs +++ b/src/dns_parser.rs @@ -282,11 +282,47 @@ pub const CLASS_MASK: u16 = 0x7FFF; /// Cache-flush bit: the most significant bit of the rrclass field of the resource record. pub const CLASS_CACHE_FLUSH: u16 = 0x8000; -/// Max size of UDP datagram payload. +/// Absolute max size of UDP datagram payload for an mDNS packet over IPv4. /// -/// It is calculated as: 9000 bytes - IP header 20 bytes - UDP header 8 bytes. -/// Reference: [RFC6762 section 17](https://datatracker.ietf.org/doc/html/rfc6762#section-17) -pub const MAX_MSG_ABSOLUTE: usize = 8972; +/// RFC 6762 section 17: +/// "Even when fragmentation is used, a Multicast DNS packet, including IP and UDP +/// headers, MUST NOT exceed 9000 bytes." +/// +/// It is calculated as: 9000 bytes - IPv4 header 20 bytes - UDP header 8 bytes. +pub const MAX_PKT_ABSOLUTE_IPV4: usize = 8972; + +/// Absolute max size of UDP datagram payload for an mDNS packet over IPv6. +/// +/// Same 9000-byte ceiling as [`MAX_PKT_ABSOLUTE_IPV4`], less the bigger IPv6 header: +/// 9000 bytes - IPv6 header 40 bytes - UDP header 8 bytes. +/// +/// Being the smaller of the two, it is legal over either IP version, and is therefore +/// the ceiling used when generating packets, at the cost of 20 unused bytes for IPv4. +/// The IPv4 value is for packets we receive, which others may legally send that big. +pub const MAX_PKT_ABSOLUTE_IPV6: usize = 8952; + +/// Absolute max size of an mDNS packet for the given IP version. +pub const fn max_pkt_absolute(is_ipv4: bool) -> usize { + if is_ipv4 { + MAX_PKT_ABSOLUTE_IPV4 + } else { + MAX_PKT_ABSOLUTE_IPV6 + } +} + +/// Default max size of a generated (i.e. outgoing) packet, i.e. the default of +/// [`ServiceDaemon::set_max_packet_size`](crate::ServiceDaemon::set_max_packet_size). +/// +/// The limit is per packet, not per message: a message too big for one packet is +/// split across several rather than truncated. +/// +/// Calculated as: 1500 bytes Ethernet MTU - IPv6 header 40 bytes - UDP header 8 bytes. +/// It is safe on both IPv4 and IPv6, at the cost of 20 unused bytes for IPv4. +/// +/// The idea is to keep generated packets unfragmented. See RFC 6762 section 17 for details. +/// This is a conservative constant rather than the real MTU of the outgoing interface: use +/// the API above to raise it on links known to support bigger packets. +pub const MAX_PKT_DEFAULT: usize = 1452; const MSG_HEADER_LEN: usize = 12; @@ -304,7 +340,8 @@ pub enum WriteError { /// A label in a name is longer than [`MAX_LABEL_BYTES`]. NameTooLong, - /// The packet would exceed [`MAX_MSG_ABSOLUTE`] with this record. + /// The packet would exceed its max size with this record. The caller can + /// retry the record in a new packet. PacketFull, } @@ -1457,14 +1494,19 @@ pub struct DnsOutPacket { /// k: name, v: offset names: HashMap, + + /// Max byte size of `data`. A question or record that would push `data` + /// past it is rejected with [`WriteError::PacketFull`]. + max_size: usize, } impl DnsOutPacket { - fn new() -> Self { + fn new(max_size: usize) -> Self { Self { data: vec![0; MSG_HEADER_LEN], state: PacketState::Init, names: HashMap::new(), + max_size, } } @@ -1477,9 +1519,20 @@ impl DnsOutPacket { } fn write_question(&mut self, question: &DnsQuestion) -> WriteResult { - self.write_name(&question.entry.name)?; + let start_size = self.size(); + + self.write_name(&question.entry.name).map_err(|e| { + self.rollback(start_size); + e + })?; self.write_short(question.entry.ty as u16); self.write_short(question.entry.class); + + if self.size() > self.max_size { + self.rollback(start_size); + return Err(WriteError::PacketFull); + } + Ok(()) } @@ -1524,9 +1577,8 @@ impl DnsOutPacket { self.insert_short(record_offset - 2, (self.size() - record_offset) as u16); - if self.size() > MAX_MSG_ABSOLUTE { + if self.size() > self.max_size { self.rollback(start_size); - self.state = PacketState::Finished; return Err(WriteError::PacketFull); } @@ -1684,6 +1736,13 @@ impl DnsOutPacket { self.data.extend(&v.to_be_bytes()); } + /// Marks this finished packet as truncated, i.e. the message continues in + /// the next packet. + fn set_truncated(&mut self) { + let flags = u16::from_be_bytes([self.data[2], self.data[3]]); + self.insert_short(2, flags | FLAGS_TC); + } + /// Writes the header fields and finish the packet. /// This function should be only called when finishing a packet. /// @@ -1726,6 +1785,178 @@ impl DnsOutPacket { } } +/// Which section of a DNS message an item belongs to. +#[derive(Clone, Copy)] +enum Section { + Question, + Answer, + Authority, + Additional, +} + +/// Encodes a [`DnsOutgoing`] into one or more [`DnsOutPacket`], starting a new +/// packet whenever the current one runs out of room. +struct PacketBuilder<'a> { + out: &'a DnsOutgoing, + + /// Max size of a packet that holds more than one record. + max_size: usize, + + /// The message id, always 0 for multicast. + id: u16, + + finished: Vec, + current: DnsOutPacket, + + /// Section counts for `current`. + question_count: u16, + answer_count: u16, + auth_count: u16, + addi_count: u16, +} + +impl<'a> PacketBuilder<'a> { + fn new(out: &'a DnsOutgoing, max_size: usize) -> Self { + Self { + out, + max_size, + id: if out.multicast { 0 } else { out.id }, + finished: Vec::new(), + current: DnsOutPacket::new(max_size), + question_count: 0, + answer_count: 0, + auth_count: 0, + addi_count: 0, + } + } + + /// True if nothing has been written into the current packet yet. + fn current_is_empty(&self) -> bool { + self.question_count == 0 + && self.answer_count == 0 + && self.auth_count == 0 + && self.addi_count == 0 + } + + fn bump(&mut self, section: Section) { + match section { + Section::Question => self.question_count += 1, + Section::Answer => self.answer_count += 1, + Section::Authority => self.auth_count += 1, + Section::Additional => self.addi_count += 1, + } + } + + /// Writes one question or record into the current packet, starting a new + /// packet if it does not fit in the current one. + /// + /// An item that cannot be encoded at all is skipped, leaving the packet as + /// it was. Sections are written in message order, so an item that spills + /// never lands ahead of one already written. + fn add(&mut self, section: Section, write: F) + where + F: Fn(&mut DnsOutPacket) -> WriteResult, + { + match write(&mut self.current) { + Ok(()) => { + self.bump(section); + return; + } + // The item can never be encoded: skip it. + Err(WriteError::NameTooLong) => return, + Err(WriteError::PacketFull) => {} + } + + // Finish the current packet and retry in a new one. If the current packet + // is already empty, a new one would be no roomier, so don't bother. + if !self.current_is_empty() { + self.flush(); + + match write(&mut self.current) { + Ok(()) => { + self.bump(section); + return; + } + Err(WriteError::NameTooLong) => return, + Err(WriteError::PacketFull) => {} + } + } + + // A question too big for an empty packet is malformed rather than merely + // oversized: there is no legitimate question of this size. + if matches!(section, Section::Question) { + return; + } + + // The record does not fit in a packet of its own either. RFC 6762 section 17: + // a record too large for one MTU-sized packet SHOULD be sent alone, in a + // single IP datagram, using multiple IP fragments. Sending it alone is not + // optional -- such a packet "MUST NOT contain more than one resource record" + // -- so this packet is flushed immediately. + // + // No ceiling is applied here: whether such a packet may go out on the wire + // is for the send path to decide, which drops one bigger than section 17 + // allows for its IP version. + self.current.max_size = usize::MAX; + + if write(&mut self.current).is_ok() { + self.bump(section); + self.flush(); + } else { + // Too big even for the hard ceiling: skip the record and carry on. + self.current.max_size = self.max_size; + } + } + + /// Finishes the current packet and starts a new empty one. + fn flush(&mut self) { + self.current.write_header( + self.id, + self.out.flags, + self.question_count, + self.answer_count, + self.auth_count, + self.addi_count, + ); + + let next = DnsOutPacket::new(self.max_size); + self.finished + .push(std::mem::replace(&mut self.current, next)); + + self.question_count = 0; + self.answer_count = 0; + self.auth_count = 0; + self.addi_count = 0; + } + + fn finish(mut self) -> Vec { + // Always produce at least one packet, even an empty one, but never leave a + // trailing empty packet behind a full one. + if !self.current_is_empty() || self.finished.is_empty() { + self.flush(); + } + + let mut packets = self.finished; + + /* + RFC 6762 section 7.2: https://datatracker.ietf.org/doc/html/rfc6762#section-7.2 + ... + When a Multicast DNS querier sends a query to which it already knows some + answers, it ... sets the TC (Truncated) bit in the header ... [so that the + responder knows] to wait for the remaining known answers before responding. + */ + if self.out.is_query() { + if let Some((_last, rest)) = packets.split_last_mut() { + for packet in rest { + packet.set_truncated(); + } + } + } + + packets + } +} + /// Representation of one outgoing DNS message that could be sent in one or more packet(s). #[derive(Debug)] pub struct DnsOutgoing { @@ -1786,10 +2017,6 @@ impl DnsOutgoing { (self.flags & FLAGS_QR_MASK) == FLAGS_QR_QUERY } - const fn is_response(&self) -> bool { - (self.flags & FLAGS_QR_MASK) == FLAGS_QR_RESPONSE - } - // Adds an additional answer // From: RFC 6763, DNS-Based Service Discovery, February 2013 @@ -1997,86 +2224,58 @@ impl DnsOutgoing { } } - /// Returns a list of actual DNS packet data to be sent on the wire. - pub fn to_data_on_wire(&self) -> Vec> { - let packet_list = self.to_packets(); + /// Returns a list of actual DNS packet data to be sent on the wire, each no + /// bigger than `max_size`. + /// + /// Most callers want [`MAX_PKT_DEFAULT`] for `max_size`. + pub fn to_data_on_wire(&self, max_size: usize) -> Vec> { + let packet_list = self.to_packets(max_size); packet_list.into_iter().map(|p| p.data).collect() } - /// Encode self into one or more packets. - pub fn to_packets(&self) -> Vec { - let mut packet_list = Vec::new(); - let mut packet = DnsOutPacket::new(); - - let mut question_count = 0; - let mut answer_count = 0; - let mut auth_count = 0; - let mut addi_count = 0; - let id = if self.multicast { 0 } else { self.id }; + /// Encode self into one or more packets, each no bigger than `max_size`. + /// + /// Questions and records are written in message order and spill into a new + /// packet whenever the current one is full, so none is dropped for lack of + /// room. The one exception is a single record too big to fit in an otherwise + /// empty packet: it is sent alone in an oversized packet, per RFC 6762 + /// section 17. + /// + /// `max_size` must be no bigger than [`MAX_PKT_ABSOLUTE_IPV6`], the RFC 6762 + /// section 17 ceiling that is legal over either IP version; + /// [`ServiceDaemon::set_max_packet_size`](crate::ServiceDaemon::set_max_packet_size) + /// caps what it accepts. Most callers want [`MAX_PKT_DEFAULT`]. + pub fn to_packets(&self, max_size: usize) -> Vec { + debug_assert!( + max_size <= MAX_PKT_ABSOLUTE_IPV6, + "max_size {} exceeds the RFC 6762 section 17 ceiling", + max_size + ); + let mut builder = PacketBuilder::new(self, max_size); for question in self.questions.iter() { - question_count += u16::from(packet.write_question(question).is_ok()); + builder.add(Section::Question, |packet| packet.write_question(question)); } for (answer, time) in self.answers.iter() { - answer_count += u16::from(packet.write_record(answer.as_ref(), *time).is_ok()); + builder.add(Section::Answer, |packet| { + packet.write_record(answer.as_ref(), *time) + }); } for auth in self.authorities.iter() { - auth_count += u16::from(packet.write_record(auth.as_ref(), 0).is_ok()); + builder.add(Section::Authority, |packet| { + packet.write_record(auth.as_ref(), 0) + }); } for addi in self.additionals.iter() { - match packet.write_record(addi.as_ref(), 0) { - Ok(()) => { - addi_count += 1; - continue; - } - // The record itself is unusable: skip it and keep the packet. - Err(WriteError::NameTooLong) => continue, - Err(WriteError::PacketFull) => {} - } - - // No more processing for response packets. - if self.is_response() { - break; - } - - // For query, the current packet exceeds its max size due to known answers, - // need to truncate. - - // finish the current packet first. - packet.write_header( - id, - self.flags | FLAGS_TC, - question_count, - answer_count, - auth_count, - addi_count, - ); - - packet_list.push(packet); - - // create a new packet and reset counts. - packet = DnsOutPacket::new(); - addi_count = u16::from(packet.write_record(addi.as_ref(), 0).is_ok()); - - question_count = 0; - answer_count = 0; - auth_count = 0; + builder.add(Section::Additional, |packet| { + packet.write_record(addi.as_ref(), 0) + }); } - packet.write_header( - id, - self.flags, - question_count, - answer_count, - auth_count, - addi_count, - ); - - packet_list.push(packet); - packet_list + builder.finish() } } @@ -2674,8 +2873,9 @@ const fn get_expiration_time(created: u64, ttl: u32, percent: u32) -> u64 { #[cfg(test)] mod tests { use super::{ - DnsAddress, DnsHostInfo, DnsIncoming, DnsOutgoing, DnsPointer, DnsTxt, RRType, - CLASS_CACHE_FLUSH, CLASS_IN, MSG_HEADER_LEN, + DnsAddress, DnsHostInfo, DnsIncoming, DnsOutPacket, DnsOutgoing, DnsPointer, DnsTxt, + RRType, CLASS_CACHE_FLUSH, CLASS_IN, FLAGS_QR_QUERY, FLAGS_QR_RESPONSE, FLAGS_TC, + MAX_PKT_ABSOLUTE_IPV6, MAX_PKT_DEFAULT, MSG_HEADER_LEN, }; use crate::InterfaceId; use std::collections::HashMap; @@ -2684,7 +2884,7 @@ mod tests { #[test] fn test_dns_outgoing_serialization_empty() { let out = DnsOutgoing::new(0); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!(packets[0].as_bytes(), &[0; 12]); let expected_names = HashMap::new(); @@ -2695,7 +2895,7 @@ mod tests { fn test_dns_outgoing_serialization_question() { let mut out = DnsOutgoing::new(0); out.add_question("123.test", RRType::A); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2729,7 +2929,7 @@ mod tests { "arm".to_string(), "linux".to_string(), ))); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2759,7 +2959,7 @@ mod tests { IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), InterfaceId::default(), )); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2789,7 +2989,7 @@ mod tests { ), 0, ); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2822,7 +3022,7 @@ mod tests { ), 0, ); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2851,7 +3051,7 @@ mod tests { out.add_question(&format!("{long_label}.local"), RRType::PTR); out.add_question("123.test", RRType::A); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2896,7 +3096,7 @@ mod tests { 0, ); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); // Header answer count is 1: the first answer was dropped. @@ -2947,8 +3147,184 @@ mod tests { // Re-emitting it must drop the question rather than panic. let mut out = DnsOutgoing::new(0); out.add_question(&name, RRType::PTR); - let packets = out.to_packets(); + let packets = out.to_packets(MAX_PKT_DEFAULT); assert_eq!(packets.len(), 1); assert_eq!(packets[0].as_bytes(), &[0; MSG_HEADER_LEN]); } + + fn test_interface_id() -> InterfaceId { + InterfaceId { + name: "test".to_string(), + index: 1, + } + } + + /// The "flags" field of a finished packet. + fn packet_flags(packet: &DnsOutPacket) -> u16 { + let bytes = packet.as_bytes(); + u16::from_be_bytes([bytes[2], bytes[3]]) + } + + fn ptr_answer(index: usize) -> DnsPointer { + DnsPointer::new( + "_spill._tcp.local.", + RRType::PTR, + CLASS_IN, + 4500, + format!("instance-{index:04}._spill._tcp.local."), + ) + } + + /// Re-parses each packet and returns the total number of answers found, which + /// checks the header counts against what each packet actually holds. + fn parsed_answer_count(packets: &[DnsOutPacket]) -> usize { + packets + .iter() + .map(|packet: &DnsOutPacket| { + let parsed = DnsIncoming::new(packet.as_bytes().to_vec(), test_interface_id()) + .expect("each packet must parse on its own"); + assert!( + !parsed.answers().is_empty(), + "a spilled packet must not be empty" + ); + parsed.answers().len() + }) + .sum() + } + + /// A response too big for one packet spills into more packets. Every record + /// must survive: before, records that did not fit were silently dropped. + #[test] + fn test_dns_outgoing_response_spills_into_packets() { + const ANSWER_COUNT: usize = 100; + + let mut out = DnsOutgoing::new(FLAGS_QR_RESPONSE); + for i in 0..ANSWER_COUNT { + out.add_answer_at_time(ptr_answer(i), 0); + } + + let packets = out.to_packets(MAX_PKT_DEFAULT); + assert!( + packets.len() > 1, + "{} answers should not fit in one packet", + ANSWER_COUNT + ); + + for packet in &packets { + assert!( + packet.size() <= MAX_PKT_DEFAULT, + "packet of {} bytes exceeds the limit", + packet.size() + ); + + // A multi-packet response is a series of independent responses: unlike + // a query's known answers, it does not use the TC bit. + assert_eq!(packet_flags(packet) & FLAGS_TC, 0); + } + + assert_eq!(parsed_answer_count(&packets), ANSWER_COUNT); + } + + /// RFC 6762 section 7.2: a querier sending known answers in more than one + /// packet sets the TC bit in every packet but the last. + #[test] + fn test_dns_outgoing_query_truncation_bit() { + let mut out = DnsOutgoing::new(FLAGS_QR_QUERY); + out.add_question("_spill._tcp.local.", RRType::PTR); + for i in 0..100 { + out.add_answer_box(Box::new(ptr_answer(i))); + } + + let packets = out.to_packets(MAX_PKT_DEFAULT); + assert!( + packets.len() > 1, + "known answers should not fit in one packet" + ); + + let (last, rest) = packets.split_last().expect("at least one packet"); + for packet in rest { + assert_ne!( + packet_flags(packet) & FLAGS_TC, + 0, + "a packet with more known answers to follow must set TC" + ); + } + assert_eq!( + packet_flags(last) & FLAGS_TC, + 0, + "the last packet must not set TC" + ); + + // The question goes in the first packet only, and no answer is lost. + assert_eq!(packets[0].as_bytes()[4..6], 1u16.to_be_bytes()); + for packet in rest.iter().skip(1) { + assert_eq!(packet.as_bytes()[4..6], [0, 0]); + } + assert_eq!(parsed_answer_count(&packets), 100); + } + + /// RFC 6762 section 17: a record too large for one MTU-sized packet is sent + /// alone in an oversized packet, rather than dropped. It must be alone, since + /// a fragmented packet "MUST NOT contain more than one resource record". + #[test] + fn test_dns_outgoing_oversized_record_sent_alone() { + let mut out = DnsOutgoing::new(FLAGS_QR_RESPONSE); + out.add_answer_at_time(ptr_answer(0), 0); + out.add_answer_at_time( + DnsTxt::new("big._spill._tcp.local.", CLASS_IN, 4500, vec![b'x'; 2000]), + 0, + ); + out.add_answer_at_time(ptr_answer(1), 0); + + let packets = out.to_packets(MAX_PKT_DEFAULT); + assert_eq!(packets.len(), 3, "the big record needs a packet to itself"); + + assert!(packets[0].size() <= MAX_PKT_DEFAULT); + assert!( + packets[1].size() > MAX_PKT_DEFAULT, + "the oversized record must not be dropped" + ); + // Still small enough that the send path will let it out. + assert!(packets[1].size() <= MAX_PKT_ABSOLUTE_IPV6); + assert!(packets[2].size() <= MAX_PKT_DEFAULT); + + // One record per packet here, the middle one being the big TXT. + let parsed = DnsIncoming::new(packets[1].as_bytes().to_vec(), test_interface_id()).unwrap(); + assert_eq!(parsed.answers().len(), 1); + assert_eq!(parsed.answers()[0].get_name(), "big._spill._tcp.local."); + assert_eq!(parsed_answer_count(&packets), 3); + } + + /// Authorities and additionals spill too, and stay in their own sections. + #[test] + fn test_dns_outgoing_all_sections_spill() { + let mut out = DnsOutgoing::new(FLAGS_QR_RESPONSE); + for i in 0..40 { + out.add_answer_at_time(ptr_answer(i), 0); + } + for i in 40..80 { + out.add_authority(Box::new(ptr_answer(i))); + } + for i in 80..120 { + out.add_additional_answer(ptr_answer(i)); + } + + let packets = out.to_packets(MAX_PKT_DEFAULT); + assert!(packets.len() > 1); + + let mut answers = 0; + let mut authorities = 0; + let mut additionals = 0; + for packet in &packets { + assert!(packet.size() <= MAX_PKT_DEFAULT); + let parsed = DnsIncoming::new(packet.as_bytes().to_vec(), test_interface_id()).unwrap(); + answers += parsed.answers().len(); + authorities += parsed.authorities().len(); + additionals += parsed.additionals().len(); + } + + assert_eq!(answers, 40); + assert_eq!(authorities, 40); + assert_eq!(additionals, 40); + } } diff --git a/src/lib.rs b/src/lib.rs index 2ffb7e1..abf9b13 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -177,7 +177,7 @@ mod error; mod service_daemon; mod service_info; -pub use dns_parser::{InterfaceId, RRType, ScopedIp, ScopedIpV4, ScopedIpV6}; +pub use dns_parser::{InterfaceId, RRType, ScopedIp, ScopedIpV4, ScopedIpV6, MAX_PKT_DEFAULT}; pub use error::{Error, Result}; pub use service_daemon::{ DaemonEvent, DaemonStatus, DnsNameChange, HostnameResolutionEvent, IfKind, IfPredicate, diff --git a/src/service_daemon.rs b/src/service_daemon.rs index f9ea01e..678e881 100644 --- a/src/service_daemon.rs +++ b/src/service_daemon.rs @@ -34,9 +34,10 @@ use crate::{ current_time_millis, dns_cache::{DnsCache, IpType}, dns_parser::{ - ip_address_rr_type, DnsAddress, DnsEntryExt, DnsIncoming, DnsOutgoing, DnsPointer, - DnsRecordBox, DnsRecordExt, DnsSrv, DnsTxt, InterfaceId, RRType, ScopedIp, - CLASS_CACHE_FLUSH, CLASS_IN, FLAGS_AA, FLAGS_QR_QUERY, FLAGS_QR_RESPONSE, MAX_MSG_ABSOLUTE, + ip_address_rr_type, max_pkt_absolute, DnsAddress, DnsEntryExt, DnsIncoming, DnsOutgoing, + DnsPointer, DnsRecordBox, DnsRecordExt, DnsSrv, DnsTxt, InterfaceId, RRType, ScopedIp, + CLASS_CACHE_FLUSH, CLASS_IN, FLAGS_AA, FLAGS_QR_QUERY, FLAGS_QR_RESPONSE, + MAX_PKT_ABSOLUTE_IPV6, MAX_PKT_DEFAULT, }, error::{e_fmt, Error, Result}, service_info::{ @@ -72,6 +73,9 @@ pub const IP_CHECK_INTERVAL_IN_SECS_DEFAULT: u32 = 5; /// [RFC 6762 section 10.4](https://datatracker.ietf.org/doc/html/rfc6762#section-10.4) pub const VERIFY_TIMEOUT_DEFAULT: Duration = Duration::from_secs(10); +/// The smallest value accepted by [`ServiceDaemon::set_max_packet_size`]. +pub const MIN_MAX_PACKET_SIZE: usize = 512; + /// The mDNS port number per RFC 6762. pub const MDNS_PORT: u16 = 5353; @@ -639,6 +643,38 @@ impl ServiceDaemon { self.send_cmd(Command::SetOption(DaemonOption::ServiceNameLenMax(len_max))) } + /// Change the max byte size of a packet this daemon generates on the interfaces + /// matching `if_kind`. Use `IfKind::All` to change it on every interface. Messages + /// that don't fit are split across multiple packets rather than truncated. + /// + /// The default is `MAX_PKT_DEFAULT` (1452 bytes), small enough to fit in one + /// Ethernet frame over either IPv4 or IPv6. + /// + /// A `size` outside the accepted range is rejected with an error. The minimum is + /// 512 bytes, the classic UDP DNS message size of RFC 1035. The maximum is 8952 + /// bytes: RFC 6762 section 17 caps an mDNS packet at 9000 bytes including the IP + /// and UDP headers, and we subtract the bigger of the two IP headers so that a + /// generated packet is legal over either IP version. + pub fn set_max_packet_size(&self, if_kind: impl IntoIfKindVec, size: usize) -> Result<()> { + if size < MIN_MAX_PACKET_SIZE { + return Err(Error::Msg(format!( + "max packet size {size} is too small, must be at least {MIN_MAX_PACKET_SIZE}" + ))); + } + + if size > MAX_PKT_ABSOLUTE_IPV6 { + return Err(Error::Msg(format!( + "max packet size {size} is too big, must be at most {MAX_PKT_ABSOLUTE_IPV6}" + ))); + } + + let if_kind_vec = if_kind.into_vec(); + self.send_cmd(Command::SetOption(DaemonOption::MaxPacketSize( + if_kind_vec.kinds, + size, + ))) + } + /// Change the interval for checking IP changes automatically. /// /// Setting the interval to 0 disables the IP check. @@ -842,7 +878,7 @@ fn _new_socket_bind(intf: &Interface, should_loop: bool) -> Result // Test if we can send packets successfully. let multicast_addr = SocketAddrV4::new(GROUP_ADDR_V4, MDNS_PORT).into(); - let test_packets = DnsOutgoing::new(0).to_data_on_wire(); + let test_packets = DnsOutgoing::new(0).to_data_on_wire(MAX_PKT_DEFAULT); for packet in test_packets { sock.send_to(&packet, &multicast_addr) .map_err(|e| e_fmt!("send multicast packet on addr {}: {}", ip, e))?; @@ -1067,6 +1103,15 @@ struct IfSelection { selected: bool, } +/// Selection of the max packet size of interfaces. +struct MaxPacketSizeSelection { + /// The interfaces to be selected. + if_kind: IfKind, + + /// Max byte size of a packet generated for the selected interfaces. + max_packet_size: usize, +} + /// A struct holding the state. It was inspired by `zeroconf` package in Python. struct Zeroconf { /// The mDNS port number to use for socket binding. @@ -1120,6 +1165,10 @@ struct Zeroconf { /// Interval in millis to check IP address changes. ip_check_interval: u64, + /// All max packet size selections called to the daemon, in call order. + /// For an interface matched by more than one, the last one wins. + max_packet_sizes: Vec, + /// All interface selections called to the daemon. if_selections: Vec, @@ -1286,6 +1335,8 @@ impl Zeroconf { name: intf.name.clone(), index: if_index, addrs: HashSet::from([intf.addr]), + max_packet_size_v4: MAX_PKT_DEFAULT, + max_packet_size_v6: MAX_PKT_DEFAULT, }); } @@ -1317,6 +1368,7 @@ impl Zeroconf { monitors, service_name_len_max, ip_check_interval, + max_packet_sizes: Vec::new(), if_selections, signal_sock, timers, @@ -1623,6 +1675,7 @@ impl Zeroconf { match daemon_opt { DaemonOption::ServiceNameLenMax(length) => self.service_name_len_max = length, DaemonOption::IpCheckInterval(interval) => self.ip_check_interval = interval, + DaemonOption::MaxPacketSize(if_kind, size) => self.set_max_packet_size(if_kind, size), DaemonOption::EnableInterface(if_kind) => self.enable_interface(if_kind), DaemonOption::DisableInterface(if_kind) => self.disable_interface(if_kind), DaemonOption::MulticastLoopV4(on) => self.set_multicast_loop_v4(on), @@ -1668,6 +1721,38 @@ impl Zeroconf { self.apply_intf_selections(interfaces); } + fn set_max_packet_size(&mut self, kinds: Vec, size: usize) { + debug!("set_max_packet_size: {:?} {}", kinds, size); + let interfaces = my_ip_interfaces_inner(true, self.include_apple_p2p); + + for if_kind in kinds { + self.max_packet_sizes.push(MaxPacketSizeSelection { + if_kind: resolve_addr_to_index(if_kind, &interfaces), + max_packet_size: size, + }); + } + + self.apply_max_packet_sizes(&interfaces); + } + + /// Resolve all max packet size selections against `interfaces` and store the + /// outcome in every interface in `my_intfs`. + fn apply_max_packet_sizes(&mut self, interfaces: &[Interface]) { + for (if_index, my_intf) in self.my_intfs.iter_mut() { + let v4 = resolve_max_packet_size(&self.max_packet_sizes, interfaces, *if_index, true); + let v6 = resolve_max_packet_size(&self.max_packet_sizes, interfaces, *if_index, false); + + if my_intf.max_packet_size_v4 != v4 || my_intf.max_packet_size_v6 != v6 { + debug!( + "interface {}: max packet size v4 {} -> {v4}, v6 {} -> {v6}", + my_intf.name, my_intf.max_packet_size_v4, my_intf.max_packet_size_v6 + ); + my_intf.max_packet_size_v4 = v4; + my_intf.max_packet_size_v6 = v6; + } + } + } + fn set_multicast_loop_v4(&mut self, on: bool) { let Some(sock) = self.ipv4_sock.as_mut() else { return; @@ -1790,15 +1875,19 @@ impl Zeroconf { } // Update `my_intfs` based on the selections. - for (idx, intf) in interfaces.into_iter().enumerate() { + for (idx, intf) in interfaces.iter().enumerate() { if intf_selections[idx] { // Add the interface - self.add_interface(intf); + self.add_interface(intf, &interfaces); } else { // Remove the interface - self.del_interface_addr(&intf); + self.del_interface_addr(intf); } } + + // An interface that lost an address may now match a different selection. + // (`add_interface` already resolved the ones that gained one.) + self.apply_max_packet_sizes(&interfaces); } fn del_ip(&mut self, ip: IpAddr) { @@ -1994,7 +2083,11 @@ impl Zeroconf { } } - fn add_interface(&mut self, intf: Interface) { + /// Add the address of `intf` to `my_intfs`, and announce our services on it. + /// + /// `interfaces` is the full list the caller is applying, needed to resolve the + /// max packet size of the interface before we send anything on it. + fn add_interface(&mut self, intf: &Interface, interfaces: &[Interface]) { let sock_opt = if intf.ip().is_ipv4() { &self.ipv4_sock } else { @@ -2018,7 +2111,7 @@ impl Zeroconf { // If intf has a new address, add it to the existing interface. let my_intf = entry.get_mut(); if !my_intf.addrs.contains(&intf.addr) { - if let Err(e) = join_multicast_group(&sock.pktinfo, &intf) { + if let Err(e) = join_multicast_group(&sock.pktinfo, intf) { debug!("add_interface: socket_config {}: {e}", &intf.name); } my_intf.addrs.insert(intf.addr.clone()); @@ -2026,7 +2119,7 @@ impl Zeroconf { } } Entry::Vacant(entry) => { - if let Err(e) = join_multicast_group(&sock.pktinfo, &intf) { + if let Err(e) = join_multicast_group(&sock.pktinfo, intf) { debug!("add_interface: socket_config {}: {e}. Skipped.", &intf.name); return; } @@ -2036,6 +2129,8 @@ impl Zeroconf { name: intf.name.clone(), index: if_index, addrs: HashSet::from([intf.addr.clone()]), + max_packet_size_v4: MAX_PKT_DEFAULT, + max_packet_size_v6: MAX_PKT_DEFAULT, }; entry.insert(new_intf); } @@ -2048,6 +2143,14 @@ impl Zeroconf { debug!("add new interface {}: {}", intf.name, intf.ip()); + // Resolve before announcing, so the first packet out already honors it. + let v4 = resolve_max_packet_size(&self.max_packet_sizes, interfaces, if_index, true); + let v6 = resolve_max_packet_size(&self.max_packet_sizes, interfaces, if_index, false); + if let Some(my_intf) = self.my_intfs.get_mut(&if_index) { + my_intf.max_packet_size_v4 = v4; + my_intf.max_packet_size_v6 = v6; + } + let Some(my_intf) = self.my_intfs.get(&if_index) else { debug!("add_interface: cannot find if_index {if_index}"); return; @@ -2063,7 +2166,7 @@ impl Zeroconf { for (_, service_info) in self.my_services.iter_mut() { if service_info.is_addr_auto() { - service_info.insert_ipaddr(&intf); + service_info.insert_ipaddr(intf); if let Ok(true) = announce_service_on_intf( dns_registry, @@ -2536,6 +2639,7 @@ impl Zeroconf { /// Returns false if failed to receive a packet, /// otherwise returns true. fn handle_read(&mut self, event_key: usize) -> bool { + let is_ipv4 = event_key == IPV4_SOCK_EVENT_KEY; let sock_opt = match event_key { IPV4_SOCK_EVENT_KEY => &mut self.ipv4_sock, IPV6_SOCK_EVENT_KEY => &mut self.ipv6_sock, @@ -2548,14 +2652,13 @@ impl Zeroconf { debug!("handle_read: socket not available for token {}", event_key); return false; }; - let mut buf = vec![0u8; MAX_MSG_ABSOLUTE]; + // The buffer is one byte bigger than the biggest legal message, so that an + // over-sized datagram can be told apart from a legal one that happens to be + // exactly at the limit. + let max_size = max_pkt_absolute(is_ipv4); + let mut buf = vec![0u8; max_size + 1]; // Read the next mDNS UDP datagram. - // - // If the datagram is larger than `buf`, excess bytes may or may not - // be truncated by the socket layer depending on the platform's libc. - // In any case, such large datagram will not be decoded properly and - // this function should return false but should not crash. let (sz, pktinfo) = match sock.pktinfo.recv(&mut buf) { Ok(sz) => sz, Err(e) => { @@ -2566,6 +2669,22 @@ impl Zeroconf { } }; + // RFC 6762 section 17 caps an mDNS packet at 9000 bytes including the IP and + // UDP headers. A datagram over that arrives truncated, and decoding a + // truncated message does not fail cleanly: names run into whatever bytes + // follow, yielding bogus records or confusing parse errors. Drop it instead. + // + // On Windows, `recv` fails with WSAEMSGSIZE for such a datagram instead of + // truncating it, so it is dropped by the error branch above. Either way it + // is never decoded. + if sz > max_size { + debug!( + "handle_read: dropping over-sized datagram of at least {} bytes (max {})", + sz, max_size + ); + return true; // We still read something. + } + // Find the interface that received the packet. let pkt_if_index = pktinfo.if_index as u32; let Some(my_intf) = self.my_intfs.get(&pkt_if_index) else { @@ -4333,6 +4452,7 @@ struct DaemonOptionVal { enum DaemonOption { ServiceNameLenMax(u8), IpCheckInterval(u64), + MaxPacketSize(Vec, usize), EnableInterface(Vec), DisableInterface(Vec), MulticastLoopV4(bool), @@ -4484,6 +4604,17 @@ fn is_apple_p2p_by_name(name: &str) -> bool { p2p_prefixes.iter().any(|prefix| name.starts_with(prefix)) } +/// How to encode and where to send outgoing messages on one interface. +#[derive(Clone, Copy, Debug)] +struct SendConfig { + /// The mDNS port to send to. + port: u16, + + /// Max byte size of a generated packet. + /// See [`ServiceDaemon::set_max_packet_size`]. + max_packet_size: usize, +} + /// Send an outgoing mDNS query or response, and returns the packet bytes. /// Returns empty vec if no valid interface address is found. fn send_dns_outgoing( @@ -4513,13 +4644,19 @@ fn send_dns_outgoing( } }; + // The limit is per address family, so read it off the address we send from. + let config = SendConfig { + port, + max_packet_size: my_intf.max_packet_size(if_addr.ip().is_ipv4()), + }; + send_dns_outgoing_impl( out, if_name, my_intf.index, if_addr, sock, - port, + config, unicast_dest, ) } @@ -4531,7 +4668,7 @@ fn send_dns_outgoing_impl( if_index: u32, if_addr: &IfAddr, sock: &PktInfoUdpSocket, - port: u16, + config: SendConfig, unicast_dest: Option, ) -> MyResult>> { let qtype = if out.is_query() { @@ -4598,11 +4735,11 @@ fn send_dns_outgoing_impl( } } - let packet_list = out.to_data_on_wire(); + let packet_list = out.to_data_on_wire(config.max_packet_size); for packet in packet_list.iter() { match unicast_dest { Some(dest) => unicast_on_intf(packet, if_name, dest, sock), - None => multicast_on_intf(packet, if_name, if_index, if_addr, sock, port), + None => multicast_on_intf(packet, if_name, if_index, if_addr, sock, config.port), } } Ok(packet_list) @@ -4611,8 +4748,9 @@ fn send_dns_outgoing_impl( /// Sends a unicast packet directly to `dest` (used for RFC 6762 §6.7 /// legacy unicast responses). fn unicast_on_intf(packet: &[u8], if_name: &str, dest: SocketAddr, socket: &PktInfoUdpSocket) { - if packet.len() > MAX_MSG_ABSOLUTE { - debug!("Drop over-sized packet ({})", packet.len()); + let max_size = max_pkt_absolute(dest.is_ipv4()); + if packet.len() > max_size { + debug!("Drop over-sized packet ({} > {max_size})", packet.len()); return; } @@ -4642,8 +4780,9 @@ fn multicast_on_intf( socket: &PktInfoUdpSocket, port: u16, ) { - if packet.len() > MAX_MSG_ABSOLUTE { - debug!("Drop over-sized packet ({})", packet.len()); + let max_size = max_pkt_absolute(if_addr.ip().is_ipv4()); + if packet.len() > max_size { + debug!("Drop over-sized packet ({} > {max_size})", packet.len()); return; } @@ -5015,6 +5154,35 @@ fn handle_expired_probes( } /// Resolves `IfKind::Addr(ip)` to `IndexV4(if_index)` or `IndexV6(if_index)`. +/// Returns the max packet size to use on the interface `if_index` for the given +/// address family, i.e. the size of the last selection matching it, or +/// [`MAX_PKT_DEFAULT`] if none does. +/// +/// A selection matches an address, so it applies as soon as any address of the +/// interface in that family matches. That keeps the two families independent: +/// e.g. [`IfKind::IPv4`] leaves the IPv6 side of the interface alone. +fn resolve_max_packet_size( + selections: &[MaxPacketSizeSelection], + interfaces: &[Interface], + if_index: u32, + is_ipv4: bool, +) -> usize { + let mut size = MAX_PKT_DEFAULT; + + for selection in selections { + let matched = interfaces.iter().any(|intf| { + intf.index.unwrap_or(0) == if_index + && intf.ip().is_ipv4() == is_ipv4 + && selection.if_kind.matches(intf) + }); + if matched { + size = selection.max_packet_size; + } + } + + size +} + fn resolve_addr_to_index(if_kind: IfKind, interfaces: &[Interface]) -> IfKind { if let IfKind::Addr(addr) = &if_kind { if let Some(intf) = interfaces.iter().find(|intf| &intf.ip() == addr) { @@ -5033,11 +5201,12 @@ fn resolve_addr_to_index(if_kind: IfKind, interfaces: &[Interface]) -> IfKind { mod tests { use super::{ _new_socket_bind, check_domain_suffix, check_service_name_length, hostname_change, - my_ip_interfaces, name_change, send_dns_outgoing_impl, valid_instance_name, - valid_ip_on_intf, DaemonEvent, HostnameResolutionEvent, MyIntf, ServiceDaemon, - ServiceEvent, ServiceInfo, GROUP_ADDR_V4, INITIAL_QUERY_DELAY_MAX_MILLIS, - INITIAL_QUERY_DELAY_MIN_MILLIS, MDNS_PORT, SHARED_RESPONSE_DELAY_MAX_MILLIS, - SHARED_RESPONSE_DELAY_MIN_MILLIS, + my_ip_interfaces, name_change, resolve_max_packet_size, send_dns_outgoing_impl, + valid_instance_name, valid_ip_on_intf, DaemonEvent, HostnameResolutionEvent, IfKind, + MaxPacketSizeSelection, MyIntf, SendConfig, ServiceDaemon, ServiceEvent, ServiceInfo, + GROUP_ADDR_V4, INITIAL_QUERY_DELAY_MAX_MILLIS, INITIAL_QUERY_DELAY_MIN_MILLIS, + MAX_PKT_ABSOLUTE_IPV6, MAX_PKT_DEFAULT, MDNS_PORT, MIN_MAX_PACKET_SIZE, + SHARED_RESPONSE_DELAY_MAX_MILLIS, SHARED_RESPONSE_DELAY_MIN_MILLIS, }; use crate::{ dns_parser::{ @@ -5046,14 +5215,141 @@ mod tests { }, service_daemon::{add_answer_of_service, check_hostname}, }; - use if_addrs::{IfAddr, Ifv4Addr}; + use if_addrs::{IfAddr, Ifv4Addr, Ifv6Addr, Interface}; use std::{ collections::HashSet, - net::{IpAddr, Ipv4Addr, UdpSocket}, + net::{IpAddr, Ipv4Addr, Ipv6Addr, UdpSocket}, time::{Duration, Instant, SystemTime}, }; use test_log::test; + /// Builds an interface address for the max packet size tests below. + fn test_interface(name: &str, index: u32, addr: IfAddr) -> Interface { + Interface { + name: name.to_string(), + addr, + index: Some(index), + oper_status: if_addrs::IfOperStatus::Up, + is_p2p: false, + #[cfg(windows)] + adapter_name: String::new(), + } + } + + fn test_ifaddr_v4(ip: Ipv4Addr) -> IfAddr { + IfAddr::V4(Ifv4Addr { + ip, + netmask: Ipv4Addr::new(255, 255, 255, 0), + broadcast: None, + prefixlen: 24, + }) + } + + fn test_ifaddr_v6(ip: Ipv6Addr) -> IfAddr { + IfAddr::V6(Ifv6Addr { + ip, + netmask: Ipv6Addr::from(u128::MAX << 64), + broadcast: None, + prefixlen: 64, + }) + } + + #[test] + fn test_resolve_max_packet_size() { + // en0 is dual-stack, en1 is IPv4 only. + let interfaces = vec![ + test_interface("en0", 1, test_ifaddr_v4(Ipv4Addr::new(192, 168, 1, 2))), + test_interface( + "en0", + 1, + test_ifaddr_v6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1)), + ), + test_interface("en1", 2, test_ifaddr_v4(Ipv4Addr::new(10, 0, 0, 2))), + ]; + + let resolve = |selections: &[MaxPacketSizeSelection], if_index, is_ipv4| { + resolve_max_packet_size(selections, &interfaces, if_index, is_ipv4) + }; + + // No selection: every interface keeps the default. + assert_eq!(resolve(&[], 1, true), MAX_PKT_DEFAULT); + assert_eq!(resolve(&[], 1, false), MAX_PKT_DEFAULT); + + // A selection by name applies to the interface it matches, both families. + let by_name = vec![MaxPacketSizeSelection { + if_kind: IfKind::Name("en0".to_string()), + max_packet_size: 8000, + }]; + assert_eq!(resolve(&by_name, 1, true), 8000); + assert_eq!(resolve(&by_name, 1, false), 8000); + assert_eq!(resolve(&by_name, 2, true), MAX_PKT_DEFAULT); + + // For an interface matched more than once, the last selection wins. + let overlapping = vec![ + MaxPacketSizeSelection { + if_kind: IfKind::All, + max_packet_size: 8000, + }, + MaxPacketSizeSelection { + if_kind: IfKind::Name("en1".to_string()), + max_packet_size: 4000, + }, + ]; + assert_eq!(resolve(&overlapping, 1, true), 8000); + assert_eq!(resolve(&overlapping, 1, false), 8000); + assert_eq!(resolve(&overlapping, 2, true), 4000); + + // A selection of one address family leaves the other one alone. + let v4_only = vec![MaxPacketSizeSelection { + if_kind: IfKind::IPv4, + max_packet_size: 8000, + }]; + assert_eq!(resolve(&v4_only, 1, true), 8000); + assert_eq!(resolve(&v4_only, 1, false), MAX_PKT_DEFAULT); + + let v6_only = vec![MaxPacketSizeSelection { + if_kind: IfKind::IPv6, + max_packet_size: 8000, + }]; + assert_eq!(resolve(&v6_only, 1, false), 8000); + assert_eq!(resolve(&v6_only, 1, true), MAX_PKT_DEFAULT); + // en1 has no IPv6 address, so the IPv6 selection cannot reach it. + assert_eq!(resolve(&v6_only, 2, true), MAX_PKT_DEFAULT); + assert_eq!(resolve(&v6_only, 2, false), MAX_PKT_DEFAULT); + + // Same for an index selection, which names a family too. + let by_index_v4 = vec![MaxPacketSizeSelection { + if_kind: IfKind::IndexV4(1), + max_packet_size: 8000, + }]; + assert_eq!(resolve(&by_index_v4, 1, true), 8000); + assert_eq!(resolve(&by_index_v4, 1, false), MAX_PKT_DEFAULT); + } + + /// A size outside [`MIN_MAX_PACKET_SIZE`]..=[`MAX_PKT_ABSOLUTE_IPV6`] is rejected + /// rather than clamped, so what reaches the encoder is always legal. + #[test] + fn test_set_max_packet_size_range() { + let daemon = ServiceDaemon::new().unwrap(); + + assert!(daemon + .set_max_packet_size(IfKind::All, MIN_MAX_PACKET_SIZE - 1) + .is_err()); + assert!(daemon + .set_max_packet_size(IfKind::All, MAX_PKT_ABSOLUTE_IPV6 + 1) + .is_err()); + + // Both ends of the range are accepted. + assert!(daemon + .set_max_packet_size(IfKind::All, MIN_MAX_PACKET_SIZE) + .is_ok()); + assert!(daemon + .set_max_packet_size(IfKind::All, MAX_PKT_ABSOLUTE_IPV6) + .is_ok()); + + daemon.shutdown().unwrap(); + } + #[test] fn test_response_source_ifaddr_match() { // When an interface has multiple IPs on unrelated subnets, @@ -5076,6 +5372,8 @@ mod tests { name: "dummy0".to_string(), index: 1, addrs: HashSet::from([ifaddr_a.clone(), ifaddr_b.clone()]), + max_packet_size_v4: MAX_PKT_DEFAULT, + max_packet_size_v6: MAX_PKT_DEFAULT, }; let pick = |querier: IpAddr| -> Option { @@ -5167,7 +5465,7 @@ mod tests { let mut query = DnsOutgoing::new(FLAGS_QR_QUERY); query.add_question(&hostname, RRType::A); let query_packet = query - .to_data_on_wire() + .to_data_on_wire(MAX_PKT_DEFAULT) .pop() .expect("query serialized to one packet"); @@ -5428,7 +5726,10 @@ mod tests { // Build the PTR query for our service type. let mut query = DnsOutgoing::new(FLAGS_QR_QUERY); query.add_question(&service_type, RRType::PTR); - let query_packet = query.to_data_on_wire().pop().expect("one packet"); + let query_packet = query + .to_data_on_wire(MAX_PKT_DEFAULT) + .pop() + .expect("one packet"); // Wait for the initial announcements and the §6 rate-limit window (1s) to // pass, so our query elicits a fresh (delayed) response instead of being @@ -5617,7 +5918,10 @@ mod tests { intf.index.unwrap_or(0), &intf.addr, &sock.pktinfo, - MDNS_PORT, + SendConfig { + port: MDNS_PORT, + max_packet_size: MAX_PKT_DEFAULT, + }, None, ) .unwrap(); @@ -5886,7 +6190,7 @@ mod tests { let mut out = DnsOutgoing::new(FLAGS_QR_RESPONSE | FLAGS_AA); // Construct a dummy DnsIncoming message - let mut dummy_data = out.to_data_on_wire(); + let mut dummy_data = out.to_data_on_wire(MAX_PKT_DEFAULT); let interface_id = InterfaceId::from(&service_intf); let incoming = DnsIncoming::new(dummy_data.pop().unwrap(), interface_id).unwrap(); diff --git a/src/service_info.rs b/src/service_info.rs index c2c5b79..3af09c1 100644 --- a/src/service_info.rs +++ b/src/service_info.rs @@ -34,6 +34,14 @@ pub(crate) struct MyIntf { /// One interface can have multiple IPv4 addresses and/or multiple IPv6 addresses. pub(crate) addrs: HashSet, + + /// Max byte size of a packet generated for the IPv4 addresses of this interface, + /// resolved from the selections given to + /// [`ServiceDaemon::set_max_packet_size`](crate::ServiceDaemon::set_max_packet_size). + pub(crate) max_packet_size_v4: usize, + + /// Same as `max_packet_size_v4`, for the IPv6 addresses of this interface. + pub(crate) max_packet_size_v6: usize, } impl MyIntf { @@ -44,6 +52,15 @@ impl MyIntf { pub(crate) fn next_ifaddr_v6(&self) -> Option<&IfAddr> { self.addrs.iter().find(|a| a.ip().is_ipv6()) } + + /// Max byte size of a packet generated for the given address family. + pub(crate) fn max_packet_size(&self, is_ipv4: bool) -> usize { + if is_ipv4 { + self.max_packet_size_v4 + } else { + self.max_packet_size_v6 + } + } } impl From<&MyIntf> for InterfaceId { From 9033bc34ae844307612888ffa2b1bf0433a2b35a Mon Sep 17 00:00:00 2001 From: Han Xu Date: Sun, 2 Aug 2026 17:40:53 -0700 Subject: [PATCH 2/4] minor changes --- src/dns_cache.rs | 5 +- src/dns_parser.rs | 153 +++++++++++++++++----------------------------- 2 files changed, 58 insertions(+), 100 deletions(-) diff --git a/src/dns_cache.rs b/src/dns_cache.rs index d673040..30d3bbe 100644 --- a/src/dns_cache.rs +++ b/src/dns_cache.rs @@ -862,6 +862,7 @@ mod tests { use crate::{ dns_parser::{DnsAddress, DnsPointer, DnsRecordExt, DnsSrv, DnsTxt, RRType, CLASS_IN}, service_info::MyIntf, + MAX_PKT_DEFAULT, }; use std::collections::HashSet; use std::net::IpAddr; @@ -871,8 +872,8 @@ mod tests { name: name.to_string(), index, addrs: HashSet::new(), - max_packet_size_v4: crate::MAX_PKT_DEFAULT, - max_packet_size_v6: crate::MAX_PKT_DEFAULT, + max_packet_size_v4: MAX_PKT_DEFAULT, + max_packet_size_v6: MAX_PKT_DEFAULT, } } diff --git a/src/dns_parser.rs b/src/dns_parser.rs index 94420fa..e0e95f2 100644 --- a/src/dns_parser.rs +++ b/src/dns_parser.rs @@ -295,10 +295,6 @@ pub const MAX_PKT_ABSOLUTE_IPV4: usize = 8972; /// /// Same 9000-byte ceiling as [`MAX_PKT_ABSOLUTE_IPV4`], less the bigger IPv6 header: /// 9000 bytes - IPv6 header 40 bytes - UDP header 8 bytes. -/// -/// Being the smaller of the two, it is legal over either IP version, and is therefore -/// the ceiling used when generating packets, at the cost of 20 unused bytes for IPv4. -/// The IPv4 value is for packets we receive, which others may legally send that big. pub const MAX_PKT_ABSOLUTE_IPV6: usize = 8952; /// Absolute max size of an mDNS packet for the given IP version. @@ -310,18 +306,12 @@ pub const fn max_pkt_absolute(is_ipv4: bool) -> usize { } } -/// Default max size of a generated (i.e. outgoing) packet, i.e. the default of -/// [`ServiceDaemon::set_max_packet_size`](crate::ServiceDaemon::set_max_packet_size). -/// -/// The limit is per packet, not per message: a message too big for one packet is -/// split across several rather than truncated. +/// Default max size of a generated (i.e. outgoing) packet. /// /// Calculated as: 1500 bytes Ethernet MTU - IPv6 header 40 bytes - UDP header 8 bytes. /// It is safe on both IPv4 and IPv6, at the cost of 20 unused bytes for IPv4. /// -/// The idea is to keep generated packets unfragmented. See RFC 6762 section 17 for details. -/// This is a conservative constant rather than the real MTU of the outgoing interface: use -/// the API above to raise it on links known to support bigger packets. +/// The idea is to keep generated packets unfragmented at IP layer. See RFC 6762 section 17. pub const MAX_PKT_DEFAULT: usize = 1452; const MSG_HEADER_LEN: usize = 12; @@ -340,8 +330,7 @@ pub enum WriteError { /// A label in a name is longer than [`MAX_LABEL_BYTES`]. NameTooLong, - /// The packet would exceed its max size with this record. The caller can - /// retry the record in a new packet. + /// The packet would exceed its max size with this record. PacketFull, } @@ -1478,10 +1467,13 @@ impl DnsRecordExt for DnsNSec { } } -#[derive(PartialEq)] -enum PacketState { - Init = 0, - Finished = 1, +/// Which section of a DNS message an item belongs to. +#[derive(Clone, Copy)] +enum Section { + Question, + Answer, + Authority, + Additional, } /// A single packet for outgoing DNS message. @@ -1489,24 +1481,29 @@ pub struct DnsOutPacket { /// All bytes in `data` is the actual packet on the wire. data: Vec, - /// An internal state, not defined by DNS. - state: PacketState, - /// k: name, v: offset names: HashMap, - /// Max byte size of `data`. A question or record that would push `data` - /// past it is rejected with [`WriteError::PacketFull`]. + /// Max byte size of `data`. i.e. the max packet size. max_size: usize, + + /// How many items `data` holds in each section, i.e. the header counts. + question_count: u16, + answer_count: u16, + auth_count: u16, + addi_count: u16, } impl DnsOutPacket { fn new(max_size: usize) -> Self { Self { data: vec![0; MSG_HEADER_LEN], - state: PacketState::Init, names: HashMap::new(), max_size, + question_count: 0, + answer_count: 0, + auth_count: 0, + addi_count: 0, } } @@ -1518,6 +1515,24 @@ impl DnsOutPacket { &self.data } + /// True if nothing has been written into this packet yet. + fn is_empty(&self) -> bool { + self.question_count == 0 + && self.answer_count == 0 + && self.auth_count == 0 + && self.addi_count == 0 + } + + /// Counts one more item in `section`. + fn bump(&mut self, section: Section) { + match section { + Section::Question => self.question_count += 1, + Section::Answer => self.answer_count += 1, + Section::Authority => self.auth_count += 1, + Section::Additional => self.addi_count += 1, + } + } + fn write_question(&mut self, question: &DnsQuestion) -> WriteResult { let start_size = self.size(); @@ -1575,7 +1590,7 @@ impl DnsOutPacket { return Err(e); } - self.insert_short(record_offset - 2, (self.size() - record_offset) as u16); + self.set_short_at(record_offset - 2, (self.size() - record_offset) as u16); if self.size() > self.max_size { self.rollback(start_size); @@ -1585,7 +1600,7 @@ impl DnsOutPacket { Ok(()) } - pub(crate) fn insert_short(&mut self, index: usize, value: u16) { + fn set_short_at(&mut self, index: usize, value: u16) { self.data[index..index + 2].copy_from_slice(&value.to_be_bytes()); } @@ -1740,7 +1755,7 @@ impl DnsOutPacket { /// the next packet. fn set_truncated(&mut self) { let flags = u16::from_be_bytes([self.data[2], self.data[3]]); - self.insert_short(2, flags | FLAGS_TC); + self.set_short_at(2, flags | FLAGS_TC); } /// Writes the header fields and finish the packet. @@ -1765,35 +1780,16 @@ impl DnsOutPacket { // | ARCOUNT | // +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ // - fn write_header( - &mut self, - id: u16, - flags: u16, - q_count: u16, - a_count: u16, - auth_count: u16, - addi_count: u16, - ) { - self.insert_short(0, id); - self.insert_short(2, flags); - self.insert_short(4, q_count); - self.insert_short(6, a_count); - self.insert_short(8, auth_count); - self.insert_short(10, addi_count); - - self.state = PacketState::Finished; + fn write_header(&mut self, id: u16, flags: u16) { + self.set_short_at(0, id); + self.set_short_at(2, flags); + self.set_short_at(4, self.question_count); + self.set_short_at(6, self.answer_count); + self.set_short_at(8, self.auth_count); + self.set_short_at(10, self.addi_count); } } -/// Which section of a DNS message an item belongs to. -#[derive(Clone, Copy)] -enum Section { - Question, - Answer, - Authority, - Additional, -} - /// Encodes a [`DnsOutgoing`] into one or more [`DnsOutPacket`], starting a new /// packet whenever the current one runs out of room. struct PacketBuilder<'a> { @@ -1807,12 +1803,6 @@ struct PacketBuilder<'a> { finished: Vec, current: DnsOutPacket, - - /// Section counts for `current`. - question_count: u16, - answer_count: u16, - auth_count: u16, - addi_count: u16, } impl<'a> PacketBuilder<'a> { @@ -1823,27 +1813,6 @@ impl<'a> PacketBuilder<'a> { id: if out.multicast { 0 } else { out.id }, finished: Vec::new(), current: DnsOutPacket::new(max_size), - question_count: 0, - answer_count: 0, - auth_count: 0, - addi_count: 0, - } - } - - /// True if nothing has been written into the current packet yet. - fn current_is_empty(&self) -> bool { - self.question_count == 0 - && self.answer_count == 0 - && self.auth_count == 0 - && self.addi_count == 0 - } - - fn bump(&mut self, section: Section) { - match section { - Section::Question => self.question_count += 1, - Section::Answer => self.answer_count += 1, - Section::Authority => self.auth_count += 1, - Section::Additional => self.addi_count += 1, } } @@ -1859,7 +1828,7 @@ impl<'a> PacketBuilder<'a> { { match write(&mut self.current) { Ok(()) => { - self.bump(section); + self.current.bump(section); return; } // The item can never be encoded: skip it. @@ -1869,12 +1838,12 @@ impl<'a> PacketBuilder<'a> { // Finish the current packet and retry in a new one. If the current packet // is already empty, a new one would be no roomier, so don't bother. - if !self.current_is_empty() { + if !self.current.is_empty() { self.flush(); match write(&mut self.current) { Ok(()) => { - self.bump(section); + self.current.bump(section); return; } Err(WriteError::NameTooLong) => return, @@ -1900,7 +1869,7 @@ impl<'a> PacketBuilder<'a> { self.current.max_size = usize::MAX; if write(&mut self.current).is_ok() { - self.bump(section); + self.current.bump(section); self.flush(); } else { // Too big even for the hard ceiling: skip the record and carry on. @@ -1910,29 +1879,17 @@ impl<'a> PacketBuilder<'a> { /// Finishes the current packet and starts a new empty one. fn flush(&mut self) { - self.current.write_header( - self.id, - self.out.flags, - self.question_count, - self.answer_count, - self.auth_count, - self.addi_count, - ); + self.current.write_header(self.id, self.out.flags); let next = DnsOutPacket::new(self.max_size); self.finished .push(std::mem::replace(&mut self.current, next)); - - self.question_count = 0; - self.answer_count = 0; - self.auth_count = 0; - self.addi_count = 0; } fn finish(mut self) -> Vec { // Always produce at least one packet, even an empty one, but never leave a // trailing empty packet behind a full one. - if !self.current_is_empty() || self.finished.is_empty() { + if !self.current.is_empty() || self.finished.is_empty() { self.flush(); } From d2a7cc8b28c9e3d2c62d59ebb55c0dca7da136ec Mon Sep 17 00:00:00 2001 From: Han Xu Date: Sun, 2 Aug 2026 21:45:04 -0700 Subject: [PATCH 3/4] add is_ipv4 to PacketBuilder --- src/dns_parser.rs | 103 +++++++++++++++++++++++++++++++----------- src/service_daemon.rs | 21 ++++++--- 2 files changed, 90 insertions(+), 34 deletions(-) diff --git a/src/dns_parser.rs b/src/dns_parser.rs index e0e95f2..ef60eb6 100644 --- a/src/dns_parser.rs +++ b/src/dns_parser.rs @@ -1798,19 +1798,20 @@ struct PacketBuilder<'a> { /// Max size of a packet that holds more than one record. max_size: usize, - /// The message id, always 0 for multicast. - id: u16, + /// IP version these packets are bound for, which decides their absolute + /// ceiling: see [`max_pkt_absolute`]. + is_ipv4: bool, finished: Vec, current: DnsOutPacket, } impl<'a> PacketBuilder<'a> { - fn new(out: &'a DnsOutgoing, max_size: usize) -> Self { + fn new(out: &'a DnsOutgoing, max_size: usize, is_ipv4: bool) -> Self { Self { out, max_size, - id: if out.multicast { 0 } else { out.id }, + is_ipv4, finished: Vec::new(), current: DnsOutPacket::new(max_size), } @@ -1863,10 +1864,9 @@ impl<'a> PacketBuilder<'a> { // optional -- such a packet "MUST NOT contain more than one resource record" // -- so this packet is flushed immediately. // - // No ceiling is applied here: whether such a packet may go out on the wire - // is for the send path to decide, which drops one bigger than section 17 - // allows for its IP version. - self.current.max_size = usize::MAX; + // Only the section 17 ceiling for this IP version still applies: a packet + // over it cannot go out on the wire at all. + self.current.max_size = max_pkt_absolute(self.is_ipv4); if write(&mut self.current).is_ok() { self.current.bump(section); @@ -1879,7 +1879,8 @@ impl<'a> PacketBuilder<'a> { /// Finishes the current packet and starts a new empty one. fn flush(&mut self) { - self.current.write_header(self.id, self.out.flags); + self.current + .write_header(self.out.wire_id(), self.out.flags); let next = DnsOutPacket::new(self.max_size); self.finished @@ -1970,6 +1971,15 @@ impl DnsOutgoing { self.id = id; } + /// The id to put in the header, always 0 for multicast. + const fn wire_id(&self) -> u16 { + if self.multicast { + 0 + } else { + self.id + } + } + pub const fn is_query(&self) -> bool { (self.flags & FLAGS_QR_MASK) == FLAGS_QR_QUERY } @@ -2182,11 +2192,11 @@ impl DnsOutgoing { } /// Returns a list of actual DNS packet data to be sent on the wire, each no - /// bigger than `max_size`. + /// bigger than `max_size`, over the IP version given by `is_ipv4`. /// /// Most callers want [`MAX_PKT_DEFAULT`] for `max_size`. - pub fn to_data_on_wire(&self, max_size: usize) -> Vec> { - let packet_list = self.to_packets(max_size); + pub fn to_data_on_wire(&self, max_size: usize, is_ipv4: bool) -> Vec> { + let packet_list = self.to_packets(max_size, is_ipv4); packet_list.into_iter().map(|p| p.data).collect() } @@ -2198,17 +2208,21 @@ impl DnsOutgoing { /// empty packet: it is sent alone in an oversized packet, per RFC 6762 /// section 17. /// + /// `is_ipv4` tells which IP version the packets are bound for, and so how big + /// that lone oversized packet may get: see [`max_pkt_absolute`]. A record too + /// big even for that could not be sent at all, and is dropped. + /// /// `max_size` must be no bigger than [`MAX_PKT_ABSOLUTE_IPV6`], the RFC 6762 /// section 17 ceiling that is legal over either IP version; /// [`ServiceDaemon::set_max_packet_size`](crate::ServiceDaemon::set_max_packet_size) /// caps what it accepts. Most callers want [`MAX_PKT_DEFAULT`]. - pub fn to_packets(&self, max_size: usize) -> Vec { + pub fn to_packets(&self, max_size: usize, is_ipv4: bool) -> Vec { debug_assert!( max_size <= MAX_PKT_ABSOLUTE_IPV6, "max_size {} exceeds the RFC 6762 section 17 ceiling", max_size ); - let mut builder = PacketBuilder::new(self, max_size); + let mut builder = PacketBuilder::new(self, max_size, is_ipv4); for question in self.questions.iter() { builder.add(Section::Question, |packet| packet.write_question(question)); @@ -2838,10 +2852,14 @@ mod tests { use std::collections::HashMap; use std::net::{IpAddr, Ipv4Addr}; + /// The `is_ipv4` argument of `to_packets`. IPv6 has the smaller of the two + /// absolute ceilings, so it is the stricter one to encode for. + const IPV6: bool = false; + #[test] fn test_dns_outgoing_serialization_empty() { let out = DnsOutgoing::new(0); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!(packets[0].as_bytes(), &[0; 12]); let expected_names = HashMap::new(); @@ -2852,7 +2870,7 @@ mod tests { fn test_dns_outgoing_serialization_question() { let mut out = DnsOutgoing::new(0); out.add_question("123.test", RRType::A); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2886,7 +2904,7 @@ mod tests { "arm".to_string(), "linux".to_string(), ))); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2916,7 +2934,7 @@ mod tests { IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), InterfaceId::default(), )); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2946,7 +2964,7 @@ mod tests { ), 0, ); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -2979,7 +2997,7 @@ mod tests { ), 0, ); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -3008,7 +3026,7 @@ mod tests { out.add_question(&format!("{long_label}.local"), RRType::PTR); out.add_question("123.test", RRType::A); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!( packets[0].as_bytes(), @@ -3053,7 +3071,7 @@ mod tests { 0, ); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); // Header answer count is 1: the first answer was dropped. @@ -3104,7 +3122,7 @@ mod tests { // Re-emitting it must drop the question rather than panic. let mut out = DnsOutgoing::new(0); out.add_question(&name, RRType::PTR); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 1); assert_eq!(packets[0].as_bytes(), &[0; MSG_HEADER_LEN]); } @@ -3160,7 +3178,7 @@ mod tests { out.add_answer_at_time(ptr_answer(i), 0); } - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert!( packets.len() > 1, "{} answers should not fit in one packet", @@ -3192,7 +3210,7 @@ mod tests { out.add_answer_box(Box::new(ptr_answer(i))); } - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert!( packets.len() > 1, "known answers should not fit in one packet" @@ -3233,7 +3251,7 @@ mod tests { ); out.add_answer_at_time(ptr_answer(1), 0); - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert_eq!(packets.len(), 3, "the big record needs a packet to itself"); assert!(packets[0].size() <= MAX_PKT_DEFAULT); @@ -3252,6 +3270,37 @@ mod tests { assert_eq!(parsed_answer_count(&packets), 3); } + /// A record over the RFC 6762 section 17 ceiling could not go out on the wire + /// even in a packet of its own, so it is dropped while its neighbors survive. + #[test] + fn test_dns_outgoing_record_over_absolute_ceiling_dropped() { + let mut out = DnsOutgoing::new(FLAGS_QR_RESPONSE); + out.add_answer_at_time(ptr_answer(0), 0); + out.add_answer_at_time( + DnsTxt::new( + "huge._spill._tcp.local.", + CLASS_IN, + 4500, + vec![b'x'; MAX_PKT_ABSOLUTE_IPV6], + ), + 0, + ); + out.add_answer_at_time(ptr_answer(1), 0); + + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); + for packet in &packets { + assert!( + packet.size() <= MAX_PKT_ABSOLUTE_IPV6, + "an unsendable packet must never be generated" + ); + } + assert_eq!( + parsed_answer_count(&packets), + 2, + "only the huge record is dropped" + ); + } + /// Authorities and additionals spill too, and stay in their own sections. #[test] fn test_dns_outgoing_all_sections_spill() { @@ -3266,7 +3315,7 @@ mod tests { out.add_additional_answer(ptr_answer(i)); } - let packets = out.to_packets(MAX_PKT_DEFAULT); + let packets = out.to_packets(MAX_PKT_DEFAULT, IPV6); assert!(packets.len() > 1); let mut answers = 0; diff --git a/src/service_daemon.rs b/src/service_daemon.rs index 678e881..c97fc95 100644 --- a/src/service_daemon.rs +++ b/src/service_daemon.rs @@ -878,7 +878,7 @@ fn _new_socket_bind(intf: &Interface, should_loop: bool) -> Result // Test if we can send packets successfully. let multicast_addr = SocketAddrV4::new(GROUP_ADDR_V4, MDNS_PORT).into(); - let test_packets = DnsOutgoing::new(0).to_data_on_wire(MAX_PKT_DEFAULT); + let test_packets = DnsOutgoing::new(0).to_data_on_wire(MAX_PKT_DEFAULT, true); for packet in test_packets { sock.send_to(&packet, &multicast_addr) .map_err(|e| e_fmt!("send multicast packet on addr {}: {}", ip, e))?; @@ -4613,6 +4613,10 @@ struct SendConfig { /// Max byte size of a generated packet. /// See [`ServiceDaemon::set_max_packet_size`]. max_packet_size: usize, + + /// Whether the packets go out over IPv4, which decides their absolute + /// ceiling: see [`max_pkt_absolute`]. + is_ipv4: bool, } /// Send an outgoing mDNS query or response, and returns the packet bytes. @@ -4644,10 +4648,12 @@ fn send_dns_outgoing( } }; - // The limit is per address family, so read it off the address we send from. + // The limits are per address family, so read them off the address we send from. + let is_ipv4 = if_addr.ip().is_ipv4(); let config = SendConfig { port, - max_packet_size: my_intf.max_packet_size(if_addr.ip().is_ipv4()), + max_packet_size: my_intf.max_packet_size(is_ipv4), + is_ipv4, }; send_dns_outgoing_impl( @@ -4735,7 +4741,7 @@ fn send_dns_outgoing_impl( } } - let packet_list = out.to_data_on_wire(config.max_packet_size); + let packet_list = out.to_data_on_wire(config.max_packet_size, config.is_ipv4); for packet in packet_list.iter() { match unicast_dest { Some(dest) => unicast_on_intf(packet, if_name, dest, sock), @@ -5465,7 +5471,7 @@ mod tests { let mut query = DnsOutgoing::new(FLAGS_QR_QUERY); query.add_question(&hostname, RRType::A); let query_packet = query - .to_data_on_wire(MAX_PKT_DEFAULT) + .to_data_on_wire(MAX_PKT_DEFAULT, true) .pop() .expect("query serialized to one packet"); @@ -5727,7 +5733,7 @@ mod tests { let mut query = DnsOutgoing::new(FLAGS_QR_QUERY); query.add_question(&service_type, RRType::PTR); let query_packet = query - .to_data_on_wire(MAX_PKT_DEFAULT) + .to_data_on_wire(MAX_PKT_DEFAULT, true) .pop() .expect("one packet"); @@ -5921,6 +5927,7 @@ mod tests { SendConfig { port: MDNS_PORT, max_packet_size: MAX_PKT_DEFAULT, + is_ipv4: intf.addr.ip().is_ipv4(), }, None, ) @@ -6190,7 +6197,7 @@ mod tests { let mut out = DnsOutgoing::new(FLAGS_QR_RESPONSE | FLAGS_AA); // Construct a dummy DnsIncoming message - let mut dummy_data = out.to_data_on_wire(MAX_PKT_DEFAULT); + let mut dummy_data = out.to_data_on_wire(MAX_PKT_DEFAULT, true); let interface_id = InterfaceId::from(&service_intf); let incoming = DnsIncoming::new(dummy_data.pop().unwrap(), interface_id).unwrap(); From ab404f3215c3739512cebbf0dfd80e689f37d6c5 Mon Sep 17 00:00:00 2001 From: Han Xu Date: Sun, 2 Aug 2026 21:52:38 -0700 Subject: [PATCH 4/4] minor fix --- src/dns_parser.rs | 31 +++++++++++++++---------------- src/service_daemon.rs | 8 +++++--- src/service_info.rs | 4 +--- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/dns_parser.rs b/src/dns_parser.rs index ef60eb6..b83bd5e 100644 --- a/src/dns_parser.rs +++ b/src/dns_parser.rs @@ -289,16 +289,16 @@ pub const CLASS_CACHE_FLUSH: u16 = 0x8000; /// headers, MUST NOT exceed 9000 bytes." /// /// It is calculated as: 9000 bytes - IPv4 header 20 bytes - UDP header 8 bytes. -pub const MAX_PKT_ABSOLUTE_IPV4: usize = 8972; +pub(crate) const MAX_PKT_ABSOLUTE_IPV4: usize = 8972; /// Absolute max size of UDP datagram payload for an mDNS packet over IPv6. /// /// Same 9000-byte ceiling as [`MAX_PKT_ABSOLUTE_IPV4`], less the bigger IPv6 header: /// 9000 bytes - IPv6 header 40 bytes - UDP header 8 bytes. -pub const MAX_PKT_ABSOLUTE_IPV6: usize = 8952; +pub(crate) const MAX_PKT_ABSOLUTE_IPV6: usize = 8952; /// Absolute max size of an mDNS packet for the given IP version. -pub const fn max_pkt_absolute(is_ipv4: bool) -> usize { +pub(crate) const fn max_pkt_absolute(is_ipv4: bool) -> usize { if is_ipv4 { MAX_PKT_ABSOLUTE_IPV4 } else { @@ -1468,7 +1468,7 @@ impl DnsRecordExt for DnsNSec { } /// Which section of a DNS message an item belongs to. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] enum Section { Question, Answer, @@ -1837,8 +1837,7 @@ impl<'a> PacketBuilder<'a> { Err(WriteError::PacketFull) => {} } - // Finish the current packet and retry in a new one. If the current packet - // is already empty, a new one would be no roomier, so don't bother. + // Packet is full. Flush the current and create a new one. if !self.current.is_empty() { self.flush(); @@ -1852,20 +1851,16 @@ impl<'a> PacketBuilder<'a> { } } - // A question too big for an empty packet is malformed rather than merely - // oversized: there is no legitimate question of this size. + // Packet is still full. A question such big is not legitimate. if matches!(section, Section::Question) { return; } - // The record does not fit in a packet of its own either. RFC 6762 section 17: - // a record too large for one MTU-sized packet SHOULD be sent alone, in a - // single IP datagram, using multiple IP fragments. Sending it alone is not - // optional -- such a packet "MUST NOT contain more than one resource record" - // -- so this packet is flushed immediately. - // - // Only the section 17 ceiling for this IP version still applies: a packet - // over it cannot go out on the wire at all. + // Packet is still full. We will send this single record. + + // RFC 6762 section 17: + // "a record too large for one MTU-sized packet SHOULD be sent alone, in a + // single IP datagram". self.current.max_size = max_pkt_absolute(self.is_ipv4); if write(&mut self.current).is_ok() { @@ -1874,6 +1869,10 @@ impl<'a> PacketBuilder<'a> { } else { // Too big even for the hard ceiling: skip the record and carry on. self.current.max_size = self.max_size; + debug!( + "Record too big for absolute max size, skipping: {:?}", + section + ); } } diff --git a/src/service_daemon.rs b/src/service_daemon.rs index c97fc95..1de0fa9 100644 --- a/src/service_daemon.rs +++ b/src/service_daemon.rs @@ -74,7 +74,7 @@ pub const IP_CHECK_INTERVAL_IN_SECS_DEFAULT: u32 = 5; pub const VERIFY_TIMEOUT_DEFAULT: Duration = Duration::from_secs(10); /// The smallest value accepted by [`ServiceDaemon::set_max_packet_size`]. -pub const MIN_MAX_PACKET_SIZE: usize = 512; +pub(crate) const MIN_MAX_PACKET_SIZE: usize = 512; /// The mDNS port number per RFC 6762. pub const MDNS_PORT: u16 = 5353; @@ -645,7 +645,9 @@ impl ServiceDaemon { /// Change the max byte size of a packet this daemon generates on the interfaces /// matching `if_kind`. Use `IfKind::All` to change it on every interface. Messages - /// that don't fit are split across multiple packets rather than truncated. + /// that don't fit are split across multiple packets. A single record that doesn't + /// fit in a packet is sent alone in a packet of up to 8952 bytes over IPv6 or 8972 + /// bytes over IPv4, per RFC 6762 section 17. /// /// The default is `MAX_PKT_DEFAULT` (1452 bytes), small enough to fit in one /// Ethernet frame over either IPv4 or IPv6. @@ -5159,7 +5161,6 @@ fn handle_expired_probes( waiting_services } -/// Resolves `IfKind::Addr(ip)` to `IndexV4(if_index)` or `IndexV6(if_index)`. /// Returns the max packet size to use on the interface `if_index` for the given /// address family, i.e. the size of the last selection matching it, or /// [`MAX_PKT_DEFAULT`] if none does. @@ -5189,6 +5190,7 @@ fn resolve_max_packet_size( size } +/// Resolves `IfKind::Addr(ip)` to `IndexV4(if_index)` or `IndexV6(if_index)`. fn resolve_addr_to_index(if_kind: IfKind, interfaces: &[Interface]) -> IfKind { if let IfKind::Addr(addr) = &if_kind { if let Some(intf) = interfaces.iter().find(|intf| &intf.ip() == addr) { diff --git a/src/service_info.rs b/src/service_info.rs index 3af09c1..aec2634 100644 --- a/src/service_info.rs +++ b/src/service_info.rs @@ -35,9 +35,7 @@ pub(crate) struct MyIntf { /// One interface can have multiple IPv4 addresses and/or multiple IPv6 addresses. pub(crate) addrs: HashSet, - /// Max byte size of a packet generated for the IPv4 addresses of this interface, - /// resolved from the selections given to - /// [`ServiceDaemon::set_max_packet_size`](crate::ServiceDaemon::set_max_packet_size). + /// Max byte size of a packet generated for the IPv4 addresses of this interface. pub(crate) max_packet_size_v4: usize, /// Same as `max_packet_size_v4`, for the IPv6 addresses of this interface.