Skip to content

fix: address ipv6 validation and filtering review comments #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mossaka merged 4 commits into from
Nov 27, 2025
Merged

fix: address ipv6 validation and filtering review comments #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
088c80e
Initial plan
Copilot Nov 26, 2025
b02a885
fix: address pr review comments for ipv6 validation and filtering
Copilot Nov 26, 2025
59cca07
fix: improve ipv6 chain comments for clarity
Copilot Nov 26, 2025
6e0de58
fix: add icmpv6 rules and ipv6 validation tests
Copilot Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 49 additions & 14 deletions containers/agent/setup-iptables.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ set -e
echo "[iptables] Setting up NAT redirection to Squid proxy..."
echo "[iptables] NOTE: Host-level DOCKER-USER chain handles egress filtering for all containers on this network"

# Function to check if an IP address is IPv6
is_ipv6() {
local ip="$1"
# Check if it contains a colon (IPv6 addresses always contain colons)
[[ "$ip" == *:* ]]
}
Comment on lines +8 to +12
Copy link
Collaborator

@Mossaka Mossaka Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this test overly simplified?

Copy link
Contributor Author

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the check is intentionally simple because the DNS servers passed to this script via AWF_DNS_SERVERS are already validated in src/cli.ts using Node.js's net.isIPv6() before the container starts. The shell script only needs to distinguish between pre-validated IPv4 and IPv6 addresses, so checking for a colon is sufficient - IPv6 addresses always contain colons, IPv4 never do.

If you'd prefer more robust validation in the shell script itself, I can add a more comprehensive check using regex or by attempting to parse with standard utilities.


# Get Squid proxy configuration from environment
SQUID_HOST="${SQUID_PROXY_HOST:-squid-proxy}"
SQUID_PORT="${SQUID_PROXY_PORT:-3128}"
Expand All @@ -18,55 +25,83 @@ if [ -z "$SQUID_IP" ]; then
fi
echo "[iptables] Squid IP resolved to: $SQUID_IP"

# Clear existing NAT rules
# Clear existing NAT rules (both IPv4 and IPv6)
iptables -t nat -F OUTPUT 2>/dev/null || true
ip6tables -t nat -F OUTPUT 2>/dev/null || true

# Allow localhost traffic (for stdio MCP servers)
echo "[iptables] Allow localhost traffic..."
iptables -t nat -A OUTPUT -o lo -j RETURN
iptables -t nat -A OUTPUT -d 127.0.0.0/8 -j RETURN
ip6tables -t nat -A OUTPUT -o lo -j RETURN
ip6tables -t nat -A OUTPUT -d ::1/128 -j RETURN

# Get DNS servers from environment (default to Google DNS)
DNS_SERVERS="${AWF_DNS_SERVERS:-8.8.8.8,8.8.4.4}"
echo "[iptables] Configuring DNS rules for trusted servers: $DNS_SERVERS"

# Allow DNS queries ONLY to trusted DNS servers (prevents DNS exfiltration)
# Separate IPv4 and IPv6 DNS servers
IPV4_DNS_SERVERS=()
IPV6_DNS_SERVERS=()
IFS=',' read -ra DNS_ARRAY <<< "$DNS_SERVERS"
for dns_server in "${DNS_ARRAY[@]}"; do
dns_server=$(echo "$dns_server" | tr -d ' ')
if [ -n "$dns_server" ]; then
echo "[iptables] Allow DNS to trusted server: $dns_server"
iptables -t nat -A OUTPUT -p udp -d "$dns_server" --dport 53 -j RETURN
iptables -t nat -A OUTPUT -p tcp -d "$dns_server" --dport 53 -j RETURN
if is_ipv6 "$dns_server"; then
IPV6_DNS_SERVERS+=("$dns_server")
else
IPV4_DNS_SERVERS+=("$dns_server")
fi
fi
done

echo "[iptables] IPv4 DNS servers: ${IPV4_DNS_SERVERS[*]:-none}"
echo "[iptables] IPv6 DNS servers: ${IPV6_DNS_SERVERS[*]:-none}"

# Allow DNS queries ONLY to trusted IPv4 DNS servers (prevents DNS exfiltration)
for dns_server in "${IPV4_DNS_SERVERS[@]}"; do
echo "[iptables] Allow DNS to trusted IPv4 server: $dns_server"
iptables -t nat -A OUTPUT -p udp -d "$dns_server" --dport 53 -j RETURN
iptables -t nat -A OUTPUT -p tcp -d "$dns_server" --dport 53 -j RETURN
done

# Allow DNS queries ONLY to trusted IPv6 DNS servers
for dns_server in "${IPV6_DNS_SERVERS[@]}"; do
echo "[iptables] Allow DNS to trusted IPv6 server: $dns_server"
ip6tables -t nat -A OUTPUT -p udp -d "$dns_server" --dport 53 -j RETURN
ip6tables -t nat -A OUTPUT -p tcp -d "$dns_server" --dport 53 -j RETURN
done

# Allow DNS to Docker's embedded DNS server (127.0.0.11) for container name resolution
echo "[iptables] Allow DNS to Docker embedded DNS (127.0.0.11)..."
iptables -t nat -A OUTPUT -p udp -d 127.0.0.11 --dport 53 -j RETURN
iptables -t nat -A OUTPUT -p tcp -d 127.0.0.11 --dport 53 -j RETURN

# Allow return traffic to trusted DNS servers
# Allow return traffic to trusted IPv4 DNS servers
echo "[iptables] Allow traffic to trusted DNS servers..."
for dns_server in "${DNS_ARRAY[@]}"; do
dns_server=$(echo "$dns_server" | tr -d ' ')
if [ -n "$dns_server" ]; then
iptables -t nat -A OUTPUT -d "$dns_server" -j RETURN
fi
for dns_server in "${IPV4_DNS_SERVERS[@]}"; do
iptables -t nat -A OUTPUT -d "$dns_server" -j RETURN
done

# Allow return traffic to trusted IPv6 DNS servers
for dns_server in "${IPV6_DNS_SERVERS[@]}"; do
ip6tables -t nat -A OUTPUT -d "$dns_server" -j RETURN
done

# Allow traffic to Squid proxy itself
echo "[iptables] Allow traffic to Squid proxy (${SQUID_IP}:${SQUID_PORT})..."
iptables -t nat -A OUTPUT -d "$SQUID_IP" -j RETURN

# Redirect HTTP traffic to Squid
# Redirect HTTP traffic to Squid (IPv4 only - Squid runs on IPv4)
echo "[iptables] Redirect HTTP (port 80) to Squid..."
iptables -t nat -A OUTPUT -p tcp --dport 80 -j DNAT --to-destination "${SQUID_IP}:${SQUID_PORT}"

# Redirect HTTPS traffic to Squid
# Redirect HTTPS traffic to Squid (IPv4 only - Squid runs on IPv4)
echo "[iptables] Redirect HTTPS (port 443) to Squid..."
iptables -t nat -A OUTPUT -p tcp --dport 443 -j DNAT --to-destination "${SQUID_IP}:${SQUID_PORT}"

echo "[iptables] NAT rules applied successfully"
echo "[iptables] Current NAT OUTPUT rules:"
echo "[iptables] Current IPv4 NAT OUTPUT rules:"
iptables -t nat -L OUTPUT -n -v
echo "[iptables] Current IPv6 NAT OUTPUT rules:"
ip6tables -t nat -L OUTPUT -n -v 2>/dev/null || echo "[iptables] (ip6tables NAT not available)"
13 changes: 13 additions & 0 deletions src/cli.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,12 +703,25 @@ describe('cli', () => {
expect(isValidIPv6('2001:0db8:85a3:0000:0000:8a2e:0370:7334')).toBe(true);
});

it('should accept IPv4-mapped IPv6 addresses', () => {
expect(isValidIPv6('::ffff:192.0.2.1')).toBe(true);
expect(isValidIPv6('::ffff:8.8.8.8')).toBe(true);
expect(isValidIPv6('::ffff:127.0.0.1')).toBe(true);
});

it('should reject invalid IPv6 addresses', () => {
expect(isValidIPv6('8.8.8.8')).toBe(false);
expect(isValidIPv6('localhost')).toBe(false);
expect(isValidIPv6('')).toBe(false);
expect(isValidIPv6('2001:4860:4860:8888')).toBe(false); // Missing ::
});

it('should reject malformed input', () => {
expect(isValidIPv6('not-an-ip')).toBe(false);
expect(isValidIPv6('192.168.1.1')).toBe(false);
expect(isValidIPv6(':::1')).toBe(false);
expect(isValidIPv6('2001:db8::g')).toBe(false); // Invalid hex character
});
});

describe('DNS servers parsing', () => {
Expand Down
12 changes: 3 additions & 9 deletions src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Command } from 'commander';
import * as path from 'path';
import * as os from 'os';
import * as fs from 'fs';
import { isIPv6 } from 'net';
import { WrapperConfig, LogLevel } from './types';
import { logger } from './logger';
import {
Expand Down Expand Up @@ -89,19 +90,12 @@ export function isValidIPv4(ip: string): boolean {
}

/**
* Validates that a string is a valid IPv6 address
* Validates that a string is a valid IPv6 address using Node.js built-in net module
* @param ip - String to validate
* @returns true if the string is a valid IPv6 address
*/
export function isValidIPv6(ip: string): boolean {
// Comprehensive IPv6 validation covering:
// - Full form: 2001:0db8:85a3:0000:0000:8a2e:0370:7334
// - Compressed form: 2001:db8:85a3::8a2e:370:7334
// - Loopback: ::1
// - Unspecified: ::
// - IPv4-mapped: ::ffff:192.0.2.1
const ipv6Regex = /^(?:(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|(?:[0-9a-fA-F]{1,4}:){1,7}:|(?:[0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|(?:[0-9a-fA-F]{1,4}:){1,5}(?::[0-9a-fA-F]{1,4}){1,2}|(?:[0-9a-fA-F]{1,4}:){1,4}(?::[0-9a-fA-F]{1,4}){1,3}|(?:[0-9a-fA-F]{1,4}:){1,3}(?::[0-9a-fA-F]{1,4}){1,4}|(?:[0-9a-fA-F]{1,4}:){1,2}(?::[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:(?::[0-9a-fA-F]{1,4}){1,6}|:(?::[0-9a-fA-F]{1,4}){1,7}|::)$/;
return ipv6Regex.test(ip);
return isIPv6(ip);
}
Comment on lines 97 to 99
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated isValidIPv6() function lacks test coverage. Given that this is a critical validation function used to separate IPv4 and IPv6 DNS servers, it should have comprehensive tests to verify:

  1. Valid IPv6 addresses (standard form, compressed, with leading zeros, etc.)
  2. IPv4-mapped IPv6 addresses (::ffff:192.0.2.1)
  3. Invalid inputs (malformed addresses, IPv4 addresses, empty strings, etc.)
  4. Edge cases (::1, ::, etc.)

Consider adding unit tests in a new file tests/unit/cli.test.ts or expanding existing tests to cover this function:

describe('isValidIPv6', () => {
  it('should return true for valid IPv6 addresses', () => {
    expect(isValidIPv6('2001:db8::1')).toBe(true);
    expect(isValidIPv6('::1')).toBe(true);
    expect(isValidIPv6('fe80::1')).toBe(true);
  });

  it('should return false for IPv4 addresses', () => {
    expect(isValidIPv6('8.8.8.8')).toBe(false);
    expect(isValidIPv6('192.168.1.1')).toBe(false);
  });

  it('should return false for invalid input', () => {
    expect(isValidIPv6('not-an-ip')).toBe(false);
    expect(isValidIPv6('')).toBe(false);
  });
});

Copilot uses AI. Check for mistakes.

/**
Expand Down
60 changes: 57 additions & 3 deletions src/host-iptables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,41 @@ export async function setupHostIptables(squidIp: string, squidPort: number, dnsS
// Set up IPv6 chain if we have IPv6 DNS servers
await setupIpv6Chain(bridgeName);

// IPv6 chain needs to mirror IPv4 chain's comprehensive filtering
// This prevents IPv6 from becoming an unfiltered bypass path

// Note: Squid proxy rule is omitted for IPv6 since Squid runs on IPv4 only

// 1. Allow established and related connections (return traffic)
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-m', 'conntrack', '--ctstate', 'ESTABLISHED,RELATED',
'-j', 'ACCEPT',
]);

// 2. Allow localhost/loopback traffic
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-o', 'lo',
'-j', 'ACCEPT',
]);

await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-d', '::1/128',
'-j', 'ACCEPT',
]);

Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IPv6 filtering rules don't explicitly allow ICMPv6 traffic. While the ESTABLISHED,RELATED rule may cover some ICMPv6 (like error messages related to existing connections), certain essential ICMPv6 traffic may be blocked:

  • Neighbor Discovery Protocol (NDP): Required for IPv6 address resolution (replaces ARP in IPv4)
  • Path MTU Discovery: ICMPv6 "Packet Too Big" messages (type 2) are critical for avoiding fragmentation
  • Router Discovery: May be needed in some network configurations

Consider adding an ICMPv6 allow rule after the localhost rules to ensure IPv6 networking functions properly:

// Allow essential ICMPv6 (required for IPv6 functionality)
await execa('ip6tables', [
  '-t', 'filter', '-A', CHAIN_NAME_V6,
  '-p', 'ipv6-icmp',
  '-j', 'ACCEPT',
]);

If you want to be more restrictive, allow only specific ICMPv6 types (1: destination unreachable, 2: packet too big, 3: time exceeded, 128-129: echo request/reply, 133-137: NDP).

Suggested change
// Allow essential ICMPv6 (required for IPv6 functionality)
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'ipv6-icmp',
'-j', 'ACCEPT',
]);

Copilot uses AI. Check for mistakes.
// 3. Allow essential ICMPv6 (required for IPv6 functionality)
// This includes: destination unreachable, packet too big, time exceeded,
// echo request/reply, and Neighbor Discovery Protocol (NDP)
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'ipv6-icmp',
'-j', 'ACCEPT',
]);

// 4. Allow DNS ONLY to specified trusted IPv6 DNS servers
for (const dnsServer of ipv6DnsServers) {
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
Expand All @@ -284,7 +319,20 @@ export async function setupHostIptables(squidIp: string, squidPort: number, dnsS
]);
}

// Block all other IPv6 UDP traffic in the IPv6 chain
// 5. Block IPv6 multicast and link-local traffic
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-d', 'ff00::/8', // IPv6 multicast range
'-j', 'REJECT', '--reject-with', 'icmp6-port-unreachable',
]);

await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-d', 'fe80::/10', // IPv6 link-local range
'-j', 'REJECT', '--reject-with', 'icmp6-port-unreachable',
]);

// 6. Block all other IPv6 UDP traffic (DNS to whitelisted servers already allowed above)
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-p', 'udp',
Expand All @@ -297,10 +345,16 @@ export async function setupHostIptables(squidIp: string, squidPort: number, dnsS
'-j', 'REJECT', '--reject-with', 'icmp6-port-unreachable',
]);

// Default allow for other IPv6 traffic (return to main chain)
// 7. Default deny all other IPv6 traffic (including TCP)
// This prevents IPv6 from being an unfiltered bypass path
await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-j', 'RETURN',
'-j', 'LOG', '--log-prefix', '[FW_BLOCKED_OTHER6] ', '--log-level', '4',
]);

await execa('ip6tables', [
'-t', 'filter', '-A', CHAIN_NAME_V6,
'-j', 'REJECT', '--reject-with', 'icmp6-port-unreachable',
]);
}

Expand Down