From 8d3f5c646ae2bf0ef3275147616f4f393efd7baa Mon Sep 17 00:00:00 2001 From: anti Date: Wed, 29 Apr 2026 11:56:40 -0400 Subject: [PATCH] fix(network): accept CAP_NET_ADMIN in lieu of euid==0 for macvlan setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The systemd unit grants AmbientCapabilities=CAP_NET_ADMIN so the API service can program host-side macvlan/ipvlan interfaces without running as root, but setup_host_macvlan/_ipvlan rejected with euid!=0 before even trying — making web-driven 'decnet deploy' impossible under the privilege model the unit advertises. Replace _require_root with _require_net_admin, which reads CapEff from /proc/self/status and accepts the cap (bit 12) as well as euid==0. No libcap dep — pure /proc parse. --- decnet/network.py | 51 +++++++++++++++++++++++++++++++------- tests/core/test_network.py | 20 ++++++++++++--- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/decnet/network.py b/decnet/network.py index c150071e..634082c6 100644 --- a/decnet/network.py +++ b/decnet/network.py @@ -303,11 +303,44 @@ def remove_bridge_network(client: docker.DockerClient, name: str) -> None: # Host-side macvlan interface (hairpin fix) # --------------------------------------------------------------------------- -def _require_root() -> None: - if os.geteuid() != 0: - raise PermissionError( - "MACVLAN host-side interface setup requires root. Run with sudo." - ) +# Linux capability bit positions — see capabilities(7). +_CAP_NET_ADMIN = 12 + + +def _has_cap_net_admin() -> bool: + """True if the current process holds CAP_NET_ADMIN in its effective set. + + Reads ``/proc/self/status`` rather than calling ``capget(2)`` so we + don't need a libcap dependency. ``CapEff`` is a 64-bit hex bitmask; + bit 12 is CAP_NET_ADMIN. + """ + try: + with open("/proc/self/status", "r") as fh: + for line in fh: + if line.startswith("CapEff:"): + bits = int(line.split()[1], 16) + return bool(bits & (1 << _CAP_NET_ADMIN)) + except OSError: + pass + return False + + +def _require_net_admin() -> None: + """Reject early if the process can't run ``ip link add ... macvlan``. + + CAP_NET_ADMIN is what the kernel actually checks for netlink RTM_NEWLINK + of a macvlan/ipvlan slave; euid==0 is sufficient (it grants every cap) + but not necessary. Prefer the cap check so the systemd unit's + ``AmbientCapabilities=CAP_NET_ADMIN`` is honoured without forcing the + whole API to run as root. + """ + if os.geteuid() == 0 or _has_cap_net_admin(): + return + raise PermissionError( + "MACVLAN host-side interface setup needs CAP_NET_ADMIN. " + "Either run as root or grant the cap (systemd: " + "AmbientCapabilities=CAP_NET_ADMIN)." + ) def setup_host_macvlan(interface: str, host_macvlan_ip: str, decky_ip_range: str) -> None: @@ -317,7 +350,7 @@ def setup_host_macvlan(interface: str, host_macvlan_ip: str, decky_ip_range: str host-helper first: the two drivers can share a parent NIC on paper but leaving the opposite helper in place is just cruft after a driver swap. """ - _require_root() + _require_net_admin() _run(["ip", "link", "del", HOST_IPVLAN_IFACE], check=False) @@ -332,7 +365,7 @@ def setup_host_macvlan(interface: str, host_macvlan_ip: str, decky_ip_range: str def teardown_host_macvlan(decky_ip_range: str) -> None: - _require_root() + _require_net_admin() _run(["ip", "route", "del", decky_ip_range, "dev", HOST_MACVLAN_IFACE], check=False) _run(["ip", "link", "del", HOST_MACVLAN_IFACE], check=False) @@ -344,7 +377,7 @@ def setup_host_ipvlan(interface: str, host_ipvlan_ip: str, decky_ip_range: str) host-helper first so a prior macvlan deploy doesn't leave its slave dangling on the parent NIC after the driver swap. """ - _require_root() + _require_net_admin() _run(["ip", "link", "del", HOST_MACVLAN_IFACE], check=False) @@ -358,7 +391,7 @@ def setup_host_ipvlan(interface: str, host_ipvlan_ip: str, decky_ip_range: str) def teardown_host_ipvlan(decky_ip_range: str) -> None: - _require_root() + _require_net_admin() _run(["ip", "route", "del", decky_ip_range, "dev", HOST_IPVLAN_IFACE], check=False) _run(["ip", "link", "del", HOST_IPVLAN_IFACE], check=False) diff --git a/tests/core/test_network.py b/tests/core/test_network.py index 8db05c49..5e5d5733 100644 --- a/tests/core/test_network.py +++ b/tests/core/test_network.py @@ -203,11 +203,23 @@ class TestSetupHostMacvlan: add_cmds = [cmd for cmd in calls if cmd[:3] == ["ip", "link", "add"]] assert not any(HOST_MACVLAN_IFACE in cmd for cmd in add_cmds) + @patch("decnet.network._has_cap_net_admin", return_value=False) @patch("decnet.network.os.geteuid", return_value=1) - def test_requires_root(self, _): + def test_rejects_when_not_root_and_no_cap(self, _eu, _cap): with pytest.raises(PermissionError): setup_host_macvlan("eth0", "192.168.1.5", "192.168.1.96/27") + @patch("decnet.network._has_cap_net_admin", return_value=True) + @patch("decnet.network.os.geteuid", return_value=1000) + @patch("decnet.network._run") + def test_accepts_cap_net_admin_without_root(self, mock_run, _eu, _cap): + # Non-root with CAP_NET_ADMIN is what systemd's + # AmbientCapabilities=CAP_NET_ADMIN gives the API service. The + # kernel will accept the netlink RTM_NEWLINK; we must not reject + # earlier on a stricter euid==0 check. + mock_run.side_effect = lambda cmd, **kw: MagicMock(returncode=1) if "show" in cmd else MagicMock(returncode=0) + setup_host_macvlan("eth0", "192.168.1.5", "192.168.1.96/27") + # --------------------------------------------------------------------------- # setup_host_ipvlan / teardown_host_ipvlan @@ -236,8 +248,9 @@ class TestSetupHostIpvlan: macvlan_refs = [cmd for cmd in calls if HOST_MACVLAN_IFACE in cmd] assert all(cmd[:3] == ["ip", "link", "del"] for cmd in macvlan_refs) + @patch("decnet.network._has_cap_net_admin", return_value=False) @patch("decnet.network.os.geteuid", return_value=1) - def test_requires_root(self, _): + def test_rejects_when_not_root_and_no_cap(self, _eu, _cap): with pytest.raises(PermissionError): setup_host_ipvlan("wlan0", "192.168.1.5", "192.168.1.96/27") @@ -396,7 +409,8 @@ class TestTeardownHostMacvlan: calls = [str(c) for c in mock_run.call_args_list] assert any(HOST_MACVLAN_IFACE in c for c in calls) + @patch("decnet.network._has_cap_net_admin", return_value=False) @patch("decnet.network.os.geteuid", return_value=1) - def test_requires_root(self, _): + def test_rejects_when_not_root_and_no_cap(self, _eu, _cap): with pytest.raises(PermissionError): teardown_host_macvlan("192.168.1.96/27")