[Security Review] Daily Security Review and Threat Modeling — 2026-03-22 #1395
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-29T13:42:50.809Z.
|
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
The gh-aw-firewall codebase demonstrates strong layered security with defense-in-depth across all major threat surfaces. The architecture correctly implements: privilege separation via separate iptables-init container, capability dropping before user code execution, domain-level L7 egress control via Squid, IPv6 bypass prevention, DNS exfiltration mitigations, and a strict default-deny seccomp profile.
No critical vulnerabilities were found. Three medium-severity observations and several low/informational items are documented below with evidence.
🛡️ Architecture Security Analysis
Network Security Assessment
Evidence gathered:
Firewall rule ordering (✅ Correct): NAT RETURN rules for dangerous ports are placed before the DNAT redirect rules for ports 80/443. This ensures dangerous ports (22, 25, 3306, 5432, etc.) are blocked even if they somehow map to web ports.
Default deny policy (✅): Both TCP and UDP are dropped at the OUTPUT filter chain:
# setup-iptables.sh lines 319-320: iptables -A OUTPUT -p tcp -j DROP iptables -A OUTPUT -p udp -j DROPHost-level DOCKER-USER chain (✅):
src/host-iptables.tscreates a dedicatedFW_WRAPPERchain with final REJECT-all. UDP and "other" traffic are both logged and rejected.IPv6 handling (✅): Both
src/host-iptables.ts(host level) andcontainers/agent/setup-iptables.sh(container level) check forip6tablesavailability. If unavailable, IPv6 is disabled viasysctl -w net.ipv6.conf.all.disable_ipv6=1as a fallback to prevent unfiltered bypass.The container's OUTPUT filter chain only drops TCP and UDP (
setup-iptables.shlines 319–320). ICMP traffic from within the agent container is not explicitly blocked. ICMP is caught by the host-levelFW_WRAPPERchain's final REJECT rule, but this is a single layer of protection (the host-level DOCKER-USER chain). If the host-level chain is bypassed or not installed, agents could use ICMP for covert out-of-band communication (e.g., ICMP tunneling/exfiltration).Recommendation: Add an explicit DROP rule for ICMP at the container level:
mountandunsharein seccomp ALLOW list:The seccomp profile explicitly allows
mount,unshare, andsetnssyscalls. While these are mitigated by capability dropping (SYS_ADMIN required for most mount operations), their presence in the ALLOW list widens the attack surface if capabilities are inadvertently granted or if a capability-bypassing kernel vulnerability is exploited. In particular:unsharewithCLONE_NEWUSER(user namespaces) may work without privileges on many kernels, potentially enabling privilege escalation within the container.setnsallows joining existing namespaces (e.g., PID namespace of another container's process).Recommendation: Add these to the
SCMP_ACT_ERRNOblock unless specifically needed:{"names": ["unshare", "setns", "mount", "umount"], "action": "SCMP_ACT_ERRNO"} ``` Note: `umount`/`umount2` are already blocked separately. **Seccomp default action** (✅ Excellent): `defaultAction: SCMP_ACT_ERRNO` — all unrecognized syscalls are denied by default. Only the large explicit ALLOW list is permitted. **Blocked critical syscalls** (✅): ``` # SCMP_ACT_ERRNO applied to: # ptrace, process_vm_readv, process_vm_writev (anti-inspection) # kexec_load, reboot, init_module, delete_module, pivot_root (kernel/system protection) # acct, swapon, swapoff, syslog, add_key, request_key, keyctl (system integrity)Privileged mode (✅): No containers use
privileged: true.UID/GID validation (✅): System UIDs/GIDs (0–999) are rejected in
docker-manager.ts:Domain Validation Assessment
ReDoS protection (✅): Wildcard patterns use
[a-zA-Z0-9.-]*character class instead of.*:// domain-patterns.ts: DOMAIN_CHAR_PATTERN = '[a-zA-Z0-9.-]*'Overly broad pattern rejection (✅): Patterns
*,*.*, and patterns with only wildcards/dots are rejected with clear error messages.Multi-wildcard segment check (✅): Patterns like
*.*.comare blocked (>1 pure wildcard segment when wildcards ≥ total_segments - 1).Input length limit (✅):
isDomainMatchedByPattern()limits domain length to 512 characters before regex matching.Protocol prefix handling (✅):
(redacted) andhttps://` prefixes are stripped correctly; protocol-specific ACLs are generated in Squid config.Input Validation Assessment
Port injection prevention (✅):
--allow-host-portsvalues go through numeric parsing with range validation and a dangerous-ports blocklist check. A finalreplace(/[^0-9-]/g, '')sanitization is applied before use in Squid config (squid-config.tslines ~480-510).Shell injection via command arguments (✅): The CLI uses
execawith array arguments (not shell interpolation) for all Docker commands. User-provided command strings are passed as arguments to the container shell, not interpolated into host shell commands.The working directory is
awf-(timestamp)based onDate.now()(millisecond precision). In a shared-user environment, a local attacker who can observe process creation timing could pre-create/tmp/awf-(N)as a symlink beforemkdirSyncruns, potentially redirecting config file writes (Squid config, Docker Compose config) to an attacker-controlled location.Recommendation: Use
fs.mkdtempSync(path.join(os.tmpdir(), 'awf-'))for cryptographically random suffix:✅ Recommendations
Medium Priority — Address in Near Term
M1 — Add explicit ICMP DROP to container OUTPUT chain (
containers/agent/setup-iptables.sh)containers/agent/setup-iptables.sh— after the UDP DROP rule (line ~320)iptables -A OUTPUT -p icmp -j DROPM2 — Block
unshare/setns/mountin seccomp profile (containers/agent/seccomp-profile.json)containers/agent/seccomp-profile.jsonmount,unshare,setnsfrom ALLOW list to a new ERRNO groupunshare CLONE_NEWUSER) can be created without privileges; reduces attack surface even with capability droppingM3 — Use
mkdtempSyncfor workDir (src/cli.ts)src/cli.tsline 1320fs.mkdtempSync(path.join(os.tmpdir(), 'awf-'))Low Priority — Consider for Future
L1 — Document GITHUB_TOKEN forwarding in security model
docker-manager.tslines 503–504 forwardGITHUB_TOKEN/GH_TOKENby default. This is necessary for agents to function but should be explicitly documented as a known credential delegation in the security model. Consider a--no-github-tokenflag for untrusted agent scenarios.L2 — Add Squid connection limits per domain
src/squid-config.tsdoesn't setmax_connectionsper ACL. A misbehaving agent could exhaust the connection pool or create a DoS condition.L3 — Consider
--env-allsecurity warning--env-allis used, since it bypasses credential isolation for non-excluded environment variables.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions