From 3092c95d5406dc07a8204c8792006c5f0a474555 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 29 Apr 2026 22:32:24 +0000 Subject: [PATCH] Filter stray punch probes on adopted UDP transports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a UDP hole-punch succeeds in only one direction and the local side adopts the punched socket, the remote end keeps retrying its own punch attempt for several seconds. Those retries arrive on the adopted socket and were forwarded to the FMP rx handler, which parsed the first byte (0x4E from PUNCH_MAGIC's "NPTC" big-endian encoding) as FMP protocol version 4 and emitted "Unknown FMP version, dropping" once per probe. The probe stream contaminated post-adoption handshake logs and added timing pressure during the handshake window. Add a transport-level filter in udp_receive_loop that silently drops any datagram whose first 4 bytes match PUNCH_MAGIC or PUNCH_ACK_MAGIC. Filter applies to all UDP transports, not just adopted ones — the magic values cannot collide with valid FMP frames (FMP version 4 is not assigned, and the protocol's versioning is wire-format breaking), so universal filtering is safe and removes any "is this an adopted socket" branching. Move PUNCH_MAGIC / PUNCH_ACK_MAGIC and the new is_punch_packet() helper from the `nostr-discovery`-gated submodule up to crate::discovery (unconditionally compiled) so the UDP transport can import them without requiring the feature. The nostr-discovery types module re-exports the constants so the existing traversal-side imports keep working unchanged. Test: pushes a probe + ack + real frame through the receive loop and asserts only the real frame is delivered to packet_tx. --- src/discovery.rs | 24 ++++++++++++ src/discovery/nostr/types.rs | 5 ++- src/transport/udp/mod.rs | 75 ++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/discovery.rs b/src/discovery.rs index 838b8f2..290d023 100644 --- a/src/discovery.rs +++ b/src/discovery.rs @@ -13,6 +13,30 @@ use crate::config::UdpConfig; use crate::{NodeAddr, TransportId}; use std::net::{SocketAddr, UdpSocket}; +/// Punch-probe magic ("NPTC", network byte order). First byte `0x4E` +/// collides with FMP's prefix-version high-nibble check, so the UDP +/// transport silently filters packets carrying this magic to keep +/// post-adoption handshake logs clean. Defined here (unconditionally +/// compiled) rather than inside the `nostr-discovery`-gated submodule so +/// the filter applies regardless of feature configuration. +pub const PUNCH_MAGIC: u32 = 0x4E505443; + +/// Punch-probe-ack magic ("NPTA", network byte order). Same filter as +/// [`PUNCH_MAGIC`]. +pub const PUNCH_ACK_MAGIC: u32 = 0x4E505441; + +/// Returns `true` if the first four bytes of `data` match a punch-probe or +/// punch-ack magic. Used by the UDP transport's receive loop to silently +/// drop stray probes that arrive on an adopted socket after the remote +/// peer's punch attempt has already timed out. +pub fn is_punch_packet(data: &[u8]) -> bool { + if data.len() < 4 { + return false; + } + let magic = u32::from_be_bytes([data[0], data[1], data[2], data[3]]); + magic == PUNCH_MAGIC || magic == PUNCH_ACK_MAGIC +} + /// Result of handing an established traversal session into FIPS. #[derive(Debug, Clone)] pub struct BootstrapHandoffResult { diff --git a/src/discovery/nostr/types.rs b/src/discovery/nostr/types.rs index fda8b48..4a5ece3 100644 --- a/src/discovery/nostr/types.rs +++ b/src/discovery/nostr/types.rs @@ -6,8 +6,9 @@ pub const ADVERT_KIND: u16 = 37195; pub const ADVERT_IDENTIFIER: &str = "fips-overlay-v1"; pub const ADVERT_VERSION: u32 = 1; pub const SIGNAL_KIND: u16 = 21059; -pub const PUNCH_MAGIC: u32 = 0x4E505443; -pub const PUNCH_ACK_MAGIC: u32 = 0x4E505441; +// Re-exported from `crate::discovery` so the UDP transport's stray-probe +// filter compiles regardless of the `nostr-discovery` cargo feature. +pub use crate::discovery::{PUNCH_ACK_MAGIC, PUNCH_MAGIC}; pub const PROTOCOL_VERSION: &str = "1"; #[derive(Debug, thiserror::Error)] diff --git a/src/transport/udp/mod.rs b/src/transport/udp/mod.rs index 50bfa05..a5b2482 100644 --- a/src/transport/udp/mod.rs +++ b/src/transport/udp/mod.rs @@ -10,6 +10,7 @@ mod socket; mod stats; use super::resolve_socket_addr; use crate::config::UdpConfig; +use crate::discovery::is_punch_packet; use socket::{AsyncUdpSocket, UdpRawSocket}; use stats::UdpStats; use std::collections::HashMap; @@ -408,6 +409,22 @@ async fn udp_receive_loop( stats.record_recv(len); stats.set_kernel_drops(kernel_drops as u64); + // Drop stray punch probes / acks. After bootstrap-handoff + // adopts a socket, the remote side may keep retrying its + // own punch attempt for several seconds; those probes + // arrive here and would otherwise be parsed as FMP frames + // (their first byte 0x4E has high-nibble 0x4 → bogus + // "FMP version 4" warnings). + if is_punch_packet(&buf[..len]) { + trace!( + transport_id = %transport_id, + remote_addr = %remote_addr, + bytes = len, + "Dropping stray punch probe/ack on UDP transport" + ); + continue; + } + let data = buf[..len].to_vec(); let addr = TransportAddr::from_string(&remote_addr.to_string()); let packet = ReceivedPacket::new(transport_id, addr, data); @@ -688,6 +705,64 @@ mod tests { assert_eq!(cong.recv_drops, Some(0)); } + #[tokio::test] + async fn test_punch_probe_dropped() { + let (tx_recv, mut rx_recv) = packet_channel(100); + let (tx_send, _rx_send) = packet_channel(100); + + let mut t_recv = UdpTransport::new(TransportId::new(1), None, make_config(0), tx_recv); + let mut t_send = UdpTransport::new(TransportId::new(2), None, make_config(0), tx_send); + + t_recv.start_async().await.unwrap(); + t_send.start_async().await.unwrap(); + + let recv_addr = t_recv.local_addr().unwrap(); + let recv_addr_str = TransportAddr::from_string(&recv_addr.to_string()); + + // Probe (PUNCH_MAGIC = "NPTC", be) followed by sequence + payload. + let mut probe = vec![0u8; 16]; + probe[..4].copy_from_slice(&0x4E505443u32.to_be_bytes()); + t_send.send_async(&recv_addr_str, &probe).await.unwrap(); + + // Ack (PUNCH_ACK_MAGIC = "NPTA", be). + let mut ack = vec![0u8; 16]; + ack[..4].copy_from_slice(&0x4E505441u32.to_be_bytes()); + t_send.send_async(&recv_addr_str, &ack).await.unwrap(); + + // A real (non-punch) packet must still arrive. + let real = b"valid-fmp-frame"; + t_send.send_async(&recv_addr_str, real).await.unwrap(); + + // First message read should be the real one — punch probe + ack + // both filtered silently. + let packet = timeout(Duration::from_secs(1), rx_recv.recv()) + .await + .expect("timeout waiting for real packet") + .expect("channel closed"); + assert_eq!(packet.data, real); + + // No further packets should be queued (probe + ack dropped). + let no_more = timeout(Duration::from_millis(200), rx_recv.recv()).await; + assert!(no_more.is_err(), "punch probe/ack leaked through filter"); + + t_recv.stop_async().await.unwrap(); + t_send.stop_async().await.unwrap(); + } + + #[test] + fn test_is_punch_packet_helper() { + use crate::discovery::is_punch_packet; + // PUNCH_MAGIC ("NPTC", be) + assert!(is_punch_packet(&[0x4E, 0x50, 0x54, 0x43, 0xAA, 0xBB])); + // PUNCH_ACK_MAGIC ("NPTA", be) + assert!(is_punch_packet(&[0x4E, 0x50, 0x54, 0x41])); + // Non-magic packet + assert!(!is_punch_packet(&[0x01, 0x02, 0x03, 0x04])); + // Too short + assert!(!is_punch_packet(&[0x4E, 0x50, 0x54])); + assert!(!is_punch_packet(&[])); + } + #[tokio::test] async fn test_send_recv_ip_string() { let (tx1, _rx1) = packet_channel(100);