diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82198ce..4decfe9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -261,6 +261,9 @@ jobs: - suite: rekey-accept-off type: rekey-accept-off topology: rekey-accept-off + - suite: rekey-outbound-only + type: rekey-outbound-only + topology: rekey-outbound-only - suite: acl-allowlist type: acl-allowlist # ── Outbound LAN gateway integration test ────────────────────── @@ -460,6 +463,41 @@ jobs: docker compose -f testing/static/docker-compose.yml \ --profile rekey-accept-off down --volumes --remove-orphans + # ── Rekey + udp.outbound_only=true variant ───────────────────────────── + - name: Generate and inject configs (rekey-outbound-only) + if: matrix.type == 'rekey-outbound-only' + env: + REKEY_TOPOLOGY: rekey-outbound-only + REKEY_OUTBOUND_ONLY_NODES: b + run: | + bash testing/static/scripts/generate-configs.sh rekey-outbound-only + bash testing/static/scripts/rekey-test.sh inject-config + + - name: Start containers (rekey-outbound-only) + if: matrix.type == 'rekey-outbound-only' + run: | + docker compose -f testing/static/docker-compose.yml \ + --profile rekey-outbound-only up -d + + - name: Run rekey test (outbound-only variant) + if: matrix.type == 'rekey-outbound-only' + env: + REKEY_TOPOLOGY: rekey-outbound-only + REKEY_OUTBOUND_ONLY_NODES: b + run: bash testing/static/scripts/rekey-test.sh + + - name: Collect logs on failure (rekey-outbound-only) + if: matrix.type == 'rekey-outbound-only' && failure() + run: | + docker compose -f testing/static/docker-compose.yml \ + --profile rekey-outbound-only logs --no-color | tail -300 + + - name: Stop containers (rekey-outbound-only) + if: matrix.type == 'rekey-outbound-only' && always() + run: | + docker compose -f testing/static/docker-compose.yml \ + --profile rekey-outbound-only down --volumes --remove-orphans + # ── ACL allowlist integration test ───────────────────────────────────── - name: Run ACL allowlist integration test if: matrix.type == 'acl-allowlist' diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index 5fe5221..1ca9a4d 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -13,11 +13,29 @@ impl Node { /// Returns true if an inbound msg1 should be admitted past the /// `accept_connections` gate. /// - /// Rekey/restart msg1 on an existing link is always admitted (the gate - /// is meant to filter fresh handshakes from strangers, not maintenance - /// traffic on established sessions). Otherwise the transport's - /// `accept_connections` config decides; absence of a registered - /// transport admits (no gate to apply). + /// Rekey/restart msg1 from an established peer is always admitted (the + /// gate is meant to filter fresh handshakes from strangers, not + /// maintenance traffic on established sessions). Two predicates cover + /// "established peer at this transport+addr": + /// + /// 1. `addr_to_link` has an entry for `(transport_id, remote_addr)`. + /// This is the fast path and matches when the peer registered with + /// the same `TransportAddr` form we observe on inbound packets + /// (e.g., both numeric when peer config uses a numeric IP). + /// + /// 2. An active peer's `current_addr()` matches `(transport_id, + /// remote_addr)`. `current_addr` is updated from inbound encrypted- + /// frame source addrs (always numeric `SocketAddr`-form), so this + /// catches established peers whose `addr_to_link` key is hostname- + /// form (because `initiate_connection` populated it from a + /// hostname-bearing peer config) while inbound rekey msg1 arrives + /// in numeric form. Without this second predicate, the carve-out + /// misses any deployment that combines a hostname-based peer config + /// with `udp.accept_connections: false` or `udp.outbound_only: true` + /// (the production trigger for the 2026-04-30 bug). + /// + /// Otherwise the transport's `accept_connections` config decides; + /// absence of a registered transport admits (no gate to apply). pub(in crate::node) fn should_admit_msg1( &self, transport_id: crate::transport::TransportId, @@ -29,6 +47,11 @@ impl Node { { return true; } + if self.peers.values().any(|p| { + p.transport_id() == Some(transport_id) && p.current_addr() == Some(remote_addr) + }) { + return true; + } self.transports .get(&transport_id) .is_none_or(|t| t.accept_connections()) diff --git a/src/node/tests/handshake.rs b/src/node/tests/handshake.rs index 2b8695e..15a2df6 100644 --- a/src/node/tests/handshake.rs +++ b/src/node/tests/handshake.rs @@ -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" + ); +} diff --git a/testing/ci-local.sh b/testing/ci-local.sh index c83887d..c1c9927 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -15,7 +15,8 @@ # -h, --help Show this help # # Integration suites (default coverage): -# static-mesh, static-chain, rekey, rekey-accept-off, gateway, +# static-mesh, static-chain, rekey, rekey-accept-off, +# rekey-outbound-only, gateway, # acl-allowlist, nat-cone, nat-symmetric, nat-lan, # chaos-smoke-10, chaos-churn-mixed-10, chaos-ethernet-mesh, # chaos-ethernet-only, chaos-tcp-mesh, chaos-bottleneck-parent, @@ -53,7 +54,7 @@ ONLY_SUITE="" # All integration suites matching ci.yml STATIC_SUITES=(static-mesh static-chain) -REKEY_SUITES=(rekey rekey-accept-off) +REKEY_SUITES=(rekey rekey-accept-off rekey-outbound-only) # Each entry: "display-name scenario [--flag value ...]" CHAOS_SUITES=( "smoke-10 smoke-10" @@ -376,6 +377,40 @@ run_rekey_accept_off() { record "rekey-accept-off" $rc } +# Run the rekey-outbound-only integration variant. Same harness as +# run_rekey but with udp.outbound_only=true on node-b plus its peer +# addrs rewritten from numeric docker IPs to docker hostnames so the +# addr_to_link key form mismatches inbound packet source addrs (the +# production trigger for the rekey-msg1 carve-out gap). +run_rekey_outbound_only() { + local compose="testing/static/docker-compose.yml" + local rc=0 + + info "[rekey-outbound-only] Generating configs" + bash testing/static/scripts/generate-configs.sh rekey-outbound-only || \ + { record "rekey-outbound-only" 1; return; } + REKEY_TOPOLOGY=rekey-outbound-only REKEY_OUTBOUND_ONLY_NODES=b \ + bash testing/static/scripts/rekey-test.sh inject-config || \ + { record "rekey-outbound-only" 1; return; } + + info "[rekey-outbound-only] Starting containers" + docker compose -f "$compose" --profile rekey-outbound-only up -d || \ + { record "rekey-outbound-only" 1; return; } + + info "[rekey-outbound-only] Running rekey test" + if REKEY_TOPOLOGY=rekey-outbound-only REKEY_OUTBOUND_ONLY_NODES=b \ + bash testing/static/scripts/rekey-test.sh; then + rc=0 + else + rc=1 + info "[rekey-outbound-only] Collecting failure logs" + docker compose -f "$compose" --profile rekey-outbound-only logs --no-color 2>&1 | tail -100 + fi + + docker compose -f "$compose" --profile rekey-outbound-only down --volumes --remove-orphans 2>/dev/null + record "rekey-outbound-only" $rc +} + # Run ACL allowlist integration test run_acl_allowlist() { info "[acl-allowlist] Running integration test" @@ -462,9 +497,10 @@ run_integration() { run_static "$topology" done - # Rekey + rekey-accept-off variant + # Rekey + rekey-accept-off + rekey-outbound-only variants run_rekey run_rekey_accept_off + run_rekey_outbound_only # Gateway run_gateway @@ -553,6 +589,8 @@ run_suite() { run_rekey ;; rekey-accept-off) run_rekey_accept_off ;; + rekey-outbound-only) + run_rekey_outbound_only ;; gateway) run_gateway ;; acl-allowlist) diff --git a/testing/static/configs/topologies/rekey-outbound-only.yaml b/testing/static/configs/topologies/rekey-outbound-only.yaml new file mode 100644 index 0000000..65b6fe5 --- /dev/null +++ b/testing/static/configs/topologies/rekey-outbound-only.yaml @@ -0,0 +1,53 @@ +# Rekey Integration Test Topology — udp.outbound_only variant. +# +# Same sparse mesh as rekey.yaml. Configs are post-processed to use +# aggressive rekey timers (35s, same as the parent) AND to set +# `transports.udp.outbound_only: true` on node b — the single-peer +# node, auto-connected to c. **Crucially**, node-b's peer-c address is +# also rewritten from the numeric docker IP to the docker hostname +# (`node-c:2121`), reproducing the production scenario where peer +# configs carry hostnames (e.g., `core-vm.tail65015.ts.net:2121`) and +# the literal hostname-form `TransportAddr` ends up as the +# `addr_to_link` key while inbound packets arrive with numeric source +# addresses. With the old `should_admit_msg1` carve-out, the +# hostname-vs-numeric form mismatch makes the lookup miss, falls +# through to `accept_connections() == false` (forced by outbound_only), +# and rejects the rekey msg1. The dual-init tie-breaker on node-c then +# stalls in a 1 Hz "we win (smaller addr), dropping their msg1" loop. +# +# This topology pins that regression alongside `rekey-accept-off`, +# which exercises the same Node-level gate but with numeric peer +# configs. The unit test +# `test_should_admit_msg1_admits_rekey_when_addr_form_differs` is the +# fast-feedback complement. + +nodes: + a: + nsec: "0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20" + npub: "npub1sjlh2c3x9w7kjsqg2ay080n2lff2uvt325vpan33ke34rn8l5jcqawh57m" + docker_ip: "172.20.0.10" + peers: [d, e] + + b: + nsec: "b102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1fb0" + npub: "npub1tdwa4vjrjl33pcjdpf2t4p027nl86xrx24g4d3avg4vwvayr3g8qhd84le" + docker_ip: "172.20.0.11" + peers: [c] + + c: + nsec: "c102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1fc0" + npub: "npub1cld9yay0u24davpu6c35l4vldrhzvaq66pcqtg9a0j2cnjrn9rtsxx2pe6" + docker_ip: "172.20.0.12" + peers: [b, d, e] + + d: + nsec: "d102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1fd0" + npub: "npub1n9lpnv0592cc2ps6nm0ca3qls642vx7yjsv35rkxqzj2vgds52sqgpverl" + docker_ip: "172.20.0.13" + peers: [a, c, e] + + e: + nsec: "e102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1fe0" + npub: "npub1wf8akf8lu2zdkjkmwhl75pqvven654mpv4sz2x2tprl5265mgrzq8nhak4" + docker_ip: "172.20.0.14" + peers: [a, c, d] diff --git a/testing/static/docker-compose.yml b/testing/static/docker-compose.yml index 68c3d78..85da0cf 100644 --- a/testing/static/docker-compose.yml +++ b/testing/static/docker-compose.yml @@ -356,6 +356,86 @@ services: fips-net: ipv4_address: 172.20.0.14 + # ── Rekey topology with udp.outbound_only=true on node b ──── + # Exercises the udp.outbound_only rekey-msg1 admission regression + # observed in production 2026-04-30. Same 5-node mesh as + # rekey-accept-off, but node-b's generated config has + # `transports.udp.outbound_only: true` (forces ephemeral bind + + # accept_connections false) AND node-b's peer-c address is + # rewritten to the docker hostname (`node-c:2121`) so the + # `addr_to_link` lookup misses on the inbound numeric source addr. + # Injected by rekey-test.sh's inject-config when + # REKEY_OUTBOUND_ONLY_NODES=b. + rekey-outbound-only-a: + <<: *fips-common + profiles: ["rekey-outbound-only"] + container_name: fips-node-a + hostname: node-a + environment: + - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug + volumes: + - ../docker/resolv.conf:/etc/resolv.conf:ro + - ./generated-configs/rekey-outbound-only/node-a.yaml:/etc/fips/fips.yaml:ro + networks: + fips-net: + ipv4_address: 172.20.0.10 + + rekey-outbound-only-b: + <<: *fips-common + profiles: ["rekey-outbound-only"] + container_name: fips-node-b + hostname: node-b + environment: + - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug + volumes: + - ../docker/resolv.conf:/etc/resolv.conf:ro + - ./generated-configs/rekey-outbound-only/node-b.yaml:/etc/fips/fips.yaml:ro + networks: + fips-net: + ipv4_address: 172.20.0.11 + + rekey-outbound-only-c: + <<: *fips-common + profiles: ["rekey-outbound-only"] + container_name: fips-node-c + hostname: node-c + environment: + - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug + volumes: + - ../docker/resolv.conf:/etc/resolv.conf:ro + - ./generated-configs/rekey-outbound-only/node-c.yaml:/etc/fips/fips.yaml:ro + networks: + fips-net: + ipv4_address: 172.20.0.12 + + rekey-outbound-only-d: + <<: *fips-common + profiles: ["rekey-outbound-only"] + container_name: fips-node-d + hostname: node-d + environment: + - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug + volumes: + - ../docker/resolv.conf:/etc/resolv.conf:ro + - ./generated-configs/rekey-outbound-only/node-d.yaml:/etc/fips/fips.yaml:ro + networks: + fips-net: + ipv4_address: 172.20.0.13 + + rekey-outbound-only-e: + <<: *fips-common + profiles: ["rekey-outbound-only"] + container_name: fips-node-e + hostname: node-e + environment: + - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug + volumes: + - ../docker/resolv.conf:/etc/resolv.conf:ro + - ./generated-configs/rekey-outbound-only/node-e.yaml:/etc/fips/fips.yaml:ro + networks: + fips-net: + ipv4_address: 172.20.0.14 + # ── TCP chain topology (A-B-C) ─────────────────────────────── tcp-a: <<: *fips-common diff --git a/testing/static/scripts/rekey-test.sh b/testing/static/scripts/rekey-test.sh index e66a660..10aaede 100755 --- a/testing/static/scripts/rekey-test.sh +++ b/testing/static/scripts/rekey-test.sh @@ -32,6 +32,16 @@ NODES="a b c d e" # initiation" log lines appear on the affected node. REKEY_ACCEPT_OFF_NODES="${REKEY_ACCEPT_OFF_NODES:-}" +# Comma-separated list of node IDs to set udp.outbound_only=true on +# during inject-config. For each such node, peer addresses are also +# rewritten from numeric docker IPs to docker hostnames (e.g. +# 172.20.0.12:2121 → node-c:2121). This reproduces the production +# scenario where peer configs carry hostnames so the `addr_to_link` +# key is hostname-form while inbound packet source addrs are numeric, +# making the should_admit_msg1 carve-out's `addr_to_link.contains_key` +# check miss. +REKEY_OUTBOUND_ONLY_NODES="${REKEY_OUTBOUND_ONLY_NODES:-}" + # Rekey timing configuration REKEY_AFTER_SECS=35 @@ -43,6 +53,9 @@ if [ "${1:-}" = "inject-config" ]; then if [ -n "$REKEY_ACCEPT_OFF_NODES" ]; then echo " Setting udp.accept_connections=false on nodes: $REKEY_ACCEPT_OFF_NODES" fi + if [ -n "$REKEY_OUTBOUND_ONLY_NODES" ]; then + echo " Setting udp.outbound_only=true + rewriting peer addrs to docker hostnames on nodes: $REKEY_OUTBOUND_ONLY_NODES" + fi for node in $NODES; do cfg="$SCRIPT_DIR/../generated-configs/$TOPOLOGY/node-$node.yaml" if [ ! -f "$cfg" ]; then @@ -57,6 +70,14 @@ if [ "${1:-}" = "inject-config" ]; then fi done fi + outbound_only="false" + if [ -n "$REKEY_OUTBOUND_ONLY_NODES" ]; then + for oo_node in ${REKEY_OUTBOUND_ONLY_NODES//,/ }; do + if [ "$oo_node" = "$node" ]; then + outbound_only="true" + fi + done + fi python3 -c " import yaml with open('$cfg') as f: @@ -74,14 +95,48 @@ if '$accept_off' == 'true': transports['udp'] = udp if isinstance(udp, dict): udp['accept_connections'] = False +if '$outbound_only' == 'true': + transports = cfg.setdefault('transports', {}) + udp = transports.get('udp') + if udp is None: + udp = {} + transports['udp'] = udp + if isinstance(udp, dict): + udp['outbound_only'] = True + # Rewrite peer addrs to docker hostnames so the addr_to_link key + # is hostname-form (mirroring production peer configs that carry + # hostnames). Without this, peer addrs are numeric and the + # carve-out's addr_to_link lookup matches inbound numeric source + # addrs, masking the bug. + ip_to_host = { + '172.20.0.10': 'node-a', + '172.20.0.11': 'node-b', + '172.20.0.12': 'node-c', + '172.20.0.13': 'node-d', + '172.20.0.14': 'node-e', + } + for peer in cfg.get('peers', []) or []: + for addr in peer.get('addresses', []) or []: + t = addr.get('transport') + if t is not None and t != 'udp': + continue + a = addr.get('addr', '') + for ip, host in ip_to_host.items(): + if a.startswith(ip + ':'): + port = a.split(':', 1)[1] + addr['addr'] = f'{host}:{port}' + break with open('$cfg', 'w') as f: yaml.dump(cfg, f, default_flow_style=False, sort_keys=False) " + suffix="" if [ "$accept_off" = "true" ]; then - echo " ✓ node-$node (accept_connections=false)" - else - echo " ✓ node-$node" + suffix=" (accept_connections=false)" fi + if [ "$outbound_only" = "true" ]; then + suffix=" (outbound_only=true, hostname peer addrs)" + fi + echo " ✓ node-$node$suffix" done echo "✓ Config injection complete" exit 0 @@ -368,6 +423,29 @@ if [ -n "$REKEY_ACCEPT_OFF_NODES" ]; then done fi +# Variant-specific: udp.outbound_only=true. The pre-fix bug fired the +# dual-init loop on the OTHER side (the peer of the outbound-only node) +# because the outbound-only side rejects the inbound rekey msg1 due to +# the addr_to_link hostname-vs-numeric mismatch, leaving the peer's +# rekey state in a 1Hz retry loop that the outbound-only side keeps +# dropping. The exact node that emits "we win" depends on which side +# has the smaller NodeAddr, so check all five nodes for the sustained- +# loop signature. +if [ -n "$REKEY_OUTBOUND_ONLY_NODES" ]; then + DUAL_INIT_THRESHOLD=10 + for n in $NODES; do + count=$(docker logs "fips-node-$n" 2>&1 \ + | grep -cE "Dual rekey initiation: we win" || true) + if [ "${count:-0}" -le "$DUAL_INIT_THRESHOLD" ]; then + echo " PASS: node-$n dual-init drops below threshold ($count <= $DUAL_INIT_THRESHOLD)" + PASSED=$((PASSED + 1)) + else + echo " FAIL: node-$n sustained dual-init drops ($count > $DUAL_INIT_THRESHOLD)" + FAILED=$((FAILED + 1)) + fi + done +fi + phase_result "Log analysis" echo ""