mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 08:14:42 +00:00
Admit rekey msg1 from established peers when addr forms differ
Companion to the ethernet `accept_connections: false` rekey-deadlock fix from earlier this release: the same dual-init failure mode shows up over UDP when peers register by hostname, and the existing addr_to_link-only carve-out in `should_admit_msg1` doesn't cover it. The carve-out's first predicate keys `addr_to_link` by the literal `TransportAddr` that `initiate_connection` inserted, which is the hostname-form when a peer config carries a hostname (e.g., `core-vm.tail65015.ts.net:2121`). Inbound packets always arrive with numeric source addrs because `udp_receive_loop` builds the `TransportAddr` from the `SocketAddr` the kernel reports via `recvfrom`. `TransportAddr` equality is byte-exact, so the two forms don't match and the lookup misses. With `udp.accept_connections: false` (or `udp.outbound_only: true`, which forces it false) the gate then rejects the rekey msg1 from an established peer. The dual-init tie-breaker stalls because the loser side never produces msg2; both sides retry indefinitely and the winner side keeps logging "Dual rekey initiation: we win, dropping their msg1" at 1Hz. The earlier ethernet fix didn't generalize to this variant because ethernet TransportAddrs are always numeric MAC bytes — both the config-time form and the inbound-arrival form match identically. Add a second predicate to `should_admit_msg1`: an active peer's `current_addr()` matching `(transport_id, remote_addr)`. `current_addr` is updated and refreshed from inbound encrypted-frame source addrs (`handlers/encrypted.rs`), which are always numeric `SocketAddr`-form, so this catches the established peer regardless of how its `addr_to_link` key was originally inserted. The fast `addr_to_link` check stays first; the iteration over peers is bounded by peer count and only runs when the first predicate misses. Regression coverage in this commit: - Unit test `test_should_admit_msg1_admits_rekey_when_addr_form_differs` in `src/node/tests/handshake.rs`. Constructs the failing scenario in-process: `addr_to_link` populated with hostname-form key, peer's `current_addr` at the resolved numeric form, query with numeric form. Without the new predicate this fails immediately. - New integration topology `rekey-outbound-only` plus matching docker-compose profile. Same 5-node mesh shape as `rekey-accept-off` but `inject-config` sets `udp.outbound_only: true` on node-b and rewrites node-b's peer-c address from the numeric docker IP to the docker hostname (`node-c:2121`), reproducing the production hostname-vs-numeric mismatch. The test asserts no sustained "Dual rekey initiation: we win" log lines on any node (>10 = bug) and the existing rekey health checks catch the connectivity loss the loop produces. - `testing/ci-local.sh` and `.github/workflows/ci.yml` extended to run the new variant in the local sweep and the GitHub CI integration matrix alongside `rekey` and `rekey-accept-off`. Verified locally: full `bash testing/ci-local.sh` sweep passes 29/29 suites (23m 12s) with the new variant green; 1084 unit tests pass.
This commit is contained in:
@@ -1056,3 +1056,80 @@ async fn test_should_admit_msg1_admits_rekey_when_udp_accept_off() {
|
||||
|
||||
assert!(node.should_admit_msg1(transport_id, &addr));
|
||||
}
|
||||
|
||||
/// Regression test for the udp.outbound_only rekey loop observed in
|
||||
/// production 2026-04-30 (parallel to ISSUE-2026-0004).
|
||||
///
|
||||
/// Production scenario: nomad runs `udp.outbound_only=true` with peer
|
||||
/// core-vm configured by hostname (`core-vm.tail65015.ts.net:2121`).
|
||||
/// `initiate_connection` populates `addr_to_link` with the literal
|
||||
/// hostname-form `TransportAddr`. core-vm's later rekey msg1 arrives at
|
||||
/// nomad with a numeric source addr (the kernel always reports
|
||||
/// `SocketAddr` in numeric form via `recvfrom`), so the `addr_to_link`
|
||||
/// lookup misses, the gate falls through to `accept_connections()`
|
||||
/// (false in outbound_only mode), and rejects. Result: dual-init
|
||||
/// tie-breaker stalls because the loser side never produces msg2.
|
||||
///
|
||||
/// The carve-out predicate must also consult peer state by source
|
||||
/// address: `current_addr()` is updated from inbound encrypted-frame
|
||||
/// source addrs (`handlers/encrypted.rs`), so an established peer can
|
||||
/// be matched even when the addr_to_link key is hostname-form and the
|
||||
/// incoming addr is numeric.
|
||||
#[tokio::test]
|
||||
async fn test_should_admit_msg1_admits_rekey_when_addr_form_differs() {
|
||||
use crate::config::UdpConfig;
|
||||
use crate::peer::ActivePeer;
|
||||
use crate::transport::udp::UdpTransport;
|
||||
|
||||
let mut node = make_node();
|
||||
let transport_id = TransportId::new(1);
|
||||
|
||||
// outbound_only mode forces accept_connections() to false.
|
||||
let cfg = UdpConfig {
|
||||
outbound_only: Some(true),
|
||||
..Default::default()
|
||||
};
|
||||
let (tx, _rx) = packet_channel(64);
|
||||
let udp = UdpTransport::new(transport_id, None, cfg, tx);
|
||||
node.transports
|
||||
.insert(transport_id, TransportHandle::Udp(udp));
|
||||
|
||||
// Simulate initiate_connection's effect when peer config carries a
|
||||
// hostname: addr_to_link is populated with hostname-form, not
|
||||
// numeric-form.
|
||||
let hostname_addr = TransportAddr::from_string("core-vm.example:2121");
|
||||
let link_id = node.allocate_link_id();
|
||||
node.addr_to_link
|
||||
.insert((transport_id, hostname_addr.clone()), link_id);
|
||||
|
||||
// Promote a peer at the hostname's resolved numeric form
|
||||
// (current_addr is set from the SocketAddr in udp_receive_loop).
|
||||
let peer_full = crate::Identity::generate();
|
||||
let peer_identity = PeerIdentity::from_pubkey(peer_full.pubkey());
|
||||
let peer_node_addr = *peer_identity.node_addr();
|
||||
let mut peer = ActivePeer::new(peer_identity, link_id, 1000);
|
||||
let numeric_addr = TransportAddr::from_string("100.64.0.5:2121");
|
||||
peer.set_current_addr(transport_id, numeric_addr.clone());
|
||||
node.peers.insert(peer_node_addr, peer);
|
||||
|
||||
// Sanity: legacy carve-out still works for the hostname-form lookup.
|
||||
assert!(node.should_admit_msg1(transport_id, &hostname_addr));
|
||||
|
||||
// The bug: incoming rekey msg1 arrives with numeric source addr.
|
||||
// Without the additional carve-out, this is rejected (addr_to_link
|
||||
// miss → accept_connections() false → drop).
|
||||
assert!(
|
||||
node.should_admit_msg1(transport_id, &numeric_addr),
|
||||
"rekey msg1 from established peer must be admitted even when \
|
||||
addr_to_link is keyed by a different addr-form (hostname vs \
|
||||
numeric); the carve-out must consult peer current_addr"
|
||||
);
|
||||
|
||||
// Negative: a stranger at a different numeric addr is still rejected
|
||||
// (no peer there, no addr_to_link entry, falls to accept_connections).
|
||||
let stranger_addr = TransportAddr::from_string("198.51.100.1:2121");
|
||||
assert!(
|
||||
!node.should_admit_msg1(transport_id, &stranger_addr),
|
||||
"fresh msg1 from unknown source must still be rejected"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user