From 19efe06622c2c009831de2ea7fac2e22c0207a03 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 21 Feb 2026 14:18:48 +0000 Subject: [PATCH] Add dest_coords to SessionAck for return-path routing SessionAck previously only carried the responder's coordinates (src_coords). When the return path diverged from the forward path (e.g., after tree reconvergence), transit nodes on the return path lacked the initiator's coordinates and couldn't route the SessionAck back, causing handshake timeouts. Add dest_coords (initiator's coordinates) to the SessionAck wire format, mirroring SessionSetup's design. Transit nodes now cache both endpoints' coordinates when forwarding a SessionAck, making the return path self-sufficient regardless of path asymmetry. Root cause confirmed by churn-20 sim log analysis: the n04-n14 handshake failure was caused by n15 (return-path transit) lacking n04's coordinates, not by stale tree routes through a downed node. --- docs/design/fips-mesh-operation.md | 10 ++++++---- docs/design/fips-session-layer.md | 8 +++++--- docs/design/fips-wire-formats.md | 6 ++++-- src/node/handlers/forwarding.rs | 6 ++++++ src/node/handlers/session.rs | 4 ++-- src/node/tests/forwarding.rs | 11 +++++++---- src/node/tests/session.rs | 5 +++-- src/protocol/session.rs | 21 +++++++++++++++++---- 8 files changed, 50 insertions(+), 21 deletions(-) diff --git a/docs/design/fips-mesh-operation.md b/docs/design/fips-mesh-operation.md index 345eebd..fb7b9d4 100644 --- a/docs/design/fips-mesh-operation.md +++ b/docs/design/fips-mesh-operation.md @@ -220,7 +220,7 @@ transit, CP-flagged data packets, LookupResponse — write to the same cache. | Source | When | What | | ------ | ---- | ---- | | SessionSetup transit | Session establishment | Both src and dest coordinates | -| SessionAck transit | Session establishment | Responder's coordinates | +| SessionAck transit | Session establishment | Both src and dest coordinates | | CP-flagged data packet | Warmup or recovery | Both src and dest coordinates (cleartext) | | LookupResponse | Discovery | Target's coordinates | @@ -329,8 +329,10 @@ As the SessionSetup transits each intermediate node: coordinate cache 3. Forwards the message using the cached destination coordinates -SessionAck returns along the reverse path, carrying the responder's -coordinates and warming caches in the other direction. +SessionAck returns along the reverse path, carrying both the responder's +and initiator's coordinates and warming caches in the other direction. This +ensures return-path transit nodes can route even when the reverse path +diverges from the forward path (e.g., after tree reconvergence). ### Result @@ -532,7 +534,7 @@ routing decisions but retains its own end-to-end encryption and identity. | LookupRequest | ~300 bytes | First contact, recovery | Yes (flood) | | LookupResponse | ~400 bytes | Response to discovery | Yes (greedy routed) | | SessionDatagram + SessionSetup | ~232–402 bytes | Session establishment | Yes (routed) | -| SessionDatagram + SessionAck | ~122 bytes | Session confirmation | Yes (routed) | +| SessionDatagram + SessionAck | ~170 bytes | Session confirmation | Yes (routed) | | SessionDatagram + Data (minimal) | 106 bytes + payload | Bulk traffic | Yes (routed) | | SessionDatagram + Data (with CP) | 106 + coords + payload | Warmup/recovery | Yes (routed) | | SessionDatagram + CoordsRequired | 70 bytes | Cache miss error | Yes (routed) | diff --git a/docs/design/fips-session-layer.md b/docs/design/fips-session-layer.md index cce3ab6..ddcbc80 100644 --- a/docs/design/fips-session-layer.md +++ b/docs/design/fips-session-layer.md @@ -116,7 +116,7 @@ The handshake is carried in SessionSetup and SessionAck messages: 1. **Initiator** sends SessionSetup containing Noise IK msg1 and both parties' tree coordinates 2. **Responder** processes msg1, learns initiator identity, sends SessionAck - containing Noise IK msg2 and its own coordinates + containing Noise IK msg2 and both parties' tree coordinates 3. Both parties derive identical symmetric session keys Packets that trigger session establishment are queued (with bounded buffer) @@ -130,8 +130,10 @@ As the message transits intermediate nodes, each node caches these coordinates, warming the path for subsequent data packets that carry only addresses (no coordinates). -SessionAck carries the responder's coordinates back along the reverse path, -warming caches in the other direction. +SessionAck carries both the responder's and initiator's coordinates back +along the reverse path, warming caches in the other direction. This ensures +return-path transit nodes can route even when the reverse path diverges from +the forward path (e.g., after tree reconvergence). ### Simultaneous Initiation diff --git a/docs/design/fips-wire-formats.md b/docs/design/fips-wire-formats.md index 2f65197..115cb5e 100644 --- a/docs/design/fips-wire-formats.md +++ b/docs/design/fips-wire-formats.md @@ -566,8 +566,10 @@ Encoded with FSP prefix: ver=0, phase=0x2, flags=0, payload_len. | Offset | Field | Size | Description | | ------ | ----- | ---- | ----------- | | 0 | flags | 1 byte | Reserved | -| 1 | src_coords_count | 2 bytes LE | Number of coordinate entries | +| 1 | src_coords_count | 2 bytes LE | Number of acknowledger coordinate entries | | 3 | src_coords | 16 x n bytes | Acknowledger's ancestry (for cache warming) | +| ... | dest_coords_count | 2 bytes LE | Number of initiator coordinate entries | +| ... | dest_coords | 16 x m bytes | Initiator's ancestry (for return-path cache warming) | | ... | handshake_len | 2 bytes LE | Noise payload length | | ... | handshake_payload | variable | Noise IK msg2 (33 bytes typical) | @@ -730,7 +732,7 @@ endpoint session keys). | Message | Typical Size | Notes | | ------- | ------------ | ----- | | SessionSetup | ~200 bytes | Depth-dependent | -| SessionAck | ~80 bytes | Depth-dependent | +| SessionAck | ~130 bytes | Depth-dependent (carries both endpoints' coords) | | Data (minimal) | 12 + 6 + payload + 16 bytes | Steady state | | Data (with coords) | 12 + ~130 + 6 + payload + 16 bytes | Warmup/recovery | | SenderReport | 12 + 6 + 46 + 16 bytes | MMP metrics | diff --git a/src/node/handlers/forwarding.rs b/src/node/handlers/forwarding.rs index d56f16c..0cc5440 100644 --- a/src/node/handlers/forwarding.rs +++ b/src/node/handlers/forwarding.rs @@ -138,8 +138,14 @@ impl Node { ack.src_coords, now_ms, ); + self.coord_cache_mut().insert( + datagram.dest_addr, + ack.dest_coords, + now_ms, + ); debug!( src = %datagram.src_addr, + dest = %datagram.dest_addr, "Cached coords from SessionAck" ); } diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 136c430..28aceec 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -364,9 +364,9 @@ impl Node { } }; - // Build and send SessionAck + // Build and send SessionAck (include initiator's coords for return-path warming) let our_coords = self.tree_state.my_coords().clone(); - let ack = SessionAck::new(our_coords).with_handshake(msg2); + let ack = SessionAck::new(our_coords, setup.src_coords).with_handshake(msg2); let ack_payload = ack.encode(); let my_addr = *self.node_addr(); let mut datagram = SessionDatagram::new(my_addr, *src_addr, ack_payload.clone()) diff --git a/src/node/tests/forwarding.rs b/src/node/tests/forwarding.rs index 1cdb2c6..1070e17 100644 --- a/src/node/tests/forwarding.rs +++ b/src/node/tests/forwarding.rs @@ -159,8 +159,9 @@ async fn test_coord_cache_warming_session_ack() { let root_addr = make_node_addr(0xF0); let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); + let dest_coords = TreeCoordinate::from_addrs(vec![dest_addr, root_addr]).unwrap(); - let ack = SessionAck::new(src_coords.clone()); + let ack = SessionAck::new(src_coords.clone(), dest_coords.clone()); let ack_payload = ack.encode(); let dg = SessionDatagram::new(src_addr, dest_addr, ack_payload); @@ -172,16 +173,18 @@ async fn test_coord_cache_warming_session_ack() { .as_millis() as u64; assert!(node.coord_cache().get(&src_addr, now_ms).is_none()); + assert!(node.coord_cache().get(&dest_addr, now_ms).is_none()); node.handle_session_datagram(&from, &encoded[1..]).await; - // SessionAck only caches src_coords (the acknowledger's coords) + // SessionAck caches both src_coords and dest_coords let cached_src = node.coord_cache().get(&src_addr, now_ms); assert!(cached_src.is_some(), "src_addr coords not cached from SessionAck"); assert_eq!(cached_src.unwrap().root_id(), &root_addr); - // dest_addr should NOT be cached (SessionAck doesn't carry dest coords) - assert!(node.coord_cache().get(&dest_addr, now_ms).is_none()); + let cached_dest = node.coord_cache().get(&dest_addr, now_ms); + assert!(cached_dest.is_some(), "dest_addr coords not cached from SessionAck"); + assert_eq!(cached_dest.unwrap().root_id(), &root_addr); } #[tokio::test] diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 74b3f32..012d115 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -424,8 +424,9 @@ async fn test_session_ack_for_unknown_session() { let node1_addr = *nodes[1].node.node_addr(); // Fabricate a SessionAck and deliver directly - let coords = nodes[1].node.tree_state().my_coords().clone(); - let ack = SessionAck::new(coords).with_handshake(vec![0u8; 33]); + let src_coords = nodes[1].node.tree_state().my_coords().clone(); + let dest_coords = nodes[0].node.tree_state().my_coords().clone(); + let ack = SessionAck::new(src_coords, dest_coords).with_handshake(vec![0u8; 33]); let datagram = SessionDatagram::new(node1_addr, node0_addr, ack.encode()); // Send through link layer diff --git a/src/protocol/session.rs b/src/protocol/session.rs index 2ab465d..8d69d89 100644 --- a/src/protocol/session.rs +++ b/src/protocol/session.rs @@ -446,8 +446,10 @@ impl SessionSetup { /// Session acknowledgement. /// /// Carried inside a SessionDatagram envelope which provides src_addr and -/// dest_addr. The SessionAck payload contains the acknowledger's coordinates -/// for route cache warming and the Noise IK handshake response. +/// dest_addr. The SessionAck payload contains both the acknowledger's and +/// initiator's coordinates for route cache warming (ensuring return-path +/// transit nodes can route independently of the forward path) and the Noise +/// IK handshake response. /// /// ## Wire Format /// @@ -457,12 +459,16 @@ impl SessionSetup { /// | 1 | flags | 1 byte | Reserved | /// | 2 | src_coords_count | 2 bytes | u16 LE | /// | 4 | src_coords | 16 × n | Acknowledger's coords (for caching) | +/// | ... | dest_coords_count| 2 bytes | u16 LE | +/// | ... | dest_coords | 16 × m | Initiator's coords (for return path)| /// | ... | handshake_len | 2 bytes | u16 LE, Noise payload length | /// | ... | handshake_payload| variable| Noise IK msg2 (33 bytes typical) | #[derive(Clone, Debug)] pub struct SessionAck { /// Acknowledger's coordinates. pub src_coords: TreeCoordinate, + /// Initiator's coordinates (for return-path cache warming). + pub dest_coords: TreeCoordinate, /// Reserved flags byte (for forward compatibility). pub flags: u8, /// Noise IK handshake message 2. @@ -471,9 +477,10 @@ pub struct SessionAck { impl SessionAck { /// Create a new session acknowledgement. - pub fn new(src_coords: TreeCoordinate) -> Self { + pub fn new(src_coords: TreeCoordinate, dest_coords: TreeCoordinate) -> Self { Self { src_coords, + dest_coords, flags: 0, handshake_payload: Vec::new(), } @@ -494,6 +501,7 @@ impl SessionAck { let mut body = Vec::new(); body.push(self.flags); encode_coords(&self.src_coords, &mut body); + encode_coords(&self.dest_coords, &mut body); let hs_len = self.handshake_payload.len() as u16; body.extend_from_slice(&hs_len.to_le_bytes()); body.extend_from_slice(&self.handshake_payload); @@ -522,6 +530,9 @@ impl SessionAck { let (src_coords, consumed) = decode_coords(&payload[offset..])?; offset += consumed; + let (dest_coords, consumed) = decode_coords(&payload[offset..])?; + offset += consumed; + if payload.len() < offset + 2 { return Err(ProtocolError::MessageTooShort { expected: offset + 2, @@ -541,6 +552,7 @@ impl SessionAck { Ok(Self { src_coords, + dest_coords, flags, handshake_payload, }) @@ -1079,7 +1091,7 @@ mod tests { #[test] fn test_session_ack_encode_decode() { let handshake = vec![0xBB; 33]; // typical Noise IK msg2 - let ack = SessionAck::new(make_coords(&[7, 8, 0])) + let ack = SessionAck::new(make_coords(&[7, 8, 0]), make_coords(&[3, 4, 0])) .with_handshake(handshake.clone()); let encoded = ack.encode(); @@ -1089,6 +1101,7 @@ mod tests { let decoded = SessionAck::decode(&encoded[4..]).unwrap(); assert_eq!(decoded.src_coords, ack.src_coords); + assert_eq!(decoded.dest_coords, ack.dest_coords); assert_eq!(decoded.handshake_payload, handshake); }