From b4c04ba8216e8f5d5dc556d2eb9e13f5fb0e11c8 Mon Sep 17 00:00:00 2001 From: op-q <141008398+op-q@users.noreply.github.com> Date: Tue, 15 Sep 2026 12:46:04 +0200 Subject: [PATCH] Keep waiting for the receiver when a stray connection attempt fails A direct-path sender ended its whole transfer if anything reached its socket first with a QUIC handshake that failed. The network lab traced the intermittent `authentication failed` to exactly that: a late packet from a relay's address-discovery port was taken for an incoming connection, failed, and ended the send before the real receiver dialled. Plain LAN passed 1 run in 5 on main. On a public address, anyone who can send one UDP packet could do the same on purpose. A failed handshake is now dropped and the wait goes on. It is not the receiver, which completes the handshake, and not a guess, which needs a completed handshake first, so the one-guess rule is untouched. After the fix, plain LAN passed 8 runs in 8. A unit test sends a junk QUIC Initial to a waiting sender and then connects a real receiver; with the old behaviour it fails 3 times in 3. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01G7Fy45hUvna94cd79WKG8S --- cli/src/transport/quic.rs | 107 ++++++++++++++++++++-- docs/plans/network-lab-plan-2026-08-31.md | 31 +++++++ 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/cli/src/transport/quic.rs b/cli/src/transport/quic.rs index 3467f01..94a2ee4 100644 --- a/cli/src/transport/quic.rs +++ b/cli/src/transport/quic.rs @@ -148,14 +148,28 @@ impl QuicEndpoint { /// the transfer with nothing to explain why. What replaces it is not a /// looser rule but a differently enforced one: the caller may come back for /// another attempt, and entry 13 requires it to ask a human first. + /// + /// **A connection attempt that fails its handshake is ignored**, and the + /// wait goes on. It is not the receiver: the receiver's endpoint completes + /// the handshake, or it could not have dialled this one by its id. It is not + /// a guess either, since a guess is only made after a completed handshake, + /// over the transfer's own key exchange, so it costs nothing against the + /// one-guess rule. It used to end the whole send. A stray packet was + /// enough, and the network lab caught one: a late reply from a relay's + /// address-discovery port arrived on the sender's socket, failed with + /// `authentication failed`, and killed the direct path in most runs. On a + /// public address anyone could do the same on purpose. pub async fn accept_transfer(&self) -> Result { - let incoming = self.endpoint.accept().await.ok_or_else(|| { - TransportError::Connect("the endpoint closed before a peer connected".into()) - })?; + let connection = loop { + let incoming = self.endpoint.accept().await.ok_or_else(|| { + TransportError::Connect("the endpoint closed before a peer connected".into()) + })?; - let connection = incoming.await.map_err(|error| { - TransportError::Connect(format!("a peer failed to complete the handshake: {error}")) - })?; + match incoming.await { + Ok(connection) => break connection, + Err(_) => continue, + } + }; let (send, recv) = connection.open_bi().await.map_err(|error| { TransportError::Io(format!("could not open a transfer stream: {error}")) @@ -542,4 +556,85 @@ mod tests { std::fs::remove_dir_all(&base).ok(); } + + /// A connection attempt that fails its handshake must not end the wait + /// for the real receiver. + /// + /// The shape the network lab caught: something that is not the receiver + /// reaches the sender's socket first with a QUIC Initial that cannot be + /// authenticated (there, a late reply from a relay's address-discovery + /// port). Here it is sent on purpose, as anyone on the network could, and + /// then the real receiver connects. **Negative control:** with + /// `accept_transfer` returning the handshake error, as it did, this fails + /// with "a peer failed to complete the handshake". + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn a_stray_handshake_does_not_end_the_wait_for_the_receiver() { + let sender = QuicEndpoint::bind_without_relays() + .await + .expect("a sender endpoint"); + let receiver = QuicEndpoint::bind_without_relays() + .await + .expect("a receiver endpoint"); + let address = sender.addr(); + + let target = *address + .ip_addrs() + .find(|addr| addr.is_ipv4()) + .expect("an IPv4 address to aim at"); + let target = std::net::SocketAddr::new(std::net::Ipv4Addr::LOCALHOST.into(), target.port()); + + // Holds the accepted connection and speaks first, as a real sender + // does, so the receiver's side resolves on a live stream rather than + // racing a connection being dropped. + let accepting = tokio::spawn(async move { + let mut transport = sender.accept_transfer().await?; + transport + .send_control(serde_json::json!({ "type": "hello" })) + .await?; + Ok::<_, crate::transport::TransportError>((sender, transport)) + }); + + // A QUIC v1 Initial that parses as one and cannot be decrypted: a + // long header, random connection ids, and a payload of noise, padded + // to the 1200 bytes an Initial must be. + let mut packet = vec![0xC3_u8, 0x00, 0x00, 0x00, 0x01, 8]; + packet.extend_from_slice(&[0x5A; 8]); + packet.push(8); + packet.extend_from_slice(&[0xA5; 8]); + packet.push(0x00); + let remaining = 1200 - packet.len() - 2; + packet.extend_from_slice(&[0x40 | ((remaining >> 8) as u8), remaining as u8]); + packet.extend((0..remaining).map(|index| (index * 131 % 251) as u8)); + + let stray = std::net::UdpSocket::bind("127.0.0.1:0").expect("a stray socket"); + for _ in 0..3 { + stray + .send_to(&packet, target) + .expect("the stray packet was sent"); + } + tokio::time::sleep(Duration::from_millis(200)).await; + + let mut connected = + tokio::time::timeout(Duration::from_secs(10), receiver.connect_transfer(address)) + .await + .expect("the receiver connected in time") + .expect("the receiver could not connect"); + + let (sender, mut accepted) = tokio::time::timeout(Duration::from_secs(10), accepting) + .await + .expect("the sender accepted in time") + .expect("the accepting task ran") + .unwrap_or_else(|error| { + panic!("a stray handshake ended the wait for the real receiver: {error}") + }); + + let Some(Frame::Control(frame)) = connected.receive().await.expect("a frame") else { + panic!("the real receiver should hear the sender over a live stream"); + }; + assert_eq!(frame["type"], "hello"); + + accepted.close().await; + sender.shutdown().await; + receiver.shutdown().await; + } } diff --git a/docs/plans/network-lab-plan-2026-08-31.md b/docs/plans/network-lab-plan-2026-08-31.md index 436fde7..16a8753 100644 --- a/docs/plans/network-lab-plan-2026-08-31.md +++ b/docs/plans/network-lab-plan-2026-08-31.md @@ -751,6 +751,37 @@ use and reports what that said, naming the refusing knob as a hint. Predicting a capability you are about to depend on irrecoverably is the bug; the fix is to try it while there is still a process left to report. +#### `authentication failed`, explained and fixed, 2026-09-15 + +The intermittent failure first seen in the full-cone row, and later in plain LAN, +was **not in iroh's traversal and not in TLS verification.** A traced run +(`iroh=debug`, from a temporary subscriber that was not committed) shows the +failing sender's last moments: + +```text +iroh::_events::conn::incoming: remote_addr=Ip(10.40.0.2:7842) +noq_proto::endpoint: failed to authenticate initial packet +error: a peer failed to complete the handshake: ... authentication failed +``` + +`10.40.0.2:7842` is the rendezvous host's QUIC address-discovery port. A late +packet from the address-discovery exchange reached the sender's socket and was +taken for an incoming connection. It failed its handshake, and +`QuicEndpoint::accept_transfer` returned that failure, **ending the whole send** +before the real receiver dialled. The receiver then timed out reaching a sender +that had already gone. + +Fixed on `fix/accept-survives-stray-handshakes`: a connection attempt that fails +its handshake is dropped and the wait goes on. It is neither the receiver, which +completes the handshake, nor a guess, which needs a completed handshake first. +Measured on this machine, plain LAN on `main` passed **1 run in 5** before the fix +and **8 in 8** after. A unit test sends a junk QUIC Initial to a waiting sender +and then connects a real receiver; with the old behaviour restored it fails 3 +times in 3. + +The same failure is reachable in production. Anyone who can send one UDP packet +to a sender waiting on a public address could end its transfer. + ### Phase 5 — Reporting and CI - [ ] `netlab/report.py` — writes `docs/validation/network-lab--YYYY-MM-DD.md`