diff --git a/modules/dbus.nix b/modules/dbus.nix deleted file mode 100644 index 000b0ff..0000000 --- a/modules/dbus.nix +++ /dev/null @@ -1,55 +0,0 @@ -{ config, lib, pkgs, ... }: - -with lib; - -let - inherit (config) nix-bitcoin-services; - dataDir = "/var/lib/dbus-hardening"; - # Mitigates a security issue that allows unprivileged users to read - # other unprivileged user's processes' credentials from CGroup using - # `systemctl status`. - dbus-hardening = pkgs.writeText "dbus.conf" '' - - - - - - - - - - - - - - ''; -in { - config = { - systemd.tmpfiles.rules = [ - "d '${dataDir}/etc/dbus-1/system.d' 0770 messagebus messagebus - -" - ]; - - services.dbus.packages = [ "${dataDir}" ]; - - systemd.services.hardeneddbus = { - description = "Install hardeneddbus"; - wantedBy = [ "multi-user.target" ]; - script = '' - cp ${dbus-hardening} ${dataDir}/etc/dbus-1/system.d/dbus.conf - chmod 640 ${dataDir}/etc/dbus-1/system.d/dbus.conf - ''; - serviceConfig = nix-bitcoin-services.defaultHardening // { - PrivateNetwork = "true"; - Type = "oneshot"; - User = "messagebus"; - ReadWritePaths = "${dataDir}"; - }; - }; - }; -} diff --git a/modules/modules.nix b/modules/modules.nix index 3625c1e..1b6e253 100644 --- a/modules/modules.nix +++ b/modules/modules.nix @@ -16,7 +16,7 @@ ./lightning-loop.nix ./secrets/secrets.nix ./netns-isolation.nix - ./dbus.nix + ./security.nix ./backups.nix ]; diff --git a/modules/presets/secure-node.nix b/modules/presets/secure-node.nix index 374dd52..133f649 100644 --- a/modules/presets/secure-node.nix +++ b/modules/presets/secure-node.nix @@ -42,8 +42,7 @@ in { networking.firewall.enable = true; - # hideProcessInformation even if hardened kernel profile is disabled - security.hideProcessInformation = true; + nix-bitcoin.security.hideProcessInformation = true; # Tor services.tor = { @@ -227,6 +226,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 new file mode 100644 index 0000000..cd5ad4e --- /dev/null +++ b/modules/security.nix @@ -0,0 +1,39 @@ +{ config, lib, pkgs, options, ... }: + +{ + options = { + nix-bitcoin.security.hideProcessInformation = options.security.hideProcessInformation; + }; + + config = lib.mkIf config.nix-bitcoin.security.hideProcessInformation { + # Only show the current user's processes in /proc. + # Users with group 'proc' can still access all processes. + security.hideProcessInformation = true; + + # This mitigates a systemd security issue leaking (sub)process + # command lines. + # 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 = lib.mkAfter [ # Apply at the end to override the default policy + (pkgs.writeTextDir "etc/dbus-1/system.d/dbus.conf" '' + + + + + + + + + '') + ]; + }; +} diff --git a/test/scenarios/lib.py b/test/scenarios/lib.py index 2989fec..bffd4bb 100644 --- a/test/scenarios/lib.py +++ b/test/scenarios/lib.py @@ -9,7 +9,7 @@ def assert_matches(cmd, regexp): raise Exception(f"Pattern '{regexp}' not found in '{out}'") -def assert_matches_exactly(cmd, regexp): +def assert_full_match(cmd, regexp): out = succeed(cmd) if not re.fullmatch(regexp, out): raise Exception(f"Pattern '{regexp}' doesn't match '{out}'") @@ -38,9 +38,7 @@ if "is_interactive" in vars(): # The argument extra_tests is a dictionary from strings to functions. The string # determines at which point of run_tests the corresponding function is executed. def run_tests(extra_tests): - assert_running("setup-secrets") - # Unused secrets should be inaccessible - succeed('[[ $(stat -c "%U:%G %a" /secrets/dummy) = "root:root 440" ]]') + test_security() assert_running("bitcoind") machine.wait_until_succeeds("bitcoin-cli getnetworkinfo") @@ -103,13 +101,6 @@ 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 - assert_matches( - "sudo -u electrs systemctl status clightning 2>&1 >/dev/null", - "Failed to dump process list for 'clightning.service', ignoring: Access denied", - ) - machine.succeed("grep -Fq hidepid=2 /proc/mounts") - ### Additional tests # Current time in µs @@ -155,3 +146,21 @@ def run_tests(extra_tests): ### Check that all extra_tests have been run assert len(extra_tests) == 0 + + +def test_security(): + assert_running("setup-secrets") + # Unused secrets should be inaccessible + succeed('[[ $(stat -c "%U:%G %a" /secrets/dummy) = "root:root 440" ]]') + + # Access to '/proc' should be restricted + machine.succeed("grep -Fq hidepid=2 /proc/mounts") + + machine.wait_for_unit("bitcoind") + # `systemctl status` run by unprivileged users shouldn't leak cgroup info + assert_matches( + "sudo -u electrs systemctl status bitcoind 2>&1 >/dev/null", + "Failed to dump process list for 'bitcoind.service', ignoring: Access denied", + ) + # The 'operator' with group 'proc' has full access + assert_full_match("sudo -u operator systemctl status bitcoind 2>&1 >/dev/null", "") diff --git a/test/scenarios/withnetns.py b/test/scenarios/withnetns.py index 7534a14..6b5df81 100644 --- a/test/scenarios/withnetns.py +++ b/test/scenarios/withnetns.py @@ -84,7 +84,7 @@ def prestop(): machine.fail("netns-exec nb-electrs ip a") # test that netns-exec drops capabilities - assert_matches_exactly( + assert_full_match( "su operator -c 'netns-exec nb-bitcoind capsh --print | grep Current '", "Current: =\n" )