[Security Review] Daily Security Review and Threat Modeling – 2026-03-20 #1382
Closed
Replies: 1 comment
-
|
This discussion was automatically closed because it expired on 2026-03-27T13:51:44.836Z.
|
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
Deep evidence-based security review of the gh-aw-firewall codebase (2,513 lines of security-critical code analyzed across 5 core files). The overall security posture is strong. No critical vulnerabilities were found, and no npm dependency vulnerabilities exist. Three low/medium findings were identified, all of which are defense-in-depth gaps rather than direct exploitable issues.
🔍 Findings from Firewall Escape Test
Workflow logs for the
security-reviewworkflow could not be fetched in this run (GitHub CLI git discovery issue in sandbox). Analysis is based entirely on static source code review below.🛡️ Architecture Security Analysis
Network Security Assessment ✅ (with caveats)
The firewall implements a layered defense using two independent enforcement points:
Layer 1 – Host-level (FORWARD chain,
src/host-iptables.ts)FW_WRAPPERchain is inserted at position 1 ofDOCKER-USER224.0.0.0/4) and link-local (169.254.0.0/16) rangesicmp-port-unreachable)Layer 2 – Container-level (OUTPUT chain,
containers/agent/setup-iptables.sh)iptables -A OUTPUT -p tcp -j DROPandiptables -A OUTPUT -p udp -j DROPL7 – Squid Proxy (
src/squid-config.ts)dstdomain) and wildcard regex (dstdom_regex)acl dst_ipv4 dstdom_regex ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$pinger_enable offforwarded_for delete+via offcache deny all(prevents cross-request side-channel via cache)http_access deny !Safe_portsContainer Security Assessment ✅
NET_ADMINis never granted to the agent container (only to the ephemeral init containerawf-iptables-init)if [ "$HOST_UID" -eq 0 ]; then … exit 1ptrace,process_vm_readv,process_vm_writev,kexec_load,reboot, kernel module operations,pivot_root, key management syscalls (add_key,request_key,keyctl)LD_PRELOAD) prevents multiple reads of sensitive env varsunset_sensitive_tokensallows agent to cache tokens via the libraryDomain Validation Assessment ✅
*and*.*explicitly rejected*rejected*.and.*rejected(redacted)https://` prefixes)Input Validation Assessment ✅
--allow-host-ports(enforced in both Squid config and iptables)setup-iptables.shhas 15 ports;squid-config.tshas 21iptables -A OUTPUT -p icmp -j DROPinsetup-iptables.shsrc/host-iptables.ts:371uses\$\{minPort}:\$\{maxPort}=10000:10004containers/agent/entrypoint.sh:641-689containers/agent/entrypoint.sh:296src/squid-config.ts:147-182– documented but high-impact if misused🎯 Attack Surface Map
/etc/shadowexcluded, home dir allow-list, chroot root is/hostsysctl disable_ipv6=1📋 Evidence Collection
Finding T1: Dangerous Port List Mismatch
Impact: Minimal. The container's final
iptables -A OUTPUT -p tcp -j DROPcatches all TCP regardless of the NAT RETURN list. However, this inconsistency could mislead operators and represents incomplete defense-in-depth documentation.Finding T2: ICMP Not Explicitly Blocked in Container OUTPUT
Impact: Very low. External ICMP exfiltration (ICMP tunneling to internet) is blocked at the host level. Intra-network ICMP (agent→squid pings) is inconsequential.
Finding T3: API Proxy Port Range Gap (10003)
Impact: Very low. No service listens on 10003 at the API proxy IP, so connections would be refused by the container. An adversary would need to compromise the API proxy container and bind a service to 10003.
Finding T4: Token Unset Timing Assumption
Design: The one-shot-token library intercepts
getenv()via LD_PRELOAD and caches the value. Subsequent calls return the cached value even after the shell unsets the variable. The 5-second window is intentional to allow the library to be loaded and the first read to occur. Tokens are never directly visible in/proc/self/environafter the parent unsets them.Finding T5: git safe.directory Disabled
Context: This is required because the workspace bind mount may be owned by a different UID than
awfuser(before UID/GID remapping takes effect). The risk is limited to the container environment where the operator controls what is mounted.Positive Finding: Zero npm Vulnerabilities
Positive Finding: Squid Security Hardening
✅ Recommendations
🔴 Critical
None identified.
🟠 High
None identified.
🟡 Medium
M1 – Synchronize Dangerous Port Lists
containers/agent/setup-iptables.shDANGEROUS_PORTS is missing 6 ports present insrc/squid-config.ts:5984(CouchDB),6984(CouchDB SSL),8086(InfluxDB HTTP),8088(InfluxDB RPC),9200(Elasticsearch HTTP),9300(Elasticsearch transport).While the final
iptables -A OUTPUT -p tcp -j DROPcatches these, the intent of the NAT RETURN list is explicit defense-in-depth documentation. Add the 6 missing ports toDANGEROUS_PORTSinsetup-iptables.sh.Files:
containers/agent/setup-iptables.shlines 222-249🔵 Low
L1 – Explicitly Block ICMP in Container OUTPUT Chain
Add
iptables -A OUTPUT -p icmp -j DROPtosetup-iptables.shafter the UDP DROP rule. While ICMP is blocked at the host level via FW_WRAPPER, adding an explicit container-level rule closes the defense-in-depth gap and prevents any intra-network ICMP leakage.Files:
containers/agent/setup-iptables.shafter line 320L2 – Use Explicit Port List Instead of min-max Range for API Proxy
Replace the
minPort:maxPortrange insrc/host-iptables.tswith explicit per-port rules matching exactlyAPI_PROXY_PORTSvalues (10000, 10001, 10002, 10004), eliminating the unintended port 10003 allowance.Files:
src/host-iptables.tslines 365-374🟢 Informational
I1 – Document 5-second Token Caching Window
The
sleep 5beforeunset_sensitive_tokensis correct but worth explicitly documenting the security model: tokens are readable by the agent during the first 5 seconds via direct env read, then only via the one-shot-token library cache. Consider making the delay configurable via an env var for high-security environments.I2 – Consider Restricting git safe.directory
The global
safe.directory='*'is necessary for workspace access but broad. Consider restricting to specific mounted paths (e.g., the workspace directory) rather than all paths.I3 – SSL Bump Operational Guidance
When
--enable-ssl-bumpis used, the Squid CA key should be treated as a highly sensitive secret. Consider adding a prominent warning in the CLI output when SSL bump is active, noting that HTTPS credentials will be visible to the Squid proxy.📈 Security Metrics
Beta Was this translation helpful? Give feedback.
All reactions