From 611f045d33503ef2addf474033ab7bf265471e9d Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 25 Jul 2026 19:01:16 +0000 Subject: [PATCH 1/4] Let the acl-allowlist suite float its docker subnet The compose pinned 172.31.0.0/24 with a per-container ipv4_address, so two concurrent runs of this suite asked docker for the same address space and the second failed with a pool-overlap error. Request no subnet instead and let docker assign one from the daemon pool, which leaves nothing for two runs to contend for. Peers now address each other by the docker hostname the compose already sets (host-a through host-f) rather than by literal IP, and docker's embedded DNS is per-network, so the same hostname in two runs resolves inside each run's own subnet. The generated config directory moves under the run suffix in the same change, because it has to: the generator does rm -rf on it, so a shared directory means one run deletes the fixtures another is still using, which fails silently rather than loudly at bring-up. Scoping it requires the generator output path, all five bind-mount sources per service, and the gitignore entry together; scoping only the generator leaves the compose reading the unscoped path. The floating subnet removes one of the two obstacles to running this suite twice at once, not both: the compose project name is still fixed, and nothing scopes it for this suite. Recorded at the site so the comment does not claim more than the change delivers. --- testing/acl-allowlist/.gitignore | 2 +- testing/acl-allowlist/README.md | 10 ++- testing/acl-allowlist/docker-compose.yml | 86 ++++++++++++----------- testing/acl-allowlist/generate-configs.sh | 35 +++++---- 4 files changed, 77 insertions(+), 56 deletions(-) diff --git a/testing/acl-allowlist/.gitignore b/testing/acl-allowlist/.gitignore index 31315f6..b06c7df 100644 --- a/testing/acl-allowlist/.gitignore +++ b/testing/acl-allowlist/.gitignore @@ -1 +1 @@ -generated-configs +generated-configs* diff --git a/testing/acl-allowlist/README.md b/testing/acl-allowlist/README.md index 6c5c231..95d0f2e 100644 --- a/testing/acl-allowlist/README.md +++ b/testing/acl-allowlist/README.md @@ -79,6 +79,12 @@ Docker service/container/hostname identifiers in this harness intentionally use `node-a` through `node-f`. For data-plane checks and operator examples, use the explicit FIPS names such as `node-a.fips` and `node-d.fips`. +The bridge network requests no subnet, so docker assigns one from its own +address pool and two concurrent runs of this harness never contend for a fixed +range. Consequently no node's IPv4 address is known before startup, and the +generated peer stanzas address each other by docker hostname (`host-a` … +`host-f`), resolved through the container's dnsmasq to docker's embedded DNS. + ACL paths are fixed in this branch: - `/etc/fips/peers.allow` @@ -93,7 +99,9 @@ Mounted ACL files in this harness: Generated fixture location: -- `testing/acl-allowlist/generated-configs/` +- `testing/acl-allowlist/generated-configs/`, or + `generated-configs/` when `FIPS_CI_NAME_SUFFIX` is set, which is how + concurrent runs keep their fixtures apart Inspect peer state: diff --git a/testing/acl-allowlist/docker-compose.yml b/testing/acl-allowlist/docker-compose.yml index abef96d..7d731d9 100644 --- a/testing/acl-allowlist/docker-compose.yml +++ b/testing/acl-allowlist/docker-compose.yml @@ -1,11 +1,21 @@ networks: + # No subnet is requested: docker assigns one from the daemon's address pool. + # A fixed request is honoured verbatim, so two runs asking for the same range + # collide on "Pool overlaps" — which is why the generated peer addresses are + # docker hostnames (host-a … host-f) rather than literal IPs. Docker's + # embedded DNS is per-network, so the same hostname in two concurrent runs + # resolves inside each run's own subnet. + # + # That removes one of the two obstacles to running this suite twice at once, + # not both. The compose project name is still fixed, so two runs that do not + # set COMPOSE_PROJECT_NAME share a project, and the second `up` recreates the + # first's containers while either `down` removes both. Nothing scopes it for + # this suite: it is run by hand only, so the caller has to. Do not read the + # floating subnet as making concurrent bare runs safe. acl-net: driver: bridge labels: - "com.corganlabs.fips-ci=1" - ipam: - config: - - subnet: 172.31.0.0/24 x-fips-common: &fips-common build: @@ -32,14 +42,13 @@ services: hostname: host-a volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-a/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-a/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-a/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-a/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-a/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.10 + - acl-net service-b: <<: *fips-common @@ -47,14 +56,13 @@ services: hostname: host-b volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-b/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-b/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-b/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-b/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-b/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.11 + - acl-net service-c: <<: *fips-common @@ -62,14 +70,13 @@ services: hostname: host-c volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-c/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-c/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-c/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-c/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-c/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-c/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.12 + - acl-net service-d: <<: *fips-common @@ -77,14 +84,13 @@ services: hostname: host-d volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-d/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-d/peers.allow:/etc/fips/peers.allow:ro - - ./generated-configs/node-d/peers.deny:/etc/fips/peers.deny:ro - - ./generated-configs/node-d/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-d/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/peers.allow:/etc/fips/peers.allow:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/peers.deny:/etc/fips/peers.deny:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-d/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.13 + - acl-net service-e: <<: *fips-common @@ -92,12 +98,11 @@ services: hostname: host-e volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-e/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-e/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-e/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-e/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-e/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-e/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.14 + - acl-net service-f: <<: *fips-common @@ -105,9 +110,8 @@ services: hostname: host-f volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-f/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-f/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-f/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-f/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-f/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-f/fips.key:/etc/fips/fips.key:ro networks: - acl-net: - ipv4_address: 172.31.0.15 + - acl-net diff --git a/testing/acl-allowlist/generate-configs.sh b/testing/acl-allowlist/generate-configs.sh index 01ced41..302b127 100755 --- a/testing/acl-allowlist/generate-configs.sh +++ b/testing/acl-allowlist/generate-configs.sh @@ -3,7 +3,12 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -GENERATED_DIR="$SCRIPT_DIR/generated-configs" + +# Scoped by the per-run suffix because this directory is wiped and rewritten +# below: two runs sharing one output directory would delete each other's +# fixtures out from under running containers. Unset (a bare hand run, or the +# GitHub-hosted path) it collapses to the historical "generated-configs". +GENERATED_DIR="$SCRIPT_DIR/generated-configs${FIPS_CI_NAME_SUFFIX:-}" write_file() { local path="$1" @@ -23,6 +28,10 @@ node-f npub1ytrut7gjncn2zfnhn56c0zgftf0w6p99gf6fu8j73hzw5603zglqc9av6c EOF } +# Peers are addressed by the docker hostname the compose file assigns +# (host-a … host-f), not by IP. The network requests no subnet so that two +# concurrent runs cannot collide on one address range, which means no node's +# address is knowable before `docker compose up`. echo "Generating ACL allowlist fixtures..." rm -rf "$GENERATED_DIR" @@ -48,31 +57,31 @@ peers: alias: "node-b" addresses: - transport: udp - addr: "172.31.0.11:2121" + addr: "host-b:2121" connect_policy: auto_connect - npub: "npub1cld9yay0u24davpu6c35l4vldrhzvaq66pcqtg9a0j2cnjrn9rtsxx2pe6" alias: "node-c" addresses: - transport: udp - addr: "172.31.0.12:2121" + addr: "host-c:2121" connect_policy: auto_connect - npub: "npub1n9lpnv0592cc2ps6nm0ca3qls642vx7yjsv35rkxqzj2vgds52sqgpverl" alias: "node-d" addresses: - transport: udp - addr: "172.31.0.13:2121" + addr: "host-d:2121" connect_policy: auto_connect - npub: "npub1x5z9rwzzm26q9verutx4aajhf2zw2pyp34c6whhde2zduxqav40qgq36l6" alias: "node-e" addresses: - transport: udp - addr: "172.31.0.14:2121" + addr: "host-e:2121" connect_policy: auto_connect - npub: "npub1ytrut7gjncn2zfnhn56c0zgftf0w6p99gf6fu8j73hzw5603zglqc9av6c" alias: "node-f" addresses: - transport: udp - addr: "172.31.0.15:2121" + addr: "host-f:2121" connect_policy: auto_connect EOF @@ -113,19 +122,19 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect - npub: "npub1cld9yay0u24davpu6c35l4vldrhzvaq66pcqtg9a0j2cnjrn9rtsxx2pe6" alias: "node-c" addresses: - transport: udp - addr: "172.31.0.12:2121" + addr: "host-c:2121" connect_policy: auto_connect - npub: "npub1n9lpnv0592cc2ps6nm0ca3qls642vx7yjsv35rkxqzj2vgds52sqgpverl" alias: "node-d" addresses: - transport: udp - addr: "172.31.0.13:2121" + addr: "host-d:2121" connect_policy: auto_connect EOF @@ -166,7 +175,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF @@ -209,7 +218,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF @@ -252,7 +261,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF @@ -282,7 +291,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.31.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF From 0e42c789beb8f4ac0743c6d6d104a4abb15b8dbc Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 25 Jul 2026 19:01:56 +0000 Subject: [PATCH 2/4] Let the firewall suite float its docker subnet The compose pinned 172.32.0.0/24 with a per-container ipv4_address, so two concurrent runs asked docker for the same address space and the second failed with a pool-overlap error. Request no subnet and let docker assign one from the daemon pool. Peers address each other by the docker hostname the compose already sets (host-a, host-b) rather than by literal IP; docker's embedded DNS is per-network, so the same hostname in two runs resolves inside each run's own subnet. Nothing this suite asserts on moves as a result: its checks run over the fips0 overlay, whose addresses are derived from node npubs and are independent of docker addressing, and the packaged nftables ruleset matches on interface rather than on any address. The generated config directory moves under the run suffix for the same reason it did in the acl suite, and the run teardown gains the matching removal so a run no longer leaves its directory behind. Case (b) also wrote curl's output to a fixed path under /tmp. Two concurrent runs shared that one file, and either run's cleanup landing between the other's write and read left an empty read, failing the http_code check for a reason having nothing to do with the firewall. It uses mktemp now. As in the acl suite, the floating subnet removes one of the two obstacles to concurrent runs. The compose project name is still fixed; the local CI runner scopes it externally, a bare hand run does not, and the comment at the site says so rather than claiming the file is self-sufficient. --- testing/ci-local.sh | 13 ++++++---- testing/firewall/.gitignore | 2 +- testing/firewall/README.md | 20 ++++++++++++---- testing/firewall/docker-compose.yml | 36 +++++++++++++++++----------- testing/firewall/generate-configs.sh | 15 +++++++++--- testing/firewall/test.sh | 12 +++++++--- 6 files changed, 68 insertions(+), 30 deletions(-) diff --git a/testing/ci-local.sh b/testing/ci-local.sh index f879ff4..3fe7796 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -390,15 +390,20 @@ ci_teardown() { --images "$CI_IMAGE_TEST $CI_IMAGE_APP" \ --veth-suffixes "${_suffixes[*]}" >/dev/null || true - # 3. The static suite's generated configs are per-run (a shared directory - # would let concurrent runs overwrite each other's node configs), so - # they are this run's to remove. Only on a green run: after a failure - # they are the evidence of what the failing nodes were actually + # 3. The static and firewall suites' generated configs are per-run (a + # shared directory would let concurrent runs overwrite each other's node + # configs), so they are this run's to remove. Only on a green run: after + # a failure they are the evidence of what the failing nodes were actually # configured with. Guarded on a non-empty suffix too, since without one # the path is the unscoped working directory a developer uses by hand, # which is not ours to delete. + # + # Every suite whose configs become per-run owes a line here. acl-allowlist + # also generates per-run now but is not in this runner's suite list, so + # this teardown never creates its directory and must not remove one. if [[ $run_status -eq 0 && -n "${CI_RUN_NAME_SUFFIX:-}" ]]; then rm -rf "$SCRIPT_DIR/static/generated-configs${CI_RUN_NAME_SUFFIX}" + rm -rf "$SCRIPT_DIR/firewall/generated-configs${CI_RUN_NAME_SUFFIX}" fi } diff --git a/testing/firewall/.gitignore b/testing/firewall/.gitignore index 31315f6..b06c7df 100644 --- a/testing/firewall/.gitignore +++ b/testing/firewall/.gitignore @@ -1 +1 @@ -generated-configs +generated-configs* diff --git a/testing/firewall/README.md b/testing/firewall/README.md index cecd186..3a6bd99 100644 --- a/testing/firewall/README.md +++ b/testing/firewall/README.md @@ -24,10 +24,18 @@ actively DROP'd by the fips chain (not silently unrouted). Two FIPS nodes peered over UDP on a Docker bridge network: -| Container | Hostname | docker IPv4 | Firewall | -|-------------------------|----------|---------------|----------| -| `fips-fw-container-a` | `host-a` | 172.32.0.10 | none (probe) | -| `fips-fw-container-b` | `host-b` | 172.32.0.11 | `fips.nft` + drop-in | +| Container | Hostname | Firewall | +|-------------------------|----------|----------| +| `fips-fw-container-a` | `host-a` | none (probe) | +| `fips-fw-container-b` | `host-b` | `fips.nft` + drop-in | + +The bridge network requests no subnet, so docker assigns one from its own +address pool and two concurrent runs never contend for a fixed range. No +node's IPv4 address is therefore known before startup, and the generated peer +stanzas address each other by docker hostname, resolved through the +container's dnsmasq to docker's embedded DNS. The firewall assertions +themselves are unaffected: they run over the fips0 overlay, whose addresses +are derived from the node npubs. `node-b` mounts the production `packaging/common/fips.nft` read-only at `/etc/fips/fips.nft`, plus a drop-in at `/etc/fips/fips.d/services.nft` @@ -108,4 +116,6 @@ docker compose -f testing/firewall/docker-compose.yml down ## Generated fixture location -`testing/firewall/generated-configs/` (gitignored). +`testing/firewall/generated-configs/` (gitignored), or +`generated-configs/` when `FIPS_CI_NAME_SUFFIX` is set, which is how +concurrent runs keep their fixtures apart. diff --git a/testing/firewall/docker-compose.yml b/testing/firewall/docker-compose.yml index 60b16ea..88110dd 100644 --- a/testing/firewall/docker-compose.yml +++ b/testing/firewall/docker-compose.yml @@ -1,11 +1,21 @@ networks: + # No subnet is requested: docker assigns one from the daemon's address pool. + # A fixed request is honoured verbatim, so two runs asking for the same range + # collide on "Pool overlaps" — which is why the generated peer addresses are + # docker hostnames (host-a, host-b) rather than literal IPs. Docker's embedded + # DNS is per-network, so the same hostname in two concurrent runs resolves + # inside each run's own subnet. + # + # That removes one of the two obstacles to running this suite twice at once, + # not both. The compose project name is still fixed, so two runs that do not + # set COMPOSE_PROJECT_NAME share a project, and the second `up` recreates the + # first's containers while either `down` removes both. The local CI runner + # scopes it externally (run_firewall in ci-local.sh); a bare hand run does + # not. Do not read the floating subnet as making concurrent bare runs safe. fw-net: driver: bridge labels: - "com.corganlabs.fips-ci=1" - ipam: - config: - - subnet: 172.32.0.0/24 x-fips-common: &fips-common build: @@ -31,12 +41,11 @@ services: hostname: host-a volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-a/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-a/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-a/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-a/fips.key:/etc/fips/fips.key:ro networks: - fw-net: - ipv4_address: 172.32.0.10 + - fw-net service-b: <<: *fips-common @@ -44,11 +53,10 @@ services: hostname: host-b volumes: - ../docker/resolv.conf:/etc/resolv.conf:ro - - ./generated-configs/node-b/hosts:/etc/fips/hosts:ro - - ./generated-configs/node-b/fips.yaml:/etc/fips/fips.yaml:ro - - ./generated-configs/node-b/fips.key:/etc/fips/fips.key:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/hosts:/etc/fips/hosts:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.yaml:/etc/fips/fips.yaml:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.key:/etc/fips/fips.key:ro - ../../packaging/common/fips.nft:/etc/fips/fips.nft:ro - - ./generated-configs/node-b/fips.d:/etc/fips/fips.d:ro + - ./generated-configs${FIPS_CI_NAME_SUFFIX:-}/node-b/fips.d:/etc/fips/fips.d:ro networks: - fw-net: - ipv4_address: 172.32.0.11 + - fw-net diff --git a/testing/firewall/generate-configs.sh b/testing/firewall/generate-configs.sh index 1fca1de..8ecd328 100755 --- a/testing/firewall/generate-configs.sh +++ b/testing/firewall/generate-configs.sh @@ -9,7 +9,12 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -GENERATED_DIR="$SCRIPT_DIR/generated-configs" + +# Scoped by the per-run suffix because this directory is wiped and rewritten +# below: two runs sharing one output directory would delete each other's +# fixtures out from under running containers. Unset (a bare hand run, or the +# GitHub-hosted path) it collapses to the historical "generated-configs". +GENERATED_DIR="$SCRIPT_DIR/generated-configs${FIPS_CI_NAME_SUFFIX:-}" # Deterministic test identities (mirrors the acl-allowlist style). NPUB_A="npub1sjlh2c3x9w7kjsqg2ay080n2lff2uvt325vpan33ke34rn8l5jcqawh57m" @@ -32,6 +37,10 @@ node-b $NPUB_B EOF } +# Peers are addressed by the docker hostname the compose file assigns (host-a, +# host-b), not by IP. The network requests no subnet so that two concurrent +# runs cannot collide on one address range, which means neither node's address +# is knowable before `docker compose up`. echo "Generating firewall fixtures..." rm -rf "$GENERATED_DIR" @@ -58,7 +67,7 @@ peers: alias: "node-b" addresses: - transport: udp - addr: "172.32.0.11:2121" + addr: "host-b:2121" connect_policy: auto_connect EOF @@ -89,7 +98,7 @@ peers: alias: "node-a" addresses: - transport: udp - addr: "172.32.0.10:2121" + addr: "host-a:2121" connect_policy: auto_connect EOF diff --git a/testing/firewall/test.sh b/testing/firewall/test.sh index 4d942e7..f4327ca 100755 --- a/testing/firewall/test.sh +++ b/testing/firewall/test.sh @@ -230,14 +230,20 @@ log "Case (b): node-b initiates outbound TCP, expects reply via conntrack" # node-b → node-a:8000 on the fips overlay. node-a has http.server on # [::]:8000 and is NOT firewalled, so this is purely a test of node-b's # outbound + ct state established,related path on the way back. +# +# mktemp rather than a fixed /tmp name: two concurrent runs of this suite +# would otherwise share one host file, and either one's `rm` between the +# other's write and read leaves an empty read that fails the http_code check +# for a reason that has nothing to do with the firewall. +CURL_OUT="$(mktemp)" set +e docker exec "$CONTAINER_B" curl -6 --silent --max-time 5 \ --output /dev/null --write-out '%{http_code}' \ - "http://[${ADDR_A}]:${OUTBOUND_TARGET_PORT}/" >/tmp/fw_b_rc 2>/dev/null + "http://[${ADDR_A}]:${OUTBOUND_TARGET_PORT}/" >"$CURL_OUT" 2>/dev/null RC=$? set -e -HTTP_CODE="$(cat /tmp/fw_b_rc 2>/dev/null || true)" -rm -f /tmp/fw_b_rc +HTTP_CODE="$(cat "$CURL_OUT" 2>/dev/null || true)" +rm -f "$CURL_OUT" if [ "$RC" -ne 0 ]; then fail "(b) outbound from node-b failed (curl rc=$RC, http=$HTTP_CODE) — conntrack reply path broken" fi From 1077fd6a7d7945396cda9cbe44bf2a6bd9669c08 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 25 Jul 2026 19:02:15 +0000 Subject: [PATCH 3/4] Stop the firewall peer reader turning a silent container into a count of zero wait_for_peers_exact read the connected-peer count through a pipeline ending in `|| echo 0`, so a container that never answered and a daemon that answered zero produced the same value. That is only harmless while every caller expects a non-zero count, which is true here today and is the reason this copy was left alone when the acl suite's copy was fixed. It leaves the trap armed for whoever adds the first caller expecting zero: the check would be satisfied on its first iteration without the property it exists to verify ever being observed. The read moves into its own function that returns the empty string when the container does not answer, and the caller treats empty as "no answer" rather than as a count. A run that never gets an answer now fails saying so, distinctly from one that answered the wrong count, and the diagnostic peer dump runs only on the latter, since dumping from a container that cannot answer prints a docker error rather than evidence. This is the shape the acl-allowlist suite already uses. The two copies had diverged on whether a silent container counts as an answer, which is the kind of disagreement that decides a security assertion in whichever file is read last. Checked by construction rather than by a green run: driving the old form against an absent container with an expected count of zero returns success on the first iteration, and the new form fails; with an expected count of one, the shape both real callers use, both forms still fail, and a genuine zero from a live daemon is still reported as zero. --- testing/firewall/test.sh | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/testing/firewall/test.sh b/testing/firewall/test.sh index f4327ca..833e9c8 100755 --- a/testing/firewall/test.sh +++ b/testing/firewall/test.sh @@ -81,21 +81,46 @@ wait_for_fips0() { fail "$container fips0 did not come up within ${timeout}s" } +# Connected-peer count for a container, or the empty string if it did not +# answer. +# +# Empty is deliberately distinct from a real 0. An `|| echo 0` fallback here +# would make "the container is unreachable, or its daemon never came up" and +# "the daemon answered, and the answer was zero" the same value, so any caller +# expecting zero would be satisfied on the first iteration without the +# property it is checking ever being observed. No caller in this file expects +# zero today, which is exactly why the fallback has to go now rather than when +# one is added. Same shape as the acl-allowlist suite's read_connected_peers. +read_connected_peers() { + local container="$1" + docker exec "$container" fipsctl show peers 2>/dev/null \ + | python3 -c 'import json,sys; data=json.load(sys.stdin); print(sum(1 for p in data.get("peers", []) if p.get("connectivity") == "connected"))' 2>/dev/null \ + || true +} + # Wait for the peer count on a container to reach the expected value. wait_for_peers_exact() { local container="$1" local expected_count="$2" local timeout="${3:-30}" + + local count="" answered=false for _ in $(seq 1 "$timeout"); do - local count - count=$(docker exec "$container" fipsctl show peers 2>/dev/null \ - | python3 -c 'import json,sys; data=json.load(sys.stdin); print(sum(1 for p in data.get("peers", []) if p.get("connectivity") == "connected"))' 2>/dev/null || echo 0) - if [ "$count" -eq "$expected_count" ]; then - return 0 + count=$(read_connected_peers "$container") + if [ -n "$count" ]; then + answered=true + if [ "$count" -eq "$expected_count" ]; then + return 0 + fi fi sleep 1 done - fail "$container did not reach $expected_count connected peers in ${timeout}s" + + if [ "$answered" = false ]; then + fail "$container never answered a peer query in ${timeout}s, so a count of $expected_count was never actually observed" + fi + docker exec "$container" fipsctl show peers >&2 || true + fail "$container did not reach $expected_count connected peers in ${timeout}s (last answer: $count)" } # Resolve `.fips` inside a container and print the AAAA answer. From 0eea7dae134c6800b6e625e39e0703b64e9a7796 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 25 Jul 2026 19:15:31 +0000 Subject: [PATCH 4/4] Make a zero-peer floor unreachable in the shared convergence wait wait_for_peers reads the connected-peer count through a pipeline ending in `|| echo 0`, so a container that never answers contributes 0. That is safe against a floor of 1 or more, where 0 reads as "not converged yet" and the wait eventually times out, and it is unsafe against a floor of 0, where the first read from a dead container satisfies the wait and the caller proceeds as though convergence had been observed. No caller passes 0 today; every one passes 1 or more, and the single variable minimum is advisory. Rather than harden the reader, reject the input: a floor below 1 is now an error. That makes the shape unreachable instead of repairing instances of it, and it costs a future caller nothing except a clear message saying that asserting "exactly zero peers" needs a reader that distinguishes no answer from zero, which this floor is not. wait_for_links goes with it. It had no caller anywhere in the tree on any branch, and it carried the identical fallback, so the only thing it could do was hand the hazard to whoever called it first. An uncalled helper cannot be wrong today, which is exactly why it was the risk worth removing rather than the one worth keeping for symmetry. The hermetic gate suite gains a case covering both directions, with docker stubbed so it stays container-free: a floor of 0 is refused with a message naming why, and the same unreachable container with a floor of 1 still polls its full budget and times out. Verified by removing the guard and confirming exactly the three zero-floor assertions red while both floor-of-one controls stay green. --- testing/lib/wait-converge-test.sh | 41 +++++++++++++++++++++++++ testing/lib/wait-converge.sh | 51 ++++++++++++++++--------------- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/testing/lib/wait-converge-test.sh b/testing/lib/wait-converge-test.sh index fda25c4..7b7c0ab 100755 --- a/testing/lib/wait-converge-test.sh +++ b/testing/lib/wait-converge-test.sh @@ -180,6 +180,47 @@ check "case4: returns 0 with 4 args" "$c4_rc_ok" "rc=$rc" c4_hold_ok=1; echo "$out" | grep -q "$HOLD_MSG" && c4_hold_ok=0 check "case4: default slack triggered near-converged hold" "$c4_hold_ok" +# --- Case 5: wait_for_peers refuses a floor of zero ------------------- +# +# The reader inside wait_for_peers falls back to 0 when a container does +# not answer. That is safe against a floor of 1 or more, where 0 reads as +# "not converged yet", and unsafe against a floor of 0, where the first +# read from a dead container satisfies the wait immediately. This case is +# the break-what-it-guards check for the rejection: drive the guard with a +# floor of 0 and confirm it refuses, then drive the same dead container +# with a floor of 1 and confirm the refusal is specific to the dangerous +# input rather than a blanket failure. +# +# `docker` is stubbed to fail, which is what an unreachable container looks +# like to this reader, so no container or network is involved and the suite +# stays hermetic. +echo +echo "== Case 5: wait_for_peers refuses a zero floor ==" +docker() { return 1; } + +out=$(wait_for_peers stub-container 0 2 2>&1); rc=$? +echo "$out" +c5_reject_ok=1; [ "$rc" -eq 2 ] && c5_reject_ok=0 +check "case5: floor of 0 is refused" "$c5_reject_ok" "rc=$rc" +c5_msg_ok=1; echo "$out" | grep -q "refusing a minimum" && c5_msg_ok=0 +check "case5: refusal names the reason" "$c5_msg_ok" +# The pre-guard behaviour, asserted so a regression is visible rather than +# quiet: without the rejection this returned 0 on its first iteration +# against a container that never answered. +c5_notpass_ok=1; [ "$rc" -ne 0 ] && c5_notpass_ok=0 +check "case5: floor of 0 does not report success" "$c5_notpass_ok" "rc=$rc" + +start=$SECONDS +out=$(wait_for_peers stub-container 1 2 2>&1); rc=$? +elapsed=$((SECONDS - start)) +echo "$out" +c5_floor1_ok=1; [ "$rc" -eq 1 ] && c5_floor1_ok=0 +check "case5: floor of 1 times out rather than being refused" "$c5_floor1_ok" "rc=$rc" +c5_polled_ok=1; [ "$elapsed" -ge 2 ] && c5_polled_ok=0 +check "case5: floor of 1 polled its full budget" "$c5_polled_ok" "elapsed=${elapsed}s >= 2s" + +unset -f docker + # --- Summary ---------------------------------------------------------- echo echo "==============================================" diff --git a/testing/lib/wait-converge.sh b/testing/lib/wait-converge.sh index 8832b67..17933a5 100644 --- a/testing/lib/wait-converge.sh +++ b/testing/lib/wait-converge.sh @@ -1,44 +1,47 @@ #!/bin/bash # Shared convergence wait helpers for FIPS integration tests. # -# Source this file to get wait_for_links(), wait_for_peers(), and -# wait_until_connected(). +# Source this file to get wait_for_peers() and wait_until_connected(). # # Usage: # source "$(dirname "$0")/../../lib/wait-converge.sh" -# wait_for_links [timeout_secs] # wait_for_peers [timeout_secs] # wait_until_connected [poll_secs] \ # [near_converged_slack] - -# Wait until a container has at least min_links active links. -# Returns 0 on success, 1 on timeout. -wait_for_links() { - local container="$1" - local min_links="$2" - local timeout="${3:-30}" - - for i in $(seq 1 "$timeout"); do - local count - count=$(docker exec "$container" fipsctl show links 2>/dev/null \ - | python3 -c "import sys,json; print(len(json.load(sys.stdin).get('links',[])))" 2>/dev/null || echo 0) - if [ "$count" -ge "$min_links" ]; then - echo " $container: $count link(s) after ${i}s" - return 0 - fi - sleep 1 - done - echo " $container: TIMEOUT waiting for $min_links link(s) after ${timeout}s" - return 1 -} +# +# There was a wait_for_links() here. It was removed rather than kept for +# symmetry: it had no caller anywhere in the tree on any branch, and its +# reader carried the same failure-to-zero fallback wait_for_peers does. An +# uncalled helper cannot be wrong today, so the risk was that the first +# caller to appear would inherit the hazard below without the reasoning +# that goes with it. `git log` has the implementation if one is needed. # Wait until a container has at least min_peers connected peers. # Returns 0 on success, 1 on timeout. +# +# The read below falls back to 0 when the container does not answer, which +# is safe ONLY because this is a floor: a fallback of 0 reads as "not +# converged yet", the loop keeps polling, and a container that never answers +# times out and returns 1. That safety is a property of the comparison, not +# of the reader. +# +# A minimum of 0 inverts it. `[ 0 -ge 0 ]` is true, so the first read from a +# dead container would satisfy the wait immediately and the caller would +# proceed as though convergence had been observed. No caller passes 0, and +# rejecting it here means none can start to — which is cheaper than auditing +# every future caller, and is why this is a hard error rather than a warning. +# A caller that genuinely wants to assert "exactly zero peers" needs a reader +# that distinguishes no-answer from zero, not this floor. wait_for_peers() { local container="$1" local min_peers="$2" local timeout="${3:-30}" + if [ "$min_peers" -lt 1 ]; then + echo " wait_for_peers: refusing a minimum of $min_peers for $container — a floor of 0 is satisfied by a container that never answered" >&2 + return 2 + fi + for i in $(seq 1 "$timeout"); do local count count=$(docker exec "$container" fipsctl show peers 2>/dev/null \