Filter stray punch probes on adopted UDP transports

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.
This commit is contained in:
Johnathan Corgan
2026-04-30 03:10:56 +00:00
parent c8502cdb97
commit 3092c95d54
3 changed files with 102 additions and 2 deletions
+24
View File
@@ -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 {
+3 -2
View File
@@ -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)]
+75
View File
@@ -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);