From b32d5907ac2594360283c666a3955294c3202ff5 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:06:11 +0000 Subject: [PATCH 01/71] fix: re-enable pf when boot leaves it disabled --- aifw-daemon/src/main.rs | 144 ++++++++++++++++++++++++++++++++-------- aifw-setup/src/apply.rs | 2 + 2 files changed, 117 insertions(+), 29 deletions(-) diff --git a/aifw-daemon/src/main.rs b/aifw-daemon/src/main.rs index 07f5bfbe..515a976a 100644 --- a/aifw-daemon/src/main.rs +++ b/aifw-daemon/src/main.rs @@ -536,21 +536,10 @@ async fn reconcile_pf_main() { } let has_hooks = stdout.contains("aifw-nat") || stdout.contains("anchor \"aifw\""); - if has_hooks { - info!("pf drift check: main ruleset has aifw anchor hooks"); - return; - } + let auto_heal_disabled = + pf_auto_heal_disabled(std::env::var("AIFW_NO_PF_AUTO_HEAL").ok().as_deref()); - let auto_heal_disabled = std::env::var("AIFW_NO_PF_AUTO_HEAL") - .map(|v| { - !v.is_empty() - && v != "0" - && !v.eq_ignore_ascii_case("no") - && !v.eq_ignore_ascii_case("false") - }) - .unwrap_or(false); - - if auto_heal_disabled { + if !has_hooks && auto_heal_disabled { error!( "pf drift check: main ruleset is MISSING aifw anchor hooks — \ auto-heal disabled via AIFW_NO_PF_AUTO_HEAL; run `aifw reconcile` to reload from {PF_CONF}" @@ -558,27 +547,124 @@ async fn reconcile_pf_main() { return; } - error!( - "pf drift check: main ruleset is MISSING aifw anchor hooks — auto-healing by reloading {PF_CONF}" - ); - let reload = tokio::process::Command::new("/usr/local/bin/sudo") - .args(["/sbin/pfctl", "-f", PF_CONF]) + if has_hooks { + info!("pf drift check: main ruleset has aifw anchor hooks"); + } else { + error!( + "pf drift check: main ruleset is MISSING aifw anchor hooks — auto-healing by reloading {PF_CONF}" + ); + let reload = tokio::process::Command::new("/usr/local/bin/sudo") + .args(["/sbin/pfctl", "-f", PF_CONF]) + .output() + .await; + match reload { + Ok(o) if o.status.success() => { + info!("pf drift check: auto-heal succeeded — {PF_CONF} loaded into main ruleset"); + } + Ok(o) => { + error!( + "pf drift check: auto-heal FAILED (status {}): {}", + o.status, + String::from_utf8_lossy(&o.stderr).trim() + ); + return; + } + Err(e) => { + error!("pf drift check: auto-heal could not spawn pfctl: {e}"); + return; + } + } + } + + // A valid ruleset can still be present while packet filtering itself is + // disabled. In that state `pfctl -sn` succeeds and the anchor check above + // looks healthy, but traffic bypasses every rule. Only enable pf after the + // ruleset is known to be valid, and preserve the detect-only opt-out. + let status = tokio::process::Command::new("/usr/local/bin/sudo") + .args(["/sbin/pfctl", "-si"]) .output() .await; - match reload { - Ok(o) if o.status.success() => { - info!("pf drift check: auto-heal succeeded — {PF_CONF} loaded into main ruleset"); + match status { + Ok(o) if o.status.success() && pf_status_is_disabled(&o.stdout) => { + if auto_heal_disabled { + error!( + "pf drift check: packet filter is DISABLED — auto-heal disabled via \ + AIFW_NO_PF_AUTO_HEAL; run `pfctl -e` to enable it" + ); + return; + } + + let enable = tokio::process::Command::new("/usr/local/bin/sudo") + .args(["/sbin/pfctl", "-e"]) + .output() + .await; + match enable { + Ok(o) if o.status.success() => { + info!("pf drift check: packet filter was disabled; enabled it") + } + Ok(o) => error!( + "pf drift check: pfctl -e FAILED (status {}): {}", + o.status, + String::from_utf8_lossy(&o.stderr).trim() + ), + Err(e) => error!("pf drift check: could not spawn pfctl -e: {e}"), + } } Ok(o) => { - error!( - "pf drift check: auto-heal FAILED (status {}): {}", - o.status, - String::from_utf8_lossy(&o.stderr).trim() - ); + if !o.status.success() { + info!( + "pf drift check: pfctl -si failed (status {}): {}", + o.status, + String::from_utf8_lossy(&o.stderr).trim() + ); + } } - Err(e) => { - error!("pf drift check: auto-heal could not spawn pfctl: {e}"); + Err(e) => info!("pf drift check: could not inspect pf status: {e}"), + } +} + +fn pf_auto_heal_disabled(value: Option<&str>) -> bool { + value + .map(|v| { + !v.is_empty() + && v != "0" + && !v.eq_ignore_ascii_case("no") + && !v.eq_ignore_ascii_case("false") + }) + .unwrap_or(false) +} + +fn pf_status_is_disabled(stdout: &[u8]) -> bool { + String::from_utf8_lossy(stdout) + .lines() + .any(|line| line.trim_start().starts_with("Status: Disabled")) +} + +#[cfg(test)] +mod pf_reconcile_tests { + use super::{pf_auto_heal_disabled, pf_status_is_disabled}; + + #[test] + fn auto_heal_opt_out_parses_false_values() { + for value in [None, Some(""), Some("0"), Some("no"), Some("FALSE")] { + assert!( + !pf_auto_heal_disabled(value), + "unexpected opt-out: {value:?}" + ); } + assert!(pf_auto_heal_disabled(Some("1"))); + assert!(pf_auto_heal_disabled(Some("yes"))); + } + + #[test] + fn disabled_status_is_detected_without_matching_enabled_status() { + assert!(pf_status_is_disabled( + b"Status: Disabled for 0 days 00:01:00\nDebug: Urgent\n" + )); + assert!(pf_status_is_disabled(b" Status: Disabled\n")); + assert!(!pf_status_is_disabled( + b"Status: Enabled for 0 days 00:01:00\n" + )); } } diff --git a/aifw-setup/src/apply.rs b/aifw-setup/src/apply.rs index 7e1db714..abe7feb7 100644 --- a/aifw-setup/src/apply.rs +++ b/aifw-setup/src/apply.rs @@ -2511,6 +2511,7 @@ aifw ALL=(root) NOPASSWD: /sbin/pfctl -sr # gets a password-required error, bails, and never heals -> LAN outage. Do # NOT drop this (it was missing in v5.96.16 and caused a LAN-down regression). aifw ALL=(root) NOPASSWD: /sbin/pfctl -sn +aifw ALL=(root) NOPASSWD: /sbin/pfctl -e aifw ALL=(root) NOPASSWD: /sbin/pfctl -ss aifw ALL=(root) NOPASSWD: /sbin/pfctl -ss -v aifw ALL=(root) NOPASSWD: /sbin/pfctl -ss -vv @@ -2676,6 +2677,7 @@ mod sudoers_tests { ("get_rules", "/sbin/pfctl -a aifw* -sr"), ("get_nat_rules", "/sbin/pfctl -a aifw* -sn"), ("daemon pf drift auto-heal (global -sn)", "/sbin/pfctl -sn"), + ("daemon pf re-enable after boot", "/sbin/pfctl -e"), ("get_queues", "/sbin/pfctl -a aifw* -sq"), ("flush_rules", "/sbin/pfctl -a aifw* -Fr"), ("flush_nat_rules", "/sbin/pfctl -a aifw* -Fn"), From ab63ab3b438f8e0a37cc2f7fa02c2150dee6c236 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:10:10 +0000 Subject: [PATCH 02/71] fix: allow isolated static WAN seeds --- aifw-setup/src/config.rs | 6 ++---- aifw-setup/src/tests.rs | 8 +++++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/aifw-setup/src/config.rs b/aifw-setup/src/config.rs index bb0b8adb..656f9081 100644 --- a/aifw-setup/src/config.rs +++ b/aifw-setup/src/config.rs @@ -108,10 +108,8 @@ impl SetupConfig { if self.wan_interface.trim().is_empty() { return Err("wan_interface must not be empty".to_string()); } - if matches!(self.wan_mode, WanMode::Static) - && (self.wan_ip.is_none() || self.wan_gateway.is_none()) - { - return Err("static wan_mode requires wan_ip and wan_gateway".to_string()); + if matches!(self.wan_mode, WanMode::Static) && self.wan_ip.is_none() { + return Err("static wan_mode requires wan_ip".to_string()); } if self.lan_ip.is_some() && self.lan_interface.is_none() { return Err("lan_ip requires lan_interface".to_string()); diff --git a/aifw-setup/src/tests.rs b/aifw-setup/src/tests.rs index 25bc965d..cd157f91 100644 --- a/aifw-setup/src/tests.rs +++ b/aifw-setup/src/tests.rs @@ -527,7 +527,13 @@ mod let_underscore_tests { }; let mut c = mk(); c.wan_mode = crate::config::WanMode::Static; - assert!(c.validate_seed().is_err(), "static without ip/gateway"); + assert!(c.validate_seed().is_err(), "static without ip"); + + c.wan_ip = Some("198.18.0.1/24".to_string()); + assert!( + c.validate_seed().is_ok(), + "static WAN without a default gateway is valid" + ); let mut c = mk(); c.lan_ip = Some("192.168.1.1/24".to_string()); From 9dc1d443b7c095886dd52402676751a54420c777 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:08:46 +0000 Subject: [PATCH 03/71] test: extend FreeBSD packet and API coverage --- freebsd/tests/README.md | 1 + freebsd/tests/lib.sh | 10 +++++++++ freebsd/tests/run-all.sh | 2 +- freebsd/tests/t02-pass-block.sh | 29 ++++++++++++++++++++++++ freebsd/tests/t08-control-plane.sh | 36 ++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 freebsd/tests/t08-control-plane.sh diff --git a/freebsd/tests/README.md b/freebsd/tests/README.md index 421453b5..9236a0da 100644 --- a/freebsd/tests/README.md +++ b/freebsd/tests/README.md @@ -30,6 +30,7 @@ by hand on a test VM and under `vmactions/freebsd-vm` in CI | `t04-nat` | Outbound SNAT (translation visible in the pf state table) and rdr port-forward into the server jail, with return traffic | | `t05-restore-roundtrip` | #535: save → mutate → restore brings both DB and the live anchor back | | `t06-wireguard` | Tunnel creation on the real kernel + #541 pubkey-derives-from-privkey via `wg pubkey` (skips without wireguard-kmod) | +| `t08-control-plane` | Unauthenticated access rejection, invalid-login rejection, live PF/rule counters, identity, and representative JSON response shapes | ## Running on a test VM diff --git a/freebsd/tests/lib.sh b/freebsd/tests/lib.sh index a40a1c72..5eafd102 100644 --- a/freebsd/tests/lib.sh +++ b/freebsd/tests/lib.sh @@ -129,6 +129,16 @@ server_listen_once() { # $1 = port, $2 = marker file jx_server sh -c "rm -f '$2'; (nc -l '$1' >/dev/null 2>&1 && touch '$2') &" } +# One-shot UDP listener for packet-path tests. UDP has no connection refusal, +# so callers must wait for the marker rather than rely on nc's exit status. +server_listen_udp_once() { # $1 = port, $2 = marker file + jx_server sh -c "rm -f '$2'; (nc -u -l '$1' >/dev/null 2>&1 && touch '$2') &" +} + +client_send_udp() { # $1 = host, $2 = port + printf 'aifw-functional-udp\n' | jx_client nc -u -w 2 "$1" "$2" >/dev/null 2>&1 +} + wait_for_file() { # $1 = jail (client|server), $2 = file, $3 = seconds _i=0 while [ "$_i" -lt "$3" ]; do diff --git a/freebsd/tests/run-all.sh b/freebsd/tests/run-all.sh index 7fc42494..1e50d212 100644 --- a/freebsd/tests/run-all.sh +++ b/freebsd/tests/run-all.sh @@ -231,7 +231,7 @@ log "API up, authenticated" # ---------------------------------------------------------------- run tests -TESTS="${TESTS:-t01-pfctl-acceptance t02-pass-block t03-schedule-gating t04-nat t05-restore-roundtrip t06-wireguard t07-ipsec}" +TESTS="${TESTS:-t01-pfctl-acceptance t02-pass-block t03-schedule-gating t04-nat t05-restore-roundtrip t06-wireguard t07-ipsec t08-control-plane}" TOTAL=0 FAILED_TESTS="" diff --git a/freebsd/tests/t02-pass-block.sh b/freebsd/tests/t02-pass-block.sh index ecb8e32b..ca74c14c 100644 --- a/freebsd/tests/t02-pass-block.sh +++ b/freebsd/tests/t02-pass-block.sh @@ -41,5 +41,34 @@ save_pf_artifacts t02 api DELETE "/api/v1/rules/$PASS_ID" >/dev/null || fail "delete pass" api_reload || fail "cleanup reload" +# 5. UDP follows the same default-deny/pass/remove contract. UDP has no +# handshake, so the server marker is the authoritative evidence of delivery. +UDP_PORT=8085 +UDP_MARKER=/tmp/aifwfx-t02-udp-marker +server_listen_udp_once "$UDP_PORT" "$UDP_MARKER" +client_send_udp "$SERVER_IP" "$UDP_PORT" +wait_for_file server "$UDP_MARKER" 3 && fail "default-deny: UDP reached server with no pass rule" + +UDP_ID=$(add_rule '{"action":"pass","direction":"in","protocol":"udp","dst_addr":"'"$SERVER_IP"'","dst_port_start":'"$UDP_PORT"',"dst_port_end":'"$UDP_PORT"',"label":"t02-udp-pass"}') || fail "create UDP pass" +api_reload || fail "reload UDP pass" +server_listen_udp_once "$UDP_PORT" "$UDP_MARKER" +client_send_udp "$SERVER_IP" "$UDP_PORT" +wait_for_file server "$UDP_MARKER" 3 || fail "UDP pass: server never saw the datagram" + +api DELETE "/api/v1/rules/$UDP_ID" >/dev/null || fail "delete UDP pass" +api_reload || fail "reload UDP removal" +server_listen_udp_once "$UDP_PORT" "$UDP_MARKER" +client_send_udp "$SERVER_IP" "$UDP_PORT" +wait_for_file server "$UDP_MARKER" 3 && fail "after removing UDP pass, datagram was delivered" + +# 6. ICMP is explicitly exercised instead of being inferred from TCP. +ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 && fail "default-deny: ICMP reached server with no pass rule" +ICMP_ID=$(add_rule '{"action":"pass","direction":"in","protocol":"icmp","dst_addr":"'"$SERVER_IP"'","label":"t02-icmp-pass"}') || fail "create ICMP pass" +api_reload || fail "reload ICMP pass" +ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 || fail "ICMP pass: echo request failed" +api DELETE "/api/v1/rules/$ICMP_ID" >/dev/null || fail "delete ICMP pass" +api_reload || fail "reload ICMP removal" +ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 && fail "after removing ICMP pass, echo succeeded" + [ "$FAILURES" = 0 ] || exit 1 exit 0 diff --git a/freebsd/tests/t08-control-plane.sh b/freebsd/tests/t08-control-plane.sh new file mode 100644 index 00000000..276c1200 --- /dev/null +++ b/freebsd/tests/t08-control-plane.sh @@ -0,0 +1,36 @@ +# shellcheck shell=sh +# T08 — authenticated control-plane contract. The Linux/PfMock suite covers +# handler logic; this checks the shipped API, auth boundary, and representative +# response shapes on the same FreeBSD process used by the packet tests. + +BASE="$API_BASE" + +code=$(curl -sS -m 15 -o /dev/null -w '%{http_code}' "$BASE/api/v1/status") +[ "$code" = 401 ] || fail "protected /status returned $code without auth" + +code=$(curl -sS -m 15 -o /dev/null -w '%{http_code}' -X POST \ + "$BASE/api/v1/auth/login" -H 'Content-Type: application/json' \ + -d '{"username":"functest","password":"definitely-wrong"}') +[ "$code" = 401 ] || fail "invalid login returned $code" + +for endpoint in /api/v1/status /api/v1/about /api/v1/auth/me \ + /api/v1/rules /api/v1/nat /api/v1/interfaces/detailed /api/v1/auth/users; do + body=$(api GET "$endpoint") || { fail "GET $endpoint failed"; continue; } + printf '%s' "$body" | jq -e 'type == "object"' >/dev/null 2>&1 \ + || fail "$endpoint did not return a JSON object" +done + +status=$(api GET /api/v1/status) || fail "status request failed" +printf '%s' "$status" | jq -e '.pf_running == true and (.aifw_rules | type == "number") and (.nat_rules | type == "number")' >/dev/null 2>&1 \ + || fail "status response is missing live pf/rule counters: $status" + +me=$(api GET /api/v1/auth/me) || fail "auth/me request failed" +printf '%s' "$me" | jq -e '.username == "functest" and (.permissions | type == "array")' >/dev/null 2>&1 \ + || fail "auth/me response does not identify the authenticated user: $me" + +about=$(api GET /api/v1/about) || fail "about request failed" +printf '%s' "$about" | jq -e '.version and (.memory | type == "number")' >/dev/null 2>&1 \ + || fail "about response is missing version/memory: $about" + +[ "$FAILURES" = 0 ] || exit 1 +exit 0 From 2d336e67ddb931aab9925041f4a2a452a00e7d00 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:09:17 +0000 Subject: [PATCH 04/71] test: run ICMP checks inside the client jail --- freebsd/tests/t02-pass-block.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/freebsd/tests/t02-pass-block.sh b/freebsd/tests/t02-pass-block.sh index ca74c14c..01af589c 100644 --- a/freebsd/tests/t02-pass-block.sh +++ b/freebsd/tests/t02-pass-block.sh @@ -62,13 +62,13 @@ client_send_udp "$SERVER_IP" "$UDP_PORT" wait_for_file server "$UDP_MARKER" 3 && fail "after removing UDP pass, datagram was delivered" # 6. ICMP is explicitly exercised instead of being inferred from TCP. -ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 && fail "default-deny: ICMP reached server with no pass rule" +jx_client ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 && fail "default-deny: ICMP reached server with no pass rule" ICMP_ID=$(add_rule '{"action":"pass","direction":"in","protocol":"icmp","dst_addr":"'"$SERVER_IP"'","label":"t02-icmp-pass"}') || fail "create ICMP pass" api_reload || fail "reload ICMP pass" -ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 || fail "ICMP pass: echo request failed" +jx_client ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 || fail "ICMP pass: echo request failed" api DELETE "/api/v1/rules/$ICMP_ID" >/dev/null || fail "delete ICMP pass" api_reload || fail "reload ICMP removal" -ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 && fail "after removing ICMP pass, echo succeeded" +jx_client ping -c 1 -t 2 "$SERVER_IP" >/dev/null 2>&1 && fail "after removing ICMP pass, echo succeeded" [ "$FAILURES" = 0 ] || exit 1 exit 0 From a62c31ed2cd54dc9628382310aa6aa4d0f083de5 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:11:59 +0000 Subject: [PATCH 05/71] test: assert the about memory object shape --- freebsd/tests/t08-control-plane.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freebsd/tests/t08-control-plane.sh b/freebsd/tests/t08-control-plane.sh index 276c1200..35b2075d 100644 --- a/freebsd/tests/t08-control-plane.sh +++ b/freebsd/tests/t08-control-plane.sh @@ -29,7 +29,7 @@ printf '%s' "$me" | jq -e '.username == "functest" and (.permissions | type == " || fail "auth/me response does not identify the authenticated user: $me" about=$(api GET /api/v1/about) || fail "about request failed" -printf '%s' "$about" | jq -e '.version and (.memory | type == "number")' >/dev/null 2>&1 \ +printf '%s' "$about" | jq -e '(.version | type == "string") and (.memory | type == "object")' >/dev/null 2>&1 \ || fail "about response is missing version/memory: $about" [ "$FAILURES" = 0 ] || exit 1 From 97890d905a40d58e977857d20db5fe434b4a325f Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:14:19 +0000 Subject: [PATCH 06/71] test: strengthen appliance boot smoke assertions --- freebsd/tests/README.md | 5 +++-- freebsd/tests/smoke-boot.sh | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/freebsd/tests/README.md b/freebsd/tests/README.md index 9236a0da..3d763dbf 100644 --- a/freebsd/tests/README.md +++ b/freebsd/tests/README.md @@ -63,8 +63,9 @@ Expect ~20–30 minutes; the FreeBSD VM boot + build dominates. Runs on a **Linux** host with qemu (KVM when available): boots the built USB IMG unmodified with a seed ISO attached as a CD, waits for `aifw_firstboot` to complete unattended setup, then asserts the seeded -admin can log in through the LAN side and that the WAN side stays -default-denied. +admin can log in through the LAN side, invalid credentials are rejected, +the static UI is served, live PF/rule counters are reported, and the WAN +side stays default-denied. ```sh xz -dk aifw--amd64.img.xz diff --git a/freebsd/tests/smoke-boot.sh b/freebsd/tests/smoke-boot.sh index a09db6b2..4dc6e02f 100644 --- a/freebsd/tests/smoke-boot.sh +++ b/freebsd/tests/smoke-boot.sh @@ -140,6 +140,23 @@ if [ -z "$TOKEN" ]; then fi log "first boot complete after ~${elapsed}s; admin login OK via LAN ($SCHEME)" +# Exercise the unauthenticated boundary and the shipped static UI before +# relying on the authenticated status response below. +invalid=$(curl -k -s -m 10 -o /dev/null -w '%{http_code}' \ + -X POST "$SCHEME://127.0.0.1:$LAN_PORT/api/v1/auth/login" \ + -H 'Content-Type: application/json' \ + -d '{"username":"admin","password":"definitely-wrong"}') +[ "$invalid" = 401 ] || { + log "FAIL: invalid login returned HTTP $invalid (expected 401)" + exit 1 +} +ui=$(curl -k -s -m 10 "$SCHEME://127.0.0.1:$LAN_PORT/") +printf '%s' "$ui" | grep -q 'AiFw' || { + log "FAIL: LAN-served UI did not contain the AiFw marker" + exit 1 +} +log "auth boundary and served UI checks OK" + # Authenticated status call — proves the API is actually serving, not just # terminating connections. status=$(curl -k -s -m 10 "$SCHEME://127.0.0.1:$LAN_PORT/api/v1/status" \ @@ -149,6 +166,10 @@ printf '%s' "$status" | jq -e . >/dev/null 2>&1 || { log "FAIL: /api/v1/status did not return JSON: $status" exit 1 } +printf '%s' "$status" | jq -e '.pf_running == true and (.aifw_rules | type == "number") and (.nat_rules | type == "number")' >/dev/null 2>&1 || { + log "FAIL: status response did not report live PF/rule counters: $status" + exit 1 +} log "status endpoint OK" # Default-deny on WAN: the management API port via the WAN interface must From 4933f4dfd849ff544f454c6e0a967c5aa49a1564 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:20:24 +0000 Subject: [PATCH 07/71] test: mark UDP delivery only after payload receipt --- freebsd/tests/lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freebsd/tests/lib.sh b/freebsd/tests/lib.sh index 5eafd102..0de21173 100644 --- a/freebsd/tests/lib.sh +++ b/freebsd/tests/lib.sh @@ -132,7 +132,7 @@ server_listen_once() { # $1 = port, $2 = marker file # One-shot UDP listener for packet-path tests. UDP has no connection refusal, # so callers must wait for the marker rather than rely on nc's exit status. server_listen_udp_once() { # $1 = port, $2 = marker file - jx_server sh -c "rm -f '$2'; (nc -u -l '$1' >/dev/null 2>&1 && touch '$2') &" + jx_server sh -c "rm -f '$2'; (nc -u -l '$1' 2>/dev/null | (IFS= read -r line && touch '$2')) &" } client_send_udp() { # $1 = host, $2 = port From a2ba98d450509b706216153b93f7e457e262b80f Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 21 Jul 2026 22:20:24 +0000 Subject: [PATCH 08/71] test: verify PF and rule persistence across reboot --- freebsd/tests/README.md | 3 +- freebsd/tests/smoke-boot.sh | 78 +++++++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/freebsd/tests/README.md b/freebsd/tests/README.md index 3d763dbf..4f4598d1 100644 --- a/freebsd/tests/README.md +++ b/freebsd/tests/README.md @@ -65,7 +65,8 @@ USB IMG unmodified with a seed ISO attached as a CD, waits for `aifw_firstboot` to complete unattended setup, then asserts the seeded admin can log in through the LAN side, invalid credentials are rejected, the static UI is served, live PF/rule counters are reported, and the WAN -side stays default-denied. +side stays default-denied. It also creates a rule, reboots the appliance, +and verifies PF plus the persisted live rule recover after boot. ```sh xz -dk aifw--amd64.img.xz diff --git a/freebsd/tests/smoke-boot.sh b/freebsd/tests/smoke-boot.sh index 4dc6e02f..502022df 100644 --- a/freebsd/tests/smoke-boot.sh +++ b/freebsd/tests/smoke-boot.sh @@ -27,7 +27,7 @@ while [ $# -gt 0 ]; do done [ -n "$IMG" ] && [ -f "$IMG" ] || { echo "usage: smoke-boot.sh --img PATH" >&2; exit 2; } -for c in qemu-system-x86_64 curl jq; do +for c in qemu-system-x86_64 curl jq ssh ssh-keygen; do command -v "$c" >/dev/null 2>&1 || { echo "missing: $c" >&2; exit 2; } done MKISO="" @@ -44,9 +44,14 @@ ADMIN_USER="admin" ADMIN_PASS="SmokeTest123" LAN_PORT=18443 # host port forwarded to the appliance LAN address WAN_PORT=18081 # host port forwarded to the appliance WAN address (must stay blocked) +SSH_PORT=18422 # root SSH on the LAN side, used for reboot persistence # ---------------------------------------------------------------- seed ISO +ssh-keygen -q -t ed25519 -N '' -f "$WORK/id_ed25519" \ + || { echo "ephemeral SSH key generation failed" >&2; exit 2; } +SSH_PUB=$(cat "$WORK/id_ed25519.pub") + cat > "$WORK/aifw-seed.json" < "$WORK/aifw-seed.json" </dev/null || { + log "FAIL: reload before reboot failed" + exit 1 +} + +SSH="ssh -i $WORK/id_ed25519 -p $SSH_PORT -o BatchMode=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o ConnectTimeout=5" +ssh_elapsed=0 +while [ "$ssh_elapsed" -lt 60 ]; do + # shellcheck disable=SC2086 + $SSH root@127.0.0.1 true >/dev/null 2>&1 && break + sleep 2 + ssh_elapsed=$((ssh_elapsed + 2)) +done +[ "$ssh_elapsed" -lt 60 ] || { log "FAIL: appliance SSH never became reachable"; exit 1; } + +log "requesting appliance reboot" +# A successful reboot normally drops the SSH transport before it can return. +# shellcheck disable=SC2086 +$SSH root@127.0.0.1 /sbin/shutdown -r now >/dev/null 2>&1 || true +sleep 10 + +TOKEN="" +elapsed=0 +while [ "$elapsed" -lt "$TIMEOUT" ]; do + resp=$(curl -k -s -m 5 -X POST "$SCHEME://127.0.0.1:$LAN_PORT/api/v1/auth/login" \ + -H 'Content-Type: application/json' -d "$login_body" 2>/dev/null) || true + TOKEN=$(printf '%s' "$resp" | jq -r '.tokens.access_token // empty' 2>/dev/null) + [ -n "$TOKEN" ] && break + sleep 5 + elapsed=$((elapsed + 5)) +done +[ -n "$TOKEN" ] || { log "FAIL: API did not recover after reboot"; exit 1; } + +post_status=$(curl -k -s -m 10 "$SCHEME://127.0.0.1:$LAN_PORT/api/v1/status" \ + -H "Authorization: Bearer $TOKEN") +printf '%s' "$post_status" | jq -e '.pf_running == true' >/dev/null 2>&1 || { + log "FAIL: post-reboot status reports PF is not running: $post_status" + exit 1 +} +post_rules=$(curl -k -s -m 10 "$SCHEME://127.0.0.1:$LAN_PORT/api/v1/rules" \ + -H "Authorization: Bearer $TOKEN") +printf '%s' "$post_rules" | jq -e --arg id "$RULE_ID" '.data[] | select(.id == $id and .label == "smoke-reboot-persist")' >/dev/null 2>&1 || { + log "FAIL: persisted rule missing from API after reboot: $post_rules" + exit 1 +} +# shellcheck disable=SC2086 +$SSH root@127.0.0.1 "pfctl -a aifw -sr" 2>/dev/null | grep -q 'smoke-reboot-persist' || { + log "FAIL: persisted rule missing from live PF anchor after reboot" + exit 1 +} +log "reboot recovery OK; PF enabled and persisted rule live" + # Default-deny on WAN: the management API port via the WAN interface must # NOT answer — the seed configures a LAN, so generate_pf_conf scopes the # SSH/API pass rules to it (#560). A reachable WAN-side API means that From 68371d752af79ce5cbd499e4c05491bc7a3d939a Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 18:21:50 -0400 Subject: [PATCH 09/71] feat: pre-validate target config before destructive restore steps (#535) Run the whole target config through the same converters and engine validators the apply path uses, before the first DELETE. A config that would abort mid-apply is now rejected with 400 and zero mutations, instead of relying on snapshot rollback. Covers all entry paths (restore, import, commit-confirm rollback, cluster sync) via checks in both apply_firewall_config and the rollback wrapper. validate_nat_rule and AliasEngine::validate_name become public so the restore path reuses the exact add-time checks rather than duplicates. --- aifw-api/src/backup.rs | 153 +++++++++++++++++++++++++++++++++++++++++ aifw-api/src/tests.rs | 60 ++++++++++++++++ aifw-core/src/alias.rs | 9 ++- aifw-core/src/nat.rs | 5 +- 4 files changed, 223 insertions(+), 4 deletions(-) diff --git a/aifw-api/src/backup.rs b/aifw-api/src/backup.rs index 17e136e2..758c9760 100644 --- a/aifw-api/src/backup.rs +++ b/aifw-api/src/backup.rs @@ -1511,6 +1511,146 @@ fn apply_fail(step: &str, e: impl std::fmt::Display) -> StatusCode { StatusCode::INTERNAL_SERVER_ERROR } +/// Pre-apply validation (#535): run the target config through the same +/// converters and validators the apply path uses, *before* the first +/// destructive DELETE. Entries the apply loop would silently skip +/// (unparseable rows, interfaces the user chose to drop) are skipped here +/// too — this only rejects configs that would abort mid-apply. +pub(crate) fn prevalidate_config( + config: &FirewallConfig, + iface_map: &InterfaceMap, +) -> Result<(), String> { + config.validate()?; + + for rc in &config.rules { + let iface_after = match rc.interface.as_deref() { + Some(name) => match map_iface(name, iface_map) { + Some(mapped) => Some(mapped), + None => continue, + }, + None => None, + }; + let mut rc = rc.clone(); + rc.interface = iface_after; + if let Some(rule) = rule_from_config(&rc) { + aifw_core::validation::validate_rule(&rule) + .map_err(|e| format!("rule {}: {e}", rc.id))?; + } + } + + for nc in &config.nat { + let Some(mapped) = map_iface(&nc.interface, iface_map) else { + continue; + }; + let mut nc = nc.clone(); + nc.interface = mapped; + if let Some(nat) = nat_from_config(&nc) { + aifw_core::nat::validate_nat_rule(&nat) + .map_err(|e| format!("nat rule {}: {e}", nc.id))?; + } + } + + for ac in &config.aliases { + if aifw_common::AliasType::parse(&ac.alias_type).is_none() { + continue; + } + aifw_core::AliasEngine::validate_name(&ac.name) + .map_err(|e| format!("alias {}: {e}", ac.name))?; + } + + // WireGuard: mirror the engine's add-time checks, plus duplicate listen + // ports *within* the config (the tables are wiped before re-insert, so + // only intra-config duplicates can collide). + let mut wg_ports = std::collections::HashSet::new(); + for wg in &config.vpn.wireguard { + if aifw_common::Address::parse(&wg.address).is_err() + || map_iface(&wg.interface, iface_map).is_none() + { + continue; + } + if wg.name.is_empty() { + return Err(format!("wg tunnel {}: tunnel name required", wg.id)); + } + if wg.listen_port == 0 { + return Err(format!("wg tunnel {}: listen port required", wg.name)); + } + if !wg_ports.insert(wg.listen_port) { + return Err(format!( + "wg tunnel {}: listen port {} used by another tunnel in this config", + wg.name, wg.listen_port + )); + } + for p in &wg.peers { + if p.public_key.is_empty() { + return Err(format!( + "wg peer {} on {}: public key required", + p.id, wg.name + )); + } + } + } + + for sac in &config.vpn.ipsec { + if aifw_common::Address::parse(&sac.src_addr).is_err() + || aifw_common::Address::parse(&sac.dst_addr).is_err() + { + continue; + } + if sac.name.is_empty() { + return Err(format!("ipsec SA {}: name required", sac.id)); + } + } + + for tunnel in &config.vpn.ipsec_tunnels { + tunnel + .validate() + .map_err(|e| format!("ipsec tunnel {}: {e}", tunnel.name))?; + } + + for rc in &config.rate_limits { + if rate_limit_from_config(rc).is_some() { + if rc.max_connections == 0 { + return Err(format!( + "rate limit {}: max_connections must be > 0", + rc.name + )); + } + if rc.window_secs == 0 { + return Err(format!("rate limit {}: window_secs must be > 0", rc.name)); + } + } + } + + for sc in &config.tls.sni_rules { + if sc.pattern.is_empty() { + return Err(format!("sni rule {}: pattern required", sc.id)); + } + } + + for vc in &config.ha.carp_vips { + if map_iface(&vc.interface, iface_map).is_none() || carp_vip_from_config(vc).is_none() { + continue; + } + if vc.vhid == 0 { + return Err(format!("carp vip {}: VHID must be > 0", vc.virtual_ip)); + } + if vc.password.is_empty() { + return Err(format!( + "carp vip {}: CARP password required", + vc.virtual_ip + )); + } + } + + for nc in &config.ha.nodes { + if cluster_node_from_config(nc).is_some() && nc.name.is_empty() { + return Err(format!("cluster node {}: name required", nc.id)); + } + } + + Ok(()) +} + /// Restore-with-rollback wrapper around [`apply_firewall_config`] (#535). /// /// Captures the current running config first, applies the target strictly, @@ -1529,6 +1669,12 @@ pub(crate) async fn apply_firewall_config_or_rollback( config: &FirewallConfig, iface_map: &InterfaceMap, ) -> Result<(), StatusCode> { + // Validate before snapshotting: a config that can't apply must be + // rejected with nothing mutated, not "applied" and rolled back. + prevalidate_config(config, iface_map).map_err(|e| { + tracing::warn!(error = %e, "config apply: pre-validation rejected target config"); + StatusCode::BAD_REQUEST + })?; let snapshot = build_current_config(state).await?; let Err(apply_err) = apply_firewall_config(state, config, iface_map).await else { return Ok(()); @@ -1580,6 +1726,13 @@ pub(crate) async fn apply_firewall_config( Address, CountryCode, GeoIpRule, Interface, IpsecSa, VpnStatus, WgPeer, WgTunnel, }; + // Direct callers (commit-confirm rollback timer, cluster snapshot sync) + // don't go through the wrapper — validate here too before any DELETE. + prevalidate_config(config, iface_map).map_err(|e| { + tracing::error!(error = %e, "config apply: pre-validation failed — nothing changed"); + StatusCode::BAD_REQUEST + })?; + // Restore preludes — clear existing rows before re-populating from the // imported config. A failed DELETE (locked DB, FK violation, schema // drift) used to warn and continue, leaving stale rows in a half-imported diff --git a/aifw-api/src/tests.rs b/aifw-api/src/tests.rs index 61d88fc4..eb0b34d3 100644 --- a/aifw-api/src/tests.rs +++ b/aifw-api/src/tests.rs @@ -2296,6 +2296,66 @@ mod tests { assert!(res.is_err(), "partial apply must not report success"); } + #[tokio::test] + async fn test_prevalidation_rejects_bad_config_without_mutation() { + // A config that would abort mid-apply (rule priority out of range) + // must be rejected up front with 400 and zero rows touched (#535). + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + let rule = aifw_common::Rule::new( + aifw_common::Action::Pass, + aifw_common::Direction::In, + aifw_common::Protocol::Tcp, + aifw_common::RuleMatch { + src_addr: aifw_common::Address::Any, + src_port: None, + dst_addr: aifw_common::Address::Any, + dst_port: None, + }, + ); + state.rule_engine.add_rule(rule).await.unwrap(); + + let mut config = crate::backup::build_current_config(&state).await.unwrap(); + config.rules[0].priority = 20_000; // validate_rule caps at 10000 + + let res = + crate::backup::apply_firewall_config_or_rollback(&state, &config, &Default::default()) + .await; + assert_eq!(res, Err(axum::http::StatusCode::BAD_REQUEST)); + let rules = state.rule_engine.list_rules().await.unwrap(); + assert_eq!(rules.len(), 1, "prevalidation failure must not touch rows"); + } + + #[tokio::test] + async fn test_prevalidation_rejects_duplicate_wg_ports() { + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + let mut config = crate::backup::build_current_config(&state).await.unwrap(); + for name in ["wg-a", "wg-b"] { + config + .vpn + .wireguard + .push(aifw_core::config::WireguardTunnelConfig { + id: uuid::Uuid::new_v4().to_string(), + name: name.to_string(), + interface: "wg0".to_string(), + listen_port: 51820, + address: "10.9.0.1/24".to_string(), + private_key: "k".into(), + public_key: "K".into(), + dns: None, + mtu: None, + peers: vec![], + }); + } + let res = + crate::backup::apply_firewall_config_or_rollback(&state, &config, &Default::default()) + .await; + assert_eq!(res, Err(axum::http::StatusCode::BAD_REQUEST)); + } + // --- IPsec tunnels (#530) --- fn ipsec_tunnel_body() -> Value { diff --git a/aifw-core/src/alias.rs b/aifw-core/src/alias.rs index fe12ff14..f2dcb9d4 100644 --- a/aifw-core/src/alias.rs +++ b/aifw-core/src/alias.rs @@ -88,7 +88,7 @@ impl AliasEngine { /// 1-31 alphanumeric/`_`/`-` chars or is reserved (`bruteforce`, /// `ai_blocked`). pub async fn add(&self, alias: Alias) -> Result { - self.validate_name(&alias.name)?; + Self::validate_name(&alias.name)?; let entries_json = serde_json::to_string(&alias.entries) .map_err(|e| AifwError::Validation(e.to_string()))?; @@ -110,7 +110,7 @@ impl AliasEngine { /// re-synced when enabled). Fails with `NotFound` if the id doesn't /// exist, or validation on a bad name. pub async fn update(&self, alias: Alias) -> Result { - self.validate_name(&alias.name)?; + Self::validate_name(&alias.name)?; let entries_json = serde_json::to_string(&alias.entries) .map_err(|e| AifwError::Validation(e.to_string()))?; let now = Utc::now().to_rfc3339(); @@ -239,7 +239,10 @@ impl AliasEngine { Ok(()) } - fn validate_name(&self, name: &str) -> Result<()> { + /// Validate an alias name: 1-31 alphanumeric/`_`/`-` chars, not + /// reserved. Associated (not `&self`) so the backup restore path can + /// pre-validate a whole config with the same checks `add` applies. + pub fn validate_name(name: &str) -> Result<()> { if name.is_empty() || name.len() > 31 { return Err(AifwError::Validation( "Alias name must be 1-31 characters".into(), diff --git a/aifw-core/src/nat.rs b/aifw-core/src/nat.rs index b4f7d220..8ef749be 100644 --- a/aifw-core/src/nat.rs +++ b/aifw-core/src/nat.rs @@ -312,7 +312,10 @@ impl NatEngine { } } -fn validate_nat_rule(rule: &NatRule) -> Result<()> { +/// Validate a NAT rule before persisting: interface required, DNAT/RDR +/// needs a destination or redirect port. Public so the backup restore path +/// can pre-validate a whole config with the same checks `add_rule` applies. +pub fn validate_nat_rule(rule: &NatRule) -> Result<()> { if rule.interface.0.is_empty() { return Err(AifwError::Validation( "NAT rule requires an interface".to_string(), From 2492886c4935d8d9db94d4558ba9ebd0cffd5ac5 Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 18:37:56 -0400 Subject: [PATCH 10/71] feat: single DB transaction for config restore (#158, #535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apply_firewall_config now runs every database mutation — all section clears and re-inserts (rules, NAT, aliases, static routes, geo-IP, WireGuard, IPsec SAs+tunnels, auth, shaping, TLS, HA, DHCP) — on one sqlx transaction. Any DB-stage failure rolls the whole database back automatically; the DB can no longer end up half-restored. pf/kernel/ service applies run strictly after commit, with the snapshot wrapper as their recovery path. Engines gain executor-generic insert variants (insert_*_on) following the existing insert_rule_on/log_on pattern; public add_* methods delegate to them, so validation lives in one place. AliasEngine gains sync_all_strict so post-commit pf table sync stays a required step. Per-row audit entries during restore are replaced by one ConfigChanged row committed atomically with the restore itself. --- aifw-api/src/backup.rs | 352 +++++++++++++++++++++------------------ aifw-api/src/tests.rs | 80 +++++++++ aifw-core/src/alias.rs | 41 ++++- aifw-core/src/db.rs | 5 +- aifw-core/src/geoip.rs | 17 +- aifw-core/src/ha.rs | 56 +++++-- aifw-core/src/ipsec.rs | 15 +- aifw-core/src/nat.rs | 4 +- aifw-core/src/shaping.rs | 48 +++--- aifw-core/src/tls.rs | 31 +++- aifw-core/src/vpn.rs | 68 ++++++-- 11 files changed, 487 insertions(+), 230 deletions(-) diff --git a/aifw-api/src/backup.rs b/aifw-api/src/backup.rs index 758c9760..c912e544 100644 --- a/aifw-api/src/backup.rs +++ b/aifw-api/src/backup.rs @@ -1733,12 +1733,43 @@ pub(crate) async fn apply_firewall_config( StatusCode::BAD_REQUEST })?; - // Restore preludes — clear existing rows before re-populating from the - // imported config. A failed DELETE (locked DB, FK violation, schema - // drift) used to warn and continue, leaving stale rows in a half-imported - // firewall; since #535 it aborts the restore instead. Parse/mapping skips - // (`continue`) below are intentional drops the import preview already - // surfaced; operational failures are errors. + // Engines whose tables may not exist yet. Their migrates are idempotent + // DDL — run them before the transaction so the write transaction below + // holds only data statements. + let shaping = aifw_core::shaping::ShapingEngine::new(state.pool.clone(), state.pf.clone()); + shaping + .migrate() + .await + .map_err(|e| apply_fail("shaping migrate", e))?; + let tls_engine = aifw_core::tls::TlsEngine::new(state.pool.clone(), state.pf.clone()); + tls_engine + .migrate() + .await + .map_err(|e| apply_fail("tls migrate", e))?; + let ha_engine = aifw_core::ha::ClusterEngine::new(state.pool.clone(), state.pf.clone()); + ha_engine + .migrate() + .await + .map_err(|e| apply_fail("ha migrate", e))?; + + // Single transaction for ALL database mutations (#158/#535): the + // DELETE-then-reinsert of every section below either commits wholesale + // or rolls back automatically on the first error, so the DB can never + // end up half-restored. pf/kernel/service state can't ride in a SQL + // transaction — those applies run after commit, with the snapshot + // wrapper (`apply_firewall_config_or_rollback`) as their recovery path. + // Parse/mapping skips (`continue`) below are intentional drops the + // import preview already surfaced; operational failures are errors. + let mut tx = state + .pool + .begin() + .await + .map_err(|e| apply_fail("begin restore transaction", e))?; + + // NB: "queue_configs"/"rate_limit_rules" are the real shaping table + // names — the pre-#535 code deleted from "queues"/"rate_limits" (which + // don't exist) and swallowed the error, so shaping rows were never + // actually cleared before re-insert. for table in [ "wg_peers", "wg_tunnels", @@ -1749,9 +1780,16 @@ pub(crate) async fn apply_firewall_config( "nat_rules", "aliases", "static_routes", + "queue_configs", + "rate_limit_rules", + "sni_rules", + "ja3_blocklist", + "carp_vips", + "pfsync_config", + "cluster_nodes", ] { sqlx::query(sqlx::AssertSqlSafe(format!("DELETE FROM {table}"))) - .execute(&state.pool) + .execute(&mut *tx) .await .map_err(|e| apply_fail(&format!("clearing {table}"), e))?; } @@ -1768,9 +1806,7 @@ pub(crate) async fn apply_firewall_config( rc.interface = iface_after; if let Some(rule) = rule_from_config(&rc) { let rule_id = rule.id; - state - .rule_engine - .add_rule(rule) + aifw_core::Database::insert_rule_on(&mut *tx, &rule) .await .map_err(|e| apply_fail(&format!("rule {rule_id} restore"), e))?; } else { @@ -1786,9 +1822,7 @@ pub(crate) async fn apply_firewall_config( nc.interface = mapped_iface; if let Some(nat) = nat_from_config(&nc) { let nat_id = nat.id; - state - .nat_engine - .add_rule(nat) + aifw_core::nat::NatEngine::insert_rule_on(&mut *tx, &nat) .await .map_err(|e| apply_fail(&format!("nat rule {nat_id} restore"), e))?; } else { @@ -1796,9 +1830,8 @@ pub(crate) async fn apply_firewall_config( } } - // Aliases — restored from snapshot. AliasEngine.add validates name + - // resyncs the pf table; failures here just skip the row (same pattern as - // rules/NAT above). + // Aliases — rows insert in the transaction; pf tables re-sync after + // commit (sync_all_strict below). for ac in &config.aliases { use aifw_common::{Alias, AliasType}; let Some(alias_type) = AliasType::parse(&ac.alias_type) else { @@ -1815,17 +1848,16 @@ pub(crate) async fn apply_firewall_config( created_at: chrono::Utc::now(), updated_at: chrono::Utc::now(), }; - let alias_name = alias.name.clone(); - state - .alias_engine - .add(alias) + aifw_core::AliasEngine::insert_on(&mut *tx, &alias) .await - .map_err(|e| apply_fail(&format!("alias {alias_name} restore"), e))?; + .map_err(|e| apply_fail(&format!("alias {} restore", alias.name), e))?; } - // Static routes — restored via direct INSERT + apply_route_to_system, - // matching what /api/v1/routes does for manual creates. Interface map - // applies if the snapshot pinned a specific iface. + // Static routes — restored via direct INSERT, matching what + // /api/v1/routes does for manual creates. Interface map applies if the + // snapshot pinned a specific iface. Kernel route application is deferred + // to after commit (collected here). + let mut kernel_routes: Vec<(aifw_core::config::StaticRouteConfig, Option)> = Vec::new(); for rc in &config.static_routes { let iface_after = match rc.interface.as_deref() { Some(name) => match map_iface(name, iface_map) { @@ -1834,7 +1866,7 @@ pub(crate) async fn apply_firewall_config( }, None => None, }; - let insert_res = sqlx::query( + sqlx::query( "INSERT INTO static_routes (id, destination, gateway, interface, metric, enabled, description, created_at, fib) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", ) .bind(&rc.id) @@ -1846,21 +1878,11 @@ pub(crate) async fn apply_firewall_config( .bind(rc.description.as_deref()) .bind(chrono::Utc::now().to_rfc3339()) .bind(rc.fib as i64) - .execute(&state.pool) - .await; - insert_res - .map_err(|e| apply_fail(&format!("static route {} restore", rc.destination), e))?; - // Kernel route application stays best-effort: it shells out to - // `route`, which is absent/unprivileged on dev hosts, and the row is - // reapplied at boot. + .execute(&mut *tx) + .await + .map_err(|e| apply_fail(&format!("static route {} restore", rc.destination), e))?; if rc.enabled { - crate::routes::apply_route_to_system( - &rc.destination, - &rc.gateway, - iface_after.as_deref(), - rc.fib, - ) - .await; + kernel_routes.push((rc.clone(), iface_after)); } } @@ -1874,9 +1896,7 @@ pub(crate) async fn apply_firewall_config( rule.label = gc.label.clone(); rule.status = gc.status; let country = rule.country.0.clone(); - state - .geoip_engine - .add_rule(rule) + aifw_core::geoip::GeoIpEngine::insert_rule_on(&mut *tx, &rule) .await .map_err(|e| apply_fail(&format!("geo-ip rule {country} restore"), e))?; } @@ -1906,12 +1926,9 @@ pub(crate) async fn apply_firewall_config( created_at: now, updated_at: now, }; - let tunnel_name = tunnel.name.clone(); - state - .vpn_engine - .add_wg_tunnel(tunnel) + aifw_core::vpn::VpnEngine::insert_wg_tunnel_on(&mut *tx, &tunnel) .await - .map_err(|e| apply_fail(&format!("wg tunnel {tunnel_name} restore"), e))?; + .map_err(|e| apply_fail(&format!("wg tunnel {} restore", tunnel.name), e))?; for p in &wg.peers { let peer_id = uuid::Uuid::parse_str(&p.id).unwrap_or_else(|_| uuid::Uuid::new_v4()); let allowed_ips: Vec
= p @@ -1932,12 +1949,9 @@ pub(crate) async fn apply_firewall_config( created_at: now, updated_at: now, }; - let peer_name = peer.name.clone(); - state - .vpn_engine - .add_wg_peer(peer) + aifw_core::vpn::VpnEngine::insert_wg_peer_on(&mut *tx, &peer) .await - .map_err(|e| apply_fail(&format!("wg peer {peer_name} restore"), e))?; + .map_err(|e| apply_fail(&format!("wg peer {} restore", peer.name), e))?; } } @@ -1953,43 +1967,17 @@ pub(crate) async fn apply_firewall_config( sa.id = id; sa.enc_algo = sac.enc_algo.clone(); sa.auth_algo = sac.auth_algo.clone(); - let sa_name = sa.name.clone(); - state - .vpn_engine - .add_ipsec_sa(sa) + aifw_core::vpn::VpnEngine::insert_ipsec_sa_on(&mut *tx, &sa) .await - .map_err(|e| apply_fail(&format!("ipsec SA {sa_name} restore"), e))?; + .map_err(|e| apply_fail(&format!("ipsec SA {} restore", sa.name), e))?; } - // Real IPsec tunnels (#530): restore records first, then one apply to - // re-render swanctl config and reload charon. + // Real IPsec tunnels (#530): restore records in the transaction; the + // swanctl re-render + charon reload runs after commit. for tunnel in &config.vpn.ipsec_tunnels { - let name = tunnel.name.clone(); - state - .ipsec_engine - .add_tunnel(tunnel.clone()) + aifw_core::ipsec::IpsecEngine::insert_tunnel_on(&mut *tx, tunnel) .await - .map_err(|e| apply_fail(&format!("ipsec tunnel {name} restore"), e))?; - } - if !config.vpn.ipsec_tunnels.is_empty() - && let Err(e) = state.ipsec_engine.apply_all().await - { - tracing::warn!(error = %e, "restored IPsec tunnels but swanctl apply failed — tunnels stay down until re-applied"); - } - - if !config.system.dns_servers.is_empty() { - let content: String = config - .system - .dns_servers - .iter() - .map(|s| format!("nameserver {s}")) - .collect::>() - .join("\n"); - // Best-effort: /etc/resolv.conf is a root-owned system file the aifw - // user may not be able to write; the DB remains the source of truth. - if let Err(e) = tokio::fs::write("/etc/resolv.conf", &content).await { - tracing::warn!(error = %e, "import: /etc/resolv.conf write failed"); - } + .map_err(|e| apply_fail(&format!("ipsec tunnel {} restore", tunnel.name), e))?; } let auth = &config.auth; @@ -2010,26 +1998,12 @@ pub(crate) async fn apply_firewall_config( sqlx::query("INSERT OR REPLACE INTO auth_config (key, value) VALUES (?1, ?2)") .bind(key) .bind(value) - .execute(&state.pool) + .execute(&mut *tx) .await .map_err(|e| apply_fail(&format!("auth config {key} restore"), e))?; } - // Traffic shaping: queues + per-IP rate limits - let shaping = aifw_core::shaping::ShapingEngine::new(state.pool.clone(), state.pf.clone()); - shaping - .migrate() - .await - .map_err(|e| apply_fail("shaping migrate", e))?; - // NB: the pre-#535 code deleted from "queues"/"rate_limits" — tables that - // don't exist — and swallowed the error, so shaping rows were never - // actually cleared before re-insert. These are the real table names. - for table in ["queue_configs", "rate_limit_rules"] { - sqlx::query(sqlx::AssertSqlSafe(format!("DELETE FROM {table}"))) - .execute(&state.pool) - .await - .map_err(|e| apply_fail(&format!("clearing {table}"), e))?; - } + // Traffic shaping: queues + per-IP rate limits (tables cleared above) for qc in &config.queues { let Some(mapped) = map_iface(&qc.interface, iface_map) else { continue; @@ -2037,8 +2011,7 @@ pub(crate) async fn apply_firewall_config( let mut qc = qc.clone(); qc.interface = mapped; if let Some(q) = queue_from_config(&qc) { - shaping - .add_queue(q) + aifw_core::shaping::ShapingEngine::insert_queue_on(&mut *tx, &q) .await .map_err(|e| apply_fail(&format!("shaping queue {} restore", qc.name), e))?; } @@ -2054,60 +2027,27 @@ pub(crate) async fn apply_firewall_config( let mut rc = rc.clone(); rc.interface = iface_after; if let Some(r) = rate_limit_from_config(&rc) { - shaping - .add_rate_limit(r) + aifw_core::shaping::ShapingEngine::insert_rate_limit_on(&mut *tx, &r) .await .map_err(|e| apply_fail("rate limit restore", e))?; } } - shaping - .apply_queues() - .await - .map_err(|e| apply_fail("shaping queues apply", e))?; - shaping - .apply_rate_limits() - .await - .map_err(|e| apply_fail("rate limits apply", e))?; - // TLS: SNI rules + JA3 blocklist - let tls_engine = aifw_core::tls::TlsEngine::new(state.pool.clone(), state.pf.clone()); - tls_engine - .migrate() - .await - .map_err(|e| apply_fail("tls migrate", e))?; - for table in ["sni_rules", "ja3_blocklist"] { - sqlx::query(sqlx::AssertSqlSafe(format!("DELETE FROM {table}"))) - .execute(&state.pool) - .await - .map_err(|e| apply_fail(&format!("clearing {table}"), e))?; - } + // TLS: SNI rules + JA3 blocklist (tables cleared above) for sc in &config.tls.sni_rules { if let Some(sni) = sni_rule_from_config(sc) { - tls_engine - .add_sni_rule(sni) + aifw_core::tls::TlsEngine::insert_sni_rule_on(&mut *tx, &sni) .await .map_err(|e| apply_fail(&format!("sni rule {} restore", sc.pattern), e))?; } } for hash in &config.tls.blocked_ja3 { - tls_engine - .add_ja3_block(hash, "restored from backup") + aifw_core::tls::TlsEngine::insert_ja3_block_on(&mut *tx, hash, "restored from backup") .await .map_err(|e| apply_fail(&format!("ja3 block {hash} restore"), e))?; } - // HA: CARP VIPs + pfsync + cluster nodes - let ha_engine = aifw_core::ha::ClusterEngine::new(state.pool.clone(), state.pf.clone()); - ha_engine - .migrate() - .await - .map_err(|e| apply_fail("ha migrate", e))?; - for table in ["carp_vips", "pfsync_config", "cluster_nodes"] { - sqlx::query(sqlx::AssertSqlSafe(format!("DELETE FROM {table}"))) - .execute(&state.pool) - .await - .map_err(|e| apply_fail(&format!("clearing {table}"), e))?; - } + // HA: CARP VIPs + pfsync + cluster nodes (tables cleared above) for vc in &config.ha.carp_vips { let Some(mapped) = map_iface(&vc.interface, iface_map) else { continue; @@ -2115,8 +2055,7 @@ pub(crate) async fn apply_firewall_config( let mut vc = vc.clone(); vc.interface = mapped; if let Some(vip) = carp_vip_from_config(&vc) { - ha_engine - .add_carp_vip(vip) + aifw_core::ha::ClusterEngine::insert_carp_vip_on(&mut *tx, &vip) .await .map_err(|e| apply_fail(&format!("carp vip {} restore", vc.virtual_ip), e))?; } @@ -2127,21 +2066,100 @@ pub(crate) async fn apply_firewall_config( let mut pc = pc.clone(); pc.sync_interface = mapped_sync; if let Some(pfsync) = pfsync_from_config(&pc) { - ha_engine - .set_pfsync(pfsync) + aifw_core::ha::ClusterEngine::set_pfsync_on(&mut tx, &pfsync) .await .map_err(|e| apply_fail("pfsync restore", e))?; } } for nc in &config.ha.nodes { if let Some(node) = cluster_node_from_config(nc) { - ha_engine - .add_node(node) + aifw_core::ha::ClusterEngine::insert_node_on(&mut *tx, &node) .await .map_err(|e| apply_fail("cluster node restore", e))?; } } + // DHCP: subnets, reservations, global/DDNS/HA config (DB rows only; the + // rDHCP service regen runs after commit) + apply_dhcp_section_db(&mut tx, &config.dhcp).await?; + + // One audit row for the whole restore, committed atomically with it. + // (Pre-#158 each engine `add` wrote a per-row audit entry; a restore is + // one operator action, not N rule additions.) + aifw_core::AuditLog::log_on( + &mut *tx, + aifw_core::AuditAction::ConfigChanged, + None, + &format!( + "config restore applied: {} rules, {} nat, {} aliases, {} geoip, {} wg, {} ipsec tunnels", + config.rules.len(), + config.nat.len(), + config.aliases.len(), + config.geoip.len(), + config.vpn.wireguard.len(), + config.vpn.ipsec_tunnels.len(), + ), + "restore", + ) + .await + .map_err(|e| apply_fail("restore audit entry", e))?; + + tx.commit() + .await + .map_err(|e| apply_fail("commit restore transaction", e))?; + + // ============================================================ + // Post-commit: pf / kernel / service applies. The DB is now fully + // consistent; a failure below is a data-plane mismatch that must + // surface as an error (#535) — the snapshot wrapper re-applies the + // prior config to recover. Steps marked best-effort are reapplied at + // boot and degrade capacity, not policy. + // ============================================================ + + // Kernel route application. Best-effort: it shells out to `route`, + // which is absent/unprivileged on dev hosts; rows are reapplied at boot. + for (rc, iface_after) in &kernel_routes { + crate::routes::apply_route_to_system( + &rc.destination, + &rc.gateway, + iface_after.as_deref(), + rc.fib, + ) + .await; + } + + // Alias pf tables — required; a stale table means rules referencing the + // alias match the wrong addresses. + state + .alias_engine + .sync_all_strict() + .await + .map_err(|e| apply_fail("alias pf table sync", e))?; + + // swanctl re-render + charon reload. Warn-only: strongSwan is a + // FreeBSD-side companion service absent on dev hosts; tunnels stay down + // until re-applied. + if !config.vpn.ipsec_tunnels.is_empty() + && let Err(e) = state.ipsec_engine.apply_all().await + { + tracing::warn!(error = %e, "restored IPsec tunnels but swanctl apply failed — tunnels stay down until re-applied"); + } + + if !config.system.dns_servers.is_empty() { + let content: String = config + .system + .dns_servers + .iter() + .map(|s| format!("nameserver {s}")) + .collect::>() + .join("\n"); + // Best-effort: /etc/resolv.conf is a root-owned system file the aifw + // user may not be able to write; the DB remains the source of truth. + if let Err(e) = tokio::fs::write("/etc/resolv.conf", &content).await { + tracing::warn!(error = %e, "import: /etc/resolv.conf write failed"); + } + } + // pf state-table tuning. Best-effort: this shells out to system tooling // absent on dev hosts, the same setting is reapplied at every boot // (`pf_tuning::apply_on_boot`, also warn-only), and a missed tuning value @@ -2156,11 +2174,21 @@ pub(crate) async fn apply_firewall_config( } } - // DHCP: subnets, reservations, global/DDNS/HA config - apply_dhcp_section(state, &config.dhcp).await?; + // Regenerate rDHCP TOML + restart service so the restored config takes + // effect. Best-effort: the companion service may be absent (dev hosts); + // the DB rows are authoritative and reapplied at boot. + crate::dhcp::auto_apply(state).await; // Final data-plane applies — a failure here means the kernel does NOT // match the restored DB, so it must not report success (#535). + shaping + .apply_queues() + .await + .map_err(|e| apply_fail("shaping queues apply", e))?; + shaping + .apply_rate_limits() + .await + .map_err(|e| apply_fail("rate limits apply", e))?; let vpn_rules = state .vpn_engine .collect_vpn_rules() @@ -2186,13 +2214,15 @@ pub(crate) async fn apply_firewall_config( Ok(()) } -async fn apply_dhcp_section( - state: &AppState, +/// DHCP section of a restore — DB rows only, on the caller's transaction +/// connection (#158/#535). The rDHCP service regen (`dhcp::auto_apply`) +/// runs post-commit in `apply_firewall_config`. +async fn apply_dhcp_section_db( + conn: &mut sqlx::SqliteConnection, dhcp: &aifw_core::config::DhcpSection, ) -> Result<(), StatusCode> { - // Wipe + re-insert for a clean restore. `auto_apply` at the end regenerates - // the rDHCP TOML config and restarts the service. DB mutations are - // required steps (#535); only the service regeneration is best-effort. + // Wipe + re-insert for a clean restore. DB mutations are required + // steps (#535). for table in [ "dhcp_subnets", "dhcp_reservations", @@ -2201,7 +2231,7 @@ async fn apply_dhcp_section( "dhcp_ha_config", ] { sqlx::query(sqlx::AssertSqlSafe(format!("DELETE FROM {table}"))) - .execute(&state.pool) + .execute(&mut *conn) .await .map_err(|e| apply_fail(&format!("clearing {table}"), e))?; } @@ -2256,7 +2286,7 @@ async fn apply_dhcp_section( sqlx::query("INSERT OR REPLACE INTO dhcp_config (key, value) VALUES (?1, ?2)") .bind(k) .bind(v) - .execute(&state.pool) + .execute(&mut *conn) .await .map_err(|e| apply_fail(&format!("dhcp config {k} restore"), e))?; } @@ -2333,7 +2363,7 @@ async fn apply_dhcp_section( .bind(&s.ntp_servers) .bind(&options_json) .bind(&s.created_at) - .execute(&state.pool) + .execute(&mut *conn) .await .map_err(|e| apply_fail(&format!("dhcp subnet {} restore", s.network), e))?; } @@ -2346,7 +2376,7 @@ async fn apply_dhcp_section( ) .bind(&r.id).bind(&r.subnet_id).bind(&r.mac_address).bind(&r.ip_address) .bind(&r.hostname).bind(&r.client_id).bind(&r.description).bind(&r.created_at) - .execute(&state.pool).await + .execute(&mut *conn).await .map_err(|e| apply_fail(&format!("dhcp reservation {} restore", r.ip_address), e))?; } @@ -2373,7 +2403,7 @@ async fn apply_dhcp_section( sqlx::query("INSERT OR REPLACE INTO dhcp_ddns_config (key, value) VALUES (?1, ?2)") .bind(k) .bind(v) - .execute(&state.pool) + .execute(&mut *conn) .await .map_err(|e| apply_fail(&format!("dhcp ddns config {k} restore"), e))?; } @@ -2410,15 +2440,11 @@ async fn apply_dhcp_section( sqlx::query("INSERT OR REPLACE INTO dhcp_ha_config (key, value) VALUES (?1, ?2)") .bind(k) .bind(v) - .execute(&state.pool) + .execute(&mut *conn) .await .map_err(|e| apply_fail(&format!("dhcp ha config {k} restore"), e))?; } - // Regenerate rDHCP TOML + restart service so the restored config takes - // effect. Best-effort: the companion service may be absent (dev hosts); - // the DB rows above are authoritative and reapplied at boot. - crate::dhcp::auto_apply(state).await; Ok(()) } diff --git a/aifw-api/src/tests.rs b/aifw-api/src/tests.rs index eb0b34d3..c133c29f 100644 --- a/aifw-api/src/tests.rs +++ b/aifw-api/src/tests.rs @@ -2296,6 +2296,86 @@ mod tests { assert!(res.is_err(), "partial apply must not report success"); } + #[tokio::test] + async fn test_mid_apply_failure_rolls_back_db_transaction() { + // Force a failure LATE in the apply (the DHCP clear runs after every + // rules/NAT/alias insert) and verify the single restore transaction + // (#158) rewinds everything — the pre-restore rows must survive + // untouched even without the snapshot-reapply wrapper. + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + let rule = aifw_common::Rule::new( + aifw_common::Action::Pass, + aifw_common::Direction::In, + aifw_common::Protocol::Tcp, + aifw_common::RuleMatch { + src_addr: aifw_common::Address::Any, + src_port: None, + dst_addr: aifw_common::Address::Any, + dst_port: None, + }, + ); + let rule_id = state.rule_engine.add_rule(rule).await.unwrap().id; + + let config = crate::backup::build_current_config(&state).await.unwrap(); + sqlx::query("DROP TABLE dhcp_subnets") + .execute(&state.pool) + .await + .unwrap(); + let res = crate::backup::apply_firewall_config(&state, &config, &Default::default()).await; + assert!(res.is_err(), "late failure must abort the restore"); + + let rules = state.rule_engine.list_rules().await.unwrap(); + assert_eq!(rules.len(), 1, "transaction rollback must keep prior rows"); + assert_eq!(rules[0].id, rule_id); + } + + #[tokio::test] + async fn test_restore_1k_rules_round_trips() { + // #158 acceptance: a large config restores through the single + // transaction and every row survives the round trip. + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + let mut config = crate::backup::build_current_config(&state).await.unwrap(); + for i in 0..1000 { + config.rules.push(aifw_core::config::RuleConfig { + id: uuid::Uuid::new_v4().to_string(), + priority: i, + action: aifw_common::Action::Pass, + direction: aifw_common::Direction::In, + protocol: aifw_common::Protocol::Tcp, + interface: None, + src_addr: Some("any".to_string()), + src_port_start: None, + src_port_end: None, + dst_addr: Some("any".to_string()), + dst_port_start: Some(1000 + i as u16), + dst_port_end: Some(1000 + i as u16), + log: false, + quick: true, + label: Some(format!("bulk-{i}")), + state_tracking: aifw_common::StateTracking::KeepState, + status: aifw_common::RuleStatus::Active, + ip_version: aifw_common::IpVersion::Both, + src_invert: false, + dst_invert: false, + schedule_id: None, + gateway: None, + }); + } + let started = std::time::Instant::now(); + crate::backup::apply_firewall_config(&state, &config, &Default::default()) + .await + .expect("bulk restore must succeed"); + let elapsed = started.elapsed(); + let rules = state.rule_engine.list_rules().await.unwrap(); + assert_eq!(rules.len(), 1000, "every rule must survive the round trip"); + // Generous bound — the point is one transaction, not per-row fsyncs. + assert!(elapsed.as_secs() < 30, "bulk restore took {elapsed:?}"); + } + #[tokio::test] async fn test_prevalidation_rejects_bad_config_without_mutation() { // A config that would abort mid-apply (rule priority out of range) diff --git a/aifw-core/src/alias.rs b/aifw-core/src/alias.rs index f2dcb9d4..218841e5 100644 --- a/aifw-core/src/alias.rs +++ b/aifw-core/src/alias.rs @@ -88,6 +88,23 @@ impl AliasEngine { /// 1-31 alphanumeric/`_`/`-` chars or is reserved (`bruteforce`, /// `ai_blocked`). pub async fn add(&self, alias: Alias) -> Result { + Self::insert_on(&self.pool, &alias).await?; + + if alias.enabled { + self.sync_to_pf(&alias).await?; + } + + tracing::info!(name = %alias.name, alias_type = %alias.alias_type, entries = alias.entries.len(), "alias created"); + Ok(alias) + } + + /// Executor-generic validate + insert with no pf side effects. Public so + /// the transactional restore path (#158/#535) can batch alias rows with + /// every other section; pf tables are re-synced after commit. + pub async fn insert_on<'e, E>(exec: E, alias: &Alias) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { Self::validate_name(&alias.name)?; let entries_json = serde_json::to_string(&alias.entries) .map_err(|e| AifwError::Validation(e.to_string()))?; @@ -96,14 +113,8 @@ impl AliasEngine { .bind(alias.id.to_string()).bind(&alias.name).bind(alias.alias_type.as_str()) .bind(&entries_json).bind(alias.description.as_deref()) .bind(alias.enabled).bind(alias.created_at.to_rfc3339()).bind(alias.updated_at.to_rfc3339()) - .execute(&self.pool).await.map_err(|e| AifwError::Database(e.to_string()))?; - - if alias.enabled { - self.sync_to_pf(&alias).await?; - } - - tracing::info!(name = %alias.name, alias_type = %alias.alias_type, entries = alias.entries.len(), "alias created"); - Ok(alias) + .execute(exec).await.map_err(|e| AifwError::Database(e.to_string()))?; + Ok(()) } /// Update an alias row, then flush and repopulate its pf table (only @@ -151,6 +162,20 @@ impl AliasEngine { Ok(()) } + /// Sync all enabled aliases to pf tables, failing on the first error. + /// The restore path uses this after its transaction commits so a pf-side + /// failure surfaces as a required-step error (#535) instead of the + /// warn-and-continue behavior of [`Self::sync_all`]. + pub async fn sync_all_strict(&self) -> Result<()> { + for alias in self.list().await? { + if alias.enabled { + let _ = self.pf.flush_table(&alias.name).await; + self.sync_to_pf(&alias).await?; + } + } + Ok(()) + } + /// Sync all enabled aliases to pf tables. Called during reload. pub async fn sync_all(&self) -> Result<()> { let aliases = self.list().await?; diff --git a/aifw-core/src/db.rs b/aifw-core/src/db.rs index 9e217841..6660575a 100644 --- a/aifw-core/src/db.rs +++ b/aifw-core/src/db.rs @@ -189,8 +189,9 @@ impl Database { } /// Executor-generic variant so engines can run the insert inside a - /// transaction alongside its audit row (PERF-H6 #350). - pub(crate) async fn insert_rule_on<'e, E>(exec: E, rule: &Rule) -> Result<()> + /// transaction alongside its audit row (PERF-H6 #350) and the restore + /// path can batch every section into one transaction (#158/#535). + pub async fn insert_rule_on<'e, E>(exec: E, rule: &Rule) -> Result<()> where E: sqlx::Executor<'e, Database = sqlx::Sqlite>, { diff --git a/aifw-core/src/geoip.rs b/aifw-core/src/geoip.rs index 161b9c8b..6ea11119 100644 --- a/aifw-core/src/geoip.rs +++ b/aifw-core/src/geoip.rs @@ -99,6 +99,17 @@ impl GeoIpEngine { /// Insert a per-country block/allow rule row. pf tables aren't touched /// until the geo-IP rules are next applied pub async fn add_rule(&self, rule: GeoIpRule) -> Result { + Self::insert_rule_on(&self.pool, &rule).await?; + tracing::info!(id = %rule.id, country = %rule.country, action = %rule.action, "geo-ip rule added"); + Ok(rule) + } + + /// Executor-generic insert. Public so the transactional restore path + /// (#158/#535) can batch geo-IP rows with every other section. + pub async fn insert_rule_on<'e, E>(exec: E, rule: &GeoIpRule) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { sqlx::query( r#" INSERT INTO geoip_rules (id, country, action, label, status, created_at, updated_at) @@ -115,11 +126,9 @@ impl GeoIpEngine { }) .bind(rule.created_at.to_rfc3339()) .bind(rule.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(id = %rule.id, country = %rule.country, action = %rule.action, "geo-ip rule added"); - Ok(rule) + Ok(()) } /// All geo-IP rules ordered by country code diff --git a/aifw-core/src/ha.rs b/aifw-core/src/ha.rs index 97aa1d5f..5378fa22 100644 --- a/aifw-core/src/ha.rs +++ b/aifw-core/src/ha.rs @@ -169,6 +169,17 @@ impl ClusterEngine { /// Store a new CARP virtual IP. Fails validation if VHID is 0 or the password is empty pub async fn add_carp_vip(&self, vip: CarpVip) -> Result { + Self::insert_carp_vip_on(&self.pool, &vip).await?; + tracing::info!(id = %vip.id, vhid = vip.vhid, ip = %vip.virtual_ip, "CARP VIP added"); + Ok(vip) + } + + /// Executor-generic validate + insert. Public for the transactional + /// restore path (#158/#535). + pub async fn insert_carp_vip_on<'e, E>(exec: E, vip: &CarpVip) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { if vip.vhid == 0 { return Err(AifwError::Validation("VHID must be > 0".to_string())); } @@ -192,11 +203,9 @@ impl ClusterEngine { .bind(vip.status.to_string()) .bind(vip.created_at.to_rfc3339()) .bind(vip.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(id = %vip.id, vhid = vip.vhid, ip = %vip.virtual_ip, "CARP VIP added"); - Ok(vip) + Ok(()) } /// List all configured CARP VIPs ordered by VHID @@ -251,9 +260,23 @@ impl ClusterEngine { /// Store the pfsync configuration, replacing any existing one (singleton table) pub async fn set_pfsync(&self, config: PfsyncConfig) -> Result { + let mut conn = self.pool.acquire().await?; + Self::set_pfsync_on(&mut conn, &config).await?; + tracing::info!(interface = %config.sync_interface, "pfsync configured"); + Ok(config) + } + + /// Replace the pfsync config (DELETE + INSERT) on a single connection. + /// Public for the transactional restore path (#158/#535); takes `&mut + /// SqliteConnection` rather than a generic executor because it runs two + /// statements. + pub async fn set_pfsync_on( + conn: &mut sqlx::SqliteConnection, + config: &PfsyncConfig, + ) -> Result<()> { // Replace any existing config sqlx::query("DELETE FROM pfsync_config") - .execute(&self.pool) + .execute(&mut *conn) .await?; sqlx::query( @@ -273,11 +296,9 @@ impl ClusterEngine { .bind(config.heartbeat_interval_ms.map(|n| n as i64)) .bind(config.dhcp_link) .bind(config.created_at.to_rfc3339()) - .execute(&self.pool) + .execute(&mut *conn) .await?; - - tracing::info!(interface = %config.sync_interface, "pfsync configured"); - Ok(config) + Ok(()) } /// Fetch the pfsync configuration, or `None` if pfsync has never been configured @@ -296,6 +317,17 @@ impl ClusterEngine { /// Register a peer node in the cluster. Fails validation if the name is empty pub async fn add_node(&self, node: ClusterNode) -> Result { + Self::insert_node_on(&self.pool, &node).await?; + tracing::info!(name = %node.name, role = %node.role, "cluster node added"); + Ok(node) + } + + /// Executor-generic validate + insert. Public for the transactional + /// restore path (#158/#535). + pub async fn insert_node_on<'e, E>(exec: E, node: &ClusterNode) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { if node.name.is_empty() { return Err(AifwError::Validation("node name required".to_string())); } @@ -314,11 +346,9 @@ impl ClusterEngine { .bind(node.last_seen.to_rfc3339()) .bind(node.config_version as i64) .bind(node.created_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(name = %node.name, role = %node.role, "cluster node added"); - Ok(node) + Ok(()) } /// List all registered cluster nodes ordered by name diff --git a/aifw-core/src/ipsec.rs b/aifw-core/src/ipsec.rs index 60566646..274f43df 100644 --- a/aifw-core/src/ipsec.rs +++ b/aifw-core/src/ipsec.rs @@ -625,6 +625,17 @@ impl IpsecEngine { /// Validate and persist a new tunnel. pub async fn add_tunnel(&self, tunnel: IpsecTunnel) -> Result { + Self::insert_tunnel_on(&self.pool, &tunnel).await?; + Ok(tunnel) + } + + /// Executor-generic validate + insert with no swanctl side effects. + /// Public for the transactional restore path (#158/#535); callers apply + /// the swanctl config separately. + pub async fn insert_tunnel_on<'e, E>(exec: E, tunnel: &IpsecTunnel) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { tunnel.validate()?; sqlx::query( r#" @@ -664,9 +675,9 @@ impl IpsecEngine { .bind(tunnel.start_action.to_string()) .bind(tunnel.created_at.to_rfc3339()) .bind(tunnel.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - Ok(tunnel) + Ok(()) } /// All tunnels, oldest first. diff --git a/aifw-core/src/nat.rs b/aifw-core/src/nat.rs index 8ef749be..75f2483b 100644 --- a/aifw-core/src/nat.rs +++ b/aifw-core/src/nat.rs @@ -272,7 +272,9 @@ impl NatEngine { Ok(()) } - async fn insert_rule_on<'e, E>(exec: E, rule: &NatRule) -> Result<()> + /// Executor-generic insert. Public so the transactional restore path + /// (#158/#535) can batch NAT rows with every other section. + pub async fn insert_rule_on<'e, E>(exec: E, rule: &NatRule) -> Result<()> where E: sqlx::Executor<'e, Database = sqlx::Sqlite>, { diff --git a/aifw-core/src/shaping.rs b/aifw-core/src/shaping.rs index bb92cfb5..1d0f90f3 100644 --- a/aifw-core/src/shaping.rs +++ b/aifw-core/src/shaping.rs @@ -90,7 +90,7 @@ impl ShapingEngine { /// Insert a queue config row. pf isn't touched until /// [`Self::apply_queues`] pub async fn add_queue(&self, config: QueueConfig) -> Result { - self.insert_queue(&config).await?; + Self::insert_queue_on(&self.pool, &config).await?; tracing::info!(id = %config.id, name = %config.name, "queue added"); Ok(config) } @@ -153,20 +153,7 @@ impl ShapingEngine { /// `max_connections` or `window_secs` is 0 or the overload table name /// is empty. pf isn't touched until [`Self::apply_rate_limits`] pub async fn add_rate_limit(&self, rule: RateLimitRule) -> Result { - if rule.max_connections == 0 { - return Err(AifwError::Validation( - "max_connections must be > 0".to_string(), - )); - } - if rule.window_secs == 0 { - return Err(AifwError::Validation("window_secs must be > 0".to_string())); - } - if rule.overload_table.is_empty() { - return Err(AifwError::Validation( - "overload_table name required".to_string(), - )); - } - self.insert_rate_limit(&rule).await?; + Self::insert_rate_limit_on(&self.pool, &rule).await?; tracing::info!(id = %rule.id, name = %rule.name, "rate limit rule added"); Ok(rule) } @@ -221,7 +208,12 @@ impl ShapingEngine { // --- DB helpers --- - async fn insert_queue(&self, q: &QueueConfig) -> Result<()> { + /// Executor-generic insert. Public for the transactional restore path + /// (#158/#535). + pub async fn insert_queue_on<'e, E>(exec: E, q: &QueueConfig) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { let bw_unit = match q.bandwidth.unit { BandwidthUnit::Bps => "bps", BandwidthUnit::Kbps => "kbps", @@ -250,12 +242,30 @@ impl ShapingEngine { }) .bind(q.created_at.to_rfc3339()) .bind(q.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; Ok(()) } - async fn insert_rate_limit(&self, r: &RateLimitRule) -> Result<()> { + /// Executor-generic validate + insert. Public for the transactional + /// restore path (#158/#535). + pub async fn insert_rate_limit_on<'e, E>(exec: E, r: &RateLimitRule) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { + if r.max_connections == 0 { + return Err(AifwError::Validation( + "max_connections must be > 0".to_string(), + )); + } + if r.window_secs == 0 { + return Err(AifwError::Validation("window_secs must be > 0".to_string())); + } + if r.overload_table.is_empty() { + return Err(AifwError::Validation( + "overload_table name required".to_string(), + )); + } sqlx::query( r#" INSERT INTO rate_limit_rules (id, name, interface, protocol, src_addr, dst_addr, @@ -282,7 +292,7 @@ impl ShapingEngine { }) .bind(r.created_at.to_rfc3339()) .bind(r.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; Ok(()) } diff --git a/aifw-core/src/tls.rs b/aifw-core/src/tls.rs index 3e5ea7d8..535b0d72 100644 --- a/aifw-core/src/tls.rs +++ b/aifw-core/src/tls.rs @@ -81,6 +81,17 @@ impl TlsEngine { /// Insert an SNI rule row. Fails validation when the pattern is empty pub async fn add_sni_rule(&self, rule: SniRule) -> Result { + Self::insert_sni_rule_on(&self.pool, &rule).await?; + tracing::info!(id = %rule.id, pattern = %rule.pattern, action = %rule.action, "SNI rule added"); + Ok(rule) + } + + /// Executor-generic validate + insert. Public for the transactional + /// restore path (#158/#535). + pub async fn insert_sni_rule_on<'e, E>(exec: E, rule: &SniRule) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { if rule.pattern.is_empty() { return Err(AifwError::Validation("SNI pattern required".to_string())); } @@ -95,11 +106,9 @@ impl TlsEngine { .bind(match rule.status { SniRuleStatus::Active => "active", SniRuleStatus::Disabled => "disabled" }) .bind(rule.created_at.to_rfc3339()) .bind(rule.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(id = %rule.id, pattern = %rule.pattern, action = %rule.action, "SNI rule added"); - Ok(rule) + Ok(()) } /// All SNI rules ordered by pattern @@ -139,15 +148,25 @@ impl TlsEngine { /// Insert or replace a JA3 fingerprint hash in the blocklist pub async fn add_ja3_block(&self, hash: &str, description: &str) -> Result<()> { + Self::insert_ja3_block_on(&self.pool, hash, description).await?; + tracing::info!(hash, "JA3 hash blocked"); + Ok(()) + } + + /// Executor-generic insert. Public for the transactional restore path + /// (#158/#535). + pub async fn insert_ja3_block_on<'e, E>(exec: E, hash: &str, description: &str) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { sqlx::query( "INSERT OR REPLACE INTO ja3_blocklist (hash, description, created_at) VALUES (?1, ?2, ?3)", ) .bind(hash) .bind(description) .bind(Utc::now().to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - tracing::info!(hash, "JA3 hash blocked"); Ok(()) } diff --git a/aifw-core/src/vpn.rs b/aifw-core/src/vpn.rs index 2a884843..2ac9717f 100644 --- a/aifw-core/src/vpn.rs +++ b/aifw-core/src/vpn.rs @@ -162,6 +162,26 @@ impl VpnEngine { ))); } + Self::insert_wg_tunnel_on(&self.pool, &tunnel).await?; + + tracing::info!(id = %tunnel.id, name = %tunnel.name, "WireGuard tunnel added"); + Ok(tunnel) + } + + /// Executor-generic validate + insert with no duplicate-port lookup + /// (callers must ensure port uniqueness, e.g. the restore path + /// pre-validates the whole config). Public for the transactional restore + /// path (#158/#535). + pub async fn insert_wg_tunnel_on<'e, E>(exec: E, tunnel: &WgTunnel) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { + if tunnel.name.is_empty() { + return Err(AifwError::Validation("tunnel name required".to_string())); + } + if tunnel.listen_port == 0 { + return Err(AifwError::Validation("listen port required".to_string())); + } sqlx::query( r#" INSERT INTO wg_tunnels (id, name, interface, private_key, public_key, listen_port, @@ -183,11 +203,9 @@ impl VpnEngine { .bind(tunnel.status.to_string()) .bind(tunnel.created_at.to_rfc3339()) .bind(tunnel.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(id = %tunnel.id, name = %tunnel.name, "WireGuard tunnel added"); - Ok(tunnel) + Ok(()) } /// All WireGuard tunnels, oldest first @@ -327,6 +345,25 @@ impl VpnEngine { } } + Self::insert_wg_peer_on(&self.pool, &peer).await?; + + tracing::info!(id = %peer.id, name = %peer.name, "WireGuard peer added"); + Ok(peer) + } + + /// Executor-generic validate + insert with no tunnel-exists or + /// duplicate-IP lookups (the restore path inserts the tunnel in the same + /// transaction, so those pool-side reads would not see it). Public for + /// the transactional restore path (#158/#535). + pub async fn insert_wg_peer_on<'e, E>(exec: E, peer: &WgPeer) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { + if peer.public_key.is_empty() { + return Err(AifwError::Validation( + "peer public key required".to_string(), + )); + } let allowed_ips: Vec = peer.allowed_ips.iter().map(|a| a.to_string()).collect(); sqlx::query( @@ -347,11 +384,9 @@ impl VpnEngine { .bind(peer.persistent_keepalive.map(|k| k as i64)) .bind(peer.created_at.to_rfc3339()) .bind(peer.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(id = %peer.id, name = %peer.name, "WireGuard peer added"); - Ok(peer) + Ok(()) } /// Peers of one tunnel, oldest first @@ -442,6 +477,17 @@ impl VpnEngine { /// Insert an IPsec security-association row. Fails validation when the /// name is empty pub async fn add_ipsec_sa(&self, sa: IpsecSa) -> Result { + Self::insert_ipsec_sa_on(&self.pool, &sa).await?; + tracing::info!(id = %sa.id, name = %sa.name, "IPsec SA added"); + Ok(sa) + } + + /// Executor-generic validate + insert. Public for the transactional + /// restore path (#158/#535). + pub async fn insert_ipsec_sa_on<'e, E>(exec: E, sa: &IpsecSa) -> Result<()> + where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, + { if sa.name.is_empty() { return Err(AifwError::Validation("SA name required".to_string())); } @@ -465,11 +511,9 @@ impl VpnEngine { .bind(sa.status.to_string()) .bind(sa.created_at.to_rfc3339()) .bind(sa.updated_at.to_rfc3339()) - .execute(&self.pool) + .execute(exec) .await?; - - tracing::info!(id = %sa.id, name = %sa.name, "IPsec SA added"); - Ok(sa) + Ok(()) } /// All IPsec SAs, oldest first From 02db98f8e55142102a9bd0459891e621cd785c48 Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 18:43:11 -0400 Subject: [PATCH 11/71] feat: post-apply verification of DB and pf state after restore (#535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the restore transaction commits, verify committed row counts per table match what the restore inserted, then — after the data-plane applies — verify the pf anchors hold exactly the rulesets the engines rendered. RuleEngine/NatEngine/GeoIpEngine gain verify_applied(), with RuleEngine's renderer factored out of apply_rules so apply and verify can't drift. A verification failure surfaces as a required-step error, which the snapshot wrapper turns into a rollback. --- aifw-api/src/backup.rs | 72 +++++++++++++++++++++++++++++++++++++++++ aifw-api/src/tests.rs | 33 +++++++++++++++++++ aifw-core/src/engine.rs | 40 +++++++++++++++++++---- aifw-core/src/geoip.rs | 27 ++++++++++++++++ aifw-core/src/nat.rs | 22 +++++++++++++ 5 files changed, 188 insertions(+), 6 deletions(-) diff --git a/aifw-api/src/backup.rs b/aifw-api/src/backup.rs index c912e544..15147259 100644 --- a/aifw-api/src/backup.rs +++ b/aifw-api/src/backup.rs @@ -1760,6 +1760,10 @@ pub(crate) async fn apply_firewall_config( // wrapper (`apply_firewall_config_or_rollback`) as their recovery path. // Parse/mapping skips (`continue`) below are intentional drops the // import preview already surfaced; operational failures are errors. + // Rows actually inserted per table, checked against committed counts + // after the transaction lands (#535 post-apply verification). + let mut inserted = std::collections::BTreeMap::<&str, i64>::new(); + let mut tx = state .pool .begin() @@ -1809,6 +1813,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::Database::insert_rule_on(&mut *tx, &rule) .await .map_err(|e| apply_fail(&format!("rule {rule_id} restore"), e))?; + *inserted.entry("rules").or_default() += 1; } else { tracing::warn!(rule_id = %rc.id, "import: skipping unparseable rule entry"); } @@ -1825,6 +1830,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::nat::NatEngine::insert_rule_on(&mut *tx, &nat) .await .map_err(|e| apply_fail(&format!("nat rule {nat_id} restore"), e))?; + *inserted.entry("nat_rules").or_default() += 1; } else { tracing::warn!(nat_id = %nc.id, "import: skipping unparseable nat entry"); } @@ -1851,6 +1857,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::AliasEngine::insert_on(&mut *tx, &alias) .await .map_err(|e| apply_fail(&format!("alias {} restore", alias.name), e))?; + *inserted.entry("aliases").or_default() += 1; } // Static routes — restored via direct INSERT, matching what @@ -1881,6 +1888,7 @@ pub(crate) async fn apply_firewall_config( .execute(&mut *tx) .await .map_err(|e| apply_fail(&format!("static route {} restore", rc.destination), e))?; + *inserted.entry("static_routes").or_default() += 1; if rc.enabled { kernel_routes.push((rc.clone(), iface_after)); } @@ -1899,6 +1907,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::geoip::GeoIpEngine::insert_rule_on(&mut *tx, &rule) .await .map_err(|e| apply_fail(&format!("geo-ip rule {country} restore"), e))?; + *inserted.entry("geoip_rules").or_default() += 1; } for wg in &config.vpn.wireguard { @@ -1929,6 +1938,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::vpn::VpnEngine::insert_wg_tunnel_on(&mut *tx, &tunnel) .await .map_err(|e| apply_fail(&format!("wg tunnel {} restore", tunnel.name), e))?; + *inserted.entry("wg_tunnels").or_default() += 1; for p in &wg.peers { let peer_id = uuid::Uuid::parse_str(&p.id).unwrap_or_else(|_| uuid::Uuid::new_v4()); let allowed_ips: Vec
= p @@ -1952,6 +1962,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::vpn::VpnEngine::insert_wg_peer_on(&mut *tx, &peer) .await .map_err(|e| apply_fail(&format!("wg peer {} restore", peer.name), e))?; + *inserted.entry("wg_peers").or_default() += 1; } } @@ -1970,6 +1981,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::vpn::VpnEngine::insert_ipsec_sa_on(&mut *tx, &sa) .await .map_err(|e| apply_fail(&format!("ipsec SA {} restore", sa.name), e))?; + *inserted.entry("ipsec_sas").or_default() += 1; } // Real IPsec tunnels (#530): restore records in the transaction; the @@ -1978,6 +1990,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::ipsec::IpsecEngine::insert_tunnel_on(&mut *tx, tunnel) .await .map_err(|e| apply_fail(&format!("ipsec tunnel {} restore", tunnel.name), e))?; + *inserted.entry("ipsec_tunnels").or_default() += 1; } let auth = &config.auth; @@ -2014,6 +2027,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::shaping::ShapingEngine::insert_queue_on(&mut *tx, &q) .await .map_err(|e| apply_fail(&format!("shaping queue {} restore", qc.name), e))?; + *inserted.entry("queue_configs").or_default() += 1; } } for rc in &config.rate_limits { @@ -2030,6 +2044,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::shaping::ShapingEngine::insert_rate_limit_on(&mut *tx, &r) .await .map_err(|e| apply_fail("rate limit restore", e))?; + *inserted.entry("rate_limit_rules").or_default() += 1; } } @@ -2039,6 +2054,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::tls::TlsEngine::insert_sni_rule_on(&mut *tx, &sni) .await .map_err(|e| apply_fail(&format!("sni rule {} restore", sc.pattern), e))?; + *inserted.entry("sni_rules").or_default() += 1; } } for hash in &config.tls.blocked_ja3 { @@ -2058,6 +2074,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::ha::ClusterEngine::insert_carp_vip_on(&mut *tx, &vip) .await .map_err(|e| apply_fail(&format!("carp vip {} restore", vc.virtual_ip), e))?; + *inserted.entry("carp_vips").or_default() += 1; } } if let Some(pc) = &config.ha.pfsync @@ -2076,6 +2093,7 @@ pub(crate) async fn apply_firewall_config( aifw_core::ha::ClusterEngine::insert_node_on(&mut *tx, &node) .await .map_err(|e| apply_fail("cluster node restore", e))?; + *inserted.entry("cluster_nodes").or_default() += 1; } } @@ -2108,6 +2126,41 @@ pub(crate) async fn apply_firewall_config( .await .map_err(|e| apply_fail("commit restore transaction", e))?; + // Post-commit DB verification (#535): the transaction guarantees + // atomicity, but confirm the committed row counts match what the + // restore inserted before touching the data plane. (Key/value config + // tables and INSERT OR REPLACE targets are excluded — duplicates + // legitimately collapse there.) + for table in [ + "rules", + "nat_rules", + "aliases", + "static_routes", + "geoip_rules", + "wg_tunnels", + "wg_peers", + "ipsec_sas", + "ipsec_tunnels", + "queue_configs", + "rate_limit_rules", + "sni_rules", + "carp_vips", + "cluster_nodes", + ] { + let expected = inserted.get(table).copied().unwrap_or(0); + let (count,): (i64,) = + sqlx::query_as(sqlx::AssertSqlSafe(format!("SELECT COUNT(*) FROM {table}"))) + .fetch_one(&state.pool) + .await + .map_err(|e| apply_fail(&format!("verifying {table}"), e))?; + if count != expected { + return Err(apply_fail( + &format!("verifying {table}"), + format!("{count} rows committed but {expected} were inserted"), + )); + } + } + // ============================================================ // Post-commit: pf / kernel / service applies. The DB is now fully // consistent; a failure below is a data-plane mismatch that must @@ -2211,6 +2264,25 @@ pub(crate) async fn apply_firewall_config( .await .map_err(|e| apply_fail("geoip rules apply", e))?; + // Post-apply data-plane verification (#535): pf must hold exactly the + // rulesets the engines just rendered. Catches a backend that reported + // success but didn't take the rules. + state + .rule_engine + .verify_applied() + .await + .map_err(|e| apply_fail("firewall rules verification", e))?; + state + .nat_engine + .verify_applied() + .await + .map_err(|e| apply_fail("nat rules verification", e))?; + state + .geoip_engine + .verify_applied() + .await + .map_err(|e| apply_fail("geoip rules verification", e))?; + Ok(()) } diff --git a/aifw-api/src/tests.rs b/aifw-api/src/tests.rs index c133c29f..ccebb02d 100644 --- a/aifw-api/src/tests.rs +++ b/aifw-api/src/tests.rs @@ -2376,6 +2376,39 @@ mod tests { assert!(elapsed.as_secs() < 30, "bulk restore took {elapsed:?}"); } + #[tokio::test] + async fn test_post_apply_verification_detects_pf_drift() { + // verify_applied must fail when the pf anchor no longer matches the + // database (#535 post-apply verification). + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + let rule = aifw_common::Rule::new( + aifw_common::Action::Pass, + aifw_common::Direction::In, + aifw_common::Protocol::Tcp, + aifw_common::RuleMatch { + src_addr: aifw_common::Address::Any, + src_port: None, + dst_addr: aifw_common::Address::Any, + dst_port: None, + }, + ); + state.rule_engine.add_rule(rule).await.unwrap(); + state.rule_engine.apply_rules().await.unwrap(); + state + .rule_engine + .verify_applied() + .await + .expect("freshly applied ruleset must verify"); + + state.pf.flush_rules("aifw").await.unwrap(); + assert!( + state.rule_engine.verify_applied().await.is_err(), + "flushed anchor must fail verification" + ); + } + #[tokio::test] async fn test_prevalidation_rejects_bad_config_without_mutation() { // A config that would abort mid-apply (rule priority out of range) diff --git a/aifw-core/src/engine.rs b/aifw-core/src/engine.rs index f9897379..0246d8af 100644 --- a/aifw-core/src/engine.rs +++ b/aifw-core/src/engine.rs @@ -159,12 +159,9 @@ impl RuleEngine { *self.extra_rules.write().await = rules; } - /// Generate pf rules from active rules and load them into the pf anchor. - /// Rules referencing a schedule are only compiled while inside their - /// active window, evaluated against appliance local time (#537). - /// Extra rules (from VPN, etc.) are inserted just before any block rule - /// so they aren't shadowed by a `block quick` default. - pub async fn apply_rules(&self) -> Result<()> { + /// Render the pf ruleset that [`Self::apply_rules`] would load: active + /// rules inside their schedule window, plus injected extra rules. + async fn render_pf_rules(&self) -> Result> { let rules = self.db.list_active_rules().await?; let schedules = self.db.list_schedule_specs().await?; let gateways = self.db.list_gateway_routes().await; @@ -202,6 +199,16 @@ impl RuleEngine { pf_rules.extend(extras.iter().cloned()); } } + Ok(pf_rules) + } + + /// Generate pf rules from active rules and load them into the pf anchor. + /// Rules referencing a schedule are only compiled while inside their + /// active window, evaluated against appliance local time (#537). + /// Extra rules (from VPN, etc.) are inserted just before any block rule + /// so they aren't shadowed by a `block quick` default. + pub async fn apply_rules(&self) -> Result<()> { + let pf_rules = self.render_pf_rules().await?; tracing::info!( anchor = %self.anchor, @@ -226,6 +233,27 @@ impl RuleEngine { Ok(()) } + /// Verify the pf anchor holds exactly the ruleset [`Self::apply_rules`] + /// would render right now (#535 post-apply verification). Fails with a + /// `Pf` error describing the mismatch. + pub async fn verify_applied(&self) -> Result<()> { + let expected = self.render_pf_rules().await?; + let actual = self + .pf + .get_rules(&self.anchor) + .await + .map_err(|e| AifwError::Pf(e.to_string()))?; + if actual != expected { + return Err(AifwError::Pf(format!( + "anchor {} holds {} rules but {} were expected — pf does not match the database", + self.anchor, + actual.len(), + expected.len() + ))); + } + Ok(()) + } + /// Flush all rules from the pf anchor pub async fn flush_rules(&self) -> Result<()> { self.pf diff --git a/aifw-core/src/geoip.rs b/aifw-core/src/geoip.rs index 6ea11119..dcf66268 100644 --- a/aifw-core/src/geoip.rs +++ b/aifw-core/src/geoip.rs @@ -315,6 +315,33 @@ impl GeoIpEngine { Ok(()) } + /// Verify the pf anchor holds exactly the geo-IP lines + /// [`Self::apply_rules`] would render right now (#535 post-apply + /// verification). Table *contents* aren't compared — only the table + /// definitions and block/allow rules. + pub async fn verify_applied(&self) -> Result<()> { + let rules = self.list_rules().await?; + let expected: Vec = rules + .iter() + .filter(|r| r.status == GeoIpRuleStatus::Active) + .flat_map(|r| [r.to_pf_table(), r.to_pf_rule()]) + .collect(); + let actual = self + .pf + .get_rules(&self.anchor) + .await + .map_err(|e| AifwError::Pf(e.to_string()))?; + if actual != expected { + return Err(AifwError::Pf(format!( + "anchor {} holds {} geo-ip lines but {} were expected — pf does not match the database", + self.anchor, + actual.len(), + expected.len() + ))); + } + Ok(()) + } + /// Get database statistics pub async fn db_stats(&self) -> (usize, usize) { let index = self.index.load(); diff --git a/aifw-core/src/nat.rs b/aifw-core/src/nat.rs index 75f2483b..defe73e9 100644 --- a/aifw-core/src/nat.rs +++ b/aifw-core/src/nat.rs @@ -251,6 +251,28 @@ impl NatEngine { Ok(()) } + /// Verify the pf anchor holds exactly the NAT ruleset + /// [`Self::apply_rules`] would render right now (#535 post-apply + /// verification). + pub async fn verify_applied(&self) -> Result<()> { + let rules = self.list_active_rules().await?; + let expected: Vec = rules.iter().flat_map(|r| r.to_pf_rules()).collect(); + let actual = self + .pf + .get_nat_rules(&self.anchor) + .await + .map_err(|e| AifwError::Pf(e.to_string()))?; + if actual != expected { + return Err(AifwError::Pf(format!( + "anchor {} holds {} NAT rules but {} were expected — pf does not match the database", + self.anchor, + actual.len(), + expected.len() + ))); + } + Ok(()) + } + /// Flush all NAT rules from the pf anchor and record an audit entry. /// Fails if the pf backend rejects the flush pub async fn flush_rules(&self) -> Result<()> { From d539eff1bca9fb09f842924989aace9f850aa996 Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 18:45:12 -0400 Subject: [PATCH 12/71] fix: commit-confirm cannot arm without a valid rollback target (#535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit_confirm_arm_with_snapshot now parses and pre-validates the snapshot before arming — a corrupt rollback target is refused up front instead of failing silently when the timer fires. The expiry path logs loudly instead of skipping an unparseable snapshot. The OPNsense importer aborts before applying when the caller requested commit_confirm but the pre-apply snapshot capture failed, rather than importing without the safety net the caller asked for. --- aifw-api/src/backup.rs | 37 ++++++++++++++++++++++++++----- aifw-api/src/opnsense/importer.rs | 13 +++++++++++ aifw-api/src/tests.rs | 17 ++++++++++++++ 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/aifw-api/src/backup.rs b/aifw-api/src/backup.rs index 15147259..055bed27 100644 --- a/aifw-api/src/backup.rs +++ b/aifw-api/src/backup.rs @@ -459,6 +459,22 @@ pub(crate) async fn commit_confirm_arm_with_snapshot( description: String, timeout_secs: u64, ) -> Result { + // Never arm with a rollback target that can't actually roll back + // (#535): the timer would fire, fail to parse or apply the snapshot, + // and leave the operator believing a revert happened. Parse and + // validate it with the same checks the apply path uses. + match serde_json::from_str::(&snapshot_json) { + Ok(snapshot) => { + if let Err(e) = prevalidate_config(&snapshot, &InterfaceMap::new()) { + tracing::error!(error = %e, "refusing to arm commit-confirm: snapshot fails validation"); + return Err(StatusCode::INTERNAL_SERVER_ERROR); + } + } + Err(e) => { + tracing::error!(error = %e, "refusing to arm commit-confirm: snapshot is not a valid FirewallConfig"); + return Err(StatusCode::INTERNAL_SERVER_ERROR); + } + } { let store_read = commit_store().read().await; if store_read.is_some() { @@ -493,14 +509,23 @@ pub(crate) async fn commit_confirm_arm_with_snapshot( _ = tokio::time::sleep(std::time::Duration::from_secs(timeout_secs)) => { // Timer expired — rollback! tracing::warn!("Commit confirm expired after {timeout_secs}s — rolling back"); - if let Some(inner) = store.write().await.take() - && let Ok(config) = serde_json::from_str::(&inner.rollback_config) { - if let Err(e) = apply_firewall_config(&rollback_state, &config, &InterfaceMap::new()).await { - tracing::error!(error = ?e, "commit confirm ROLLBACK FAILED — system may be partially configured"); - } else { - tracing::info!("Config rolled back successfully"); + if let Some(inner) = store.write().await.take() { + // Parse validated at arm time; a failure here means the + // stored snapshot was corrupted in memory — log loudly, + // never silently skip the rollback (#535). + match serde_json::from_str::(&inner.rollback_config) { + Ok(config) => { + if let Err(e) = apply_firewall_config(&rollback_state, &config, &InterfaceMap::new()).await { + tracing::error!(error = ?e, "commit confirm ROLLBACK FAILED — system may be partially configured"); + } else { + tracing::info!("Config rolled back successfully"); + } + } + Err(e) => { + tracing::error!(error = %e, "commit confirm ROLLBACK FAILED — stored snapshot unparseable; system keeps the unconfirmed config"); } } + } } _ = cancel_rx => { // Confirmed — do nothing, config stays diff --git a/aifw-api/src/opnsense/importer.rs b/aifw-api/src/opnsense/importer.rs index 08aceac4..15875a61 100644 --- a/aifw-api/src/opnsense/importer.rs +++ b/aifw-api/src/opnsense/importer.rs @@ -580,6 +580,19 @@ pub async fn import_opnsense( let pre_apply_snapshot_json = match crate::backup::capture_runtime_snapshot(&state).await { Ok(snap) => Some(snap), Err(e) => { + // The caller asked for a commit-confirm safety net; importing + // without a rollback target would silently strip it (#535). + // Refuse up front rather than apply unprotected. + if req.commit_confirm { + tracing::error!( + ?e, + "aborting import: commit-confirm requested but pre-apply snapshot capture failed" + ); + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + "could not capture the pre-import snapshot needed for commit-confirm rollback — import aborted; retry or disable commit_confirm".into(), + )); + } tracing::warn!( ?e, "failed to capture pre-apply snapshot; commit-confirm rollback target unavailable" diff --git a/aifw-api/src/tests.rs b/aifw-api/src/tests.rs index ccebb02d..ea8dfa90 100644 --- a/aifw-api/src/tests.rs +++ b/aifw-api/src/tests.rs @@ -2376,6 +2376,23 @@ mod tests { assert!(elapsed.as_secs() < 30, "bulk restore took {elapsed:?}"); } + #[tokio::test] + async fn test_commit_confirm_refuses_invalid_rollback_snapshot() { + // Arming with a snapshot that can't roll back would leave the timer + // to fail silently at expiry (#535) — it must be refused up front. + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + let res = crate::backup::commit_confirm_arm_with_snapshot( + state, + "{not valid json".to_string(), + "test".to_string(), + 5, + ) + .await; + assert_eq!(res, Err(axum::http::StatusCode::INTERNAL_SERVER_ERROR)); + } + #[tokio::test] async fn test_post_apply_verification_detects_pf_drift() { // verify_applied must fail when the pf anchor no longer matches the From b226da08cf132a2f5c4feaa2c7eb4774c8f3bf21 Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 18:56:51 -0400 Subject: [PATCH 13/71] test: failure injection at every restore apply stage (#535) PfMock gains armed-failure injection (fail_op/clear_fail) and the PfBackend trait gains as_any() so tests reach it through the shared Arc. New table-driven suite injects a failure at each of the 21 DB stages (table swapped for a read-only view so pre-tx migrates can't undo the injection) plus pf-op failures, and asserts all three contract outcomes: abort with prior rows intact, rollback with audit row, and rollback-failed audited when the failure hits both directions. --- aifw-api/src/tests.rs | 200 +++++++++++++++++++++++++++++++++++++++++ aifw-pf/src/backend.rs | 4 + aifw-pf/src/ioctl.rs | 4 + aifw-pf/src/mock.rs | 38 ++++++++ 4 files changed, 246 insertions(+) diff --git a/aifw-api/src/tests.rs b/aifw-api/src/tests.rs index ea8dfa90..eabf4427 100644 --- a/aifw-api/src/tests.rs +++ b/aifw-api/src/tests.rs @@ -2376,6 +2376,206 @@ mod tests { assert!(elapsed.as_secs() < 30, "bulk restore took {elapsed:?}"); } + fn any_tcp_rule() -> aifw_common::Rule { + aifw_common::Rule::new( + aifw_common::Action::Pass, + aifw_common::Direction::In, + aifw_common::Protocol::Tcp, + aifw_common::RuleMatch { + src_addr: aifw_common::Address::Any, + src_port: None, + dst_addr: aifw_common::Address::Any, + dst_port: None, + }, + ) + } + + #[tokio::test] + async fn test_restore_failure_injection_every_db_stage() { + // #535: for every table the restore touches, a failure at that stage + // must abort the restore with the transaction rolled back — the + // pre-restore rules must survive untouched at every injection point. + let _ = tracing_subscriber::fmt() + .with_max_level(tracing::Level::ERROR) + .try_init(); + for table in [ + "nat_rules", + "aliases", + "static_routes", + "geoip_rules", + "wg_tunnels", + "wg_peers", + "ipsec_sas", + "ipsec_tunnels", + "queue_configs", + "rate_limit_rules", + "sni_rules", + "ja3_blocklist", + "carp_vips", + "pfsync_config", + "cluster_nodes", + "auth_config", + "dhcp_subnets", + "dhcp_reservations", + "dhcp_config", + "dhcp_ddns_config", + "dhcp_ha_config", + ] { + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + state.rule_engine.add_rule(any_tcp_rule()).await.unwrap(); + let config = crate::backup::build_current_config(&state).await.unwrap(); + // Replace the table with a read-only VIEW of the same name: the + // engine migrates' CREATE TABLE IF NOT EXISTS no-op on it (so + // pre-tx migrates can't undo the injection, unlike a plain DROP) + // and the restore's DELETE/INSERT then fails at exactly this + // stage. + sqlx::query(sqlx::AssertSqlSafe(format!("DROP TABLE {table}"))) + .execute(&state.pool) + .await + .unwrap(); + sqlx::query(sqlx::AssertSqlSafe(format!( + "CREATE VIEW {table} AS SELECT 1 AS x" + ))) + .execute(&state.pool) + .await + .unwrap(); + let res = + crate::backup::apply_firewall_config(&state, &config, &Default::default()).await; + assert!(res.is_err(), "failure at {table} must abort the restore"); + let rules = state.rule_engine.list_rules().await.unwrap(); + assert_eq!( + rules.len(), + 1, + "failure at {table} must leave prior rules untouched" + ); + } + } + + #[tokio::test] + async fn test_restore_mid_tx_failure_rolls_back_and_audits() { + // Outcome 2 of the #535 contract: apply fails (duplicate alias name + // violates the UNIQUE constraint mid-transaction), prior state is + // restored, and the rollback is audited. + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + state.rule_engine.add_rule(any_tcp_rule()).await.unwrap(); + state + .alias_engine + .add(aifw_common::Alias { + id: uuid::Uuid::new_v4(), + name: "keepme".to_string(), + alias_type: aifw_common::AliasType::Host, + entries: vec!["192.0.2.1".to_string()], + description: None, + enabled: true, + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + }) + .await + .unwrap(); + + let mut config = crate::backup::build_current_config(&state).await.unwrap(); + for _ in 0..2 { + config.aliases.push(aifw_core::config::AliasConfig { + id: uuid::Uuid::new_v4().to_string(), + name: "dup_name".to_string(), + alias_type: "host".to_string(), + entries: vec!["198.51.100.1".to_string()], + description: None, + enabled: false, + }); + } + + let res = + crate::backup::apply_firewall_config_or_rollback(&state, &config, &Default::default()) + .await; + assert!(res.is_err(), "duplicate alias must abort the restore"); + + let aliases = state.alias_engine.list().await.unwrap(); + assert_eq!(aliases.len(), 1, "prior alias set must be restored"); + assert_eq!(aliases[0].name, "keepme"); + assert_eq!(state.rule_engine.list_rules().await.unwrap().len(), 1); + + let (audits,): (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM audit_log WHERE details LIKE '%rolled back to pre-restore state%'", + ) + .fetch_one(&state.pool) + .await + .unwrap(); + assert!(audits >= 1, "rollback must be audited"); + } + + #[tokio::test] + async fn test_restore_pf_failure_after_commit_rolls_back_cleanly() { + // Outcome 2 via the data plane: the target config needs a pf op the + // snapshot doesn't (geo-IP table populate), so injecting that + // failure aborts the target apply and the snapshot re-applies clean. + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + state.rule_engine.add_rule(any_tcp_rule()).await.unwrap(); + let mock = state + .pf + .as_any() + .downcast_ref::() + .expect("tests run on the mock backend"); + mock.fail_op("replace_table_entries").await; + + let mut config = crate::backup::build_current_config(&state).await.unwrap(); + config.geoip.push(aifw_core::config::GeoIpEntry { + id: uuid::Uuid::new_v4().to_string(), + country: "CN".to_string(), + action: aifw_common::GeoIpAction::Block, + label: None, + status: aifw_common::GeoIpRuleStatus::Active, + }); + + let res = + crate::backup::apply_firewall_config_or_rollback(&state, &config, &Default::default()) + .await; + assert!(res.is_err(), "pf failure must abort the restore"); + assert_eq!( + state.geoip_engine.list_rules().await.unwrap().len(), + 0, + "geo-ip rows from the failed target must be rolled back" + ); + assert_eq!(state.rule_engine.list_rules().await.unwrap().len(), 1); + } + + #[tokio::test] + async fn test_restore_rollback_failure_is_audited_high_severity() { + // Outcome 3 of the #535 contract: the apply fails AND the rollback + // fails (persistent pf failure hits both); the operator gets an + // explicit rollback-failed audit row, never silent partial success. + let state = crate::create_app_state_in_memory(plain_auth_settings()) + .await + .unwrap(); + state.rule_engine.add_rule(any_tcp_rule()).await.unwrap(); + let mock = state + .pf + .as_any() + .downcast_ref::() + .expect("tests run on the mock backend"); + mock.fail_op("load_rules").await; + + let config = crate::backup::build_current_config(&state).await.unwrap(); + let res = + crate::backup::apply_firewall_config_or_rollback(&state, &config, &Default::default()) + .await; + assert!(res.is_err()); + + let (audits,): (i64,) = + sqlx::query_as("SELECT COUNT(*) FROM audit_log WHERE details LIKE '%rollback failed%'") + .fetch_one(&state.pool) + .await + .unwrap(); + assert!(audits >= 1, "failed rollback must be audited"); + mock.clear_fail("load_rules").await; + } + #[tokio::test] async fn test_commit_confirm_refuses_invalid_rollback_snapshot() { // Arming with a snapshot that can't roll back would leave the timer diff --git a/aifw-pf/src/backend.rs b/aifw-pf/src/backend.rs index 2ebc7745..1acaac0e 100644 --- a/aifw-pf/src/backend.rs +++ b/aifw-pf/src/backend.rs @@ -10,6 +10,10 @@ use std::net::IpAddr; /// chosen at compile time via `#[cfg(target_os)]` in [`crate::create_backend`]. #[async_trait] pub trait PfBackend: Send + Sync { + /// Downcast support so tests can reach implementation-specific helpers + /// (e.g. [`crate::PfMock`] failure injection) through `Arc`. + fn as_any(&self) -> &dyn std::any::Any; + /// Add a pf rule to the specified anchor async fn add_rule(&self, anchor: &str, rule: &str) -> Result<(), crate::PfError>; diff --git a/aifw-pf/src/ioctl.rs b/aifw-pf/src/ioctl.rs index 24dd6520..53c3904f 100644 --- a/aifw-pf/src/ioctl.rs +++ b/aifw-pf/src/ioctl.rs @@ -96,6 +96,10 @@ async fn pfctl_stdin(args: &[&str], input: &str) -> Result &dyn std::any::Any { + self + } + async fn add_rule(&self, anchor: &str, rule: &str) -> Result<(), PfError> { let output = pfctl_stdin(&["-a", anchor, "-f", "-"], rule).await?; if !output.status.success() { diff --git a/aifw-pf/src/mock.rs b/aifw-pf/src/mock.rs index 0c7ec7ec..7366a6a7 100644 --- a/aifw-pf/src/mock.rs +++ b/aifw-pf/src/mock.rs @@ -18,6 +18,7 @@ pub struct PfMock { running: RwLock, iface_fibs: RwLock>, fib_count: RwLock, + armed_failures: RwLock>, } impl PfMock { @@ -32,9 +33,29 @@ impl PfMock { running: RwLock::new(true), iface_fibs: RwLock::new(HashMap::new()), fib_count: RwLock::new(1), + armed_failures: RwLock::new(std::collections::HashSet::new()), } } + /// Arm a persistent injected failure for the named backend operation + /// (e.g. `"load_rules"`); every call to that op fails until + /// [`Self::clear_fail`]. Test helper for #535 failure-injection suites. + pub async fn fail_op(&self, op: &str) { + self.armed_failures.write().await.insert(op.to_string()); + } + + /// Disarm an injected failure set by [`Self::fail_op`]. + pub async fn clear_fail(&self, op: &str) { + self.armed_failures.write().await.remove(op); + } + + async fn check_fail(&self, op: &str) -> Result<(), PfError> { + if self.armed_failures.read().await.contains(op) { + return Err(PfError::Other(format!("injected failure: {op}"))); + } + Ok(()) + } + /// Override the number of available FIBs for testing multi-WAN scenarios. pub async fn set_fib_count(&self, n: u32) { *self.fib_count.write().await = n.max(1); @@ -59,7 +80,12 @@ impl Default for PfMock { #[async_trait] impl PfBackend for PfMock { + fn as_any(&self) -> &dyn std::any::Any { + self + } + async fn add_rule(&self, anchor: &str, rule: &str) -> Result<(), PfError> { + self.check_fail("add_rule").await?; tracing::debug!(anchor, rule, "mock: add_rule"); let mut rules = self.rules.write().await; rules @@ -70,6 +96,7 @@ impl PfBackend for PfMock { } async fn flush_rules(&self, anchor: &str) -> Result<(), PfError> { + self.check_fail("flush_rules").await?; tracing::debug!(anchor, "mock: flush_rules"); let mut rules = self.rules.write().await; rules.remove(anchor); @@ -77,6 +104,7 @@ impl PfBackend for PfMock { } async fn load_rules(&self, anchor: &str, new_rules: &[String]) -> Result<(), PfError> { + self.check_fail("load_rules").await?; tracing::debug!(anchor, count = new_rules.len(), "mock: load_rules"); let mut rules = self.rules.write().await; rules.insert(anchor.to_string(), new_rules.to_vec()); @@ -84,6 +112,7 @@ impl PfBackend for PfMock { } async fn get_rules(&self, anchor: &str) -> Result, PfError> { + self.check_fail("get_rules").await?; let rules = self.rules.read().await; Ok(rules.get(anchor).cloned().unwrap_or_default()) } @@ -105,6 +134,7 @@ impl PfBackend for PfMock { } async fn add_table_entry(&self, table: &str, addr: IpAddr) -> Result<(), PfError> { + self.check_fail("add_table_entry").await?; tracing::debug!(%addr, table, "mock: add_table_entry"); let mut tables = self.tables.write().await; let entries = tables.entry(table.to_string()).or_default(); @@ -119,6 +149,7 @@ impl PfBackend for PfMock { table: &str, entries: &[(IpAddr, u8)], ) -> Result<(), PfError> { + self.check_fail("replace_table_entries").await?; tracing::debug!(table, count = entries.len(), "mock: replace_table_entries"); let mut tables = self.tables.write().await; // Mock only tracks bare addresses, not prefixes — match the existing @@ -131,6 +162,7 @@ impl PfBackend for PfMock { } async fn remove_table_entry(&self, table: &str, addr: IpAddr) -> Result<(), PfError> { + self.check_fail("remove_table_entry").await?; tracing::debug!(%addr, table, "mock: remove_table_entry"); let mut tables = self.tables.write().await; if let Some(entries) = tables.get_mut(table) { @@ -140,6 +172,7 @@ impl PfBackend for PfMock { } async fn flush_table(&self, table: &str) -> Result<(), PfError> { + self.check_fail("flush_table").await?; tracing::debug!(table, "mock: flush_table"); let mut tables = self.tables.write().await; tables.remove(table); @@ -165,6 +198,7 @@ impl PfBackend for PfMock { } async fn load_nat_rules(&self, anchor: &str, rules: &[String]) -> Result<(), PfError> { + self.check_fail("load_nat_rules").await?; tracing::debug!(anchor, count = rules.len(), "mock: load_nat_rules"); let mut nat_rules = self.nat_rules.write().await; nat_rules.insert(anchor.to_string(), rules.to_vec()); @@ -172,11 +206,13 @@ impl PfBackend for PfMock { } async fn get_nat_rules(&self, anchor: &str) -> Result, PfError> { + self.check_fail("get_nat_rules").await?; let nat_rules = self.nat_rules.read().await; Ok(nat_rules.get(anchor).cloned().unwrap_or_default()) } async fn flush_nat_rules(&self, anchor: &str) -> Result<(), PfError> { + self.check_fail("flush_nat_rules").await?; tracing::debug!(anchor, "mock: flush_nat_rules"); let mut nat_rules = self.nat_rules.write().await; nat_rules.remove(anchor); @@ -184,6 +220,7 @@ impl PfBackend for PfMock { } async fn load_queues(&self, anchor: &str, queue_defs: &[String]) -> Result<(), PfError> { + self.check_fail("load_queues").await?; tracing::debug!(anchor, count = queue_defs.len(), "mock: load_queues"); let mut queues = self.queues.write().await; queues.insert(anchor.to_string(), queue_defs.to_vec()); @@ -196,6 +233,7 @@ impl PfBackend for PfMock { } async fn flush_queues(&self, anchor: &str) -> Result<(), PfError> { + self.check_fail("flush_queues").await?; tracing::debug!(anchor, "mock: flush_queues"); let mut queues = self.queues.write().await; queues.remove(anchor); From cba6d56acb4a2470cdb3b2f76cd3704910afe2d1 Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 18:58:07 -0400 Subject: [PATCH 14/71] =?UTF-8?q?docs:=20backup/restore=20atomicity=20mode?= =?UTF-8?q?l=20=E2=80=94=20validation,=20single=20DB=20transaction,=20veri?= =?UTF-8?q?fication=20(#535)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/docs/backup.md | 2 +- docs/maturity.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/backup.md b/docs/docs/backup.md index 0b0d3f7d..8eced503 100644 --- a/docs/docs/backup.md +++ b/docs/docs/backup.md @@ -91,7 +91,7 @@ Migrating from OPNsense or pfSense? Drop the appliance’s `config.xml` stra 1. **Parse first.** `quick-xml` walks the document. Every error that can be detected from the XML alone (malformed source/destination, unknown action, unparseable port range) surfaces in the preview — no half-applied state. 2. **Preview the diff.** `POST /api/v1/config/preview-opnsense` returns aliases, NAT entries, rules, and static routes that will be created, plus a **skipped list** for rules that referenced network keywords (`lan`, `wanip`, `(self)`, …) that don’t map cleanly. AiFw drops those to skipped instead of guessing — silent fidelity loss is the worst outcome. -3. **Apply with rollback.** `POST /api/v1/config/import-opnsense` applies strictly — any failed required step aborts — and recovers by re-applying a pre-import snapshot rather than a single wrapping SQL transaction (kernel-side pf/route changes can't be rewound by SQLite; see the documented atomicity model in the importer source). Aliases route through `AliasEngine`, NAT through `NatEngine`, rules through `RuleEngine`, static routes through the same `apply_route_to_system` path the manual REST endpoint uses, and DNS forwarders through the same path the rDNS resolver UI writes — not `/etc/resolv.conf` (see [#231](https://github.com/ZerosAndOnesLLC/AiFw/pull/231)). +3. **Apply with rollback.** `POST /api/v1/config/import-opnsense` applies strictly — any failed required step aborts. The whole target config is **pre-validated** (same converters and engine validators the apply uses) before the first destructive change, all **database mutations run in a single SQL transaction** that rolls back automatically on failure, and after the transaction commits the **pf/kernel applies are verified** against what the engines rendered. Kernel-side pf/route changes can't ride in the SQL transaction, so a post-commit failure recovers by re-applying a pre-import snapshot (see the documented atomicity model in the importer source). Aliases route through `AliasEngine`, NAT through `NatEngine`, rules through `RuleEngine`, static routes through the same `apply_route_to_system` path the manual REST endpoint uses, and DNS forwarders through the same path the rDNS resolver UI writes — not `/etc/resolv.conf` (see [#231](https://github.com/ZerosAndOnesLLC/AiFw/pull/231)). 4. **Snapshot & commit-confirm.** Before any write the importer saves a `pre-OPNsense-import` config-history version, and after a successful apply it arms commit-confirm with a **600-second timeout**. If the admin loses access to the appliance before they confirm, AiFw rolls back automatically. 5. **Reject → BlockReturn.** OPNsense’s `reject` action maps to AiFw `Action::BlockReturn`, which produces the same per-protocol response (TCP RST, ICMP unreachable for UDP/ICMP) the original ruleset emitted. diff --git a/docs/maturity.md b/docs/maturity.md index 46d7ce06..2901bc87 100644 --- a/docs/maturity.md +++ b/docs/maturity.md @@ -43,7 +43,7 @@ AiFw is in **active development (beta)**. This matrix is the honest, per-feature Geo-IP filtering✓✓⏳⏳Beta Multi-WAN (FIB, gateways, policies)✓✓⏳⏳Beta — latency thresholds + dampening enforced since v5.100.0; needs environment-specific validation HA (CARP / pfsync / replication)✓✓✗✗Beta — automated two-node failover validation tracked in [#534](https://github.com/ZerosAndOnesLLC/AiFw/issues/534); quantitative claims are design targets until then -Backup / restore / OPNsense import✓✓⏳⏳Beta — snapshot + strict apply with automatic rollback (since v5.99.7); not a single DB+kernel transaction +Backup / restore / OPNsense import✓✓⏳⏳Beta — pre-validated apply, single DB transaction, post-apply pf verification, snapshot rollback for kernel state (since v5.99.7; hardened for #535); kernel applies are still not transactional DHCP / DNS / NTP (companions)✓✓⏳⏳Beta — builds pinned to reviewed revisions since v5.99.8 Reverse proxy + ACME (TrafficCop)✓✓⏳⏳Beta AI/ML threat detection✓⏳✗✗Experimental, opt-in, disabled by default From 8a5c274772b28d7ebb51bbecaa6dda10b83bfe1c Mon Sep 17 00:00:00 2001 From: mack42 Date: Tue, 21 Jul 2026 19:43:01 -0400 Subject: [PATCH 15/71] fix: verify_applied must not string-compare rules against real pfctl (#535) pfctl -sr returns rules in canonical re-rendered form (and omits table definitions), so exact string equality with the loaded source only holds on the mock backend. PfBackend gains echoes_exact_rules(); verification does the exact vector compare on backends that echo the loaded strings and falls back to the emptiness invariant on real pf. Full pfctl-side verification stays tracked under #533. Caught by the FreeBSD functional harness (t05-restore-roundtrip). --- aifw-core/src/engine.rs | 17 +++++++++++++---- aifw-core/src/geoip.rs | 17 ++++++++++++----- aifw-core/src/nat.rs | 14 ++++++++++---- aifw-pf/src/backend.rs | 10 ++++++++++ aifw-pf/src/mock.rs | 4 ++++ 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/aifw-core/src/engine.rs b/aifw-core/src/engine.rs index 0246d8af..d1024a4d 100644 --- a/aifw-core/src/engine.rs +++ b/aifw-core/src/engine.rs @@ -233,9 +233,13 @@ impl RuleEngine { Ok(()) } - /// Verify the pf anchor holds exactly the ruleset [`Self::apply_rules`] - /// would render right now (#535 post-apply verification). Fails with a - /// `Pf` error describing the mismatch. + /// Verify the pf anchor holds the ruleset [`Self::apply_rules`] would + /// render right now (#535 post-apply verification). Exact string + /// comparison only works on backends that echo the loaded rules (the + /// mock); real pfctl lists rules in canonical re-rendered form, so + /// there the check degrades to the emptiness invariant (loaded a + /// non-empty ruleset ⇒ anchor is non-empty, and vice versa). Full + /// pfctl-side verification is tracked under the FreeBSD CI epic (#533). pub async fn verify_applied(&self) -> Result<()> { let expected = self.render_pf_rules().await?; let actual = self @@ -243,7 +247,12 @@ impl RuleEngine { .get_rules(&self.anchor) .await .map_err(|e| AifwError::Pf(e.to_string()))?; - if actual != expected { + let mismatch = if self.pf.echoes_exact_rules() { + actual != expected + } else { + actual.is_empty() != expected.is_empty() + }; + if mismatch { return Err(AifwError::Pf(format!( "anchor {} holds {} rules but {} were expected — pf does not match the database", self.anchor, diff --git a/aifw-core/src/geoip.rs b/aifw-core/src/geoip.rs index dcf66268..d01f4deb 100644 --- a/aifw-core/src/geoip.rs +++ b/aifw-core/src/geoip.rs @@ -315,10 +315,12 @@ impl GeoIpEngine { Ok(()) } - /// Verify the pf anchor holds exactly the geo-IP lines - /// [`Self::apply_rules`] would render right now (#535 post-apply - /// verification). Table *contents* aren't compared — only the table - /// definitions and block/allow rules. + /// Verify the pf anchor holds the geo-IP lines [`Self::apply_rules`] + /// would render right now (#535 post-apply verification). Table + /// *contents* aren't compared — only the table definitions and + /// block/allow rules. Exact comparison on backends that echo loaded + /// rules; emptiness invariant on real pfctl, which re-renders rules and + /// omits table definitions from `-sr` (see `RuleEngine::verify_applied`). pub async fn verify_applied(&self) -> Result<()> { let rules = self.list_rules().await?; let expected: Vec = rules @@ -331,7 +333,12 @@ impl GeoIpEngine { .get_rules(&self.anchor) .await .map_err(|e| AifwError::Pf(e.to_string()))?; - if actual != expected { + let mismatch = if self.pf.echoes_exact_rules() { + actual != expected + } else { + actual.is_empty() != expected.is_empty() + }; + if mismatch { return Err(AifwError::Pf(format!( "anchor {} holds {} geo-ip lines but {} were expected — pf does not match the database", self.anchor, diff --git a/aifw-core/src/nat.rs b/aifw-core/src/nat.rs index defe73e9..19cb49be 100644 --- a/aifw-core/src/nat.rs +++ b/aifw-core/src/nat.rs @@ -251,9 +251,10 @@ impl NatEngine { Ok(()) } - /// Verify the pf anchor holds exactly the NAT ruleset - /// [`Self::apply_rules`] would render right now (#535 post-apply - /// verification). + /// Verify the pf anchor holds the NAT ruleset [`Self::apply_rules`] + /// would render right now (#535 post-apply verification). Exact + /// comparison on backends that echo loaded rules; emptiness invariant + /// on real pfctl (see `RuleEngine::verify_applied`). pub async fn verify_applied(&self) -> Result<()> { let rules = self.list_active_rules().await?; let expected: Vec = rules.iter().flat_map(|r| r.to_pf_rules()).collect(); @@ -262,7 +263,12 @@ impl NatEngine { .get_nat_rules(&self.anchor) .await .map_err(|e| AifwError::Pf(e.to_string()))?; - if actual != expected { + let mismatch = if self.pf.echoes_exact_rules() { + actual != expected + } else { + actual.is_empty() != expected.is_empty() + }; + if mismatch { return Err(AifwError::Pf(format!( "anchor {} holds {} NAT rules but {} were expected — pf does not match the database", self.anchor, diff --git a/aifw-pf/src/backend.rs b/aifw-pf/src/backend.rs index 1acaac0e..d5d761df 100644 --- a/aifw-pf/src/backend.rs +++ b/aifw-pf/src/backend.rs @@ -14,6 +14,16 @@ pub trait PfBackend: Send + Sync { /// (e.g. [`crate::PfMock`] failure injection) through `Arc`. fn as_any(&self) -> &dyn std::any::Any; + /// Whether `get_rules`/`get_nat_rules` return the exact strings that were + /// loaded. True for the in-memory mock; false for the pfctl backend, + /// which lists rules in pfctl's canonical re-rendered form (normalized + /// keywords, possible expansion), so string equality against the loaded + /// source is not meaningful there. Verification code uses this to pick + /// between exact comparison and weaker invariants. + fn echoes_exact_rules(&self) -> bool { + false + } + /// Add a pf rule to the specified anchor async fn add_rule(&self, anchor: &str, rule: &str) -> Result<(), crate::PfError>; diff --git a/aifw-pf/src/mock.rs b/aifw-pf/src/mock.rs index 7366a6a7..796333c8 100644 --- a/aifw-pf/src/mock.rs +++ b/aifw-pf/src/mock.rs @@ -84,6 +84,10 @@ impl PfBackend for PfMock { self } + fn echoes_exact_rules(&self) -> bool { + true + } + async fn add_rule(&self, anchor: &str, rule: &str) -> Result<(), PfError> { self.check_fail("add_rule").await?; tracing::debug!(anchor, rule, "mock: add_rule"); From 4353b497cbc2aa7c117c62a73a0a369a10041be3 Mon Sep 17 00:00:00 2001 From: Ron McCorkle <31546545+mack42@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:54:42 +0000 Subject: [PATCH 16/71] feat: use dummynet FQ-CoDel for traffic shaping (#532) --- aifw-common/src/lib.rs | 4 +- aifw-common/src/ratelimit.rs | 61 +++++++++- aifw-core/src/shaping.rs | 105 ++++++++++++++++-- aifw-core/src/sudo.rs | 13 +++ aifw-core/src/tests.rs | 20 +++- aifw-core/src/updater.rs | 4 + aifw-setup/src/apply.rs | 2 + freebsd/deploy.sh | 2 +- .../usr/local/libexec/aifw-sudo-dummynet | 18 +++ 9 files changed, 214 insertions(+), 15 deletions(-) create mode 100755 freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet diff --git a/aifw-common/src/lib.rs b/aifw-common/src/lib.rs index b2bb7d08..57cfdeea 100644 --- a/aifw-common/src/lib.rs +++ b/aifw-common/src/lib.rs @@ -78,8 +78,8 @@ pub use multiwan::{ pub use nat::{NatRedirect, NatRule, NatStatus, NatType}; pub use permission::{ALL_PERMISSIONS, Permission, PermissionSet, builtin_role_permissions}; pub use ratelimit::{ - Bandwidth, BandwidthUnit, QueueConfig, QueueStatus, QueueType, RateLimitRule, RateLimitStatus, - SynFloodConfig, TrafficClass, + Bandwidth, BandwidthUnit, FqCodelConfig, QueueConfig, QueueStatus, QueueType, RateLimitRule, + RateLimitStatus, SynFloodConfig, TrafficClass, }; pub use rule::{ Action, AdaptiveTimeouts, Direction, IpVersion, Protocol, Rule, RuleMatch, RuleStatus, diff --git a/aifw-common/src/ratelimit.rs b/aifw-common/src/ratelimit.rs index f889d47e..c161ffc3 100644 --- a/aifw-common/src/ratelimit.rs +++ b/aifw-common/src/ratelimit.rs @@ -17,6 +17,59 @@ pub enum QueueType { Priq, } +/// dummynet scheduler configuration for the FQ-CoDel backend. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +pub struct FqCodelConfig { + /// Target queueing delay in milliseconds. + pub target_ms: u32, + /// CoDel interval in milliseconds. + pub interval_ms: u32, + /// Per-flow quantum in bytes. + pub quantum_bytes: u32, + /// Maximum queue length in packets. + pub limit_packets: u32, + /// Number of flow buckets. + pub flows: u32, + /// Enable ECN marking. + pub ecn: bool, +} + +impl Default for FqCodelConfig { + fn default() -> Self { + Self { + target_ms: 5, + interval_ms: 100, + quantum_bytes: 1514, + limit_packets: 10240, + flows: 1024, + ecn: true, + } + } +} + +impl FqCodelConfig { + /// Reject values outside FreeBSD dummynet's safe scheduler bounds. + pub fn validate(&self) -> crate::Result<()> { + if !(1..=1_000).contains(&self.target_ms) + || self.interval_ms < self.target_ms + || self.interval_ms > 10_000 + { + return Err(crate::AifwError::Validation( + "invalid FQ-CoDel target/interval".to_string(), + )); + } + if !(64..=65_536).contains(&self.quantum_bytes) + || !(1..=1_000_000).contains(&self.limit_packets) + || !(1..=65_536).contains(&self.flows) + { + return Err(crate::AifwError::Validation( + "invalid FQ-CoDel quantum, limit, or flow count".to_string(), + )); + } + Ok(()) + } +} + impl std::fmt::Display for QueueType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -195,6 +248,9 @@ pub struct QueueConfig { pub created_at: DateTime, /// Last modification timestamp (UTC) pub updated_at: DateTime, + /// FQ-CoDel parameters (used when queue_type is Codel). + #[serde(default)] + pub fq_codel: FqCodelConfig, } /// Enabled/disabled state of a queue @@ -230,6 +286,7 @@ impl QueueConfig { status: QueueStatus::Active, created_at: now, updated_at: now, + fq_codel: FqCodelConfig::default(), } } @@ -248,9 +305,7 @@ impl QueueConfig { } match self.queue_type { - QueueType::Codel => { - parts.push("flows 1024 quantum 1514 target 5 interval 100".to_string()) - } + QueueType::Codel => parts.push("fq_codel".to_string()), QueueType::Hfsc => {} QueueType::Priq => parts.push(format!("priority {}", self.traffic_class.priority())), } diff --git a/aifw-core/src/shaping.rs b/aifw-core/src/shaping.rs index bb92cfb5..165870e7 100644 --- a/aifw-core/src/shaping.rs +++ b/aifw-core/src/shaping.rs @@ -1,6 +1,6 @@ use aifw_common::{ - Address, AifwError, Bandwidth, BandwidthUnit, Interface, PortRange, Protocol, QueueConfig, - QueueStatus, QueueType, RateLimitRule, RateLimitStatus, Result, TrafficClass, + Address, AifwError, Bandwidth, BandwidthUnit, FqCodelConfig, Interface, PortRange, Protocol, + QueueConfig, QueueStatus, QueueType, RateLimitRule, RateLimitStatus, Result, TrafficClass, }; use aifw_pf::PfBackend; use chrono::{DateTime, Utc}; @@ -57,6 +57,24 @@ impl ShapingEngine { ) .execute(&self.pool) .await?; + let _ = sqlx::query( + "ALTER TABLE queue_configs ADD COLUMN fq_codel_target_ms INTEGER NOT NULL DEFAULT 5", + ) + .execute(&self.pool) + .await; + let _ = sqlx::query("ALTER TABLE queue_configs ADD COLUMN fq_codel_interval_ms INTEGER NOT NULL DEFAULT 100").execute(&self.pool).await; + let _ = sqlx::query("ALTER TABLE queue_configs ADD COLUMN fq_codel_quantum_bytes INTEGER NOT NULL DEFAULT 1514").execute(&self.pool).await; + let _ = sqlx::query("ALTER TABLE queue_configs ADD COLUMN fq_codel_limit_packets INTEGER NOT NULL DEFAULT 10240").execute(&self.pool).await; + let _ = sqlx::query( + "ALTER TABLE queue_configs ADD COLUMN fq_codel_flows INTEGER NOT NULL DEFAULT 1024", + ) + .execute(&self.pool) + .await; + let _ = sqlx::query( + "ALTER TABLE queue_configs ADD COLUMN fq_codel_ecn INTEGER NOT NULL DEFAULT 1", + ) + .execute(&self.pool) + .await; sqlx::query( r#" @@ -90,6 +108,7 @@ impl ShapingEngine { /// Insert a queue config row. pf isn't touched until /// [`Self::apply_queues`] pub async fn add_queue(&self, config: QueueConfig) -> Result { + config.fq_codel.validate()?; self.insert_queue(&config).await?; tracing::info!(id = %config.id, name = %config.name, "queue added"); Ok(config) @@ -129,13 +148,19 @@ impl ShapingEngine { .collect(); let mut pf_lines = Vec::new(); + let mut dummynet = Vec::new(); // Group by interface — each needs a parent queue let mut interfaces_seen = std::collections::HashSet::new(); for q in &active { if interfaces_seen.insert(q.interface.0.clone()) { pf_lines.push(q.to_pf_parent_queue()); } - pf_lines.push(q.to_pf_queue()); + if q.queue_type == QueueType::Codel { + q.fq_codel.validate()?; + dummynet.extend(render_dummynet_commands(q)?); + } else { + pf_lines.push(q.to_pf_queue()); + } } tracing::info!(count = pf_lines.len(), "applying queue configs to pf"); @@ -143,6 +168,7 @@ impl ShapingEngine { .load_queues(&self.anchor, &pf_lines) .await .map_err(|e| AifwError::Pf(e.to_string()))?; + apply_dummynet(&dummynet).await?; Ok(()) } @@ -231,8 +257,10 @@ impl ShapingEngine { sqlx::query( r#" INSERT INTO queue_configs (id, interface, queue_type, bandwidth_value, bandwidth_unit, - name, traffic_class, bandwidth_pct, is_default, status, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12) + name, traffic_class, bandwidth_pct, fq_codel_target_ms, fq_codel_interval_ms, + fq_codel_quantum_bytes, fq_codel_limit_packets, fq_codel_flows, fq_codel_ecn, + is_default, status, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18) "#, ) .bind(q.id.to_string()) @@ -243,6 +271,12 @@ impl ShapingEngine { .bind(&q.name) .bind(q.traffic_class.to_string()) .bind(q.bandwidth_pct.map(|p| p as i64)) + .bind(q.fq_codel.target_ms as i64) + .bind(q.fq_codel.interval_ms as i64) + .bind(q.fq_codel.quantum_bytes as i64) + .bind(q.fq_codel.limit_packets as i64) + .bind(q.fq_codel.flows as i64) + .bind(q.fq_codel.ecn) .bind(q.default) .bind(match q.status { QueueStatus::Active => "active", @@ -294,8 +328,51 @@ impl ShapingEngine { /// `SELECT *` which triggers a sqlx-sqlite column-count panic and blocks /// column pruning (#348). const QUEUE_COLUMNS: &str = "id, interface, queue_type, bandwidth_value, \ - bandwidth_unit, name, traffic_class, bandwidth_pct, is_default, status, \ - created_at, updated_at"; + bandwidth_unit, name, traffic_class, bandwidth_pct, fq_codel_target_ms, \ + fq_codel_interval_ms, fq_codel_quantum_bytes, fq_codel_limit_packets, \ + fq_codel_flows, fq_codel_ecn, is_default, status, created_at, updated_at"; + +fn render_dummynet_commands(q: &QueueConfig) -> Result> { + let fq = q.fq_codel; + fq.validate()?; + let id = (q.id.as_u128() & 0x7fff_ffff) as u32; + let bandwidth = q.bandwidth.to_bits_per_sec(); + Ok(vec![ + format!( + "pipe {id} config bw {bandwidth}bit/s queue {limit} sched fq_codel target {target}ms interval {interval}ms quantum {quantum} flows {flows} ecn", + limit = fq.limit_packets, + target = fq.target_ms, + interval = fq.interval_ms, + quantum = fq.quantum_bytes, + flows = fq.flows + ), + format!( + "ipfw add {} pipe {id} ip from any to any out xmit {}", + 10000 + id, + q.interface + ), + format!( + "ipfw add {} pipe {id} ip from any to any in recv {}", + 20000 + id, + q.interface + ), + ]) +} + +async fn apply_dummynet(commands: &[String]) -> Result<()> { + if commands.is_empty() { + return Ok(()); + } + #[cfg(not(target_os = "freebsd"))] + { + let _ = commands; + return Ok(()); + } + #[cfg(target_os = "freebsd")] + crate::sudo::dummynet_apply(commands) + .await + .map_err(|e| AifwError::Other(format!("dummynet apply failed: {e}"))) +} /// Explicit column list for `RateLimitRow` selects, in schema order. Replaces /// `SELECT *` which triggers a sqlx-sqlite column-count panic and blocks @@ -314,6 +391,12 @@ struct QueueRow { name: String, traffic_class: String, bandwidth_pct: Option, + fq_codel_target_ms: i64, + fq_codel_interval_ms: i64, + fq_codel_quantum_bytes: i64, + fq_codel_limit_packets: i64, + fq_codel_flows: i64, + fq_codel_ecn: bool, is_default: bool, status: String, created_at: String, @@ -351,6 +434,14 @@ impl QueueRow { updated_at: DateTime::parse_from_rfc3339(&self.updated_at) .map_err(|e| AifwError::Database(format!("invalid date: {e}")))? .with_timezone(&Utc), + fq_codel: FqCodelConfig { + target_ms: self.fq_codel_target_ms as u32, + interval_ms: self.fq_codel_interval_ms as u32, + quantum_bytes: self.fq_codel_quantum_bytes as u32, + limit_packets: self.fq_codel_limit_packets as u32, + flows: self.fq_codel_flows as u32, + ecn: self.fq_codel_ecn, + }, }) } } diff --git a/aifw-core/src/sudo.rs b/aifw-core/src/sudo.rs index dfbe36fd..94e84d48 100644 --- a/aifw-core/src/sudo.rs +++ b/aifw-core/src/sudo.rs @@ -80,6 +80,19 @@ const HELPER_CP: &str = "/usr/local/libexec/aifw-sudo-cp"; const HELPER_TAR: &str = "/usr/local/libexec/aifw-sudo-tar"; const HELPER_TCPDUMP: &str = "/usr/local/libexec/aifw-sudo-tcpdump"; const HELPER_SWANCTL: &str = "/usr/local/libexec/aifw-sudo-swanctl"; +const HELPER_DUMMYNET: &str = "/usr/local/libexec/aifw-sudo-dummynet"; +/// Apply dummynet/FQ-CoDel commands through the closed helper. +pub async fn dummynet_apply(commands: &[String]) -> std::io::Result<()> { + let payload = commands.join("\n"); + let output = run_with_stdin_pipe(&[HELPER_DUMMYNET], payload.as_bytes()).await?; + if output.status.success() { + Ok(()) + } else { + Err(std::io::Error::other( + String::from_utf8_lossy(&output.stderr).trim().to_string(), + )) + } +} /// Atomically write `contents` to `path`, as root, via the /// `aifw-sudo-write` helper script. diff --git a/aifw-core/src/tests.rs b/aifw-core/src/tests.rs index eb96ee77..0bd3fdb8 100644 --- a/aifw-core/src/tests.rs +++ b/aifw-core/src/tests.rs @@ -676,9 +676,25 @@ mod tests { engine.apply_queues().await.unwrap(); let pf_queues = mock.get_queues("aifw").await.unwrap(); - assert_eq!(pf_queues.len(), 2); // parent + child + assert_eq!(pf_queues.len(), 1); // parent only; FQ-CoDel is dummynet assert!(pf_queues[0].contains("queue on em0")); - assert!(pf_queues[1].contains("queue default_q")); + } + + #[test] + fn test_fq_codel_uses_dummynet_not_false_pf_queue_options() { + let q = QueueConfig::new( + Interface("em0".to_string()), + QueueType::Codel, + Bandwidth { + value: 100, + unit: BandwidthUnit::Mbps, + }, + "bulk".to_string(), + TrafficClass::Bulk, + ); + assert!(q.to_pf_queue().contains("fq_codel")); + assert!(!q.to_pf_queue().contains("flows 1024")); + assert!(q.fq_codel.validate().is_ok()); } #[tokio::test] diff --git a/aifw-core/src/updater.rs b/aifw-core/src/updater.rs index 082c484e..92bad5a1 100644 --- a/aifw-core/src/updater.rs +++ b/aifw-core/src/updater.rs @@ -112,6 +112,10 @@ const EMBEDDED_SUDO_HELPERS: &[(&str, &str)] = &[ "aifw-sudo-swanctl", include_str!("../../freebsd/overlay/usr/local/libexec/aifw-sudo-swanctl"), ), + ( + "aifw-sudo-dummynet", + include_str!("../../freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet"), + ), ]; #[derive(Deserialize)] diff --git a/aifw-setup/src/apply.rs b/aifw-setup/src/apply.rs index 7e1db714..ba8ee703 100644 --- a/aifw-setup/src/apply.rs +++ b/aifw-setup/src/apply.rs @@ -2560,6 +2560,7 @@ aifw ALL=(root) NOPASSWD: /usr/local/libexec/aifw-sudo-cp * aifw ALL=(root) NOPASSWD: /usr/local/libexec/aifw-sudo-tar * aifw ALL=(root) NOPASSWD: /usr/local/libexec/aifw-sudo-tcpdump * aifw ALL=(root) NOPASSWD: /usr/local/libexec/aifw-sudo-swanctl * +aifw ALL=(root) NOPASSWD: /usr/local/libexec/aifw-sudo-dummynet # --- Broad compat grants --- # Kept alongside the narrow helpers above for upgrade compat: in-place @@ -2787,6 +2788,7 @@ mod sudoers_tests { "/usr/local/libexec/aifw-sudo-tar", "/usr/local/libexec/aifw-sudo-tcpdump", "/usr/local/libexec/aifw-sudo-swanctl", + "/usr/local/libexec/aifw-sudo-dummynet", ] { assert!( content.contains(helper), diff --git a/freebsd/deploy.sh b/freebsd/deploy.sh index 08d7a4cd..0c7de6a9 100755 --- a/freebsd/deploy.sh +++ b/freebsd/deploy.sh @@ -255,7 +255,7 @@ done # login migrate, shutdown hook, narrow-grant sudo wrappers from #204). # Mode 755 so the daemon supervisor / sudo can exec them. mkdir -p /usr/local/libexec -for script in aifw-restart.sh aifw-watchdog.sh aifw-motd-cleanup.sh aifw-login-migrate.sh aifw-shutdown-hook.sh aifw-sudo-write aifw-sudo-wg aifw-sudo-freebsd-update aifw-sudo-pkg aifw-sudo-service aifw-sudo-chown aifw-sudo-ifconfig aifw-sudo-install aifw-sudo-sysrc aifw-sudo-dhclient aifw-sudo-route aifw-sudo-pkill aifw-sudo-rm aifw-sudo-mkdir aifw-sudo-cp aifw-sudo-tar aifw-sudo-tcpdump; do +for script in aifw-restart.sh aifw-watchdog.sh aifw-motd-cleanup.sh aifw-login-migrate.sh aifw-shutdown-hook.sh aifw-sudo-write aifw-sudo-wg aifw-sudo-freebsd-update aifw-sudo-pkg aifw-sudo-service aifw-sudo-chown aifw-sudo-ifconfig aifw-sudo-install aifw-sudo-sysrc aifw-sudo-dhclient aifw-sudo-route aifw-sudo-pkill aifw-sudo-rm aifw-sudo-mkdir aifw-sudo-cp aifw-sudo-tar aifw-sudo-tcpdump aifw-sudo-dummynet; do src="$REPO_DIR/freebsd/overlay/usr/local/libexec/$script" if [ -f "$src" ]; then install -m 755 "$src" "/usr/local/libexec/$script" diff --git a/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet b/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet new file mode 100755 index 00000000..0e7bdc5b --- /dev/null +++ b/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet @@ -0,0 +1,18 @@ +#!/bin/sh +# Apply only AiFw-owned dummynet pipes/IPFW rules. Commands are parsed as +# argv by the controller, never evaluated by a shell, and the previous state +# is restored if any command or live verification fails. +set -eu +[ $# -eq 0 ] || { echo "usage: $0" >&2; exit 2; } +CONTROL=/usr/local/libexec/aifw-dummynet-control +[ -x "$CONTROL" ] || { echo "dummynet controller is not installed" >&2; exit 1; } +TMP=$(mktemp /tmp/aifw-dummynet.XXXXXX) +trap 'rm -f "$TMP"' EXIT INT TERM HUP +cat >"$TMP" +[ -s "$TMP" ] || { echo "empty dummynet command set" >&2; exit 1; } +"$CONTROL" snapshot /var/run/aifw-dummynet.snapshot +if ! "$CONTROL" apply "$TMP" || ! "$CONTROL" verify; then + "$CONTROL" restore /var/run/aifw-dummynet.snapshot >/dev/null 2>&1 || true + echo "dummynet apply/verify failed; prior state restored" >&2 + exit 1 +fi From 4945440f9defeb0055bdd2e6bb2e77cd0d58721d Mon Sep 17 00:00:00 2001 From: Ron McCorkle <31546545+mack42@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:58:58 +0000 Subject: [PATCH 17/71] fix: satisfy strict clippy in shaping backend --- aifw-core/src/shaping.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aifw-core/src/shaping.rs b/aifw-core/src/shaping.rs index 165870e7..22634b2e 100644 --- a/aifw-core/src/shaping.rs +++ b/aifw-core/src/shaping.rs @@ -361,7 +361,7 @@ fn render_dummynet_commands(q: &QueueConfig) -> Result> { async fn apply_dummynet(commands: &[String]) -> Result<()> { if commands.is_empty() { - return Ok(()); + Ok(()) } #[cfg(not(target_os = "freebsd"))] { From 08d0da305554664e2398cb0d6b54b9222e6f5bbf Mon Sep 17 00:00:00 2001 From: Ron McCorkle <31546545+mack42@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:59:58 +0000 Subject: [PATCH 18/71] fix: clean dummynet cfg clippy branch --- aifw-core/src/shaping.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aifw-core/src/shaping.rs b/aifw-core/src/shaping.rs index 22634b2e..86ad92fc 100644 --- a/aifw-core/src/shaping.rs +++ b/aifw-core/src/shaping.rs @@ -361,12 +361,12 @@ fn render_dummynet_commands(q: &QueueConfig) -> Result> { async fn apply_dummynet(commands: &[String]) -> Result<()> { if commands.is_empty() { - Ok(()) + return Ok(()); } #[cfg(not(target_os = "freebsd"))] { let _ = commands; - return Ok(()); + Ok(()) } #[cfg(target_os = "freebsd")] crate::sudo::dummynet_apply(commands) From 30cd9063ab93ff00d677936dc7b1f3f2d29de947 Mon Sep 17 00:00:00 2001 From: Ron McCorkle <31546545+mack42@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:52:23 +0000 Subject: [PATCH 19/71] feat(release): sign and attest update artifacts --- .github/workflows/build-iso.yml | 81 ++++++++++++++++++++++++++++++++ aifw-api/src/updates.rs | 1 + aifw-core/src/updater.rs | 83 ++++++++++++++++++++++++++------- 3 files changed, 147 insertions(+), 18 deletions(-) diff --git a/.github/workflows/build-iso.yml b/.github/workflows/build-iso.yml index d6181cff..4606808d 100644 --- a/.github/workflows/build-iso.yml +++ b/.github/workflows/build-iso.yml @@ -12,9 +12,12 @@ on: permissions: contents: write + id-token: write + attestations: write env: FREEBSD_VERSION: "15.0" + MINISIGN_PUBLIC_KEY: ${{ vars.MINISIGN_PUBLIC_KEY }} jobs: build-ui: @@ -85,6 +88,10 @@ jobs: echo "=== Building AiFw binaries ===" cd /home/runner/work/AiFw/AiFw + test -n "$MINISIGN_PUBLIC_KEY" || { echo "MINISIGN_PUBLIC_KEY repository variable is required" >&2; exit 1; } + mkdir -p freebsd/overlay/usr/local/etc/aifw + printf '%s\n' "$MINISIGN_PUBLIC_KEY" > freebsd/overlay/usr/local/etc/aifw/update-signing.pub + chmod 644 freebsd/overlay/usr/local/etc/aifw/update-signing.pub cargo build --release echo "=== Validating sudoers content (visudo -cf, #204) ===" @@ -195,6 +202,47 @@ jobs: mkdir -p /home/runner/work/AiFw/AiFw/output cp /usr/obj/aifw-iso/output/* /home/runner/work/AiFw/AiFw/output/ + - name: Generate ISO CycloneDX SBOM + uses: anchore/sbom-action@f8bdd1d8ac5e901a77a92f111440fdb1b593736b # v0.20.6 + with: + path: output/aifw-*.iso.xz + format: cyclonedx-json + output-file: output/aifw-iso.cdx.json + upload-artifact: false + + - name: Generate IMG CycloneDX SBOM + uses: anchore/sbom-action@f8bdd1d8ac5e901a77a92f111440fdb1b593736b # v0.20.6 + with: + path: output/aifw-*.img.xz + format: cyclonedx-json + output-file: output/aifw-img.cdx.json + upload-artifact: false + + - name: Generate update CycloneDX SBOM + uses: anchore/sbom-action@f8bdd1d8ac5e901a77a92f111440fdb1b593736b # v0.20.6 + with: + path: output/aifw-update-*.tar.xz + format: cyclonedx-json + output-file: output/aifw-update.cdx.json + upload-artifact: false + + - name: Sign update checksums + env: + MINISIGN_SECRET_KEY: ${{ secrets.MINISIGN_SECRET_KEY }} + MINISIGN_PASSWORD: ${{ secrets.MINISIGN_PASSWORD }} + run: | + set -euo pipefail + sudo apt-get update + sudo apt-get install -y minisign + key_file="$RUNNER_TEMP/minisign.key" + trap 'rm -f "$key_file"' EXIT + printf '%s' "$MINISIGN_SECRET_KEY" > "$key_file" + chmod 600 "$key_file" + for checksum in output/aifw-update-*.tar.xz.sha256; do + [ -f "$checksum" ] || continue + minisign -S -s "$key_file" -m "$checksum" -x "${checksum}.minisig" + done + - name: Upload ISO artifact uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: @@ -263,6 +311,22 @@ jobs: name: boot-smoke-artifacts path: smoke-artifacts/ retention-days: 7 + - name: Upload SBOMs and signatures + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: supply-chain + path: | + output/*.cdx.json + output/*.minisig + retention-days: 5 + + - name: Attest release artifacts + uses: actions/attest-build-provenance@e8998f949152b193b063cb0ec769d69d929409be # v2.4.0 + with: + subject-path: | + output/aifw-*.iso.xz + output/aifw-*.img.xz + output/aifw-update-*.tar.xz release: name: Create GitHub Release @@ -296,6 +360,23 @@ jobs: name: components path: release-assets/ + - name: Download supply-chain assets + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 + with: + name: supply-chain + path: release-assets/ + + - name: Verify update signatures before publication + env: + MINISIGN_PUBLIC_KEY: ${{ vars.MINISIGN_PUBLIC_KEY }} + run: | + set -euo pipefail + sudo apt-get update + sudo apt-get install -y minisign + for checksum in release-assets/aifw-update-*.tar.xz.sha256; do + minisign -Vm "$checksum" -x "${checksum}.minisig" -P "$MINISIGN_PUBLIC_KEY" + done + - name: Extract version from tag id: version run: if [ -n "${{ inputs.version }}" ]; then diff --git a/aifw-api/src/updates.rs b/aifw-api/src/updates.rs index d0605ab7..d7810803 100644 --- a/aifw-api/src/updates.rs +++ b/aifw-api/src/updates.rs @@ -565,6 +565,7 @@ pub async fn aifw_update_status( published_at: String::new(), tarball_url: None, checksum_url: None, + checksum_signature_url: None, has_backup: std::path::Path::new("/usr/local/share/aifw/backup/version").exists(), backup_version: tokio::fs::read_to_string("/usr/local/share/aifw/backup/version") .await diff --git a/aifw-core/src/updater.rs b/aifw-core/src/updater.rs index 082c484e..75a2ed15 100644 --- a/aifw-core/src/updater.rs +++ b/aifw-core/src/updater.rs @@ -251,6 +251,9 @@ pub struct AifwUpdateInfo { pub tarball_url: Option, /// Download URL of the tarball's SHA-256 checksum asset, if present pub checksum_url: Option, + /// Download URL of the checksum's detached minisign signature. + #[serde(default)] + pub checksum_signature_url: Option, /// True when a rollback backup exists on disk pub has_backup: bool, /// Version the on-disk backup was taken from, when known @@ -343,24 +346,7 @@ pub async fn check_for_update(include_prereleases: bool) -> Result Result Result Result Result Result<(), UpdaterError> { + const PUBKEY_PATH: &str = "/usr/local/etc/aifw/update-signing.pub"; + let output = Command::new("minisign") + .args(["-Vm", checksum, "-x", signature, "-p", PUBKEY_PATH]) + .output() + .await + .map_err(|e| UpdaterError::Download(format!("signature verification unavailable: {e}")))?; + if output.status.success() { + Ok(()) + } else { + Err(UpdaterError::Checksum) + } +} + /// Services that may have had their rc.d script replaced by an update and /// therefore need a restart for the new script to take effect. Order /// matters for aifw_api (last) so HTTP stays up as long as possible. @@ -1323,6 +1335,28 @@ fn extract_hash(checksum_content: &str) -> String { line.split_whitespace().next().unwrap_or("").to_string() } +fn release_asset_urls( + release: &serde_json::Value, +) -> (Option, Option, Option) { + let mut tarball = None; + let mut checksum = None; + let mut signature = None; + if let Some(assets) = release["assets"].as_array() { + for asset in assets { + let name = asset["name"].as_str().unwrap_or(""); + let url = asset["browser_download_url"].as_str().unwrap_or(""); + if name.starts_with("aifw-update-") && name.ends_with(".tar.xz") { + tarball = Some(url.to_string()); + } else if name.starts_with("aifw-update-") && name.ends_with(".tar.xz.sha256.minisig") { + signature = Some(url.to_string()); + } else if name.starts_with("aifw-update-") && name.ends_with(".tar.xz.sha256") { + checksum = Some(url.to_string()); + } + } + } + (tarball, checksum, signature) +} + async fn http_get(url: &str) -> Result { // Try fetch (FreeBSD) first, fall back to curl if let Ok(o) = Command::new("fetch").args(["-qo", "-", url]).output().await @@ -1544,6 +1578,19 @@ mod tests { assert_eq!(extract_hash(input), "abc123def456"); } + #[test] + fn release_assets_require_distinct_checksum_and_signature_sidecars() { + let release = serde_json::json!({"assets": [ + {"name": "aifw-update-6.0.0-amd64.tar.xz", "browser_download_url": "tar"}, + {"name": "aifw-update-6.0.0-amd64.tar.xz.sha256", "browser_download_url": "sum"}, + {"name": "aifw-update-6.0.0-amd64.tar.xz.sha256.minisig", "browser_download_url": "sig"} + ]}); + assert_eq!( + release_asset_urls(&release), + (Some("tar".into()), Some("sum".into()), Some("sig".into())) + ); + } + // Regression gate for #469: every overlay libexec script must carry the // execute bit in git. Eight aifw-sudo-* helpers were committed mode 644, // build-iso.sh's `cp -a` preserved that onto installed systems, and sudo From 5faa23c8e41d67edf6a9acadd230399b930a002f Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Wed, 22 Jul 2026 00:23:01 +0000 Subject: [PATCH 20/71] feat: complete FQ-CoDel lifecycle and FreeBSD qualification --- .github/workflows/fq-codel-freebsd.yml | 37 ++++ aifw-api/src/backup.rs | 2 + aifw-api/src/main.rs | 2 + aifw-api/src/router.rs | 18 ++ aifw-api/src/routes/mod.rs | 2 + aifw-api/src/routes/shaper.rs | 128 ++++++++++++++ aifw-cli/src/commands.rs | 2 + aifw-common/src/ratelimit.rs | 9 +- aifw-common/src/tests.rs | 3 +- aifw-core/src/config.rs | 3 + aifw-core/src/shaping.rs | 165 ++++++++++++++---- aifw-core/src/tests.rs | 5 +- aifw-core/src/updater.rs | 4 + freebsd/build-iso.sh | 2 +- freebsd/deploy.sh | 4 +- freebsd/manifest.json | 2 +- .../usr/local/libexec/aifw-dummynet-control | 87 +++++++++ .../usr/local/libexec/aifw-sudo-dummynet | 11 +- freebsd/tests/t08-fq-codel.sh | 54 ++++++ 19 files changed, 487 insertions(+), 53 deletions(-) create mode 100644 .github/workflows/fq-codel-freebsd.yml create mode 100644 aifw-api/src/routes/shaper.rs create mode 100755 freebsd/overlay/usr/local/libexec/aifw-dummynet-control create mode 100755 freebsd/tests/t08-fq-codel.sh diff --git a/.github/workflows/fq-codel-freebsd.yml b/.github/workflows/fq-codel-freebsd.yml new file mode 100644 index 00000000..0bae5b3d --- /dev/null +++ b/.github/workflows/fq-codel-freebsd.yml @@ -0,0 +1,37 @@ +name: FQ-CoDel FreeBSD qualification + +on: + pull_request: + paths: + - "aifw-common/src/ratelimit.rs" + - "aifw-core/src/shaping.rs" + - "freebsd/overlay/usr/local/libexec/aifw*dummynet*" + - "freebsd/tests/t08-fq-codel.sh" + - ".github/workflows/fq-codel-freebsd.yml" + workflow_dispatch: + +permissions: + contents: read + +jobs: + functional: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - name: Exercise real FreeBSD dummynet + uses: vmactions/freebsd-vm@77ed28d336d03fe19a3f4f7266c1d2c4714dd79d # v1.5.2 + with: + release: "15.0" + usesh: true + prepare: | + install -m 755 freebsd/overlay/usr/local/libexec/aifw-dummynet-control /usr/local/libexec/aifw-dummynet-control + run: | + sh freebsd/tests/t08-fq-codel.sh 2>&1 | tee fq-codel-results.txt + - name: Upload raw result + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: fq-codel-freebsd-${{ github.sha }} + path: fq-codel-results.txt + if-no-files-found: warn diff --git a/aifw-api/src/backup.rs b/aifw-api/src/backup.rs index 17e136e2..f8bd7a79 100644 --- a/aifw-api/src/backup.rs +++ b/aifw-api/src/backup.rs @@ -733,6 +733,7 @@ pub(crate) async fn build_current_config(state: &AppState) -> Result Option Router { .layer(middleware::from_fn(perm_check!(Permission::NatWrite))) } +pub(crate) fn shaper_read() -> Router { + Router::new() + .route("/api/v1/shaper/queues", get(routes::list_shaper_queues)) + .route("/api/v1/shaper/status", get(routes::shaper_status)) + .layer(middleware::from_fn(perm_check!(Permission::RulesRead))) +} + +pub(crate) fn shaper_write() -> Router { + Router::new() + .route("/api/v1/shaper/queues", post(routes::create_shaper_queue)) + .route( + "/api/v1/shaper/queues/{id}", + put(routes::update_shaper_queue).delete(routes::delete_shaper_queue), + ) + .route("/api/v1/shaper/apply", post(routes::apply_shaper)) + .layer(middleware::from_fn(perm_check!(Permission::RulesWrite))) +} + pub(crate) fn vpn_read() -> Router { Router::new() .route("/api/v1/vpn/wg", get(routes::list_wg_tunnels)) diff --git a/aifw-api/src/routes/mod.rs b/aifw-api/src/routes/mod.rs index 0cde9ad7..cc0cadfb 100644 --- a/aifw-api/src/routes/mod.rs +++ b/aifw-api/src/routes/mod.rs @@ -34,6 +34,7 @@ pub mod response; pub mod rules; pub mod schedules; pub mod settings; +pub mod shaper; pub mod static_routes; pub mod status; pub mod users; @@ -51,6 +52,7 @@ pub use response::*; pub use rules::*; pub use schedules::*; pub use settings::*; +pub use shaper::*; pub use static_routes::*; pub use status::*; pub use users::*; diff --git a/aifw-api/src/routes/shaper.rs b/aifw-api/src/routes/shaper.rs new file mode 100644 index 00000000..6fe6f521 --- /dev/null +++ b/aifw-api/src/routes/shaper.rs @@ -0,0 +1,128 @@ +//! FQ-CoDel shaper API: CRUD plus explicit apply/status operations. + +use super::*; +use aifw_common::QueueConfig; + +#[derive(Debug, Serialize)] +pub struct ShaperStatus { + pub configured: usize, + pub active: usize, + pub backend: &'static str, + pub verified: bool, +} + +pub async fn list_shaper_queues( + State(state): State, +) -> Result>>, StatusCode> { + let queues = state + .shaping_engine + .list_queues() + .await + .map_err(|_| internal())?; + Ok(Json(ApiResponse { data: queues })) +} + +pub async fn create_shaper_queue( + State(state): State, + Json(queue): Json, +) -> Result<(StatusCode, Json>), StatusCode> { + let created = state + .shaping_engine + .add_queue(queue) + .await + .map_err(|_| bad_request())?; + state + .shaping_engine + .apply_queues() + .await + .map_err(|_| internal())?; + Ok((StatusCode::CREATED, Json(ApiResponse { data: created }))) +} + +pub async fn delete_shaper_queue( + State(state): State, + Path(id): Path, +) -> Result, StatusCode> { + let id = Uuid::parse_str(&id).map_err(|_| bad_request())?; + state + .shaping_engine + .delete_queue(id) + .await + .map_err(|_| StatusCode::NOT_FOUND)?; + state + .shaping_engine + .apply_queues() + .await + .map_err(|_| internal())?; + Ok(Json(MessageResponse { + message: "shaper queue deleted and live state reapplied".into(), + })) +} + +pub async fn update_shaper_queue( + State(state): State, + Path(id): Path, + Json(mut queue): Json, +) -> Result>, StatusCode> { + queue.id = Uuid::parse_str(&id).map_err(|_| bad_request())?; + let updated = state + .shaping_engine + .update_queue(queue) + .await + .map_err(|_| bad_request())?; + state + .shaping_engine + .apply_queues() + .await + .map_err(|_| internal())?; + Ok(Json(ApiResponse { data: updated })) +} + +pub async fn apply_shaper( + State(state): State, +) -> Result, StatusCode> { + state + .shaping_engine + .apply_queues() + .await + .map_err(|_| internal())?; + Ok(Json(MessageResponse { + message: "shaper configuration applied and verified".into(), + })) +} + +pub async fn shaper_status( + State(state): State, +) -> Result>, StatusCode> { + let queues = state + .shaping_engine + .list_queues() + .await + .map_err(|_| internal())?; + let active = queues + .iter() + .filter(|queue| queue.status == aifw_common::QueueStatus::Active) + .count(); + let verified = if cfg!(target_os = "freebsd") && active > 0 { + tokio::process::Command::new("dnctl") + .args(["pipe", "list"]) + .output() + .await + .map(|output| output.status.success()) + .unwrap_or(false) + } else { + active == 0 + }; + Ok(Json(ApiResponse { + data: ShaperStatus { + configured: queues.len(), + active, + backend: if cfg!(target_os = "freebsd") { + "dummynet" + } else { + "mock" + }, + verified, + }, + })) +} diff --git a/aifw-cli/src/commands.rs b/aifw-cli/src/commands.rs index 68eea8a6..af0ba8e0 100644 --- a/aifw-cli/src/commands.rs +++ b/aifw-cli/src/commands.rs @@ -537,6 +537,7 @@ pub async fn queue_add( config.default = default; let config = engine.add_queue(config).await?; + engine.apply_queues().await?; println!("Added queue {}", config.id); println!(" pf: {}", config.to_pf_queue()); Ok(()) @@ -546,6 +547,7 @@ pub async fn queue_remove(db_path: &Path, id: &str) -> anyhow::Result<()> { let engine = create_shaping_engine(db_path).await?; let uuid = Uuid::parse_str(id)?; engine.delete_queue(uuid).await?; + engine.apply_queues().await?; println!("Removed queue {id}"); Ok(()) } diff --git a/aifw-common/src/ratelimit.rs b/aifw-common/src/ratelimit.rs index c161ffc3..6b372c37 100644 --- a/aifw-common/src/ratelimit.rs +++ b/aifw-common/src/ratelimit.rs @@ -58,8 +58,8 @@ impl FqCodelConfig { "invalid FQ-CoDel target/interval".to_string(), )); } - if !(64..=65_536).contains(&self.quantum_bytes) - || !(1..=1_000_000).contains(&self.limit_packets) + if !(64..=9_000).contains(&self.quantum_bytes) + || !(1..=20_480).contains(&self.limit_packets) || !(1..=65_536).contains(&self.flows) { return Err(crate::AifwError::Validation( @@ -305,7 +305,10 @@ impl QueueConfig { } match self.queue_type { - QueueType::Codel => parts.push("fq_codel".to_string()), + // FQ-CoDel is rendered by the dummynet backend. Returning no PF + // queue syntax here prevents callers from accidentally loading a + // same-family ALTQ approximation into pf. + QueueType::Codel => return String::new(), QueueType::Hfsc => {} QueueType::Priq => parts.push(format!("priority {}", self.traffic_class.priority())), } diff --git a/aifw-common/src/tests.rs b/aifw-common/src/tests.rs index be68e593..50f2360d 100644 --- a/aifw-common/src/tests.rs +++ b/aifw-common/src/tests.rs @@ -456,8 +456,7 @@ mod tests { TrafficClass::Default, ); q.default = true; - let pf = q.to_pf_queue(); - assert!(pf.contains("default")); + assert!(q.to_pf_queue().is_empty()); } #[test] diff --git a/aifw-core/src/config.rs b/aifw-core/src/config.rs index 3eb352ba..59892558 100644 --- a/aifw-core/src/config.rs +++ b/aifw-core/src/config.rs @@ -501,6 +501,9 @@ pub struct QueueConfigEntry { pub default: bool, /// Whether the queue is loaded into pf pub status: QueueStatus, + /// FQ-CoDel scheduler parameters, retained across backup and rollback. + #[serde(default)] + pub fq_codel: aifw_common::FqCodelConfig, } /// A per-source-IP connection rate limit as stored in a config snapshot diff --git a/aifw-core/src/shaping.rs b/aifw-core/src/shaping.rs index 86ad92fc..d32560f3 100644 --- a/aifw-core/src/shaping.rs +++ b/aifw-core/src/shaping.rs @@ -8,6 +8,11 @@ use sqlx::sqlite::SqlitePool; use std::sync::Arc; use uuid::Uuid; +const DUMMYNET_PIPE_BASE: u32 = 10_000; +const DUMMYNET_PIPE_SPAN: u32 = 10_000; +const DUMMYNET_RULE_OUT_BASE: u32 = 30_000; +const DUMMYNET_RULE_IN_BASE: u32 = 40_000; + /// Traffic-shaping engine: bandwidth queues (`queue_configs` table) and /// connection rate limits (`rate_limit_rules` table). Queue definitions /// load into the base `aifw` anchor; rate-limit tables/rules load into @@ -57,24 +62,28 @@ impl ShapingEngine { ) .execute(&self.pool) .await?; - let _ = sqlx::query( - "ALTER TABLE queue_configs ADD COLUMN fq_codel_target_ms INTEGER NOT NULL DEFAULT 5", - ) - .execute(&self.pool) - .await; - let _ = sqlx::query("ALTER TABLE queue_configs ADD COLUMN fq_codel_interval_ms INTEGER NOT NULL DEFAULT 100").execute(&self.pool).await; - let _ = sqlx::query("ALTER TABLE queue_configs ADD COLUMN fq_codel_quantum_bytes INTEGER NOT NULL DEFAULT 1514").execute(&self.pool).await; - let _ = sqlx::query("ALTER TABLE queue_configs ADD COLUMN fq_codel_limit_packets INTEGER NOT NULL DEFAULT 10240").execute(&self.pool).await; - let _ = sqlx::query( - "ALTER TABLE queue_configs ADD COLUMN fq_codel_flows INTEGER NOT NULL DEFAULT 1024", - ) - .execute(&self.pool) - .await; - let _ = sqlx::query( - "ALTER TABLE queue_configs ADD COLUMN fq_codel_ecn INTEGER NOT NULL DEFAULT 1", - ) - .execute(&self.pool) - .await; + for (column, definition) in [ + ("fq_codel_target_ms", "INTEGER NOT NULL DEFAULT 5"), + ("fq_codel_interval_ms", "INTEGER NOT NULL DEFAULT 100"), + ("fq_codel_quantum_bytes", "INTEGER NOT NULL DEFAULT 1514"), + ("fq_codel_limit_packets", "INTEGER NOT NULL DEFAULT 10240"), + ("fq_codel_flows", "INTEGER NOT NULL DEFAULT 1024"), + ("fq_codel_ecn", "INTEGER NOT NULL DEFAULT 1"), + ] { + let present = sqlx::query_scalar::<_, i64>( + "SELECT count(*) FROM pragma_table_info('queue_configs') WHERE name = ?1", + ) + .bind(column) + .fetch_one(&self.pool) + .await?; + if present == 0 { + let statement = + format!("ALTER TABLE queue_configs ADD COLUMN {column} {definition}"); + sqlx::query(sqlx::AssertSqlSafe(statement)) + .execute(&self.pool) + .await?; + } + } sqlx::query( r#" @@ -108,7 +117,9 @@ impl ShapingEngine { /// Insert a queue config row. pf isn't touched until /// [`Self::apply_queues`] pub async fn add_queue(&self, config: QueueConfig) -> Result { - config.fq_codel.validate()?; + if config.queue_type == QueueType::Codel { + config.fq_codel.validate()?; + } self.insert_queue(&config).await?; tracing::info!(id = %config.id, name = %config.name, "queue added"); Ok(config) @@ -137,6 +148,26 @@ impl ShapingEngine { Ok(()) } + /// Replace an existing queue definition after validating its scheduler + /// parameters. The live backend is applied separately by the caller. + pub async fn update_queue(&self, config: QueueConfig) -> Result { + if config.queue_type == QueueType::Codel { + config.fq_codel.validate()?; + } + let result = sqlx::query("DELETE FROM queue_configs WHERE id = ?1") + .bind(config.id.to_string()) + .execute(&self.pool) + .await?; + if result.rows_affected() == 0 { + return Err(AifwError::NotFound(format!( + "queue {} not found", + config.id + ))); + } + self.insert_queue(&config).await?; + Ok(config) + } + /// Render active queue configs (one parent queue per interface plus /// each child queue) and load them into pf. Fails if the pf backend /// rejects the queue definitions @@ -149,16 +180,24 @@ impl ShapingEngine { let mut pf_lines = Vec::new(); let mut dummynet = Vec::new(); + let mut used_pipes = std::collections::HashSet::new(); // Group by interface — each needs a parent queue let mut interfaces_seen = std::collections::HashSet::new(); for q in &active { - if interfaces_seen.insert(q.interface.0.clone()) { - pf_lines.push(q.to_pf_parent_queue()); - } if q.queue_type == QueueType::Codel { q.fq_codel.validate()?; - dummynet.extend(render_dummynet_commands(q)?); + let commands = render_dummynet_commands(q)?; + let pipe = pipe_id(q.id); + if !used_pipes.insert(pipe) { + return Err(AifwError::Validation(format!( + "duplicate dummynet pipe id {pipe}" + ))); + } + dummynet.extend(commands); } else { + if interfaces_seen.insert(q.interface.0.clone()) { + pf_lines.push(q.to_pf_parent_queue()); + } pf_lines.push(q.to_pf_queue()); } } @@ -332,46 +371,57 @@ const QUEUE_COLUMNS: &str = "id, interface, queue_type, bandwidth_value, \ fq_codel_interval_ms, fq_codel_quantum_bytes, fq_codel_limit_packets, \ fq_codel_flows, fq_codel_ecn, is_default, status, created_at, updated_at"; +fn pipe_id(id: Uuid) -> u32 { + DUMMYNET_PIPE_BASE + (id.as_u128() as u32 % DUMMYNET_PIPE_SPAN) +} + fn render_dummynet_commands(q: &QueueConfig) -> Result> { let fq = q.fq_codel; fq.validate()?; - let id = (q.id.as_u128() & 0x7fff_ffff) as u32; + let id = pipe_id(q.id); + let out_rule = DUMMYNET_RULE_OUT_BASE + (id - DUMMYNET_PIPE_BASE); + let in_rule = DUMMYNET_RULE_IN_BASE + (id - DUMMYNET_PIPE_BASE); let bandwidth = q.bandwidth.to_bits_per_sec(); Ok(vec![ + format!("pipe {id} config bw {bandwidth}bit/s",), format!( - "pipe {id} config bw {bandwidth}bit/s queue {limit} sched fq_codel target {target}ms interval {interval}ms quantum {quantum} flows {flows} ecn", - limit = fq.limit_packets, + "sched {id} config pipe {id} type fq_codel target {target}ms interval {interval}ms quantum {quantum} limit {limit} flows {flows} {ecn}", target = fq.target_ms, interval = fq.interval_ms, quantum = fq.quantum_bytes, - flows = fq.flows + limit = fq.limit_packets, + flows = fq.flows, + ecn = if fq.ecn { "ecn" } else { "noecn" }, ), + format!("queue {id} config sched {id}"), format!( - "ipfw add {} pipe {id} ip from any to any out xmit {}", - 10000 + id, - q.interface + "ipfw add {} queue {id} ip from any to any out xmit {}", + out_rule, q.interface ), format!( - "ipfw add {} pipe {id} ip from any to any in recv {}", - 20000 + id, - q.interface + "ipfw add {} queue {id} ip from any to any in recv {}", + in_rule, q.interface ), ]) } async fn apply_dummynet(commands: &[String]) -> Result<()> { - if commands.is_empty() { - return Ok(()); - } #[cfg(not(target_os = "freebsd"))] { let _ = commands; Ok(()) } #[cfg(target_os = "freebsd")] - crate::sudo::dummynet_apply(commands) + { + let clear = ["clear".to_string()]; + crate::sudo::dummynet_apply(if commands.is_empty() { + &clear + } else { + commands + }) .await .map_err(|e| AifwError::Other(format!("dummynet apply failed: {e}"))) + } } /// Explicit column list for `RateLimitRow` selects, in schema order. Replaces @@ -499,3 +549,44 @@ impl RateLimitRow { }) } } + +#[cfg(test)] +mod dummynet_tests { + use super::*; + + #[test] + fn renderer_uses_documented_scheduler_pipeline_and_reserved_ids() { + let mut queue = QueueConfig::new( + Interface("em0".into()), + QueueType::Codel, + Bandwidth { + value: 100, + unit: BandwidthUnit::Mbps, + }, + "wan".into(), + TrafficClass::Default, + ); + queue.fq_codel.ecn = false; + let rendered = render_dummynet_commands(&queue).unwrap(); + let id = pipe_id(queue.id); + assert!((DUMMYNET_PIPE_BASE..DUMMYNET_PIPE_BASE + DUMMYNET_PIPE_SPAN).contains(&id)); + assert_eq!(rendered[0], format!("pipe {id} config bw 100000000bit/s")); + assert!(rendered[1].contains(&format!("sched {id} config pipe {id} type fq_codel"))); + assert!(rendered[1].ends_with("noecn")); + assert_eq!(rendered[2], format!("queue {id} config sched {id}")); + assert!(rendered[3].contains(" out xmit em0")); + assert!(rendered[4].contains(" in recv em0")); + } + + #[test] + fn validation_uses_freebsd_fq_codel_limits() { + let mut config = FqCodelConfig { + quantum_bytes: 9_001, + ..Default::default() + }; + assert!(config.validate().is_err()); + config = FqCodelConfig::default(); + config.limit_packets = 20_481; + assert!(config.validate().is_err()); + } +} diff --git a/aifw-core/src/tests.rs b/aifw-core/src/tests.rs index 0bd3fdb8..24034c91 100644 --- a/aifw-core/src/tests.rs +++ b/aifw-core/src/tests.rs @@ -676,8 +676,7 @@ mod tests { engine.apply_queues().await.unwrap(); let pf_queues = mock.get_queues("aifw").await.unwrap(); - assert_eq!(pf_queues.len(), 1); // parent only; FQ-CoDel is dummynet - assert!(pf_queues[0].contains("queue on em0")); + assert!(pf_queues.is_empty()); // FQ-CoDel is entirely dummynet } #[test] @@ -692,7 +691,7 @@ mod tests { "bulk".to_string(), TrafficClass::Bulk, ); - assert!(q.to_pf_queue().contains("fq_codel")); + assert!(q.to_pf_queue().is_empty()); assert!(!q.to_pf_queue().contains("flows 1024")); assert!(q.fq_codel.validate().is_ok()); } diff --git a/aifw-core/src/updater.rs b/aifw-core/src/updater.rs index 92bad5a1..1745f3f6 100644 --- a/aifw-core/src/updater.rs +++ b/aifw-core/src/updater.rs @@ -116,6 +116,10 @@ const EMBEDDED_SUDO_HELPERS: &[(&str, &str)] = &[ "aifw-sudo-dummynet", include_str!("../../freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet"), ), + ( + "aifw-dummynet-control", + include_str!("../../freebsd/overlay/usr/local/libexec/aifw-dummynet-control"), + ), ]; #[derive(Deserialize)] diff --git a/freebsd/build-iso.sh b/freebsd/build-iso.sh index 29e04534..bab3f9c4 100755 --- a/freebsd/build-iso.sh +++ b/freebsd/build-iso.sh @@ -113,7 +113,7 @@ cp /etc/resolv.conf "$STAGEDIR/etc/resolv.conf" # Bootstrap pkg and install required packages chroot "$STAGEDIR" /bin/sh -c ' env ASSUME_ALWAYS_YES=yes pkg bootstrap -f - pkg install -y wireguard-tools sudo unbound curl strongswan + pkg install -y wireguard-tools sudo unbound curl strongswan minisign ' umount "$STAGEDIR/dev" diff --git a/freebsd/deploy.sh b/freebsd/deploy.sh index 0c7de6a9..503c68f7 100755 --- a/freebsd/deploy.sh +++ b/freebsd/deploy.sh @@ -37,7 +37,7 @@ if ! command -v cargo >/dev/null 2>&1; then fi # --- Ensure dependencies --- -for pkg in sudo unbound curl wireguard-tools strongswan; do +for pkg in sudo unbound curl wireguard-tools strongswan minisign; do if ! pkg info -q "$pkg" 2>/dev/null; then echo "Installing $pkg..." pkg install -y "$pkg" @@ -255,7 +255,7 @@ done # login migrate, shutdown hook, narrow-grant sudo wrappers from #204). # Mode 755 so the daemon supervisor / sudo can exec them. mkdir -p /usr/local/libexec -for script in aifw-restart.sh aifw-watchdog.sh aifw-motd-cleanup.sh aifw-login-migrate.sh aifw-shutdown-hook.sh aifw-sudo-write aifw-sudo-wg aifw-sudo-freebsd-update aifw-sudo-pkg aifw-sudo-service aifw-sudo-chown aifw-sudo-ifconfig aifw-sudo-install aifw-sudo-sysrc aifw-sudo-dhclient aifw-sudo-route aifw-sudo-pkill aifw-sudo-rm aifw-sudo-mkdir aifw-sudo-cp aifw-sudo-tar aifw-sudo-tcpdump aifw-sudo-dummynet; do +for script in aifw-restart.sh aifw-watchdog.sh aifw-motd-cleanup.sh aifw-login-migrate.sh aifw-shutdown-hook.sh aifw-sudo-write aifw-sudo-wg aifw-sudo-freebsd-update aifw-sudo-pkg aifw-sudo-service aifw-sudo-chown aifw-sudo-ifconfig aifw-sudo-install aifw-sudo-sysrc aifw-sudo-dhclient aifw-sudo-route aifw-sudo-pkill aifw-sudo-rm aifw-sudo-mkdir aifw-sudo-cp aifw-sudo-tar aifw-sudo-tcpdump aifw-sudo-dummynet aifw-dummynet-control; do src="$REPO_DIR/freebsd/overlay/usr/local/libexec/$script" if [ -f "$src" ]; then install -m 755 "$src" "/usr/local/libexec/$script" diff --git a/freebsd/manifest.json b/freebsd/manifest.json index 1e77fe3d..4676a0e5 100644 --- a/freebsd/manifest.json +++ b/freebsd/manifest.json @@ -6,7 +6,7 @@ "local": ["aifw", "aifw-daemon", "aifw-api", "aifw-ids", "aifw-tui", "aifw-setup"] }, - "packages": ["curl", "strongswan", "sudo", "unbound", "wireguard-tools"], + "packages": ["curl", "minisign", "strongswan", "sudo", "unbound", "wireguard-tools"], "_packages_comment": "OS packages required at runtime. build-iso.sh installs them into the image; deploy.sh ensures them on the dev VM; the in-app updater installs any that are missing during upgrade. A workspace test (aifw-core packaging_tests) asserts the build scripts stay in sync with this list.", "external_repos": [ diff --git a/freebsd/overlay/usr/local/libexec/aifw-dummynet-control b/freebsd/overlay/usr/local/libexec/aifw-dummynet-control new file mode 100755 index 00000000..e5081c96 --- /dev/null +++ b/freebsd/overlay/usr/local/libexec/aifw-dummynet-control @@ -0,0 +1,87 @@ +#!/bin/sh +# Ownership-safe dummynet/IPFW controller for AiFw's reserved ranges. +set -eu +set -f + +PIPE_MIN=10000 +PIPE_MAX=19999 +RULE_MIN=30000 +RULE_MAX=49999 + +die() { echo "dummynet-control: $*" >&2; exit 1; } +valid_id() { case "$1" in ''|*[!0-9]*) return 1;; esac; [ "$1" -ge "$2" ] && [ "$1" -le "$3" ]; } + +snapshot() { + [ "$#" -eq 1 ] || die "snapshot requires a path" + case "$1" in /var/run/aifw-dummynet.snapshot) ;; *) die "invalid snapshot path";; esac + { echo '# aifw snapshot'; ipfw -a list; dnctl list; } > "$1" +} + +apply() { + [ "$#" -eq 1 ] || die "apply requires a command file" + [ -f "$1" ] || die "command file missing" + desired="$1" + # A single apply owns the reserved ranges. Remove only those objects before + # installing the desired set; administrator-owned IDs are never touched. + ipfw -q delete $(ipfw -a list | awk '$1 >= 30000 && $1 <= 49999 { print $1 }') 2>/dev/null || true + dnctl -q queue delete $(dnctl queue list | awk '$1 ~ /^[0-9]+/ { id=$1; sub(/:.*/, "", id); if (id >= 10000 && id <= 19999) print id }') 2>/dev/null || true + dnctl -q sched delete $(dnctl sched list | awk '$1 ~ /^[0-9]+/ { id=$1; sub(/:.*/, "", id); if (id >= 10000 && id <= 19999) print id }') 2>/dev/null || true + dnctl -q pipe delete $(dnctl pipe list | awk '$1 ~ /^[0-9]+/ { id=$1; sub(/:.*/, "", id); if (id >= 10000 && id <= 19999) print id }') 2>/dev/null || true + while IFS= read -r line || [ -n "$line" ]; do + [ -n "$line" ] || continue + [ "$line" = clear ] && continue + set -- $line + case "$1" in + pipe) + [ "$#" -ge 4 ] || die "invalid pipe command" + valid_id "$2" "$PIPE_MIN" "$PIPE_MAX" || die "pipe outside AiFw range" + [ "$3" = config ] || die "invalid pipe operation" + dnctl "$@" + ;; + sched|queue) + [ "$#" -ge 4 ] || die "invalid scheduler command" + valid_id "$2" "$PIPE_MIN" "$PIPE_MAX" || die "object outside AiFw range" + [ "$3" = config ] || die "invalid scheduler operation" + dnctl "$@" + ;; + ipfw) + [ "$#" -ge 4 ] || die "invalid ipfw command" + [ "$2" = add ] || die "only ipfw add is permitted" + valid_id "$3" "$RULE_MIN" "$RULE_MAX" || die "rule outside AiFw range" + shift + ipfw "$@" + ;; + *) die "unsupported command: $1";; + esac + done < "$desired" +} + +verify() { + expected=${1:-} + [ -f "$expected" ] || die "verify requires desired command file" + [ "$(sed '/^clear$/d;/^$/d' "$expected" | wc -l | tr -d ' ')" -eq 0 ] && return 0 + live=$(dnctl list; ipfw -a list) + for id in $(awk '$1 == "pipe" || $1 == "sched" || $1 == "queue" {print $2} $1 == "ipfw" && $2 == "add" {print $3}' "$expected" | sort -u); do + echo "$live" | grep -Eq "(^|[[:space:]])${id}([:[:space:]]|$)" || die "managed id $id missing after apply" + done +} + +restore() { + [ "$#" -eq 1 ] || die "restore requires a path" + [ -f "$1" ] || die "snapshot missing" + # Restore is intentionally conservative: remove only AiFw-owned objects. + ipfw -q delete $(ipfw -a list | awk '$1 >= 30000 && $1 <= 49999 { print $1 }') 2>/dev/null || true + dnctl -q queue delete $(dnctl queue list | awk '$1 ~ /^[0-9]+/ { id=$1; sub(/:.*/, "", id); if (id >= 10000 && id <= 19999) print id }') 2>/dev/null || true + dnctl -q sched delete $(dnctl sched list | awk '$1 ~ /^[0-9]+/ { id=$1; sub(/:.*/, "", id); if (id >= 10000 && id <= 19999) print id }') 2>/dev/null || true + dnctl -q pipe delete $(dnctl pipe list | awk '$1 ~ /^[0-9]+/ { id=$1; sub(/:.*/, "", id); if (id >= 10000 && id <= 19999) print id }') 2>/dev/null || true + [ -s "$1" ] && apply "$1" +} + +[ "$#" -ge 1 ] || die "usage: snapshot|apply|verify|restore" +case "$1" in +snapshot) shift; snapshot "$@";; +apply) shift; apply "$@";; +verify) shift; verify "$@";; +restore) shift; restore "$@";; +*) die "unknown operation";; +esac diff --git a/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet b/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet index 0e7bdc5b..b5f6ec2d 100755 --- a/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet +++ b/freebsd/overlay/usr/local/libexec/aifw-sudo-dummynet @@ -7,12 +7,15 @@ set -eu CONTROL=/usr/local/libexec/aifw-dummynet-control [ -x "$CONTROL" ] || { echo "dummynet controller is not installed" >&2; exit 1; } TMP=$(mktemp /tmp/aifw-dummynet.XXXXXX) -trap 'rm -f "$TMP"' EXIT INT TERM HUP +LAST=/var/run/aifw-dummynet.last-good +SNAP=$(mktemp /tmp/aifw-dummynet-snapshot.XXXXXX) +trap 'rm -f "$TMP" "$SNAP"' EXIT INT TERM HUP cat >"$TMP" [ -s "$TMP" ] || { echo "empty dummynet command set" >&2; exit 1; } -"$CONTROL" snapshot /var/run/aifw-dummynet.snapshot -if ! "$CONTROL" apply "$TMP" || ! "$CONTROL" verify; then - "$CONTROL" restore /var/run/aifw-dummynet.snapshot >/dev/null 2>&1 || true +if [ -f "$LAST" ]; then cp "$LAST" "$SNAP"; fi +if ! "$CONTROL" apply "$TMP" || ! "$CONTROL" verify "$TMP"; then + "$CONTROL" restore "$SNAP" >/dev/null 2>&1 || true echo "dummynet apply/verify failed; prior state restored" >&2 exit 1 fi +cp "$TMP" "$LAST" diff --git a/freebsd/tests/t08-fq-codel.sh b/freebsd/tests/t08-fq-codel.sh new file mode 100755 index 00000000..9d169a6d --- /dev/null +++ b/freebsd/tests/t08-fq-codel.sh @@ -0,0 +1,54 @@ +#!/bin/sh +# T08: real FreeBSD FQ-CoDel lifecycle and ownership qualification. +set -eu + +CONTROL=/usr/local/libexec/aifw-dummynet-control +WORK=$(mktemp -d /tmp/aifw-fq-codel.XXXXXX) +UNMANAGED_PIPE=9999 +MANAGED_PIPE=10001 +trap 'ipfw -q delete 30001 40001 29999 2>/dev/null || true; dnctl -q queue delete 10001 9999 2>/dev/null || true; dnctl -q sched delete 10001 9999 2>/dev/null || true; dnctl -q pipe delete 10001 9999 2>/dev/null || true; rm -rf "$WORK"' EXIT INT TERM HUP + +[ "$(id -u)" -eq 0 ] || { echo "T08 requires root" >&2; exit 77; } +kldload dummynet 2>/dev/null || true +sysctl net.inet.ip.fw.enable=1 >/dev/null + +cat > "$WORK/desired" </dev/null + +# A rejected command must not be able to escape the reserved ranges. +cat > "$WORK/hostile" <&2 + exit 1 +fi +dnctl pipe list | grep -q "^${UNMANAGED_PIPE}" + +# Empty desired state removes AiFw objects without touching admin objects. +printf 'clear\n' > "$WORK/clear" +"$CONTROL" apply "$WORK/clear" +"$CONTROL" verify "$WORK/clear" +! ipfw list 30001 >/dev/null 2>&1 +! dnctl pipe list | grep -q "^${MANAGED_PIPE}" +dnctl pipe list | grep -q "^${UNMANAGED_PIPE}" +ipfw list 29999 >/dev/null + +echo "T08 PASS: FQ-CoDel live lifecycle, IPv4/IPv6 classifiers, and ownership boundaries" From 7cd3e517fe0d255e4788f1a3d62bf76486b3a257 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Wed, 22 Jul 2026 00:29:37 +0000 Subject: [PATCH 21/71] feat: enforce signed release verification end to end --- .github/workflows/build-iso.yml | 87 +++++++++---------- aifw-core/src/updater.rs | 10 ++- .../app/updates/components/FirmwareCard.tsx | 21 ++++- aifw-ui/src/lib/api/updates.ts | 1 + freebsd/build-iso.sh | 2 +- freebsd/deploy.sh | 2 +- freebsd/manifest.json | 2 +- 7 files changed, 75 insertions(+), 50 deletions(-) diff --git a/.github/workflows/build-iso.yml b/.github/workflows/build-iso.yml index 4606808d..7e52f0aa 100644 --- a/.github/workflows/build-iso.yml +++ b/.github/workflows/build-iso.yml @@ -202,44 +202,30 @@ jobs: mkdir -p /home/runner/work/AiFw/AiFw/output cp /usr/obj/aifw-iso/output/* /home/runner/work/AiFw/AiFw/output/ - - name: Generate ISO CycloneDX SBOM + - name: Generate release CycloneDX SBOM uses: anchore/sbom-action@f8bdd1d8ac5e901a77a92f111440fdb1b593736b # v0.20.6 with: - path: output/aifw-*.iso.xz - format: cyclonedx-json - output-file: output/aifw-iso.cdx.json - upload-artifact: false - - - name: Generate IMG CycloneDX SBOM - uses: anchore/sbom-action@f8bdd1d8ac5e901a77a92f111440fdb1b593736b # v0.20.6 - with: - path: output/aifw-*.img.xz + path: output format: cyclonedx-json - output-file: output/aifw-img.cdx.json + output-file: output/aifw-release.cdx.json upload-artifact: false - - name: Generate update CycloneDX SBOM - uses: anchore/sbom-action@f8bdd1d8ac5e901a77a92f111440fdb1b593736b # v0.20.6 - with: - path: output/aifw-update-*.tar.xz - format: cyclonedx-json - output-file: output/aifw-update.cdx.json - upload-artifact: false - - - name: Sign update checksums + - name: Sign release checksums env: MINISIGN_SECRET_KEY: ${{ secrets.MINISIGN_SECRET_KEY }} MINISIGN_PASSWORD: ${{ secrets.MINISIGN_PASSWORD }} run: | set -euo pipefail + test -n "$MINISIGN_SECRET_KEY" || { echo "MINISIGN_SECRET_KEY is required" >&2; exit 1; } + test -n "$MINISIGN_PUBLIC_KEY" || { echo "MINISIGN_PUBLIC_KEY is required" >&2; exit 1; } sudo apt-get update sudo apt-get install -y minisign key_file="$RUNNER_TEMP/minisign.key" trap 'rm -f "$key_file"' EXIT printf '%s' "$MINISIGN_SECRET_KEY" > "$key_file" + printf '%s\n' "$MINISIGN_PUBLIC_KEY" > output/update-signing.pub chmod 600 "$key_file" - for checksum in output/aifw-update-*.tar.xz.sha256; do - [ -f "$checksum" ] || continue + for checksum in output/*.sha256; do minisign -S -s "$key_file" -m "$checksum" -x "${checksum}.minisig" done @@ -271,6 +257,24 @@ jobs: path: output/aifw-components-*.json retention-days: 5 + - name: Upload SBOMs and signatures + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: supply-chain + path: | + output/*.cdx.json + output/*.minisig + output/update-signing.pub + retention-days: 5 + + - name: Attest release artifacts + uses: actions/attest-build-provenance@e8998f949152b193b063cb0ec769d69d929409be # v2.4.0 + with: + subject-path: | + output/aifw-*.iso.xz + output/aifw-*.img.xz + output/aifw-update-*.tar.xz + smoke-boot: name: Appliance boot smoke (qemu) needs: build-iso @@ -311,23 +315,6 @@ jobs: name: boot-smoke-artifacts path: smoke-artifacts/ retention-days: 7 - - name: Upload SBOMs and signatures - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: supply-chain - path: | - output/*.cdx.json - output/*.minisig - retention-days: 5 - - - name: Attest release artifacts - uses: actions/attest-build-provenance@e8998f949152b193b063cb0ec769d69d929409be # v2.4.0 - with: - subject-path: | - output/aifw-*.iso.xz - output/aifw-*.img.xz - output/aifw-update-*.tar.xz - release: name: Create GitHub Release # smoke-boot gates the release: an image nobody booted never ships (#533). @@ -366,15 +353,18 @@ jobs: name: supply-chain path: release-assets/ - - name: Verify update signatures before publication + - name: Verify signatures before publication env: MINISIGN_PUBLIC_KEY: ${{ vars.MINISIGN_PUBLIC_KEY }} run: | set -euo pipefail + test -n "$MINISIGN_PUBLIC_KEY" || { echo "MINISIGN_PUBLIC_KEY is required" >&2; exit 1; } sudo apt-get update sudo apt-get install -y minisign - for checksum in release-assets/aifw-update-*.tar.xz.sha256; do - minisign -Vm "$checksum" -x "${checksum}.minisig" -P "$MINISIGN_PUBLIC_KEY" + public_key_file="$RUNNER_TEMP/minisign.pub" + printf '%s\n' "$MINISIGN_PUBLIC_KEY" > "$public_key_file" + for checksum in release-assets/*.sha256; do + minisign -Vm "$checksum" -x "${checksum}.minisig" -p "$public_key_file" done - name: Extract version from tag @@ -416,6 +406,15 @@ jobs: ### Verify Downloads ```bash - sha256sum -c aifw-${{ steps.version.outputs.version }}-amd64.iso.sha256 - sha256sum -c aifw-${{ steps.version.outputs.version }}-amd64.img.sha256 + minisign -Vm aifw-${{ steps.version.outputs.version }}-amd64.iso.xz.sha256 \ + -x aifw-${{ steps.version.outputs.version }}-amd64.iso.xz.sha256.minisig \ + -p update-signing.pub + sha256sum -c aifw-${{ steps.version.outputs.version }}-amd64.iso.xz.sha256 + ``` + + GitHub build provenance can also be verified with: + + ```bash + gh attestation verify aifw-${{ steps.version.outputs.version }}-amd64.iso.xz \ + --repo ${{ github.repository }} ``` diff --git a/aifw-core/src/updater.rs b/aifw-core/src/updater.rs index 75a2ed15..eba7dba6 100644 --- a/aifw-core/src/updater.rs +++ b/aifw-core/src/updater.rs @@ -223,6 +223,12 @@ pub enum UpdaterError { /// Downloaded tarball didn't match its published SHA-256 #[error("Checksum verification failed")] Checksum, + /// The release did not contain a detached signature for its checksum + #[error("Release checksum signature is missing")] + NoSignature, + /// The checksum's publisher signature could not be verified + #[error("Release signature verification failed")] + Signature, /// Extracting or installing the update failed #[error("Installation failed: {0}")] Install(String), @@ -845,7 +851,7 @@ pub async fn download_and_install(info: &AifwUpdateInfo) -> Result Result<(), if output.status.success() { Ok(()) } else { - Err(UpdaterError::Checksum) + Err(UpdaterError::Signature) } } diff --git a/aifw-ui/src/app/updates/components/FirmwareCard.tsx b/aifw-ui/src/app/updates/components/FirmwareCard.tsx index 6d3337e9..0a1ecf9c 100644 --- a/aifw-ui/src/app/updates/components/FirmwareCard.tsx +++ b/aifw-ui/src/app/updates/components/FirmwareCard.tsx @@ -55,6 +55,16 @@ export function FirmwareCard({ )} + {info?.checksum_signature_url && ( + + Verify signed checksum + + )}