[Security Review] Daily Security Review and Threat Modeling – 2026-03-21 #1392
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-28T13:40:27.445Z.
|
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
📊 Executive Summary
This review analyzed ~19,971 lines of security-critical code across the AWF firewall codebase (TypeScript sources, Bash entrypoints, iptables scripts, Squid configuration generation, and the seccomp profile). The overall security posture is strong: the architecture is defense-in-depth, input validation is thorough, and credential isolation is well-implemented. No critical vulnerabilities were found. Three medium findings and four lower-severity observations are noted below.
Key metrics:
src/*.ts,containers/agent/entrypoint.sh,containers/agent/setup-iptables.sh,containers/squid/entrypoint.sh,containers/agent/seccomp-profile.jsonnpm audit)🔍 Findings from Firewall Escape Test
No dedicated "firewall-escape-test" workflow was found in the current workflow list. The
security-reviewandsecret-digger-*workflows (Claude, Codex, Copilot variants scheduled hourly/daily) serve overlapping purposes. This review is therefore standalone.🛡️ Architecture Security Analysis
Network Security Assessment ✅ Strong
Evidence gathered:
cat containers/agent/setup-iptables.sh cat src/host-iptables.ts grep -n "dst_ipv4\|dst_ipv6" src/squid-config.tsThe iptables architecture is layered and sound:
Host-level (
src/host-iptables.ts): TheDOCKER-USERchain enforces egress filtering for all traffic leaving theawf-netbridge (172.30.0.0/24). A dedicatedFW_WRAPPERchain is created and cleaned up on every run. Only Squid IP (172.30.0.10), DNS servers, and localhost are permitted; everything else drops.Container-level (
containers/agent/setup-iptables.sh): NAT OUTPUT rules DNAT port 80/443 to Squid. Docker embedded DNS (127.0.0.11) rules are preserved across NAT flushes. Dangerous ports (SSH 22, databases, RDP 3389, etc.) are explicitly returned from NAT (not redirected) and then dropped by the filter chain's finalDROPrule.Squid-level (
src/squid-config.ts): Domain ACLs, IP-literal blocking (dst_ipv4,dst_ipv6via dstdom_regex), dangerous-port ACLs, and default-denyhttp_access deny allas final rule.IPv6: When
ip6tablesis unavailable,sysctl net.ipv6.conf.all.disable_ipv6=1is applied to prevent IPv6 bypass paths. When available, mirror rules are applied.One concern (Medium severity): See Finding M1 below.
Container Security Assessment ✅ Good with caveats
Evidence gathered:
Capability management:
awf-iptables-init):cap_add: [NET_ADMIN, NET_RAW],cap_drop: [ALL]— correct, narrow scope.cap_add: [SYS_CHROOT, SYS_ADMIN]during startup (needed for procfs mount + chroot), thencapsh --dropremoves them before user code runs.NET_RAWis explicitly dropped to prevent raw socket creation.cap_drop: [ALL]with only network capabilities retained.Seccomp profile (
containers/agent/seccomp-profile.json):SCMP_ACT_ERRNO(deny-by-default) ✅ptrace,process_vm_readv,process_vm_writevare blocked ✅pivot_rootis blocked ✅mount,unshare,setns,clone,clone3, andchrootare ALLOWED — see Finding M2.Domain Validation Assessment ✅ Strong
Evidence gathered:
*and*.*patterns are explicitly rejected with clear errors.*.*.com) are rejected.wildcardToRegex()uses character class[a-zA-Z0-9.-]*instead of.*to prevent ReDoS.(redacted)https://`) is well-handled.*.,.*), and empty inputs are all validated.No issues found in domain validation. The implementation reflects careful security thinking.
Input Validation Assessment ✅ Good
Evidence gathered:
execawith argument arrays (no shell interpolation).escapeShellArg()andjoinShellArgs()exist and are used for shell-context arguments.agentCommandis passed as['/bin/bash', '-c', ...]with$replaced with$$to prevent Docker Compose variable interpolation:config.agentCommand.replace(/\$/g, '$$$$').port.replace(/[^0-9-]/g, '')and validated as integers in 1–65535.AWF_USER_UID/AWF_USER_GIDare validated as numeric and non-zero beforeusermod/groupmod.One concern (Low severity): See Finding L1 below.
Credential Isolation Assessment ✅ Strong
Evidence gathered:
EXCLUDED_ENV_VARS (api-proxy disabled path)
✅ Recommendations
🔴 Critical
(None found)
🟠 High
(None found)
🟡 Medium
M1 — IPv6 DNAT gap in container NAT chain
In
setup-iptables.sh, whenIP6TABLES_AVAILABLE=true, IPv6 NAT rules only cover DNS exemptions. Port 80 and 443 DNAT rules are IPv4-only (iptables -t nat -A OUTPUT -p tcp --dport 80 -j DNAT ...). An agent using IPv6-capable connections could bypass Squid for HTTP/HTTPS traffic if the destination resolves to an AAAA record (and the container has an IPv6 route).Mitigation: Add
ip6tables -t nat -A OUTPUT -p tcp --dport 80 -j DNAT ...andip6tables -t nat -A OUTPUT -p tcp --dport 443 -j DNAT ...mirrors after the IPv4 DNAT rules insetup-iptables.sh. Also add matching ip6tables DNAT for user-specifiedAWF_ALLOW_HOST_PORTS.M2 —
unshareandsetnsallowed in seccomp profileThe seccomp profile (
containers/agent/seccomp-profile.json) allowsunshareandsetns. While these require capabilities (e.g.,CAP_SYS_ADMIN,CAP_NET_ADMIN) that are dropped before user code runs, their presence in the allow-list is an unnecessary attack surface. A future misconfiguration that leaks a capability could be exploited.Mitigation: Add
unshareandsetnsto theSCMP_ACT_ERRNOdeny group in the seccomp profile, alongsidepivot_root. The iptables-init sidecar pattern means the agent container itself no longer needs these.M3 — Docker-in-Docker enables full firewall bypass
When
--enable-dockeris passed, the host Docker socket is bind-mounted read-write into the agent container (/host/var/run/docker.sock). This allows the agent to start new containers on arbitrary networks (bypassingawf-net), pull images from anywhere, and execute code outside the firewall. The code logs a warning (src/docker-manager.ts:788) but does not restrict outbound access from those child containers.Mitigation: Document clearly that
--enable-dockeris an escape hatch that disables network isolation guarantees. Consider adding a separate--enable-docker-read-onlyoption that mounts the socket read-only. At minimum, add a prominent CLI warning printed to stderr.🔵 Low
L1 — GITHUB_TOKEN forwarded unconditionally
GITHUB_TOKENandGH_TOKENare forwarded to the agent container in all cases (lines 501-502 indocker-manager.ts), including when--enable-api-proxyis active. Unlike the API keys that are withheld in api-proxy mode, these GitHub tokens are always present. A compromised agent could use them to push code or create issues.Mitigation: Consider adding
GITHUB_TOKEN/GH_TOKENto the one-shot-token list (AWF_ONE_SHOT_TOKENS) so they are consumed on first use, or expose them only when the caller explicitly opts in.L2 — No CPU/memory resource limits on agent container
The generated docker-compose does not set
deploy.resources.limits. A runaway agent could exhaust host resources, causing a denial-of-service to the host.Mitigation: Add configurable defaults (e.g.,
--cpu-limit,--memory-limitflags) and apply them viadeploy.resources.limitsin the generated compose file.L3 —
unshare/setnsin seccomp (also listed above as M2)L4 — Squid logs require
sudoto readSquid container runs as user
proxy; log files written to/var/log/squid/are owned byproxy. On the host, reading preserved logs requiressudo. This makes automated forensic pipelines (e.g.,awf logs stats) require privilege escalation.Mitigation: Consider setting Squid log file permissions to
0644incontainers/squid/entrypoint.sh, or use a shared volume group that matches the host runner UID.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions