[Security Review] Daily Security Review and Threat Model — gh-aw-firewall #8317
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-09-15T12:38:42.841Z.
|
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
Daily automated review of
github/gh-aw-firewall(branch: main-equivalent checkout at run time). Overall security posture is strong: layered defense-in-depth (iptables NAT + filter chain, Squid L7 ACL, capability drop, seccomp, no-new-privileges, chroot, selective bind mounts), extensive test coverage for injection-prone code paths (domain validation, port-spec parsing), and IPv6 egress is explicitly disabled to prevent proxy bypass. No critical vulnerabilities were found in this pass. A few medium/low items are noted below for hardening.🔍 Findings from Firewall Escape Test
/tmp/gh-aw/escape-test-summary.txtcontains CI log output from a separate "Secret Digger (Copilot)" workflow run (github/gh-aw-firewall/actions/runs/29286879560), not a dedicated firewall-escape harness. Key facts extracted:noop, refusing to perform any secret discovery or exfiltration ("Refused prompt injection attack... No investigation was performed").GH_AW_DETECTION_CONCLUSION: warning,GH_AW_DETECTION_REASON: threat_detected) and opened tracking issue [aw] Detection Runs #6205, and a no-op record was posted to issue [aw] No-Op Runs #5883.🛡️ Architecture Security Analysis
Network Security (
containers/agent/setup-iptables.sh, 540 lines;src/host-iptables-rules.ts, 340 lines)172.30.0.10:3128) + FILTER (default-deny DROP for all other TCP/UDP), evidenced atconfigure_http_dnat()(lines ~410-431) andconfigure_filter_chain()(lines ~433-476).DANGEROUS_PORTSarray (SSH 22, SMTP 25, DB ports, Redis, MongoDB, RDP, etc.) is NAT-RETURNed then filter-DROPped — defense-in-depth even if Squid ACL fails.disable_ipv6()(~line 133) explicitly disables IPv6 in-container to prevent proxy-bypass via::1or IPv6 upstream paths — a good mitigation for a previously tracked issue (Squid proxy rejects IPv6 localhost connections from chroot (transaction-end-before-headers) #1543).-m limit --limit 5-10/min) on dangerous-port and default-deny drops provide audit trail without log-flooding DoS.allow_host_access_to_gateway) is scoped to ports 80/443 plus explicit--allow-host-ports, not a blanket bypass — reduces risk of the MCP/host-access convenience feature becoming an egress hole.parseValidPortSpecs()pre-validates before container start, andis_valid_port_spec()in bash re-validates as a fail-closed guard against a compromised/legacy env var reachingiptablesdirectly (comment at lines 24-32) — solid defense-in-depth against shell/iptables argument injection via crafted port specs.Container Security (
src/services/agent-service.ts,containers/agent/entrypoint.sh)SYS_CHROOT+SYS_ADMIN(needed for chroot + procfs mount), explicitly dropsNET_RAW,SYS_PTRACE,SYS_MODULE,SYS_RAWIO,MKNOD(agent-service.ts:80-83).NET_ADMINis never granted to the agent — only to the separateawf-iptables-initcontainer, which itself iscap_drop: ['ALL']+cap_add: ['NET_ADMIN','NET_RAW']only (test evidence:agent-service-build.test.ts:110-118).security_optincludesno-new-privileges:trueand a custom seccomp profile (414 lines,containers/agent/seccomp-profile.json) — verified viaagent-security-config.test.ts:47-55.entrypoint.shUID/GID remap (setup_user_identity()) explicitly rejects UID/GID 0 (lines ~44-52), preventing privilege-drop bypass via a craftedAWF_USER_UID=0./host/procwithhidepid=2, limiting cross-process/proc/[pid]/environcredential leakage while still supporting JVM/.NET runtimes that read/proc/self/exe.Domain Validation (
src/domain-validation.ts,src/domain-patterns.ts)SQUID_DANGEROUS_CHARS/DOMAIN_DANGEROUS_CHARSregexes reject whitespace, NUL, quotes, backtick,;,#, and backslash to prevent Squid-config-injection via crafted--allow-domainsvalues (domain-validation.ts:22-42).wildcardToRegex(),domain-patterns.ts:79-119) uses a bounded character class ([a-zA-Z0-9.-]*) instead of.*, explicitly to avoid ReDoS/catastrophic backtracking — a thoughtful mitigation.*,*.*, all-wildcard patterns) and structurally invalid domains (double dots, excessive wildcard segments) are explicitly rejected (checkOverBroadPattern,checkStructuralValidity).Input Validation / Injection Surface
exec/spawnstring-interpolation calls found insrc/cli.ts; process execution goes throughexeca, which by default avoids shell interpretation of arguments (array-based invocation), reducing shell-injection risk versuschild_process.exec.evalusage found in agent/squid container shell scripts.SQUID_PROXY_HOSTresolve_squid_ip()trusts env-provided host/IP for DNAT target--allow-domains/--allow-urlsSQUID_DANGEROUS_CHARSchecks indomain-validation.tsAWF_ALLOW_HOST_PORTSis_valid_port_spec)LOGGING.md)/proc/[pid]/environcross-process readhidepid=2procfs mount--enable-api-proxyactive-m limitrate limiting on LOG rulescapshbefore user code runs (per architecture doc) — should verify this drop cannot be skipped by a malicious command that races the dropdisable_ipv6()in setup-iptables.sh🎯 Attack Surface Map
containers/agent/setup-iptables.sh(iptables-init container, shares agent netns)NET_ADMINbeing scoped only to init container; agent must not regainNET_ADMINpost-startupsrc/services/agent-service.ts:72-92src/domain-validation.ts,src/domain-patterns.tsconfigure_host_access_rules()in setup-iptables.shAWF_ENABLE_HOST_ACCESSsrc/cli.ts,src/option-parsers.tsexeca(array-based, no shell string interpolation found)execa/spawn call sites not exhaustively completed in this pass — recommend follow-up dependency/CLI-arg fuzz pass📋 Evidence Collection
Escape test summary excerpt
Capability configuration (agent-service.ts)
Confirmed via
src/services/agent-service-build.test.ts:23-118andsrc/services/agent-security-config.test.ts:25-55.iptables default-deny (setup-iptables.sh)
Preceded by rate-limited LOG rules with prefixes
[FW_BLOCKED_TCP]/[FW_BLOCKED_UDP_AGENT].Domain injection char blacklist (domain-validation.ts:22-42)
Rejects whitespace, NUL, quotes, backtick, semicolon, hash, and backslash.
✅ Recommendations
capsh --drop=cap_sys_chroot,cap_sys_adminexecutes atomically before the user command starts (race-condition risk if a malicious command could execute in the window before capabilities are dropped).LOGGING.mdnotes.execa/spawncall-site audit acrosssrc/cli.tsandsrc/docker-manager.tsin a follow-up pass to formally rule out shell-interpolation risks (none found this pass, but not exhaustively enumerated).docs/reference acontainers/agent/docker-wrapper.shin some historical guidance; verify no dangling documentation references non-existent files (none found in current tree during this scan).📈 Security Metrics
All reactions