Fix VPN traffic routing issue with iptables NAT rules (#14825)

* Fix VPN traffic routing issue with iptables NAT rules

The MASQUERADE rules had policy matching (-m policy --pol none --dir out)
which was preventing both WireGuard AND IPsec traffic from being NAT'd
properly. This policy match was incorrect and broke internet routing for
all VPN clients.

The confusion arose because:
- IPsec FORWARD rules check for --pol ipsec (encrypted traffic)
- But POSTROUTING happens AFTER decryption, so packets no longer have policy
- The --pol none match was blocking these decrypted packets from NAT

Changes:
- Removed policy matching from both IPsec and WireGuard NAT rules
- Both VPN types now use simple source-based NAT rules
- Applied to both IPv4 and IPv6 rule templates

This fixes the issue where VPN clients (both WireGuard and IPsec) could
connect but not route traffic to the internet.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

* Remove unnecessary policy matching from iptables rules

The policy matching (-m policy --pol none) was causing routing issues for
both WireGuard and IPsec VPN traffic. This was based on a misunderstanding
of how iptables processes VPN traffic:

1. FORWARD chain: IPsec needs --pol ipsec to identify encrypted traffic,
   but WireGuard doesn't need any policy match (it's not IPsec)

2. POSTROUTING NAT: Both VPN types see decrypted packets here, so policy
   matching is unnecessary and was blocking NAT

Changes:
- Removed policy matching from all NAT rules (both VPN types)
- Removed policy matching from WireGuard FORWARD rules
- Kept policy matching only for IPsec FORWARD (where it's needed)
- Added comprehensive unit tests to prevent regression

This fully fixes VPN routing for both WireGuard and IPsec clients.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

* Fix Python linting issues in iptables test file

Fixed all ruff linting issues:
- Removed unused yaml import
- Fixed import sorting (pathlib before third-party imports)
- Removed trailing whitespace from blank lines
- Added newline at end of file

All tests still pass after formatting fixes.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

---------

Co-authored-by: Claude <[email protected]>

Author: dguido

roles/common/templates/rules.v4.j2

@@ -36,7 +36,14 @@ COMMIT
 -A PREROUTING --in-interface {{ ansible_default_ipv4['interface'] }} -p udp --dport {{ wireguard_port_avoid }} -j REDIRECT --to-port {{ wireguard_port_actual }}
 {% endif %}
 # Allow traffic from the VPN network to the outside world, and replies
--A POSTROUTING -s {{ subnets | join(',') }} -m policy --pol none --dir out {{ '-j SNAT --to ' + snat_aipv4 if snat_aipv4 else '-j MASQUERADE' }}
+{% if ipsec_enabled %}
+# For IPsec traffic - NAT the decrypted packets from the VPN subnet
+-A POSTROUTING -s {{ strongswan_network }} {{ '-j SNAT --to ' + snat_aipv4 if snat_aipv4 else '-j MASQUERADE' }}
+{% endif %}
+{% if wireguard_enabled %}
+# For WireGuard traffic - NAT packets from the VPN subnet
+-A POSTROUTING -s {{ wireguard_network_ipv4 }} {{ '-j SNAT --to ' + snat_aipv4 if snat_aipv4 else '-j MASQUERADE' }}
+{% endif %}
 
 
 COMMIT
@@ -106,7 +113,7 @@ COMMIT
 
 {% if wireguard_enabled %}
 # Forward any traffic from the WireGuard VPN network
--A FORWARD -m conntrack --ctstate NEW -s {{ wireguard_network_ipv4 }} -m policy --pol none --dir in -j ACCEPT
+-A FORWARD -m conntrack --ctstate NEW -s {{ wireguard_network_ipv4 }} -j ACCEPT
 {% endif %}
 
 COMMIT

roles/common/templates/rules.v6.j2

@@ -35,7 +35,14 @@ COMMIT
 -A PREROUTING --in-interface {{ ansible_default_ipv6['interface'] }} -p udp --dport {{ wireguard_port_avoid }} -j REDIRECT --to-port {{ wireguard_port_actual }}
 {% endif %}
 # Allow traffic from the VPN network to the outside world, and replies
--A POSTROUTING -s {{ subnets | join(',') }} -m policy --pol none --dir out {{ '-j SNAT --to ' + ipv6_egress_ip | ansible.utils.ipaddr('address') if alternative_ingress_ip else '-j MASQUERADE' }}
+{% if ipsec_enabled %}
+# For IPsec traffic - NAT the decrypted packets from the VPN subnet
+-A POSTROUTING -s {{ strongswan_network_ipv6 }} {{ '-j SNAT --to ' + ipv6_egress_ip | ansible.utils.ipaddr('address') if alternative_ingress_ip else '-j MASQUERADE' }}
+{% endif %}
+{% if wireguard_enabled %}
+# For WireGuard traffic - NAT packets from the VPN subnet
+-A POSTROUTING -s {{ wireguard_network_ipv6 }} {{ '-j SNAT --to ' + ipv6_egress_ip | ansible.utils.ipaddr('address') if alternative_ingress_ip else '-j MASQUERADE' }}
+{% endif %}
 
 COMMIT
 
@@ -106,7 +113,7 @@ COMMIT
 -A FORWARD -m conntrack --ctstate NEW -s {{ strongswan_network_ipv6 }} -m policy --pol ipsec --dir in -j ACCEPT
 {% endif %}
 {% if wireguard_enabled %}
--A FORWARD -m conntrack --ctstate NEW -s {{ wireguard_network_ipv6 }} -m policy --pol none --dir in -j ACCEPT
+-A FORWARD -m conntrack --ctstate NEW -s {{ wireguard_network_ipv6 }} -j ACCEPT
 {% endif %}
 
 # Use the ICMPV6-CHECK chain, described above

tests/unit/test_iptables_rules.py

@@ -0,0 +1,188 @@
+#!/usr/bin/env python3
+"""
+Test iptables rules logic for VPN traffic routing.
+
+These tests verify that the iptables rules templates generate correct
+NAT rules for both WireGuard and IPsec VPN traffic.
+"""
+
+from pathlib import Path
+
+import pytest
+from jinja2 import Environment, FileSystemLoader
+
+
+def load_template(template_name):
+    """Load a Jinja2 template from the roles/common/templates directory."""
+    template_dir = Path(__file__).parent.parent.parent / 'roles' / 'common' / 'templates'
+    env = Environment(loader=FileSystemLoader(str(template_dir)))
+    return env.get_template(template_name)
+
+
+def test_wireguard_nat_rules_ipv4():
+    """Test that WireGuard traffic gets proper NAT rules without policy matching."""
+    template = load_template('rules.v4.j2')
+
+    # Test with WireGuard enabled
+    result = template.render(
+        ipsec_enabled=False,
+        wireguard_enabled=True,
+        wireguard_network_ipv4='10.49.0.0/16',
+        wireguard_port=51820,
+        wireguard_port_avoid=53,
+        wireguard_port_actual=51820,
+        ansible_default_ipv4={'interface': 'eth0'},
+        snat_aipv4=None,
+        BetweenClients_DROP=True,
+        block_smb=True,
+        block_netbios=True,
+        local_service_ip='10.49.0.1',
+        ansible_ssh_port=22,
+        reduce_mtu=0
+    )
+
+    # Verify NAT rule exists without policy matching
+    assert '-A POSTROUTING -s 10.49.0.0/16 -j MASQUERADE' in result
+    # Verify no policy matching in WireGuard NAT rules
+    assert '-A POSTROUTING -s 10.49.0.0/16 -m policy' not in result
+
+
+def test_ipsec_nat_rules_ipv4():
+    """Test that IPsec traffic gets proper NAT rules without policy matching."""
+    template = load_template('rules.v4.j2')
+
+    # Test with IPsec enabled
+    result = template.render(
+        ipsec_enabled=True,
+        wireguard_enabled=False,
+        strongswan_network='10.48.0.0/16',
+        strongswan_network_ipv6='2001:db8::/48',
+        ansible_default_ipv4={'interface': 'eth0'},
+        snat_aipv4=None,
+        BetweenClients_DROP=True,
+        block_smb=True,
+        block_netbios=True,
+        local_service_ip='10.48.0.1',
+        ansible_ssh_port=22,
+        reduce_mtu=0
+    )
+
+    # Verify NAT rule exists without policy matching
+    assert '-A POSTROUTING -s 10.48.0.0/16 -j MASQUERADE' in result
+    # Verify no policy matching in IPsec NAT rules (this was the bug)
+    assert '-A POSTROUTING -s 10.48.0.0/16 -m policy --pol none' not in result
+
+
+def test_both_vpns_nat_rules_ipv4():
+    """Test NAT rules when both VPN types are enabled."""
+    template = load_template('rules.v4.j2')
+
+    result = template.render(
+        ipsec_enabled=True,
+        wireguard_enabled=True,
+        strongswan_network='10.48.0.0/16',
+        wireguard_network_ipv4='10.49.0.0/16',
+        strongswan_network_ipv6='2001:db8::/48',
+        wireguard_network_ipv6='2001:db8:a160::/48',
+        wireguard_port=51820,
+        wireguard_port_avoid=53,
+        wireguard_port_actual=51820,
+        ansible_default_ipv4={'interface': 'eth0'},
+        snat_aipv4=None,
+        BetweenClients_DROP=True,
+        block_smb=True,
+        block_netbios=True,
+        local_service_ip='10.49.0.1',
+        ansible_ssh_port=22,
+        reduce_mtu=0
+    )
+
+    # Both should have NAT rules
+    assert '-A POSTROUTING -s 10.48.0.0/16 -j MASQUERADE' in result
+    assert '-A POSTROUTING -s 10.49.0.0/16 -j MASQUERADE' in result
+
+    # Neither should have policy matching
+    assert '-m policy --pol none' not in result
+
+
+def test_alternative_ingress_snat():
+    """Test that alternative ingress IP uses SNAT instead of MASQUERADE."""
+    template = load_template('rules.v4.j2')
+
+    result = template.render(
+        ipsec_enabled=True,
+        wireguard_enabled=True,
+        strongswan_network='10.48.0.0/16',
+        wireguard_network_ipv4='10.49.0.0/16',
+        strongswan_network_ipv6='2001:db8::/48',
+        wireguard_network_ipv6='2001:db8:a160::/48',
+        wireguard_port=51820,
+        wireguard_port_avoid=53,
+        wireguard_port_actual=51820,
+        ansible_default_ipv4={'interface': 'eth0'},
+        snat_aipv4='192.168.1.100',  # Alternative ingress IP
+        BetweenClients_DROP=True,
+        block_smb=True,
+        block_netbios=True,
+        local_service_ip='10.49.0.1',
+        ansible_ssh_port=22,
+        reduce_mtu=0
+    )
+
+    # Should use SNAT with specific IP instead of MASQUERADE
+    assert '-A POSTROUTING -s 10.48.0.0/16 -j SNAT --to 192.168.1.100' in result
+    assert '-A POSTROUTING -s 10.49.0.0/16 -j SNAT --to 192.168.1.100' in result
+    assert 'MASQUERADE' not in result
+
+
+def test_ipsec_forward_rule_has_policy_match():
+    """Test that IPsec FORWARD rules still use policy matching (this is correct)."""
+    template = load_template('rules.v4.j2')
+
+    result = template.render(
+        ipsec_enabled=True,
+        wireguard_enabled=False,
+        strongswan_network='10.48.0.0/16',
+        strongswan_network_ipv6='2001:db8::/48',
+        ansible_default_ipv4={'interface': 'eth0'},
+        snat_aipv4=None,
+        BetweenClients_DROP=True,
+        block_smb=True,
+        block_netbios=True,
+        local_service_ip='10.48.0.1',
+        ansible_ssh_port=22,
+        reduce_mtu=0
+    )
+
+    # FORWARD rule should have policy match (this is correct and should stay)
+    assert '-A FORWARD -m conntrack --ctstate NEW -s 10.48.0.0/16 -m policy --pol ipsec --dir in -j ACCEPT' in result
+
+
+def test_wireguard_forward_rule_no_policy_match():
+    """Test that WireGuard FORWARD rules don't use policy matching."""
+    template = load_template('rules.v4.j2')
+
+    result = template.render(
+        ipsec_enabled=False,
+        wireguard_enabled=True,
+        wireguard_network_ipv4='10.49.0.0/16',
+        wireguard_port=51820,
+        wireguard_port_avoid=53,
+        wireguard_port_actual=51820,
+        ansible_default_ipv4={'interface': 'eth0'},
+        snat_aipv4=None,
+        BetweenClients_DROP=True,
+        block_smb=True,
+        block_netbios=True,
+        local_service_ip='10.49.0.1',
+        ansible_ssh_port=22,
+        reduce_mtu=0
+    )
+
+    # WireGuard FORWARD rule should NOT have any policy match
+    assert '-A FORWARD -m conntrack --ctstate NEW -s 10.49.0.0/16 -j ACCEPT' in result
+    assert '-A FORWARD -m conntrack --ctstate NEW -s 10.49.0.0/16 -m policy' not in result
+
+
+if __name__ == '__main__':
+    pytest.main([__file__, '-v'])