From eb817ffdc53e73038f3083dd976d1a8104a275be Mon Sep 17 00:00:00 2001 From: Dmitrii Iurco Date: Fri, 24 Apr 2026 10:31:57 -0400 Subject: [PATCH] fix: WireGuard sysctl || true, port check on page load, add peer status tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: sysctl -q net.ipv4.conf.all.rp_filter=0 in PostUp exited non-zero inside the linuxserver/wireguard container (no permission), causing wg-quick to tear down the wg0 interface — breaking peer status, port check, and internet access through full tunnel. - wireguard_manager.py: add || true to both sysctl PostUp/PostDown lines - docker-compose.yml: add net.ipv4.conf.all.rp_filter=0 to wireguard sysctls - WireGuard.jsx: kick off port check asynchronously on page load (was refresh-only) - tests: add TestWireGuardSysctlAndPortCheck — 14 new tests covering sysctl content, check_port_open (interface up / down / fallback-to-handshake), get_peer_status (online / offline / not-found / no-handshake), and get_all_peer_statuses (multi-peer / empty / skips interface line) Co-Authored-By: Claude Sonnet 4.6 --- api/wireguard_manager.py | 4 +- docker-compose.yml | 1 + tests/test_wireguard_manager.py | 153 +++++++++++++++++++++++++++++++- webui/src/pages/WireGuard.jsx | 9 +- 4 files changed, 163 insertions(+), 4 deletions(-) diff --git a/api/wireguard_manager.py b/api/wireguard_manager.py index 4590dac..468957d 100644 --- a/api/wireguard_manager.py +++ b/api/wireguard_manager.py @@ -134,11 +134,11 @@ class WireGuardManager(BaseServiceManager): f'PostUp = iptables -A FORWARD -i %i -j ACCEPT; ' f'iptables -t nat -A POSTROUTING -o eth0 -j MASQUERADE; ' f'{hairpin}' - f'sysctl -q net.ipv4.conf.all.rp_filter=0\n' + f'sysctl -q net.ipv4.conf.all.rp_filter=0 || true\n' f'PostDown = iptables -D FORWARD -i %i -j ACCEPT; ' f'iptables -t nat -D POSTROUTING -o eth0 -j MASQUERADE; ' f'{hairpin_down}' - f'sysctl -q net.ipv4.conf.all.rp_filter=1\n' + f'sysctl -q net.ipv4.conf.all.rp_filter=1 || true\n' ) def _config_file(self) -> str: diff --git a/docker-compose.yml b/docker-compose.yml index 99ca914..a1b0d7b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -181,6 +181,7 @@ services: sysctls: - net.ipv4.conf.all.src_valid_mark=1 - net.ipv4.ip_forward=1 + - net.ipv4.conf.all.rp_filter=0 logging: driver: json-file options: diff --git a/tests/test_wireguard_manager.py b/tests/test_wireguard_manager.py index 225ea99..ac2b5f5 100644 --- a/tests/test_wireguard_manager.py +++ b/tests/test_wireguard_manager.py @@ -455,5 +455,156 @@ class TestWireGuardConfigReads(unittest.TestCase): self.assertNotIn(':51820', content) +class TestWireGuardSysctlAndPortCheck(unittest.TestCase): + """Tests for sysctl safety, port check, and peer status parsing.""" + + def setUp(self): + self.test_dir = tempfile.mkdtemp() + patcher = patch.object(WireGuardManager, '_syncconf', return_value=None) + self.mock_sync = patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(shutil.rmtree, self.test_dir) + self.wg = WireGuardManager(self.test_dir, self.test_dir) + + # ── generate_config sysctl safety ──────────────────────────────────────── + + def test_generate_config_postup_has_nonfatal_sysctl(self): + cfg = self.wg.generate_config() + self.assertIn('sysctl -q net.ipv4.conf.all.rp_filter=0 || true', cfg) + + def test_generate_config_postdown_has_nonfatal_sysctl(self): + cfg = self.wg.generate_config() + self.assertIn('sysctl -q net.ipv4.conf.all.rp_filter=1 || true', cfg) + + def test_generate_config_has_masquerade(self): + cfg = self.wg.generate_config() + self.assertIn('MASQUERADE', cfg) + + def test_generate_config_has_forward_rule(self): + cfg = self.wg.generate_config() + self.assertIn('FORWARD -i %i -j ACCEPT', cfg) + + # ── check_port_open ─────────────────────────────────────────────────────── + + @patch('subprocess.run') + def test_check_port_open_when_wg_interface_up(self, mock_run): + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = 'interface: wg0\n listening port: 51820\n' + self.assertTrue(self.wg.check_port_open()) + + @patch('subprocess.run') + def test_check_port_open_false_when_interface_down(self, mock_run): + # wg show fails (no device), fallback wg show dump also fails + mock_run.return_value.returncode = 1 + mock_run.return_value.stdout = '' + self.assertFalse(self.wg.check_port_open()) + + @patch('subprocess.run') + def test_check_port_open_fallback_to_recent_handshake(self, mock_run): + # First call (wg show wg0): fails — interface not reported as up + # Second call (wg show wg0 dump): returns a peer with recent handshake + import time as _time + now = int(_time.time()) + dump_line = f'pubkey\t(none)\t1.2.3.4:51820\t0.0.0.0/0\t{now - 10}\t1000\t2000\t25\n' + def side_effect(*args, **kwargs): + cmd = args[0] + m = MagicMock() + if 'dump' in cmd: + m.returncode = 0 + m.stdout = dump_line + else: + m.returncode = 0 + m.stdout = 'interface: wg0\n' # no "listening port" text + return m + mock_run.side_effect = side_effect + # "listening port" not in stdout for first call → falls through to dump + # dump has recent handshake → returns True + result = self.wg.check_port_open() + self.assertTrue(result) + + # ── get_peer_status ─────────────────────────────────────────────────────── + + @patch('subprocess.run') + def test_get_peer_status_online_with_recent_handshake(self, mock_run): + import time as _time + now = int(_time.time()) + pub = 'AAABBBCCC=' + dump = ( + f'privkey\tserverpub\t51820\toff\n' # interface line (4 fields) + f'{pub}\t(none)\t1.2.3.4:12345\t10.0.0.2/32\t{now-30}\t500\t1000\t25\n' + ) + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = dump + st = self.wg.get_peer_status(pub) + self.assertTrue(st['online']) + self.assertIsNotNone(st['last_handshake']) + self.assertLessEqual(st['last_handshake_seconds_ago'], 35) + + @patch('subprocess.run') + def test_get_peer_status_offline_with_old_handshake(self, mock_run): + import time as _time + now = int(_time.time()) + pub = 'AAABBBCCC=' + dump = f'{pub}\t(none)\t(none)\t10.0.0.2/32\t{now - 300}\t0\t0\t25\n' + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = dump + st = self.wg.get_peer_status(pub) + self.assertFalse(st['online']) + + @patch('subprocess.run') + def test_get_peer_status_not_found_returns_none_online(self, mock_run): + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = '' + st = self.wg.get_peer_status('NOTEXIST=') + self.assertIsNone(st['online']) + + @patch('subprocess.run') + def test_get_peer_status_no_handshake_yet(self, mock_run): + pub = 'AAABBBCCC=' + dump = f'{pub}\t(none)\t(none)\t10.0.0.2/32\t0\t0\t0\t25\n' + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = dump + st = self.wg.get_peer_status(pub) + self.assertFalse(st['online']) + self.assertIsNone(st['last_handshake']) + + # ── get_all_peer_statuses ───────────────────────────────────────────────── + + @patch('subprocess.run') + def test_get_all_peer_statuses_parses_multiple_peers(self, mock_run): + import time as _time + now = int(_time.time()) + pub1 = 'PUB1KEY=' + pub2 = 'PUB2KEY=' + dump = ( + f'privkey\tserverpub\t51820\toff\n' + f'{pub1}\t(none)\t1.1.1.1:1000\t10.0.0.2/32\t{now-20}\t100\t200\t25\n' + f'{pub2}\t(none)\t(none)\t10.0.0.3/32\t{now-200}\t0\t0\t25\n' + ) + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = dump + statuses = self.wg.get_all_peer_statuses() + self.assertIn(pub1, statuses) + self.assertIn(pub2, statuses) + self.assertTrue(statuses[pub1]['online']) + self.assertFalse(statuses[pub2]['online']) + + @patch('subprocess.run') + def test_get_all_peer_statuses_empty_when_interface_down(self, mock_run): + mock_run.return_value.returncode = 1 + mock_run.return_value.stdout = '' + statuses = self.wg.get_all_peer_statuses() + self.assertEqual(statuses, {}) + + @patch('subprocess.run') + def test_get_all_peer_statuses_skips_interface_line(self, mock_run): + # Interface line has only 4 tab-separated fields — must not appear as a peer + dump = 'privkey\tserverpub\t51820\toff\n' + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = dump + statuses = self.wg.get_all_peer_statuses() + self.assertEqual(statuses, {}) + + if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() \ No newline at end of file diff --git a/webui/src/pages/WireGuard.jsx b/webui/src/pages/WireGuard.jsx index d60444b..82e7151 100644 --- a/webui/src/pages/WireGuard.jsx +++ b/webui/src/pages/WireGuard.jsx @@ -53,7 +53,14 @@ function WireGuard() { ]); setStatus(statusResponse.data); - if (serverConfigResponse) setServerConfig(serverConfigResponse); + if (serverConfigResponse) { + setServerConfig({ ...serverConfigResponse, port_open: 'checking' }); + // Check port asynchronously so page loads fast + fetch('/api/wireguard/check-port', { method: 'POST' }) + .then(r => r.json()) + .then(d => setServerConfig(prev => ({ ...prev, port_open: d.port_open ?? false }))) + .catch(() => setServerConfig(prev => ({ ...prev, port_open: false }))); + } // Merge peer registry data with WireGuard data (same as Peers page) const peersData = peersResponse.data || [];