fix: address ipv6 validation and filtering review comments

[Copilot]

Addresses review feedback on PR #68 regarding IPv6 handling vulnerabilities and validation issues.

Changes

• IPv6 validation (src/cli.ts): Replace custom regex with Node.js net.isIPv6() for reliable validation of all IPv6 formats including IPv4-mapped addresses

• Container iptables IPv6 (containers/agent/setup-iptables.sh): Add ip6tables rules for IPv6 DNS servers

  • Separate IPv4/IPv6 DNS server handling
  • Clear both IPv4 and IPv6 NAT rules on startup
  • Allow IPv6 loopback traffic

• IPv6 chain filtering (src/host-iptables.ts): Fix FW_WRAPPER_V6 chain security bypass where IPv6 only filtered UDP DNS then returned. Now mirrors IPv4 chain:

  • Allow established/related connections
  • Allow localhost traffic
  • Allow essential ICMPv6 (required for NDP, Path MTU Discovery, etc.)
  • Allow DNS to trusted IPv6 servers only
  • Block multicast (ff00::/8) and link-local (fe80::/10)
  • Block all other UDP with logging
  • Default deny all other traffic (including TCP)

• IPv6 validation tests (src/cli.test.ts): Added comprehensive test coverage for isValidIPv6():

  • IPv4-mapped IPv6 addresses (::ffff:192.0.2.1)
  • Malformed input edge cases

// Before: complex regex with edge case failures
const ipv6Regex = /^(?:(?:[0-9a-fA-F]{1,4}:){7}...$/;
return ipv6Regex.test(ip);

// After: reliable built-in validation
import { isIPv6 } from 'net';
return isIPv6(ip);

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Mossaka]

@copilot

[Mossaka]

isn't this test overly simplified?

[Copilot]

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.

[Copilot]

Pull request overview

This PR addresses IPv6 validation and filtering vulnerabilities identified in PR #68 review feedback. The changes improve IPv6 support across the firewall implementation but introduce a critical security vulnerability.

Key Changes

• Replaced custom IPv6 regex validation with Node.js built-in net.isIPv6() for more reliable validation
• Added IPv6 support to container-level iptables NAT rules for DNS traffic
• Implemented comprehensive IPv6 filtering rules in host-level iptables that mirror IPv4 chain security

Reviewed changes

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

File	Description
src/cli.ts	Replaced custom IPv6 regex with net.isIPv6() for robust validation of all IPv6 formats including IPv4-mapped addresses
containers/agent/setup-iptables.sh	Added IPv6 DNS server support with is_ipv6() helper, ip6tables NAT rules for IPv6 DNS servers, and IPv6 loopback traffic handling
src/host-iptables.ts	Added comprehensive IPv6 chain filtering (established/related, localhost, DNS, multicast/link-local blocking, default deny) but only when IPv6 DNS servers are configured - this creates a critical security bypass

---

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

[Copilot]

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]

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);
  });
});

[Mossaka]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Applied in 6e0de58:

• Added ICMPv6 allow rule for essential IPv6 functionality (NDP, Path MTU Discovery, etc.)
• Added comprehensive test coverage for isValidIPv6() including IPv4-mapped addresses and malformed input edge cases