From 588a0b240515f7c104914d5b20e3fc5fc68e2a69 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Thu, 20 Aug 2020 13:11:08 +0200
Subject: [PATCH] security: enable full systemd-status for group 'proc'

Previously, systemd-status was broken for all users except root.

Use a 'default' deny policy, which is overridden for group 'proc'.

Add operator to group 'proc'.

Also, remove redundant XML boilerplate.
---
 modules/presets/secure-node.nix |  1 +
 modules/security.nix            | 36 +++++++++++++++++++--------------
 test/scenarios/lib.py           |  4 +++-
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/modules/presets/secure-node.nix b/modules/presets/secure-node.nix
index 374dd52..7933487 100644
--- a/modules/presets/secure-node.nix
+++ b/modules/presets/secure-node.nix
@@ -227,6 +227,7 @@ in {
       isNormalUser = true;
       extraGroups = [
           "systemd-journal"
+          "proc" # Enable full /proc access and systemd-status
           cfg.bitcoind.group
         ]
         ++ (optionals cfg.clightning.enable [ "clightning" ])
diff --git a/modules/security.nix b/modules/security.nix
index 89efc66..f4d2c78 100644
--- a/modules/security.nix
+++ b/modules/security.nix
@@ -7,21 +7,27 @@
 
   # This mitigates a systemd security issue leaking (sub)process
   # command lines.
-  # Only allow root to retrieve systemd unit information like
+  # Only allow users with group 'proc' to retrieve systemd unit information like
   # cgroup paths (i.e. (sub)process command lines) via D-Bus.
   # This D-Bus call is used by `systemctl status`.
-  services.dbus.packages = [ (pkgs.writeTextDir "etc/dbus-1/system.d/dbus.conf" ''
-    <?xml version="1.0" encoding="UTF-8"?> <!-- -*- XML -*- -->
-
-    <!DOCTYPE busconfig PUBLIC
-     "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
-     "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
-
-      <policy context="mandatory">
-        <deny send_destination="org.freedesktop.systemd1"
-          send_interface="org.freedesktop.systemd1.Manager"
-          send_member="GetUnitProcesses"/>
-      </policy>
-    </busconfig>
-  '') ];
+  services.dbus.packages = lib.mkAfter [ # Apply at the end to override the default policy
+    (pkgs.writeTextDir "etc/dbus-1/system.d/dbus.conf" ''
+      <busconfig>
+        <policy context="default">
+          <deny
+            send_destination="org.freedesktop.systemd1"
+            send_interface="org.freedesktop.systemd1.Manager"
+            send_member="GetUnitProcesses"
+          />
+        </policy>
+        <policy group="proc">
+          <allow
+            send_destination="org.freedesktop.systemd1"
+            send_interface="org.freedesktop.systemd1.Manager"
+            send_member="GetUnitProcesses"
+          />
+        </policy>
+      </busconfig>
+    '')
+  ];
 }
diff --git a/test/scenarios/lib.py b/test/scenarios/lib.py
index 78c28a8..2ebda30 100644
--- a/test/scenarios/lib.py
+++ b/test/scenarios/lib.py
@@ -103,11 +103,13 @@ def run_tests(extra_tests):
     machine.wait_until_succeeds(log_has_string("bitcoind-import-banlist", "Importing node banlist"))
     assert_no_failure("bitcoind-import-banlist")
 
-    # test that `systemctl status` can't leak credentials
+    # `systemctl status` run by unprivileged users shouldn't leak cgroup info
     assert_matches(
         "sudo -u electrs systemctl status clightning 2>&1 >/dev/null",
         "Failed to dump process list for 'clightning.service', ignoring: Access denied",
     )
+    # The 'operator' with group 'proc' has full access
+    assert_full_match("sudo -u operator systemctl status clightning 2>&1 >/dev/null", "")
     machine.succeed("grep -Fq hidepid=2 /proc/mounts")
 
     ### Additional tests