[Security Review] Daily Security Review and Threat Modeling — 2026-03-28 #1480
Closed
Replies: 2 comments
-
|
This discussion was automatically closed because it expired on 2026-04-04T13:46:23.115Z.
|
Beta Was this translation helpful? Give feedback.
0 replies
-
|
🔮 The ancient spirits stir, and the smoke-test oracle has walked this thread.
|
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 is an evidence-based security review of the
gh-aw-firewallrepository, covering ~6,986 lines of security-critical code across 6 core files. The overall security posture is strong with well-layered defenses: mandatory Squid proxy, iptables NAT redirection, selective bind mounts, capability dropping, seccomp filtering, one-shot token protection, and DNS exfiltration controls. Three items warrant attention.🔍 Findings from Firewall Escape Test
Previous run data was unavailable due to
gitnot being in PATH during the workflow-log download. The analysis below is based entirely on static codebase review.🛡️ Architecture Security Analysis
Network Security Assessment ✅ Strong
Evidence collected:
All TCP/UDP from the agent that isn't DNAT'd to Squid hits the REJECT rule. The host-side DOCKER-USER chain is the second line of defense (after iptables NAT inside the container namespace).
Container-level iptables (containers/agent/setup-iptables.sh):
Gap identified: ICMP is not explicitly covered by the DROP rules in
setup-iptables.sh(lines 404–407 only drop TCP and UDP). However,NET_RAWis dropped from the agent container (docker-manager.ts:1152), which prevents raw socket creation for ICMP tunneling. The host'sFW_WRAPPERchain terminates unrecognized traffic withicmp-port-unreachable, which effectively blocks ICMP egress at the network level as well.IPv6 handling: Both host (
src/host-iptables.ts) and container (setup-iptables.sh) detect ip6tables availability and fall back to disabling IPv6 via sysctl when ip6tables is unavailable — preventing an unfiltered IPv6 bypass path.DNS exfiltration prevention: Container
resolv.confis overwritten to contain only127.0.0.11(Docker embedded DNS). iptables only allows UDP/TCP port 53 to the explicitly configured upstream servers. This is well-designed.Container Security Assessment⚠️ Adequate with noted gaps
Capability model (src/docker-manager.ts:1149–1157):
SYS_ADMINandSYS_CHROOTare added for the entrypoint (procfs mount + chroot), then explicitly dropped bycapshbefore user code runs (containers/agent/entrypoint.sh:338):CAPS_TO_DROP="cap_sys_chroot,cap_sys_admin"AppArmor disabled (Medium finding):
src/docker-manager.ts:1165:AppArmor / seccomp settings per container
✅ Recommendations
🔴 High Priority
H1 — Harden AppArmor unconfined with fallback capability check
The agent uses
apparmor:unconfinedso the entrypoint can runmount -t proc. WhileSYS_ADMINis dropped bycapshbefore user code runs, ifcapshis unavailable on the host, the entrypoint logs an error and exits (entrypoint.sh:431) — but in a degraded failure mode. A defense-in-depth improvement would be to write and ship a custom AWF AppArmor profile that allows only the specificmountcall (tmpfs/proc) rather than disabling AppArmor entirely.Effort: Medium | Impact: High
🟡 Medium Priority
M1 — Document
--enable-dindrisk more prominently--enable-dindmounts the live Docker socket into the agent's chroot. A compromised agent with Docker socket access can launch new containers bypassing AWF network restrictions entirely. The CLI currently logs a warning (docker-manager.ts:924) but there is no prominent documentation warning in the help text.Recommendation: Add a visible
[SECURITY RISK]marker to the--enable-dindhelp string insrc/cli.ts, and consider requiring an additional acknowledgment flag (e.g.,--enable-dind --i-understand-dind-bypasses-firewall).M2 — Add additional sensitive env var exclusions to
EXCLUDED_ENV_VARSThe current
EXCLUDED_ENV_VARSset (docker-manager.ts:467) only excludes shell metadata (PATH,PWD, sudo vars). When--env-allis used, host vars likeAWS_SECRET_ACCESS_KEY,AZURE_CLIENT_SECRET,VAULT_TOKEN, or SSH keys stored as environment variables are forwarded to the agent container.Recommendation: Expand the base
EXCLUDED_ENV_VARSwith common cloud credential patterns (AWS_*_KEY,AZURE_*_SECRET,ARM_*_KEY,VAULT_TOKEN, etc.), independent of the api-proxy flag. These are never needed in the agent container.M3 — Increase LOG rate limits or use connection tracking for audit completeness
Current rate limits (
--limit 10/min --limit-burst 20) on DROP rules insetup-iptables.shmean a burst of >20 blocked packets generates only 10 log entries per minute. For security audit purposes, consider using--limit 60/min --limit-burst 60or usingconntrackstate-based logging (log only new connections) to capture the first blocked attempt per flow.🟢 Low Priority
L1 — Explicitly block ICMP in agent OUTPUT chain
While
NET_RAWis dropped (preventing raw socket ICMP construction), adding an explicitiptables -A OUTPUT -p icmp -j DROPrule tosetup-iptables.shwould eliminate any residual ICMP covert channel risk and make the intent explicit in the ruleset.L2 — Address transitive dependency vulnerabilities
Run
npm audit fixor pin the transitive dependency versions:brace-expansion: update to>=2.0.2which resolves the ReDoS issuehandlebarsis not a runtime dependency (not independenciesordevDependenciesdirectly) — identify which dev tool chain introduces it and pin or removeL3 — Squid log timestamp readability
Squid uses Unix timestamps in access logs (
src/squid-config.ts:40). Consider adding a human-readable timestamp field to thefirewall_detailedlogformat to ease forensic analysis without requiringdate -d@TIMESTAMP`` conversion.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions