From 34a18f92eeacc754e5857249aaf8decf24e98cde Mon Sep 17 00:00:00 2001
From: Jonas Nick <jonasd.nick@gmail.com>
Date: Sun, 9 Aug 2026 19:40:39 +0000
Subject: [PATCH] netns-isolation: fix netns-exec being executable by all
 normal users

The `netns-exec` wrapper was created with `group = ""`, which makes the
`security.wrappers` activation run `chown ${allowedUser}:`. GNU chown
resolves the trailing colon to the owner's login group, and because the
operator is defined with `isNormalUser = true`, that group is the shared
group `users`. Together with mode 550, this made the wrapper executable
by every normal user on the host instead of only by `allowedUser`.

The wrapper carries `cap_sys_admin=ep` and the NixOS security wrapper
performs no owner check, so any normal user could enter `nb-joinmarket`,
the only netns that `netns-exec` permits. This exposes joinmarketd's
unauthenticated control port on 127.0.0.1 inside that netns and allows
sending packets from the joinmarket netns address. This can only ever become a
problem if there's a second, normal user on the host.

Use mode 500 so that only `allowedUser` can execute the wrapper. Also
set the group to `root`, so that `users` doesn't silently become an
authorized group again if the mode is ever loosened.

The netns-isolation test asserted this property with
`runuser -u clightning -- netns-exec nb-bitcoind ip a`, which fails
regardless of the file mode, because `clightning` is a system user with
its own group and `nb-bitcoind` is not in netns-exec's allowlist.
Replace it with a check that a normal user is denied by the exec
permissions, and assert the reason for each failure instead of only the
exit status.
---
 modules/netns-isolation.nix |  7 ++++---
 test/tests.nix              |  4 ++++
 test/tests.py               | 22 +++++++++++++++-------
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 683331b..1f83186 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -37,7 +37,6 @@ let
       type = types.str;
       description = ''
         User that is allowed to execute commands in the service network namespaces.
-        The user's group is also authorized.
       '';
       default = config.nix-bitcoin.operator.name;
     };
@@ -122,8 +121,10 @@ in {
       source = config.nix-bitcoin.pkgs.netns-exec;
       capabilities = "cap_sys_admin=ep";
       owner = cfg.allowedUser;
-      group = ""; # Set to the group of `owner`
-      permissions = "550";
+      # Don't authorize the group of `owner`. For normal users, this group is
+      # `users`, which is shared by all normal users.
+      group = "root";
+      permissions = "500";
     };
 
     systemd.services = {
diff --git a/test/tests.nix b/test/tests.nix
index 53c01d4..1c0eaad 100644
--- a/test/tests.nix
+++ b/test/tests.nix
@@ -279,6 +279,10 @@ let
       test.data.netns = config.nix-bitcoin.netns-isolation.netns;
       tests.netns-isolation = true;
       environment.systemPackages = [ pkgs.fping ];
+
+      # Used for testing that `netns-exec` is not executable by users other than
+      # the operator. Like all normal users, this user is a member of group `users`.
+      users.users.unauthorized.isNormalUser = true;
     };
 
     regtestBase = { config, pkgs, ... }: {
diff --git a/test/tests.py b/test/tests.py
index 3509f4b..cee5ca2 100644
--- a/test/tests.py
+++ b/test/tests.py
@@ -331,6 +331,21 @@ def _():
         f"nc -l {ip('bitcoind')} 1080 2>&1 || true", "nc: Cannot assign requested address"
     )
 
+    # netns-exec should fail for unauthorized namespaces
+    assert_matches(
+        "runuser -u operator -- netns-exec nb-clightning ip a 2>&1 || true",
+        "nb-clightning is not an allowed netns",
+    )
+
+    # netns-exec should only be executable by the operator user.
+    # User `unauthorized` is a member of group `users`, like all normal users.
+    # Netns `nb-clightning` is rejected by netns-exec before it accesses the
+    # netns, so the error below can only originate from the exec permissions.
+    assert_matches(
+        "runuser -u unauthorized -- netns-exec nb-clightning ip a 2>&1 || true",
+        "Permission denied",
+    )
+
     if "joinmarket" in enabled_tests:
         # netns-exec should drop capabilities
         assert_matches(
@@ -338,13 +353,6 @@ def _():
             re.compile("^Current: =$", re.MULTILINE),
         )
 
-    if "clightning" in enabled_tests:
-        # netns-exec should fail for unauthorized namespaces
-        machine.fail("netns-exec nb-clightning ip a")
-
-        # netns-exec should only be executable by the operator user
-        machine.fail("runuser -u clightning -- netns-exec nb-bitcoind ip a")
-
 
 # Impure: stops bitcoind (and dependent services)
 @test("backups")