diff --git a/docs/design/fips-mesh-operation.md b/docs/design/fips-mesh-operation.md index 91ccd88e..c38d1581 100644 --- a/docs/design/fips-mesh-operation.md +++ b/docs/design/fips-mesh-operation.md @@ -384,8 +384,8 @@ source. whose signals all arrive over one link never demotes this way; its verified coordinates last until discovery replaces them or their verification ages out after 300 seconds. -3. Initiate discovery for the destination, whether or not its identity is - cached +3. Initiate discovery for the destination. If its identity is not cached, + cache it first from the session's key, so the response can be verified 4. Reset CP warmup counter The source also counts, without refusing anything, a PathBroken that diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 2b897aab..4d4c1f7b 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -2024,6 +2024,7 @@ impl Node { "PathBroken quorum reached; demoted verified coordinates to a hint"); } FspAction::InitiateLookup { dest } => { + self.cache_session_identity(&dest); self.maybe_initiate_lookup(&dest).await; } _ => {} @@ -2071,6 +2072,23 @@ impl Node { } } + /// Cache the identity of `dest` from its session entry if the identity + /// cache has none. + /// + /// A lookup's answer is verified against the target's cached key, so a + /// lookup for a destination whose identity has been evicted runs to its + /// timeout and drops the packets queued for it as unreachable. A session + /// already holds the key: the one this node initiated to, or the one the + /// responder handshake authenticated. + fn cache_session_identity(&mut self, dest: &NodeAddr) { + if self.has_cached_identity(dest) { + return; + } + if let Some(pubkey) = self.sessions.get(dest).map(|e| *e.remote_pubkey()) { + self.register_identity(*dest, pubkey); + } + } + /// Count, without acting on, the two ways an admitted PathBroken can /// disagree with this node's own view of the path. /// diff --git a/src/node/tests/coord_forgery.rs b/src/node/tests/coord_forgery.rs index bf772fd3..879eb20e 100644 --- a/src/node/tests/coord_forgery.rs +++ b/src/node/tests/coord_forgery.rs @@ -542,6 +542,106 @@ async fn an_admitted_path_broken_starts_a_lookup_without_a_cached_identity() { cleanup_nodes(&mut fx.nodes).await; } +/// The lookup a PathBroken starts must be answerable when the destination's +/// identity is not cached. The session already holds the destination's key, +/// so the lookup's answer can be verified with it: the position is learned, +/// the lookup does not run to its timeout, and packets queued for the +/// destination are not dropped as unreachable when it would have. +#[tokio::test] +async fn a_path_broken_without_a_cached_identity_starts_a_lookup_this_node_can_verify() { + // V - P1 - D, with D a real node V has a session with but whose identity + // V has not cached. + let mut nodes = run_tree_test(3, &[(0, 1), (1, 2)], false).await; + let p1 = *nodes[1].node.node_addr(); + let dest = *nodes[2].node.node_addr(); + let dest_pubkey = nodes[2].node.identity().pubkey_full(); + let real: Vec = nodes[2] + .node + .tree_state() + .my_coords() + .node_addrs() + .copied() + .collect(); + let handshake = crate::noise::HandshakeState::new_xk_initiator( + nodes[0].node.identity().keypair(), + dest_pubkey, + ); + nodes[0].node.sessions.insert( + dest, + crate::node::session::SessionEntry::new( + dest, + dest_pubkey, + EndToEndState::Initiating(handshake), + 1000, + true, + ), + ); + assert!( + !nodes[0].node.has_cached_identity(&dest), + "precondition: the destination's identity is not cached" + ); + nodes[0] + .node + .queue_pending_tun_packet_for_test(dest, vec![0x60; 40]); + + let lookup = &nodes[0].node.metrics().lookup; + let (accepted, miss, timed_out) = ( + lookup.resp_accepted.get(), + lookup.resp_identity_miss.get(), + lookup.resp_timed_out.get(), + ); + + let victim = *nodes[0].node.node_addr(); + let payload = PathBroken::new(dest, p1).encode(); + let encoded = SessionDatagram::new(p1, victim, payload).encode(); + let start = wall_ms(); + nodes[0] + .node + .handle_session_datagram(&p1, &encoded[1..], false) + .await; + for _ in 0..10 { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + crate::node::tests::spanning_tree::process_available_packets(&mut nodes).await; + } + + let lookup = &nodes[0].node.metrics().lookup; + assert_eq!( + lookup.resp_identity_miss.get(), + miss, + "the lookup's answer could not be verified for want of an identity" + ); + assert!( + lookup.resp_accepted.get() > accepted, + "the lookup's answer must be verified and accepted" + ); + let entry = nodes[0].node.coord_cache().get_entry(&dest).unwrap(); + assert_eq!( + entry.coords().node_addrs().copied().collect::>(), + real + ); + assert!(entry.is_verified(wall_ms())); + + // Run the lookup schedule well past its end: an answered lookup has + // nothing left to time out, so the queued packet survives. + for step in 1..=6 { + nodes[0] + .node + .check_pending_lookups(start + step * 20_000) + .await; + } + assert_eq!( + nodes[0].node.metrics().lookup.resp_timed_out.get(), + timed_out, + "the lookup ran to its timeout" + ); + assert_eq!( + nodes[0].node.pending_tun_total_packets(), + 1, + "the packet queued for the destination was dropped" + ); + cleanup_nodes(&mut nodes).await; +} + /// The healthy path for keeping a verified entry below quorum: a genuine /// failure reported once is still recovered, because the lookup the report /// starts replaces the stale value with the destination's real position.