From 21054eeb749df3b49beac4e6f75faf84aa1914fe Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:28:40 +0000 Subject: [PATCH 1/6] Keep the macOS ordered sender moving when a worker or sender thread dies With FIPS_MACOS_ORDERED_SENDER set, each packet reserves a slot in its destination's send order and that destination's sender thread sends the slots strictly in turn. A slot that was never completed stopped the sender for good: completions for later slots then filled the flow, the encrypt workers blocked waiting for room, their queues filled, and the rx loop blocked behind them. Two ways led there: an encrypt worker thread that exited dropped every job it held without completing its slot, and a sender thread that exited left its flow open, so completions kept waiting for room it would never make, and later packets for that destination were still handed to it. Each reserved slot is now owned by a value that completes it as a skip when dropped unsent, whether it is dropped in a worker's batch, in its queue, in an undelivered completion group, or on the rx loop when a worker's queue has closed. That skip never waits for room, so the rx loop cannot block on it. A sender thread that leaves its loop, a panic included, now closes its flow; completions waiting for room return when their flow closes; and a closed flow is replaced the next time its destination is used and pruned from the flow map. The flow's lock is taken poison-tolerantly wherever another thread reads it, and a closed worker queue drops a refused job only after releasing its own lock. After an encrypt worker has died, the packets round-robined to it are still lost for the rest of the process, but the loss no longer holds up every later packet to the same destination. The default macOS sender is unchanged. --- src/node/encrypt_worker.rs | 406 ++++++++++++++++++++++++++++++++----- 1 file changed, 360 insertions(+), 46 deletions(-) diff --git a/src/node/encrypt_worker.rs b/src/node/encrypt_worker.rs index 8f5e462d..d8cd4da7 100644 --- a/src/node/encrypt_worker.rs +++ b/src/node/encrypt_worker.rs @@ -156,9 +156,7 @@ pub(crate) struct FspSealJob { struct QueuedFmpSendJob { job: FmpSendJob, #[cfg(target_os = "macos")] - macos_flow: Option>, - #[cfg(target_os = "macos")] - macos_seq: u64, + macos_ticket: Option, } impl QueuedFmpSendJob { @@ -167,19 +165,15 @@ impl QueuedFmpSendJob { Self { job, #[cfg(target_os = "macos")] - macos_flow: None, - #[cfg(target_os = "macos")] - macos_seq: 0, + macos_ticket: None, } } #[cfg(target_os = "macos")] fn macos_sequenced(job: FmpSendJob, macos_flow: Arc) -> Self { - let macos_seq = macos_flow.reserve_seq(); Self { job, - macos_flow: Some(macos_flow), - macos_seq, + macos_ticket: Some(MacSeqTicket::reserve(macos_flow)), } } } @@ -283,6 +277,8 @@ impl MacWorkerSender { .lock() .expect("encrypt worker queue poisoned"); if state.closed { + // Outside the lock: dropping a sequenced job completes its slot. + drop(state); drop(job); return Err(MacWorkerTryPushError::Closed); } @@ -307,6 +303,7 @@ impl MacWorkerSender { .expect("encrypt worker queue poisoned"); loop { if state.closed { + drop(state); drop(job); return Err(MacWorkerPushError); } @@ -610,7 +607,11 @@ impl MacSequencedSendFlows { let mut flows = self.flows.lock().expect("mac send flow map poisoned"); self.prune_idle_locked(&mut flows, now_ms); - if let Some(flow) = flows.get(&key) { + // A closed flow's sender thread has exited and sends nothing more, so + // it is replaced rather than handed further jobs. + if let Some(flow) = flows.get(&key) + && !flow.is_closed() + { flow.mark_used(now_ms); return Arc::clone(flow); } @@ -652,7 +653,7 @@ impl MacSequencedSendFlows { let idle_ms = mac_send_flow_idle_ms(); flows.retain(|_, flow| { - if flow.is_idle(now_ms, idle_ms) { + if flow.is_closed() || flow.is_idle(now_ms, idle_ms) { flow.close(); false } else { @@ -764,6 +765,77 @@ struct MacCompletionGroup { items: Vec<(u64, MacSendItem)>, } +#[cfg(target_os = "macos")] +impl MacCompletionGroup { + /// Hand every item to the flow's sender. + fn deliver(mut self) { + let items = std::mem::take(&mut self.items); + self.flow.complete_many(items); + } +} + +/// A group dropped before delivery, when its worker unwinds, still completes +/// its slots, each as a skip, so the flow moves past them. +#[cfg(target_os = "macos")] +impl Drop for MacCompletionGroup { + fn drop(&mut self) { + for (seq, _) in self.items.drain(..) { + self.flow.complete_skip(seq); + } + } +} + +/// One reserved slot in a flow's send order, owed a completion. +/// +/// Every slot the flow hands out must be completed, or its sender waits at +/// the gap for ever and every later packet for that destination piles up +/// behind it. A ticket dropped without being taken, because its job never +/// reached a worker or its worker died, completes its slot as a skip. +#[cfg(target_os = "macos")] +struct MacSeqTicket { + flow: Option>, + seq: u64, +} + +#[cfg(target_os = "macos")] +impl MacSeqTicket { + fn reserve(flow: Arc) -> Self { + let seq = flow.reserve_seq(); + Self { + flow: Some(flow), + seq, + } + } + + /// Take the slot, leaving its completion to the caller. + fn take(mut self) -> (Arc, u64) { + let flow = self.flow.take().expect("a ticket is taken once"); + (flow, self.seq) + } +} + +#[cfg(target_os = "macos")] +impl Drop for MacSeqTicket { + fn drop(&mut self) { + if let Some(flow) = self.flow.take() { + flow.complete_skip(self.seq); + } + } +} + +/// Closes its flow when the flow's sender thread leaves `run`, including by +/// a panic, so completions waiting for room are released instead of waiting +/// on a sender that will never drain. +#[cfg(target_os = "macos")] +struct MacSenderExit<'a>(&'a MacSequencedSendFlow); + +#[cfg(target_os = "macos")] +impl Drop for MacSenderExit<'_> { + fn drop(&mut self) { + self.0.close(); + } +} + #[cfg(target_os = "macos")] enum MacSendItem { Packet { @@ -817,37 +889,63 @@ impl MacSequencedSendFlow { return false; } - let state = self.state.lock().expect("mac send flow state poisoned"); + let state = self.state.lock().unwrap_or_else(PoisonError::into_inner); state.pending.is_empty() && state.next_send_seq == self.next_seq.load(std::sync::atomic::Ordering::Relaxed) } fn close(&self) { - let mut state = self.state.lock().expect("mac send flow state poisoned"); + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); state.closed = true; drop(state); self.ready_cv.notify_one(); self.space_cv.notify_all(); } + fn is_closed(&self) -> bool { + self.state + .lock() + .unwrap_or_else(PoisonError::into_inner) + .closed + } + + /// Complete one slot as a skip without waiting for room, so a caller on + /// the rx_loop never blocks here. + fn complete_skip(&self, seq: u64) { + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); + if state.closed { + return; + } + let wakes_sender = seq == state.next_send_seq; + state.pending.insert(seq, MacSendItem::Skip); + drop(state); + if wakes_sender { + self.ready_cv.notify_one(); + } + } + fn complete_many(&self, items: Vec<(u64, MacSendItem)>) { const PENDING_CAP: usize = 4096; if items.is_empty() { return; } - let mut state = self.state.lock().expect("mac send flow state poisoned"); - if state.closed { - return; - } + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); let mut wakes_sender = false; for (seq, item) in items { - while state.pending.len() >= PENDING_CAP && seq != state.next_send_seq && !wakes_sender + while !state.closed + && state.pending.len() >= PENDING_CAP + && seq != state.next_send_seq + && !wakes_sender { state = self .space_cv .wait(state) - .expect("mac send flow state poisoned"); + .unwrap_or_else(PoisonError::into_inner); + } + // The sender has exited; what is left of `items` is dropped. + if state.closed { + return; } if seq == state.next_send_seq { wakes_sender = true; @@ -861,6 +959,8 @@ impl MacSequencedSendFlow { } fn run(self: Arc) { + // Closes the flow however this returns, a panic included. + let _exit = MacSenderExit(&self); trace!( socket_fd = self.key.socket_fd, connected_fd = ?self.key.connected_fd, @@ -927,10 +1027,10 @@ impl MacSequencedSendFlow { #[cfg(target_os = "macos")] fn push_mac_completion( groups: &mut Vec, - flow: Arc, - seq: u64, + ticket: MacSeqTicket, item: MacSendItem, ) { + let (flow, seq) = ticket.take(); if let Some(group) = groups .iter_mut() .find(|group| Arc::ptr_eq(&group.flow, &flow)) @@ -1058,11 +1158,7 @@ fn flush_batch_sync( for queued in batch.drain(..) { #[cfg(target_os = "macos")] - let QueuedFmpSendJob { - job, - macos_flow, - macos_seq, - } = queued; + let QueuedFmpSendJob { job, macos_ticket } = queued; #[cfg(not(target_os = "macos"))] let QueuedFmpSendJob { job } = queued; @@ -1087,13 +1183,8 @@ fn flush_batch_sync( || fsp.plaintext_offset > wire_buf.len() { #[cfg(target_os = "macos")] - if let Some(flow) = macos_flow.as_ref() { - push_mac_completion( - &mut macos_completions, - Arc::clone(flow), - macos_seq, - MacSendItem::Skip, - ); + if let Some(ticket) = macos_ticket { + push_mac_completion(&mut macos_completions, ticket, MacSendItem::Skip); } continue; } @@ -1111,13 +1202,8 @@ fn flush_batch_sync( Ok(tag) => tag, Err(_) => { #[cfg(target_os = "macos")] - if let Some(flow) = macos_flow.as_ref() { - push_mac_completion( - &mut macos_completions, - Arc::clone(flow), - macos_seq, - MacSendItem::Skip, - ); + if let Some(ticket) = macos_ticket { + push_mac_completion(&mut macos_completions, ticket, MacSendItem::Skip); } continue; } @@ -1142,8 +1228,8 @@ fn flush_batch_sync( Ok(tag) => tag, Err(_) => { #[cfg(target_os = "macos")] - if let Some(flow) = macos_flow { - push_mac_completion(&mut macos_completions, flow, macos_seq, MacSendItem::Skip); + if let Some(ticket) = macos_ticket { + push_mac_completion(&mut macos_completions, ticket, MacSendItem::Skip); } continue; } @@ -1152,11 +1238,10 @@ fn flush_batch_sync( wire_buf.extend_from_slice(tag.as_ref()); #[cfg(target_os = "macos")] - if let Some(flow) = macos_flow { + if let Some(ticket) = macos_ticket { push_mac_completion( &mut macos_completions, - flow, - macos_seq, + ticket, MacSendItem::Packet { packet: wire_buf, drop_on_backpressure, @@ -1215,7 +1300,7 @@ fn flush_batch_sync( #[cfg(target_os = "macos")] for group in macos_completions { - group.flow.complete_many(group.items); + group.deliver(); } drop(_t); // close the encrypt timer before we open the send timer @@ -2578,3 +2663,232 @@ mod mac_queue_tests { assert!(dropped.is_ok(), "receiver drop panicked on a poisoned lock"); } } + +/// The opt-in ordered sender's completion contract: every reserved slot is +/// completed, a dead sender releases its waiters, and nothing on the rx_loop +/// waits on either. Every wait is bounded so a regression fails, not hangs. +#[cfg(all(test, target_os = "macos"))] +mod mac_ordered_tests { + use super::*; + use crate::transport::udp::io::UdpRawSocket; + use ring::aead::{LessSafeKey, UnboundKey}; + use std::net::UdpSocket; + use std::sync::mpsc; + use std::thread; + use std::time::{Duration, Instant}; + + const WAIT: Duration = Duration::from_secs(5); + const PENDING_CAP: u64 = 4096; + + fn job(socket: &AsyncUdpSocket, dest: SocketAddr, counter: u64) -> FmpSendJob { + let key = UnboundKey::new(&ring::aead::CHACHA20_POLY1305, &[7u8; 32]).expect("key"); + let mut wire_buf = Vec::with_capacity(ESTABLISHED_HEADER_SIZE + 4 + crate::noise::TAG_SIZE); + wire_buf.extend_from_slice(&[0xA5; ESTABLISHED_HEADER_SIZE]); + wire_buf.extend_from_slice(&counter.to_le_bytes()[..4]); + FmpSendJob { + cipher: LessSafeKey::new(key), + counter, + wire_buf, + fsp_seal: None, + socket: socket.clone(), + dest_addr: dest, + connected_socket: None, + drop_on_backpressure: false, + queued_at: None, + } + } + + /// Reserve `n` slots on `flow` and complete them as skips, which fills + /// it to `n` pending items when an earlier slot is still open. + fn skip_reserved(flow: &MacSequencedSendFlow, n: u64) { + let skips = (0..n) + .map(|_| (flow.reserve_seq(), MacSendItem::Skip)) + .collect(); + flow.complete_many(skips); + } + + struct Rig { + _rt: tokio::runtime::Runtime, + recv: UdpSocket, + dest: SocketAddr, + socket: AsyncUdpSocket, + flows: MacSequencedSendFlows, + } + + impl Rig { + fn new() -> Self { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_io() + .build() + .expect("tokio rt"); + let enter = rt.enter(); + let recv = UdpSocket::bind("127.0.0.1:0").expect("bind recv"); + recv.set_read_timeout(Some(WAIT)).expect("read timeout"); + let dest = recv.local_addr().expect("recv addr"); + let socket = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 1 << 20, 1 << 20) + .expect("open send socket") + .into_async() + .expect("into_async"); + drop(enter); + Self { + _rt: rt, + recv, + dest, + socket, + flows: MacSequencedSendFlows::default(), + } + } + + fn job(&self, counter: u64) -> FmpSendJob { + job(&self.socket, self.dest, counter) + } + + fn flow(&self) -> Arc { + self.flows.flow_for(&self.job(0)) + } + + fn sequenced(&self, counter: u64) -> QueuedFmpSendJob { + let job = self.job(counter); + let flow = self.flows.flow_for(&job); + QueuedFmpSendJob::macos_sequenced(job, flow) + } + + fn received(&self) -> bool { + let mut buf = [0u8; 256]; + self.recv.recv_from(&mut buf).is_ok() + } + } + + #[test] + fn a_sequenced_job_dropped_unsent_lets_its_flow_send_what_follows() { + let rig = Rig::new(); + drop(rig.sequenced(1)); + let mut batch = vec![rig.sequenced(2)]; + flush_batch_sync(&mut batch).expect("flush"); + assert!(rig.received(), "the flow stalled at the dropped job's slot"); + } + + #[test] + fn a_dead_workers_queue_releases_the_slots_it_held() { + let rig = Rig::new(); + let (tx, rx) = mac_worker_channel::(4); + assert!(tx.try_push(rig.sequenced(1)).is_ok()); + assert!(tx.try_push(rig.sequenced(2)).is_ok()); + drop(rx); + assert!(matches!( + tx.try_push(rig.sequenced(3)), + Err(MacWorkerTryPushError::Closed) + )); + let mut batch = vec![rig.sequenced(4)]; + flush_batch_sync(&mut batch).expect("flush"); + assert!( + rig.received(), + "the flow stalled at a slot the dead queue held" + ); + } + + #[test] + fn a_completion_group_dropped_undelivered_lets_its_flow_send_what_follows() { + let rig = Rig::new(); + let flow = rig.flow(); + let group = MacCompletionGroup { + flow: Arc::clone(&flow), + items: vec![( + flow.reserve_seq(), + MacSendItem::Packet { + packet: b"undelivered".to_vec(), + drop_on_backpressure: false, + }, + )], + }; + drop(group); + let mut batch = vec![rig.sequenced(2)]; + flush_batch_sync(&mut batch).expect("flush"); + assert!( + rig.received(), + "the flow stalled at a slot an undelivered group held" + ); + } + + #[test] + fn a_completion_waiting_for_room_returns_when_its_flow_closes() { + let rig = Rig::new(); + let flow = rig.flow(); + let gap = QueuedFmpSendJob::macos_sequenced(rig.job(0), Arc::clone(&flow)); + skip_reserved(&flow, PENDING_CAP); + let late = flow.reserve_seq(); + let (done_tx, done_rx) = mpsc::channel(); + let waiter = Arc::clone(&flow); + thread::spawn(move || { + waiter.complete_many(vec![(late, MacSendItem::Skip)]); + let _ = done_tx.send(()); + }); + thread::sleep(Duration::from_millis(200)); + assert!( + done_rx.try_recv().is_err(), + "a full flow took a completion without room" + ); + flow.close(); + done_rx + .recv_timeout(WAIT) + .expect("the completion still waited after its flow closed"); + drop(gap); + } + + #[test] + fn a_sender_thread_that_panics_closes_its_flow() { + let rig = Rig::new(); + let flow = rig.flow(); + // Poison the flow's state lock, then wake the sender: its own + // `expect` on the lock panics inside `run`. + let poisoner = Arc::clone(&flow); + let poisoned = thread::spawn(move || { + let _state = poisoner.state.lock().unwrap(); + panic!("poison the flow state lock"); + }) + .join(); + assert!(poisoned.is_err()); + flow.complete_skip(0); + let deadline = Instant::now() + WAIT; + while !flow.is_closed() && Instant::now() < deadline { + thread::sleep(Duration::from_millis(10)); + } + assert!( + flow.is_closed(), + "a sender thread that panicked left its flow open" + ); + } + + #[test] + fn a_flow_whose_sender_has_exited_is_replaced() { + let rig = Rig::new(); + let first = rig.flow(); + first.close(); + let second = rig.flow(); + assert!( + !Arc::ptr_eq(&first, &second), + "a closed flow was handed out again" + ); + let mut batch = vec![QueuedFmpSendJob::macos_sequenced(rig.job(3), second)]; + flush_batch_sync(&mut batch).expect("flush"); + assert!(rig.received(), "the replacement flow sent nothing"); + } + + #[test] + fn dropping_a_sequenced_job_never_waits_for_room() { + let rig = Rig::new(); + let flow = rig.flow(); + let gap = QueuedFmpSendJob::macos_sequenced(rig.job(0), Arc::clone(&flow)); + skip_reserved(&flow, PENDING_CAP); + let dropped = QueuedFmpSendJob::macos_sequenced(rig.job(1), Arc::clone(&flow)); + let (done_tx, done_rx) = mpsc::channel(); + thread::spawn(move || { + drop(dropped); + let _ = done_tx.send(()); + }); + done_rx + .recv_timeout(WAIT) + .expect("dropping a job waited for room in a full flow"); + drop(gap); + } +} From dcca22ecdd61d2535d8e04f9b90c510d99cb653a Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:31:59 +0000 Subject: [PATCH 2/6] Show an Ethernet peer address as a MAC whatever its bytes decode as MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transport address printed its bytes as text whenever they were valid UTF-8, and as a colon-separated MAC only when they were not. A MAC is six arbitrary bytes, so some Ethernet peers showed up in `fipsctl show links` and in log lines as garbled text such as "2|ѬZd" instead of 32:7c:d1:ac:5a:64. An address built from a MAC now records that it is one, and always displays as a MAC. The Ethernet receive path, the beacon buffer and configured-peer resolution build their addresses that way. Equality and hashing still compare the bytes only, so lookups and comparisons behave as before, and every other transport's addresses display as they did. --- src/node/mod.rs | 2 +- src/transport/ethernet/mod.rs | 2 +- src/transport/ethernet/neighbor.rs | 13 ++++- src/transport/mod.rs | 25 +++++++- src/transport/types.rs | 93 +++++++++++++++++++++--------- 5 files changed, 104 insertions(+), 31 deletions(-) diff --git a/src/node/mod.rs b/src/node/mod.rs index 571c035f..c53d83cc 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -1328,7 +1328,7 @@ impl Node { NodeError::NoTransportForType(format!("invalid MAC in '{}': {}", addr_str, e)) })?; - Ok((transport_id, TransportAddr::from_bytes(&mac))) + Ok((transport_id, TransportAddr::from_mac(mac))) } #[cfg(not(any(target_os = "linux", target_os = "macos")))] { diff --git a/src/transport/ethernet/mod.rs b/src/transport/ethernet/mod.rs index a1531c65..056125f4 100644 --- a/src/transport/ethernet/mod.rs +++ b/src/transport/ethernet/mod.rs @@ -465,7 +465,7 @@ async fn ethernet_receive_loop( } }; let bytes = data.len(); - let addr = TransportAddr::from_bytes(&src_mac); + let addr = TransportAddr::from_mac(src_mac); let packet = ReceivedPacket::new(transport_id, addr, data); trace!( diff --git a/src/transport/ethernet/neighbor.rs b/src/transport/ethernet/neighbor.rs index 1c543bf0..9bb73d8d 100644 --- a/src/transport/ethernet/neighbor.rs +++ b/src/transport/ethernet/neighbor.rs @@ -144,7 +144,7 @@ impl NeighborBuffer { /// Build the buffered peer record for one beacon. fn peer(&self, src_mac: [u8; 6], pubkey: XOnlyPublicKey) -> DiscoveredPeer { - let addr = TransportAddr::from_bytes(&src_mac); + let addr = TransportAddr::from_mac(src_mac); DiscoveredPeer::with_hint(self.transport_id, addr, pubkey) } } @@ -234,6 +234,17 @@ mod tests { assert!(peers.is_empty()); } + #[test] + fn a_discovered_peer_whose_mac_bytes_are_valid_utf8_still_displays_as_a_mac() { + // "2|\u{46c}Zd": six bytes that decode as UTF-8, seen on a veth MAC. + let mac = [0x32, 0x7c, 0xd1, 0xac, 0x5a, 0x64]; + assert!(core::str::from_utf8(&mac).is_ok()); + let buffer = NeighborBuffer::new(TransportId::new(1)); + assert!(buffer.add_peer(mac, test_pubkey())); + let peers = buffer.take(); + assert_eq!(peers[0].addr.to_string(), "32:7c:d1:ac:5a:64"); + } + #[test] fn test_neighbor_buffer_dedup() { let buffer = NeighborBuffer::new(TransportId::new(1)); diff --git a/src/transport/mod.rs b/src/transport/mod.rs index bc839500..79f55fe8 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -1182,12 +1182,33 @@ mod tests { #[test] fn test_transport_addr_mac_display() { - // Raw 6-byte MACs (as Ethernet stores via from_bytes) display in - // standard colon-separated notation, not bare hex. + // Raw 6-byte non-UTF-8 values from from_bytes display in standard + // colon-separated notation, not bare hex. let mac = TransportAddr::from_bytes(&[0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]); assert_eq!(format!("{}", mac), "aa:bb:cc:dd:ee:ff"); } + #[test] + fn a_mac_address_whose_bytes_are_valid_utf8_displays_as_a_mac() { + let bytes = [0x32, 0x7c, 0xd1, 0xac, 0x5a, 0x64]; + assert!(core::str::from_utf8(&bytes).is_ok()); + let mac = TransportAddr::from_mac(bytes); + assert_eq!(mac.to_string(), "32:7c:d1:ac:5a:64"); + assert_eq!(format!("{:?}", mac), "TransportAddr(32:7c:d1:ac:5a:64)"); + assert_eq!(mac.as_bytes(), &bytes); + } + + #[test] + fn a_mac_address_equals_and_hashes_as_its_bytes() { + use std::hash::BuildHasher; + let bytes = [0x32, 0x7c, 0xd1, 0xac, 0x5a, 0x64]; + let mac = TransportAddr::from_mac(bytes); + let raw = TransportAddr::from_bytes(&bytes); + assert_eq!(mac, raw); + let hasher = std::collections::hash_map::RandomState::new(); + assert_eq!(hasher.hash_one(&mac), hasher.hash_one(&raw)); + } + #[test] fn test_transport_addr_non_mac_binary_is_bare_hex() { // Non-6-byte non-UTF-8 payloads stay bare hex (no separators). diff --git a/src/transport/types.rs b/src/transport/types.rs index 4e95f4ce..2dac9fd3 100644 --- a/src/transport/types.rs +++ b/src/transport/types.rs @@ -90,75 +90,116 @@ impl fmt::Display for LinkDirection { /// /// Each transport type interprets this differently: /// - UDP/TCP: "host:port" (IP address or DNS hostname) -/// - Ethernet: MAC address (6 bytes) -#[derive(Clone, PartialEq, Eq, Hash)] -pub struct TransportAddr(Vec); +/// - Ethernet: MAC address (6 bytes), built with [`TransportAddr::from_mac`] +/// so it always displays as a MAC, whatever its bytes decode as +#[derive(Clone)] +pub struct TransportAddr { + bytes: Vec, + /// Set only by [`TransportAddr::from_mac`]: the bytes are a MAC address + /// and always display as one. Not part of equality or hashing. + mac: bool, +} + +impl PartialEq for TransportAddr { + fn eq(&self, other: &Self) -> bool { + self.bytes == other.bytes + } +} + +impl Eq for TransportAddr {} + +impl core::hash::Hash for TransportAddr { + fn hash(&self, state: &mut H) { + self.bytes.hash(state); + } +} impl TransportAddr { /// Create a transport address from raw bytes. pub fn new(bytes: Vec) -> Self { - Self(bytes) + Self { bytes, mac: false } } /// Create a transport address from a byte slice. pub fn from_bytes(bytes: &[u8]) -> Self { - Self(bytes.to_vec()) + Self::new(bytes.to_vec()) + } + + /// Create an Ethernet transport address from a MAC address. + pub fn from_mac(mac: [u8; 6]) -> Self { + Self { + bytes: mac.to_vec(), + mac: true, + } } /// Create a transport address from a string. pub fn from_string(s: &str) -> Self { - Self(s.as_bytes().to_vec()) + Self::new(s.as_bytes().to_vec()) } /// Get the raw bytes. pub fn as_bytes(&self) -> &[u8] { - &self.0 + &self.bytes } /// Try to interpret as a UTF-8 string. pub fn as_str(&self) -> Option<&str> { - core::str::from_utf8(&self.0).ok() + core::str::from_utf8(&self.bytes).ok() } /// Get the length in bytes. pub fn len(&self) -> usize { - self.0.len() + self.bytes.len() } /// Check if empty. pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.bytes.is_empty() + } +} + +impl TransportAddr { + /// Write the bytes as a colon-separated MAC address. + fn write_mac(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for (i, byte) in self.bytes.iter().enumerate() { + if i > 0 { + write!(f, ":")?; + } + write!(f, "{:02x}", byte)?; + } + Ok(()) } } impl fmt::Debug for TransportAddr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.mac { + write!(f, "TransportAddr(")?; + self.write_mac(f)?; + return write!(f, ")"); + } match self.as_str() { Some(s) => write!(f, "TransportAddr(\"{}\")", s), - None => write!(f, "TransportAddr({:?})", self.0), + None => write!(f, "TransportAddr({:?})", self.bytes), } } } impl fmt::Display for TransportAddr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // Best-effort display as string if valid UTF-8. Otherwise render a - // 6-byte payload as a colon-separated MAC (standard Unix notation, - // matching BLE addrs, `ip link`/`ip neigh`, and packet logs), and - // any other non-UTF-8 byte string as bare hex. + // An Ethernet address is a MAC whatever its bytes decode as. Any + // other address displays as a string if it is valid UTF-8; otherwise + // a 6-byte payload is rendered as a colon-separated MAC and any other + // byte string as bare hex. + if self.mac { + return self.write_mac(f); + } match self.as_str() { Some(s) => write!(f, "{}", s), - None if self.0.len() == 6 => { - for (i, byte) in self.0.iter().enumerate() { - if i > 0 { - write!(f, ":")?; - } - write!(f, "{:02x}", byte)?; - } - Ok(()) - } + None if self.bytes.len() == 6 => self.write_mac(f), None => { - for byte in &self.0 { + for byte in &self.bytes { write!(f, "{:02x}", byte)?; } Ok(()) @@ -175,7 +216,7 @@ impl From<&str> for TransportAddr { impl From for TransportAddr { fn from(s: String) -> Self { - Self(s.into_bytes()) + Self::new(s.into_bytes()) } } From 26b733ca95015532e3424b49273e6d84823daa3c Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:38:16 +0000 Subject: [PATCH 3/6] Clear a connection's handshake slot when its whole handshake state leaves the machine Taking the handshake state off a connection's control machine moved it out with Option::take, which writes only the None marker and leaves the old bytes in place. On promotion the machine lives on as the active peer's control machine, so its slot kept the session's two traffic keys for as long as the peer stayed connected; the cross-connection and stale-connection paths left the same copy behind. The slot is now cleared as the state leaves, as it already is when a completed handshake or a rekeyed session leaves its own slot. The three callers unwrap the result at once, which the clearing helper's documentation says can leave a second copy on the stack. That copy is a move like any other and is outside what the clearing covers; clearing in the one shared method still covers the heap slot for every caller. The security reference now lists this move among the cleared ones. The new residue test reads the emptied slot's bytes, which only means something in an optimised build: in a debug build the None written by Option::take carries uninitialised stack bytes either way. It is therefore release-only, and the release library-test step in GitHub CI and local CI, which only compiled the tests before, now also runs it by exact name and fails if it did not run. --- .github/workflows/ci.yml | 16 ++++++++++---- docs/reference/security.md | 13 +++++------ src/peer/machine.rs | 45 +++++++++++++++++++++++++++++++++++++- testing/ci-local.sh | 20 ++++++++++++----- 4 files changed, 76 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdd51582..9b714ce0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -384,10 +384,18 @@ jobs: # Debug-only helpers (anything behind #[cfg(debug_assertions)]) vanish in # a release build, so a test calling one without the same gate breaks a # build no other job performs: every run above compiles the test target - # in debug. Compile it in release too, without running it — the point is - # that it builds at all. Mirrored in testing/ci-local.sh. - - name: Compile the library tests in release mode - run: cargo test --release --lib --no-run + # in debug. Compile it in release too. Of what it builds, run only the + # leg-slot residue test, which is release-only because only an optimised + # build leaves the slot in a state worth measuring; the grep fails the + # step if that test did not run, since a name filter that matches + # nothing passes. Mirrored in testing/ci-local.sh. + - name: Compile the library tests in release mode and run the leg-slot residue test + shell: bash + env: + RESIDUE_TEST: peer::machine::tests::take_leg_leaves_no_session_keys_in_the_slot_it_empties + run: | + cargo test --release --lib -- --exact "$RESIDUE_TEST" | tee "$RUNNER_TEMP/release-lib-tests.log" + grep -q '^test result: ok\. 1 passed;' "$RUNNER_TEMP/release-lib-tests.log" # ───────────────────────────────────────────────────────────────────────────── # Job 2b – Unit tests (macOS) diff --git a/docs/reference/security.md b/docs/reference/security.md index 71a2b13f..197fd262 100644 --- a/docs/reference/security.md +++ b/docs/reference/security.md @@ -123,14 +123,13 @@ reach: dropped. Each of those moves leaves behind, in a stack frame that is no longer in use, a copy of the node's long-term private key and, once the handshake has started, of its ephemeral key and chaining - key. Two moves out of the slots on a connection's control machine + key. Three moves out of the slots on a connection's control machine are cleared: when a completed handshake leaves the slot that held it, - and when a session is taken out of its slot for a rekey, the slot is - overwritten as the value leaves. Other moves are not. When a - connection is promoted to an active peer, or reaped as stale, its - whole handshake state is moved off its control machine, which leaves - the session's two traffic keys, or an unfinished handshake's private - keys, in the heap memory the machine occupies. + when a session is taken out of its slot for a rekey, and when the + whole handshake state leaves the machine because the connection is + promoted to an active peer, resolved as one side of a + cross-connection, or reaped as stale. In each case the slot is + overwritten as the value leaves. Other moves are not. - **Loading the identity from a secret string.** Building the node's identity from its key file or from `node.identity.nsec` leaves copies of the private key, among them a whole intermediate identity, diff --git a/src/peer/machine.rs b/src/peer/machine.rs index 90ca055d..1c4a8792 100644 --- a/src/peer/machine.rs +++ b/src/peer/machine.rs @@ -594,7 +594,12 @@ impl PeerMachine { /// Take the handshake crypto carrier off the machine (promotion and /// teardown consume it by value). pub(crate) fn take_leg(&mut self) -> Option { - self.leg.take() + // On promotion the machine lives on as the peer's control machine, so + // the slot is cleared as the leg leaves, or it would keep the session's + // traffic keys. Callers unwrap the result at once, which `take_cleared` + // warns may leave a second stack copy; clearing here still covers every + // caller's heap slot, and a stack copy is a move like any other. + noise::take_cleared(&mut self.leg) } /// Attach a handshake crypto carrier to the machine. @@ -3405,6 +3410,44 @@ mod tests { assert_eq!(decrypted, plaintext); } + /// Non-zero bytes left in a leg slot, read in place. + #[cfg(not(debug_assertions))] + fn leg_slot_nonzero_bytes(slot: &Option) -> usize { + let base = (slot as *const Option).cast::(); + (0..std::mem::size_of::>()) + // SAFETY: `base` comes from a live reference, so every byte of the + // slot is in bounds; volatile so the read is of memory as it is. + .filter(|&i| unsafe { std::ptr::read_volatile(base.add(i)) } != 0) + .count() + } + + /// Release only: in a debug build `Option::take` leaves stack garbage in + /// the payload of the `None` it writes, so the count means nothing there. + #[cfg(not(debug_assertions))] + #[test] + fn take_leg_leaves_no_session_keys_in_the_slot_it_empties() { + let initiator = Identity::generate(); + let responder = Identity::generate(); + let responder_id = PeerIdentity::from_pubkey_full(responder.pubkey_full()); + let mut ini = outbound_leg(LinkId::new(1), responder_id, 1000); + let mut res = Box::new(inbound_leg(LinkId::new(2), 1000)); + let msg1 = ini + .start_handshake(initiator.keypair(), make_epoch(), 1100) + .unwrap(); + res.receive_handshake_init(responder.keypair(), make_epoch(), &msg1, 1200) + .unwrap(); + assert!(res.has_session(), "the responder's leg holds the session"); + assert!(leg_slot_nonzero_bytes(&res.leg) > 64); + + let leg = res.take_leg().expect("the leg was attached"); + assert!(leg.noise_session.is_some(), "the session left with the leg"); + let left = leg_slot_nonzero_bytes(&res.leg); + assert!( + left <= 8, + "{left} non-zero bytes of the session stayed in the emptied leg slot" + ); + } + #[test] fn test_connection_failure() { // `mark_failed` releases the leg's Noise handshake handle. The failure diff --git a/testing/ci-local.sh b/testing/ci-local.sh index d4526341..8359b40e 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -676,16 +676,24 @@ run_tests() { # Debug-only helpers (anything behind #[cfg(debug_assertions)]) vanish in a # release build, so a test calling one without the same gate breaks a build # nothing here ever performs: every run above compiles the test target in - # debug. Compile it in release too, without running it — the point is that - # it builds at all. Mirrored in .github/workflows/ci.yml; check-ci-parity.sh - # compares integration suites only and would not catch a stage added to one - # runner and not the other. - info "cargo test --release --lib --no-run" - if cargo test --release --lib --no-run 2>&1; then + # debug. Compile it in release too. Of what it builds, run only the leg-slot + # residue test, which is release-only because only an optimised build + # leaves the slot in a state worth measuring; the grep fails the stage if + # that test did not run, since a name filter that matches nothing passes. + # Mirrored in .github/workflows/ci.yml; check-ci-parity.sh compares + # integration suites only and would not catch a stage added to one runner + # and not the other. + local residue_test="peer::machine::tests::take_leg_leaves_no_session_keys_in_the_slot_it_empties" + local release_log + release_log=$(mktemp "/tmp/ci-release-lib-tests.XXXXXX") + info "cargo test --release --lib -- --exact $residue_test" + if cargo test --release --lib -- --exact "$residue_test" 2>&1 | tee "$release_log" \ + && grep -q '^test result: ok\. 1 passed;' "$release_log"; then record "release-test-compile" 0 else record "release-test-compile" 1 fi + rm -f "$release_log" } # ── Stage 3: Integration Tests ───────────────────────────────────────────── From 361dabd8235e19480cf7f6054d69096efeda3de1 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:40:34 +0000 Subject: [PATCH 4/6] Stop ICMP port-unreachable reports from failing UDP receives on Windows By default Windows reports an ICMP port-unreachable that answers any datagram a UDP socket sent as WSAECONNRESET on that socket's next receive. The UDP transport's socket is shared by every UDP peer, so one unreachable peer address put receive errors, each logged as a warning and counted, into the path every other peer's traffic arrives through. Sockets the transport opens or adopts now have SIO_UDP_CONNRESET turned off. If the ioctl is refused the socket still works as before, so the failure is logged and the transport starts anyway. windows-sys becomes a direct Windows dependency at the version socket2 and tokio already use, so the lock file gains no package. --- Cargo.lock | 1 + Cargo.toml | 3 +++ src/transport/udp/io/mod.rs | 37 ++++++++++++++++++++++++++++ src/transport/udp/io/windows.rs | 43 +++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 1ff965e3..58938052 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1208,6 +1208,7 @@ dependencies = [ "tracing-subscriber", "tun", "windows-service", + "windows-sys 0.61.2", "wintun", "zeroize", ] diff --git a/Cargo.toml b/Cargo.toml index ae71c12e..785c3094 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,6 +78,9 @@ bluer = { version = "0.17", features = ["bluetoothd", "l2cap"] } [target.'cfg(windows)'.dependencies] wintun = "0.5" windows-service = "0.8.1" +# For WSAIoctl(SIO_UDP_CONNRESET) on the UDP socket. The version socket2 and +# tokio already pull in, so the lock file gains no package. +windows-sys = { version = "0.61", features = ["Win32_Networking_WinSock", "Win32_System_IO"] } [package.metadata.deb] maintainer = "Johnathan Corgan " diff --git a/src/transport/udp/io/mod.rs b/src/transport/udp/io/mod.rs index 64a25cd3..a62bde02 100644 --- a/src/transport/udp/io/mod.rs +++ b/src/transport/udp/io/mod.rs @@ -143,6 +143,43 @@ mod tests { assert!(reuse_port, "the adopted socket must carry SO_REUSEPORT"); } + /// A datagram sent to a port nobody holds draws an ICMP port-unreachable. + /// Windows reports one, by default, as `WSAECONNRESET` on the socket's next + /// receive; this socket is shared by every peer, so the next datagram from + /// any of them must come through instead. Checked for both ways a socket + /// is set up. + #[cfg(windows)] + #[tokio::test] + async fn a_port_unreachable_answer_is_not_a_receive_error_on_windows() { + let opened = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 65536, 65536) + .expect("failed to open"); + let plain = std::net::UdpSocket::bind("127.0.0.1:0").expect("failed to bind"); + let adopted = UdpRawSocket::adopt(plain, 65536, 65536).expect("failed to adopt"); + for (how, raw) in [("open", opened), ("adopt", adopted)] { + let addr = raw.local_addr(); + let sock = raw.into_async().expect("into_async"); + + let gone = std::net::UdpSocket::bind("127.0.0.1:0").expect("bind the closed port"); + let gone_addr = gone.local_addr().expect("closed port address"); + drop(gone); + sock.send_to(b"nobody", &gone_addr).await.expect("send_to"); + // Let the port-unreachable arrive before the datagram that follows. + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + + let peer = std::net::UdpSocket::bind("127.0.0.1:0").expect("bind the peer"); + peer.send_to(b"peer", addr).expect("peer send"); + + let mut buf = [0u8; 16]; + let got = + tokio::time::timeout(std::time::Duration::from_secs(5), sock.recv_from(&mut buf)) + .await + .unwrap_or_else(|_| panic!("{how}: nothing received")); + let (n, from, _) = got.unwrap_or_else(|e| panic!("{how}: receive failed: {e}")); + assert_eq!(&buf[..n], b"peer", "{how}"); + assert_eq!(from, peer.local_addr().unwrap(), "{how}"); + } + } + #[tokio::test] async fn test_async_udp_socket_send_recv() { let sock1 = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 65536, 65536) diff --git a/src/transport/udp/io/windows.rs b/src/transport/udp/io/windows.rs index bf71de98..abfe31ae 100644 --- a/src/transport/udp/io/windows.rs +++ b/src/transport/udp/io/windows.rs @@ -8,7 +8,10 @@ use crate::transport::TransportError; use socket2::{Domain, Protocol, Socket, Type}; use std::net::SocketAddr; +use std::os::windows::io::AsRawSocket; use std::sync::Arc; +use tracing::warn; +use windows_sys::Win32::Networking::WinSock::{SIO_UDP_CONNRESET, SOCKET, SOCKET_ERROR, WSAIoctl}; /// UDP socket wrapper (Windows). /// @@ -44,6 +47,7 @@ impl UdpRawSocket { sock.bind(&bind_addr.into()) .map_err(|e| TransportError::StartFailed(format!("bind failed: {}", e)))?; + ignore_port_unreachable(&sock); // Set socket buffer sizes sock.set_recv_buffer_size(recv_buf_size) @@ -75,6 +79,7 @@ impl UdpRawSocket { sock.set_nonblocking(true) .map_err(|e| TransportError::StartFailed(format!("set nonblocking failed: {}", e)))?; + ignore_port_unreachable(&sock); sock.set_recv_buffer_size(recv_buf_size) .map_err(|e| TransportError::StartFailed(format!("set recv buffer: {}", e)))?; @@ -126,6 +131,44 @@ impl UdpRawSocket { } } +/// Stop an ICMP port-unreachable from failing the socket's next receive. +/// +/// By default Windows reports a port-unreachable answering any datagram the +/// socket sent as `WSAECONNRESET` on its next `recv_from`. The socket is shared +/// by every peer, so one unreachable address would put errors into the receive +/// loop that all the other peers' traffic arrives through. `SIO_UDP_CONNRESET` +/// set to false turns the report off. A socket that refuses it still works and +/// still sees the errors, so a failure is logged rather than returned. +fn ignore_port_unreachable(sock: &Socket) { + let report: u32 = 0; + let mut returned: u32 = 0; + // SAFETY: the socket is open for the duration of the call, the input + // buffer is a live u32 of the stated size, no output buffer is passed, and + // the call is synchronous (no OVERLAPPED, no completion routine). + let rc = unsafe { + WSAIoctl( + sock.as_raw_socket() as SOCKET, + SIO_UDP_CONNRESET, + (&report as *const u32).cast(), + std::mem::size_of::() as u32, + std::ptr::null_mut(), + 0, + &mut returned, + std::ptr::null_mut(), + None, + ) + }; + if rc == SOCKET_ERROR { + // Read before `warn!`, whose level and dispatcher checks run first and + // could replace the error code WSAIoctl left. + let err = std::io::Error::last_os_error(); + warn!( + error = %err, + "Could not turn off ICMP port-unreachable reports on the UDP socket" + ); + } +} + /// Async UDP socket wrapper (Windows). /// /// Uses `tokio::net::UdpSocket` directly. Kernel drop counting From 70b5b06b57aa10f6ea980deba78d1331cf557cf9 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:41:56 +0000 Subject: [PATCH 5/6] Keep native client test descriptors alive until the client holds them, and correct a stale test comment Three native client tests stand in for the daemon: they send a socket to the client and then dropped their own copy straight away. Until the client read the message, the message was the socket's only reference, and Darwin's descriptor collector can flush such a socket, so the tests failed now and then on macOS. Each now keeps its copy until the client holds the descriptor, as the daemon itself does. Two of them also provoke a collection in that window, so a regression can show on macOS without waiting for a stray collection; in the bind test the stand-in daemon keeps its copy until the client closes the connection. A comment in the native API tests still described the earlier wait for a closed flow, which watched only the reader's close flag. It now says what the wait observes, a flagged or forgotten flow, and why neither means the release has been served yet. --- src/native/client/mod.rs | 20 +++++++++++++++++--- src/native/mod.rs | 10 ++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/native/client/mod.rs b/src/native/client/mod.rs index f02ec7f1..90dd9d64 100644 --- a/src/native/client/mod.rs +++ b/src/native/client/mod.rs @@ -1092,7 +1092,12 @@ mod tests { // already queued and no read here waits on anything. (&daemon).write_all(REFUSAL).unwrap(); fdpass::send_once(daemon.as_raw_fd(), CONNECT_REPLY, Some(passed.as_fd())).unwrap(); - drop(passed); + // `passed` stays alive until the client holds the descriptor: until + // then the message would be the socket's only reference, and Darwin's + // collector flushes such a socket. The provoked collection is there so + // a regression shows on macOS without waiting for a stray collection; + // whether it shows every run rests on `dgram_probe`'s fixed wait. + crate::native::dgram_probe::provoke_collection(); // **How many reads this takes is the platform's business, and asserting // it was wrong.** Linux coalesces the plain write with the sendmsg that @@ -1133,6 +1138,7 @@ mod tests { let (second, second_fd) = wire.line().unwrap(); assert!(second.starts_with(br#"{"status":"ok""#)); let received = second_fd.expect("the reply must carry the descriptor"); + drop(passed); // A live socket rather than merely a number: the far half sees it. let mut received = UnixStream::from(received); @@ -1207,7 +1213,6 @@ mod tests { let worker = thread::spawn(move || { let command = read_line(&daemon); fdpass::send_once(daemon.as_raw_fd(), LISTEN_REPLY, Some(passed.as_fd())).unwrap(); - drop(passed); // As for connect: setup is over, so the connection is over. A // listener that held one would take its flows down with it. // A deadline, or the defect this asserts against fails as a hang @@ -1220,6 +1225,9 @@ mod tests { let read = (&daemon) .read(&mut rest) .expect("the RPC connection outlived the setup call that opened it"); + // Only now: the client closes once it holds the descriptor, and + // until then the message would be the socket's only reference. + drop(passed); (command, read) }); @@ -1308,9 +1316,15 @@ mod tests { // client could read the arrival is already there when it holds it. (&held).write_all(b"opening").unwrap(); fdpass::send_once(theirs.as_raw_fd(), ARRIVAL, Some(passed.as_fd())).unwrap(); - drop(passed); + // `passed` stays alive until the client holds the descriptor: until + // then the message would be the socket's only reference, and Darwin's + // collector flushes such a socket. The provoked collection is there so + // a regression shows on macOS without waiting for a stray collection; + // whether it shows every run rests on `dgram_probe`'s fixed wait. + crate::native::dgram_probe::provoke_collection(); let (flow, peer) = listener.accept().unwrap(); + drop(passed); assert_eq!(peer.to_string(), format!("{PEER}:5001")); assert_eq!(flow.peer_addr(), peer); // From the arrival's own `node`, not from the listener: an accepted diff --git a/src/native/mod.rs b/src/native/mod.rs index 305c5760..9f599588 100644 --- a/src/native/mod.rs +++ b/src/native/mod.rs @@ -1595,10 +1595,12 @@ mod tests { drop(client); connection.settle_closed(flow).await; - // `settle_closed` observes the reader's flag, which it sets before it - // sends the release, so the registry may not have processed it yet. - // Retry rather than sleep: without the reclaim every attempt fails and - // the loop runs out, which is the failure this test exists to produce. + // `settle_closed` returns once the reader has flagged the flow closed or + // forgotten it. Neither means the registry has served the release: the + // flag is set before the release is queued, and the flow is forgotten + // once it is queued, not once it is processed. Retry rather than sleep: + // without the reclaim every attempt fails and the loop runs out, which + // is the failure this test exists to produce. let mut last = serde_json::Value::Null; for _ in 0..1000 { // Not `ask`: a connect that succeeds carries a descriptor, and that From 5c2f52f8cc5e202f7c87bbc180d1ba1f654e27bd Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:42:24 +0000 Subject: [PATCH 6/6] Changelog: third v0.5.3 landing batch Entries for the Ethernet address display fix, the macOS ordered sender fix, the Windows UDP port-unreachable fix, and clearing a connection's handshake slot when its whole handshake state leaves the machine. --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f251afa..985fa651 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,12 +82,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 leaked one descriptor each time. - TCP connections try the remaining addresses for a hostname after a connection fails, within the existing overall connection timeout. +- An Ethernet peer's address is always shown as a colon-separated MAC address + in `fipsctl show links` and in log lines. An address whose six bytes happened + to be valid UTF-8 was printed as text instead. #### macOS - If an encrypt worker thread exits, the daemon no longer stops once that worker's send queue fills. Packets for that worker are now dropped instead of blocking forever. This applies to the default sender. +- With the opt-in ordered sender (`FIPS_MACOS_ORDERED_SENDER`), the daemon no + longer stops forwarding when an encrypt worker thread or a destination's send + thread exits. Packets that thread would have handled are dropped instead of + holding up every later packet to the same destination. #### Native datagram API @@ -153,6 +160,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 foreground run reads, which it omitted, and says to stop the service before rerunning `install-service.ps1` to upgrade: with the service running, the installer fails copying `fips.exe`. +- An ICMP port-unreachable answering a datagram the node sent no longer shows + up as a receive error on the UDP transport's socket. Windows reports one on + the socket's next receive by default, and that socket is shared by every UDP + peer, so one unreachable peer address put errors into the receive path that + every other UDP peer's traffic uses. ### Security @@ -210,6 +222,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 handshake. The security reference now states what clearing key material in memory does and does not cover in a release build, including copies left behind by moves and the identity loaded from a secret string. +- A connection's handshake state is now cleared from its control machine when + the connection is promoted to an active peer, resolved as one side of a + cross-connection, or reaped as stale. After a promotion, the memory it left + held the session's two traffic keys for as long as the peer stayed + connected. The security reference is updated to match. ## [0.5.2] - 2026-09-28