diff --git a/api/wireguard_manager.py b/api/wireguard_manager.py index b327447..b1cac5b 100644 --- a/api/wireguard_manager.py +++ b/api/wireguard_manager.py @@ -115,15 +115,23 @@ class WireGuardManager(BaseServiceManager): def _get_dnat_container_ips(self) -> tuple: """Return (dns_ip, caddy_ip) by inspecting running containers.""" + import re as _re + _ip_re = _re.compile(r'^\d{1,3}(?:\.\d{1,3}){3}$') + def _inspect(name, fallback): try: r = subprocess.run( ['docker', 'inspect', '--format', - '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}', name], + '{{range .NetworkSettings.Networks}}{{.IPAddress}} {{end}}', name], capture_output=True, text=True, check=False) - return r.stdout.strip() or fallback + # Network range may return multiple IPs — pick the first valid one. + for candidate in r.stdout.split(): + candidate = candidate.strip() + if candidate and _ip_re.match(candidate): + return candidate except Exception: - return fallback + pass + return fallback return _inspect('cell-dns', '172.20.0.3'), _inspect('cell-caddy', '172.20.0.2') def generate_config(self, interface: str = 'wg0', port: int = DEFAULT_PORT) -> str: @@ -176,11 +184,28 @@ class WireGuardManager(BaseServiceManager): f'sysctl -q net.ipv4.conf.all.rp_filter=1 || true\n' ) - def ensure_postup_dnat(self) -> bool: - """Update wg0.conf PostUp/PostDown to include DNS (53) and service (80) DNAT rules. + @staticmethod + def _is_dnat_rule(token: str) -> bool: + """Return True if this semicolon-split token is a DNAT/FORWARD rule managed by us.""" + t = token.strip() + if not t.startswith('iptables'): + return False + # PREROUTING DNAT on ports 53 or 80 + if 'PREROUTING' in t and 'DNAT' in t and ('--dport 53' in t or '--dport 80' in t): + return True + # FORWARD accept to eth0 for ports 53 or 80 (service traffic forwarding) + if 'FORWARD' in t and '-o eth0' in t and ('--dport 53' in t or '--dport 80' in t): + return True + return False - Called at startup so rules persist across WireGuard interface restarts. - Returns True if the file was changed (caller should reload WG config). + def ensure_postup_dnat(self) -> bool: + """Rewrite wg0.conf PostUp/PostDown with the correct DNS/service DNAT rules. + + Strips ALL managed DNAT and FORWARD rules (any IP, any port 53/80) and + replaces them with a single correct set for the current container IPs. + This is fully idempotent — stale IPs and duplicates are always cleaned. + + Returns True if the file was changed. """ cf = self._config_file() if not os.path.exists(cf): @@ -189,44 +214,47 @@ class WireGuardManager(BaseServiceManager): content = f.read() dns_ip, caddy_ip = self._get_dnat_container_ips() - dnat_marker = f'--dport 53 -j DNAT --to-destination {dns_ip}:53' - if dnat_marker in content: - return False dnat_up = ( - f'iptables -t nat -A PREROUTING -i %i -p udp --dport 53 -j DNAT --to-destination {dns_ip}:53; ' - f'iptables -t nat -A PREROUTING -i %i -p tcp --dport 53 -j DNAT --to-destination {dns_ip}:53; ' - f'iptables -t nat -A PREROUTING -i %i -p tcp --dport 80 -j DNAT --to-destination {caddy_ip}:80; ' - f'iptables -I FORWARD -i %i -o eth0 -p tcp --dport 80 -j ACCEPT; ' - f'iptables -I FORWARD -i %i -o eth0 -p udp --dport 53 -j ACCEPT; ' - f'iptables -I FORWARD -i %i -o eth0 -p tcp --dport 53 -j ACCEPT' + f'iptables -t nat -A PREROUTING -i %i -p udp --dport 53 -j DNAT --to-destination {dns_ip}:53' + f'; iptables -t nat -A PREROUTING -i %i -p tcp --dport 53 -j DNAT --to-destination {dns_ip}:53' + f'; iptables -t nat -A PREROUTING -i %i -p tcp --dport 80 -j DNAT --to-destination {caddy_ip}:80' + f'; iptables -I FORWARD -i %i -o eth0 -p tcp --dport 80 -j ACCEPT' + f'; iptables -I FORWARD -i %i -o eth0 -p udp --dport 53 -j ACCEPT' + f'; iptables -I FORWARD -i %i -o eth0 -p tcp --dport 53 -j ACCEPT' ) dnat_down = ( - f'iptables -t nat -D PREROUTING -i %i -p udp --dport 53 -j DNAT --to-destination {dns_ip}:53 2>/dev/null || true; ' - f'iptables -t nat -D PREROUTING -i %i -p tcp --dport 53 -j DNAT --to-destination {dns_ip}:53 2>/dev/null || true; ' - f'iptables -t nat -D PREROUTING -i %i -p tcp --dport 80 -j DNAT --to-destination {caddy_ip}:80 2>/dev/null || true; ' - f'iptables -D FORWARD -i %i -o eth0 -p tcp --dport 80 -j ACCEPT 2>/dev/null || true; ' - f'iptables -D FORWARD -i %i -o eth0 -p udp --dport 53 -j ACCEPT 2>/dev/null || true; ' - f'iptables -D FORWARD -i %i -o eth0 -p tcp --dport 53 -j ACCEPT 2>/dev/null || true' + f'iptables -t nat -D PREROUTING -i %i -p udp --dport 53 -j DNAT --to-destination {dns_ip}:53 2>/dev/null || true' + f'; iptables -t nat -D PREROUTING -i %i -p tcp --dport 53 -j DNAT --to-destination {dns_ip}:53 2>/dev/null || true' + f'; iptables -t nat -D PREROUTING -i %i -p tcp --dport 80 -j DNAT --to-destination {caddy_ip}:80 2>/dev/null || true' + f'; iptables -D FORWARD -i %i -o eth0 -p tcp --dport 80 -j ACCEPT 2>/dev/null || true' + f'; iptables -D FORWARD -i %i -o eth0 -p udp --dport 53 -j ACCEPT 2>/dev/null || true' + f'; iptables -D FORWARD -i %i -o eth0 -p tcp --dport 53 -j ACCEPT 2>/dev/null || true' ) lines = content.split('\n') updated = [] changed = False for line in lines: - if line.startswith('PostUp = ') and dnat_marker not in line: - updated.append(line + '; ' + dnat_up) - changed = True - elif line.startswith('PostDown = ') and '--dport 53 -j DNAT' not in line: - updated.append(line + '; ' + dnat_down) - changed = True + if line.startswith('PostUp = ') or line.startswith('PostDown = '): + prefix, _, rest = line.partition(' = ') + # Split on ';', strip managed DNAT rules, rejoin + tokens = [t for t in rest.split(';') if not self._is_dnat_rule(t)] + clean = '; '.join(t.strip() for t in tokens if t.strip()) + new_rules = dnat_up if line.startswith('PostUp') else dnat_down + new_line = f'{prefix} = {clean}; {new_rules}' + if new_line != line: + updated.append(new_line) + changed = True + else: + updated.append(line) else: updated.append(line) if changed: with open(cf, 'w') as f: f.write('\n'.join(updated)) - logger.info(f'ensure_postup_dnat: updated wg0.conf with DNAT rules ' + logger.info(f'ensure_postup_dnat: rewrote wg0.conf DNAT rules ' f'(dns={dns_ip}, caddy={caddy_ip})') return changed diff --git a/tests/test_wireguard_manager.py b/tests/test_wireguard_manager.py index 0d5272d..fca501c 100644 --- a/tests/test_wireguard_manager.py +++ b/tests/test_wireguard_manager.py @@ -565,11 +565,13 @@ class TestWireGuardSysctlAndPortCheck(unittest.TestCase): def test_ensure_postup_dnat_idempotent_when_rules_present(self, mock_run): mock_run.return_value.returncode = 0 mock_run.return_value.stdout = '172.20.0.3' - self._write_wg_conf_postup( - extra_postup='iptables -t nat -A PREROUTING -i %i -p udp --dport 53 -j DNAT --to-destination 172.20.0.3:53' - ) - changed = self.wg.ensure_postup_dnat() - self.assertFalse(changed) + self._write_wg_conf_postup() + # First call: writes all 6 DNAT rules + first = self.wg.ensure_postup_dnat() + self.assertTrue(first) + # Second call: rules already correct, no change + second = self.wg.ensure_postup_dnat() + self.assertFalse(second) def test_ensure_postup_dnat_returns_false_when_no_conf(self): changed = self.wg.ensure_postup_dnat()