From 9f45952605e8e1b2139e5e6f6fd75a71694a0fb2 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Tue, 27 Oct 2026 09:10:42 +0100 Subject: [PATCH] fix: WireGuard peers keep their tunnel key when others come and go (#32) get_vpn_tunnels() keyed WireGuard peers with a running counter over the peer rows, so a peer added ahead of an existing one renumbered every peer after it. Whatever a consumer stores per key moved with it: netOrk's site link and tunnel check ended up on a new, offline peer and alarmed while the real tunnel was up. The key is now the interface and the start of the public key, as in napalm-linux, made URL-safe because the key is a path segment. Two peers sharing the prefix get the whole key; a row without a public key keeps its position. IPsec and OpenVPN keys were already stable. Every WireGuard key changes once; netOrk carries its per-tunnel metadata across (netork#1654). --- CHANGELOG.md | 6 ++ napalm_opnsense/opnsense.py | 28 ++++++++- tests/unit/test_vpn_tunnels.py | 106 +++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_vpn_tunnels.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ed4ea8d..e35fd11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to this driver. The format follows ## [Unreleased] +### Fixed +- `get_vpn_tunnels()` keys a WireGuard peer by its interface and the start of its + public key (`wireguard-wg0-aBcD1234`, URL-safe) instead of its position, so a peer + added ahead of another no longer renumbers it (#32). Every WireGuard key changes + once with this release. + ## [0.5.0] - 2026-10-23 The first release on PyPI. diff --git a/napalm_opnsense/opnsense.py b/napalm_opnsense/opnsense.py index 5d2eb0a..39656c8 100644 --- a/napalm_opnsense/opnsense.py +++ b/napalm_opnsense/opnsense.py @@ -93,6 +93,28 @@ def _carp_master(entry: dict[str, Any]) -> bool: return str(entry.get("status") or "").strip().upper() == "MASTER" +#: How much of a public key names a peer, the same length as napalm-linux uses. +_WG_KEY_PREFIX = 8 + + +def _wireguard_key(row: dict[str, Any], position: int, taken: dict[str, Any]) -> str: + """A WireGuard peer's tunnel key: its interface and the start of its public key. + + Not its position: a peer added ahead of another would renumber it, and + whatever a consumer keeps per key would move to a different peer (#32). The + base64 is made URL-safe because the key is used as a path segment. Two peers + sharing a prefix get the whole key; one without a key keeps its position. + """ + public_key = (row.get("public-key") or "").strip() + if not public_key: + return f"wireguard-{position}" + safe = public_key.rstrip("=").replace("+", "-").replace("/", "_") + key = f"wireguard-{row.get('if') or 'wg'}-{safe[:_WG_KEY_PREFIX]}" + if key in taken: + key = f"wireguard-{row.get('if') or 'wg'}-{safe}" + return key + + class OPNsenseDriver( OPNsensePingMixin, ConfigSyncMixin, @@ -2172,8 +2194,8 @@ class OPNsenseDriver( # {"if":"wg0", "type":"interface", ...}, # {"if":"wg0", "type":"peer", "public-key":"...", # "endpoint":"1.2.3.4:51820", "transfer-rx":N, "transfer-tx":N, - # "name":"HCQ", "peer-status":"online", - # "latest-handshake-age":45, "ifname":"HCQ"}, ... + # "name":"office", "peer-status":"online", + # "latest-handshake-age":45, "ifname":"office"}, ... # ]} data = self._get("/api/wireguard/service/show") rows = data.get("rows", []) if isinstance(data, dict) else [] @@ -2202,7 +2224,7 @@ class OPNsenseDriver( or f"wg-peer-{wg_idx}" ) - tunnels[f"wireguard-{wg_idx}"] = { + tunnels[_wireguard_key(row, wg_idx, tunnels)] = { "type": "WireGuard", "local_endpoint": "", "remote_endpoint": remote_ip, diff --git a/tests/unit/test_vpn_tunnels.py b/tests/unit/test_vpn_tunnels.py new file mode 100644 index 0000000..e241330 --- /dev/null +++ b/tests/unit/test_vpn_tunnels.py @@ -0,0 +1,106 @@ +"""WireGuard peers in ``get_vpn_tunnels()`` keep their key when others come and go (#32). + +The key was a running counter over the peer rows, so a peer added ahead of an +existing one renumbered every peer after it, and whatever a consumer stored per +key (netOrk's site link, its tunnel check) silently moved to another peer. A +peer is its interface and its public key, so that is what the key is made of. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from napalm_opnsense.opnsense import OPNsenseDriver + +OFFICE = { + "if": "wg0", + "type": "peer", + "name": "office", + "public-key": "aBcD1234efGH5678ijKL9012mnOP3456qrST7890uvw=", + "endpoint": "198.51.100.7:51820", + "peer-status": "online", + "transfer-rx": 10, + "transfer-tx": 20, + "latest-handshake-age": 45, +} +LAPTOP = { + "if": "wg0", + "type": "peer", + "name": "laptop", + "public-key": "Zz9/+yXw8765vuTS4321rqPO0987nmLK6543jiHG210=", + "endpoint": "(none)", + "peer-status": "offline", +} +SERVER = { + "if": "wg1", + "type": "peer", + "name": "server", + "public-key": "QqQqWwWwEeEeRrRrTtTtYyYyUuUuIiIiOoOoPpPp123=", + "endpoint": "203.0.113.9:51821", + "peer-status": "online", +} + + +def _driver(*rows: dict) -> OPNsenseDriver: + with patch("napalm_opnsense.opnsense.requests.Session"): + drv = OPNsenseDriver(hostname="fw", username="k", password="s") + drv.session = MagicMock() + show = {"rows": [{"if": "wg0", "type": "interface"}, *rows]} + + def get(path: str) -> dict: + if path == "/api/wireguard/service/show": + return show + raise RuntimeError("not installed") # IPsec and OpenVPN are not part of this + + drv._get = get # type: ignore[method-assign] + return drv + + +def _by_name(tunnels: dict) -> dict[str, str]: + return {t["description"]: key for key, t in tunnels.items()} + + +class TestTheKey: + def test_is_the_interface_and_the_start_of_the_public_key(self): + assert list(_driver(OFFICE).get_vpn_tunnels()) == ["wireguard-wg0-aBcD1234"] + + def test_survives_a_peer_added_ahead_of_it(self): + before = _by_name(_driver(OFFICE, SERVER).get_vpn_tunnels()) + after = _by_name(_driver(OFFICE, LAPTOP, SERVER).get_vpn_tunnels()) + assert after["server"] == before["server"] + assert after["office"] == before["office"] + + def test_is_safe_in_a_url_path(self): + # The key is a path segment (PATCH /devices/{id}/vpn-tunnels/{key}), and + # a "/" from base64 would split it even when encoded. + key = _by_name(_driver(LAPTOP).get_vpn_tunnels())["laptop"] + assert key == "wireguard-wg0-Zz9_-yXw" + + def test_two_peers_with_the_same_key_start_both_stay(self): + twin = { + **SERVER, + "if": "wg0", + "name": "twin", + "public-key": OFFICE["public-key"][:8] + "x=", + } + tunnels = _driver(OFFICE, twin).get_vpn_tunnels() + assert len(tunnels) == 2 + + def test_without_a_public_key_falls_back_to_the_position(self): + keyless = {k: v for k, v in OFFICE.items() if k != "public-key"} + assert list(_driver(keyless).get_vpn_tunnels()) == ["wireguard-1"] + + +class TestTheEntry: + def test_carries_the_peer_s_reading(self): + tunnel = _driver(OFFICE).get_vpn_tunnels()["wireguard-wg0-aBcD1234"] + assert tunnel == { + "type": "WireGuard", + "local_endpoint": "", + "remote_endpoint": "198.51.100.7", + "is_up": True, + "uptime": 45, + "bytes_in": 10, + "bytes_out": 20, + "description": "office", + }