Merge #315: Use doas instead of sudo

47d257a docs: add rationale for doas to README and FAQ (nixbitcoin)
b0039d6 docs: discourage users from ssh'ing into the root user (nixbitcoin)
2ca92a3 services: use doas if enabled (nixbitcoin)
ce2b445 treewide: use runuser for dropping privileges (Erik Arvstedt)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK 47d257a

Tree-SHA512: 84bab7de1cc6fb3d405a4fc4589f2be825275f69e48dede6ff62d1a27e3a386fea530c91a234d006ec6305a46d0ec54ca836f52f197541a3df215369c9b7c1e2

README.md

@@ -81,7 +81,7 @@ NixOS modules
 
 Security
 ---
-* **Simplicity:** Only services you select in `configuration.nix` and their dependencies are installed, packages and dependencies are [pinned](pkgs/nixpkgs-pinned.nix), most packages are built from the [NixOS stable channel](https://github.com/NixOS/nixpkgs/tree/nixos-20.09), with a few exceptions that are built from the nixpkgs unstable channel, builds happen in a [sandboxed environment](https://nixos.org/manual/nix/stable/#conf-sandbox), code is continuously reviewed and refined.
+* **Simplicity:** Only services you select in `configuration.nix` and their dependencies are installed, packages and dependencies are [pinned](pkgs/nixpkgs-pinned.nix), support for [doas](https://github.com/Duncaen/OpenDoas) ([sudo alternative](https://lobste.rs/s/efsvqu/heap_based_buffer_overflow_sudo_cve_2021#c_c6fcfa)), most packages are built from the [NixOS stable channel](https://github.com/NixOS/nixpkgs/tree/nixos-20.09), with a few exceptions that are built from the nixpkgs unstable channel, builds happen in a [sandboxed environment](https://nixos.org/manual/nix/stable/#conf-sandbox), code is continuously reviewed and refined.
 * **Integrity:** Nix package manager, NixOS and packages can be built from source to reduce reliance on binary caches, nix-bitcoin merge commits are signed, all commits are approved by multiple nix-bitcoin developers, upstream packages are cryptographically verified where possible, we use this software ourselves.
 * **Principle of Least Privilege:** Services operate with least privileges; they each have their own user and are restricted further with [systemd options](pkgs/lib.nix), [RPC whitelisting](modules/bitcoind-rpc-public-whitelist.nix), and [netns-isolation](modules/netns-isolation.nix). There's a non-root user *operator* to interact with the various services.
 * **Defense-in-depth:** nix-bitcoin is built with a [hardened kernel](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/profiles/hardened.nix) by default, services are confined through discretionary access control, Linux namespaces, [dbus firewall](modules/security.nix) and seccomp-bpf with continuous improvements.

docs/faq.md

@@ -32,3 +32,5 @@
     * **A:** Check your clightning logs with `journalctl -eu clightning`. Do you see something like `bitcoin-cli getblock ... false` failed? Are you using pruned mode? That means that clightning hasn't seen all the blocks it needs to and it can't get that block because your node is pruned. If you're just setting up a new node you can `systemctl stop clightning` and wipe your `/var/lib/clightning` directory. Otherwise you need to reindex the Bitcoin node.
 * **Q:** My disk space is getting low due to nix.
     * **A:** run `nix-collect-garbage -d`
+* **Q:** Where is `sudo`???
+    * **A:** we replaced sudo with OpenBSD's doas after [CVE-2021-3156](https://www.openwall.com/lists/oss-security/2021/01/26/3). It has greatly reduced complexity, and is therefore less likely to be a source of severe vulnerabilities in the future.

docs/install.md

@@ -147,6 +147,8 @@ You can also build Nix from source by following the instructions at https://nixo
     nixops ssh operator@bitcoin-node
     ```
 
+For security reasons, all normal system management tasks can and should be performed with the `operator` user. Logging in as `root` should be done as rarely as possible.
+
 See [usage.md](usage.md) for usage instructions, such as how to update.
 
 To resize the VM disk image, you can use this helper script from within nix-shell:
@@ -426,4 +428,6 @@ Follow the [Setup deployment directory](#3-setup-deployment-directory) instructi
     nixops ssh operator@bitcoin-node
     ```
 
+For security reasons, all normal system management tasks can and should be performed with the `operator` user. Logging in as `root` should be done as rarely as possible.
+
 See [usage.md](usage.md) for usage instructions, such as how to update.

modules/joinmarket.nix

@@ -7,6 +7,7 @@ let
   nbLib = config.nix-bitcoin.lib;
   nbPkgs = config.nix-bitcoin.pkgs;
   secretsDir = config.nix-bitcoin.secretsDir;
+  runAsUser = config.nix-bitcoin.runAsUserCmd;
 
   inherit (config.services) bitcoind;
   torAddress = builtins.head (builtins.split ":" config.services.tor.client.socksListenAddress);
@@ -84,7 +85,7 @@ let
      for bin in jm-*; do
        {
          echo "#!${pkgs.bash}/bin/bash";
-         echo "cd '${cfg.dataDir}' && ${cfg.cliExec} sudo -u ${cfg.user} $jm/$bin --datadir='${cfg.dataDir}' \"\$@\"";
+         echo "cd '${cfg.dataDir}' && ${cfg.cliExec} ${runAsUser} ${cfg.user} $jm/$bin --datadir='${cfg.dataDir}' \"\$@\"";
        } > $out/bin/$bin
      done
      chmod -R +x $out/bin
@@ -166,7 +167,6 @@ in {
       wantedBy = [ "multi-user.target" ];
       requires = [ "bitcoind.service" ];
       after = [ "bitcoind.service" ];
-      path = [ pkgs.sudo ];
       serviceConfig = nbLib.defaultHardening // {
         ExecStartPre = nbLib.privileged "joinmarket-create-config" ''
           install -o '${cfg.user}' -g '${cfg.group}' -m 640 ${configFile} ${cfg.dataDir}/joinmarket.cfg
@@ -183,7 +183,8 @@ in {
               echo "Create wallet"
               pw=$(cat "${secretsDir}"/jm-wallet-password)
               cd ${cfg.dataDir}
-              if ! sudo -u ${cfg.user} ${nbPkgs.joinmarket}/bin/jm-genwallet --datadir=${cfg.dataDir} $walletname $pw \
+              if ! ${pkgs.utillinux}/bin/runuser -u ${cfg.user} -- \
+                     ${nbPkgs.joinmarket}/bin/jm-genwallet --datadir=${cfg.dataDir} $walletname $pw \
                      | grep 'recovery_seed' \
                      | cut -d ':' -f2 \
                      | (umask u=r,go=; cat > "${secretsDir}/jm-wallet-seed"); then
@@ -211,7 +212,7 @@ in {
     users.groups.${cfg.group} = {};
     nix-bitcoin.operator = {
       groups = [ cfg.group ];
-      sudoUsers = [ cfg.group ];
+      allowRunAsUsers = [ cfg.group ];
     };
 
     nix-bitcoin.secrets.jm-wallet-password.user = cfg.user;

modules/lnd-rest-onion-service.nix

@@ -6,11 +6,12 @@ let
   cfg = config.services.lnd.restOnionService;
   nbLib = config.nix-bitcoin.lib;
   secretsDir = config.nix-bitcoin.secretsDir;
+  runAsUser = config.nix-bitcoin.runAsUserCmd;
 
   lnd = config.services.lnd;
 
   bin = pkgs.writeScriptBin "lndconnect-rest-onion" ''
-    #!/usr/bin/env -S sudo -u lnd ${pkgs.bash}/bin/bash
+    #!/usr/bin/env -S ${runAsUser} lnd ${pkgs.bash}/bin/bash
 
     exec ${cfg.package}/bin/lndconnect \
      --host=$(cat ${config.nix-bitcoin.onionAddresses.dataDir}/lnd/lnd-rest) \

modules/lnd.nix

@@ -6,6 +6,7 @@ let
   cfg = config.services.lnd;
   nbLib = config.nix-bitcoin.lib;
   secretsDir = config.nix-bitcoin.secretsDir;
+  runAsUser = config.nix-bitcoin.runAsUserCmd;
 
   bitcoind = config.services.bitcoind;
   bitcoindRpcAddress = bitcoind.rpc.address;
@@ -123,7 +124,7 @@ in {
       default = pkgs.writeScriptBin "lncli"
       # Switch user because lnd makes datadir contents readable by user only
       ''
-        sudo -u lnd ${cfg.package}/bin/lncli \
+        ${runAsUser} lnd ${cfg.package}/bin/lncli \
           --rpcserver ${cfg.rpcAddress}:${toString cfg.rpcPort} \
           --tlscertpath '${secretsDir}/lnd-cert' \
           --macaroonpath '${networkDir}/admin.macaroon' "$@"
@@ -270,7 +271,7 @@ in {
     users.groups.lnd = {};
     nix-bitcoin.operator = {
       groups = [ "lnd" ];
-      sudoUsers = [ "lnd" ];
+      allowRunAsUsers = [ "lnd" ];
     };
 
     nix-bitcoin.secrets = {

modules/modules.nix

@@ -57,6 +57,14 @@ with lib;
             "$@"
         '';
       };
+
+      # A helper for using doas instead of sudo when doas is enabled
+      runAsUserCmd = mkOption {
+        readOnly = true;
+        default = if config.security.doas.enable
+                  then "doas -u"
+                  else "sudo -u";
+      };
     };
   };
 

modules/operator.nix

@@ -22,7 +22,7 @@ in {
       default = [];
       description = "Extra groups.";
     };
-    sudoUsers = mkOption {
+    allowRunAsUsers = mkOption {
       type = with types; listOf str;
       default = [];
       description = "Users as which the operator is allowed to run commands.";
@@ -38,10 +38,14 @@ in {
       ] ++ cfg.groups;
     };
 
-    security.sudo.extraConfig = mkIf (cfg.sudoUsers != []) (let
-      users = builtins.concatStringsSep "," cfg.sudoUsers;
-    in ''
-      ${cfg.name} ALL=(${users}) NOPASSWD: ALL
-    '');
+    security = mkIf (cfg.allowRunAsUsers != []) {
+      # Use doas instead of sudo if enabled
+      doas.extraConfig = mkIf config.security.doas.enable ''
+        ${lib.concatMapStrings (user: "permit nopass ${cfg.name} as ${user}\n") cfg.allowRunAsUsers}
+      '';
+      sudo.extraConfig = mkIf (!config.security.doas.enable) ''
+        ${cfg.name} ALL=(${builtins.concatStringsSep "," cfg.allowRunAsUsers}) NOPASSWD: ALL
+      '';
+    };
   };
 }

modules/presets/secure-node.nix

@@ -20,6 +20,10 @@ in {
 
     nix-bitcoin.security.hideProcessInformation = true;
 
+    # Use doas instead of sudo
+    security.doas.enable = true;
+    security.sudo.enable = false;
+
     environment.systemPackages = with pkgs; [
       jq
     ];

modules/recurring-donations.nix

@@ -78,7 +78,7 @@ in {
     systemd.services.recurring-donations = {
       requires = [ "clightning.service" ];
       after = [ "clightning.service" ];
-      path = with pkgs; [ nix-bitcoin.clightning curl sudo jq ];
+      path = with pkgs; [ nix-bitcoin.clightning curl jq ];
       serviceConfig = nbLib.defaultHardening // {
         ExecStart = "${pkgs.bash}/bin/bash ${recurring-donations-script}";
         User = "recurring-donations";

test/tests.py

@@ -91,18 +91,18 @@ def _():
         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",
+            "runuser -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", "")
+        assert_full_match("runuser -u operator -- systemctl status bitcoind 2>&1 >/dev/null", "")
 
 
 @test("bitcoind")
 def _():
     assert_running("bitcoind")
     machine.wait_until_succeeds("bitcoin-cli getnetworkinfo")
-    assert_matches("su operator -c 'bitcoin-cli getnetworkinfo' | jq", '"version"')
+    assert_matches("runuser -u operator -- bitcoin-cli getnetworkinfo | jq", '"version"')
     # RPC access for user 'public' should be restricted
     machine.fail(
         "bitcoin-cli -rpcuser=public -rpcpassword=$(cat /secrets/bitcoin-rpcpassword-public) stop"
@@ -131,14 +131,14 @@ def _():
 def _():
     assert_running("liquidd")
     machine.wait_until_succeeds("elements-cli getnetworkinfo")
-    assert_matches("su operator -c 'elements-cli getnetworkinfo' | jq", '"version"')
-    succeed("su operator -c 'liquidswap-cli --help'")
+    assert_matches("runuser -u operator -- elements-cli getnetworkinfo | jq", '"version"')
+    succeed("runuser -u operator -- liquidswap-cli --help")
 
 
 @test("clightning")
 def _():
     assert_running("clightning")
-    assert_matches("su operator -c 'lightning-cli getinfo' | jq", '"id"')
+    assert_matches("runuser -u operator -- lightning-cli getinfo | jq", '"id"')
     if test_data["clightning-plugins"]:
         plugin_list = succeed("lightning-cli plugin list")
         plugins = json.loads(plugin_list)["plugins"]
@@ -158,7 +158,7 @@ def _():
 @test("lnd")
 def _():
     assert_running("lnd")
-    assert_matches("su operator -c 'lncli getinfo' | jq", '"version"')
+    assert_matches("runuser -u operator -- lncli getinfo | jq", '"version"')
     assert_no_failure("lnd")
 
 
@@ -170,7 +170,7 @@ def _():
 @test("lightning-loop")
 def _():
     assert_running("lightning-loop")
-    assert_matches("su operator -c 'loop --version'", "version")
+    assert_matches("runuser -u operator -- loop --version", "version")
     # Check that lightning-loop fails with the right error, making sure
     # lightning-loop can connect to lnd
     machine.wait_until_succeeds(
@@ -191,7 +191,7 @@ def _():
     wait_for_open_port(ip("btcpayserver"), 23000)
     # test lnd custom macaroon
     assert_matches(
-        "sudo -u btcpayserver curl -s --cacert /secrets/lnd-cert "
+        "runuser -u btcpayserver -- curl -s --cacert /secrets/lnd-cert "
         '--header "Grpc-Metadata-macaroon: $(xxd -ps -u -c 1000 /run/lnd/btcpayserver.macaroon)" '
         f"-X GET https://{ip('lnd')}:8080/v1/getinfo | jq",
         '"version"',
@@ -232,7 +232,7 @@ def _():
     status, _ = machine.execute("systemctl is-enabled --quiet onion-addresses 2> /dev/null")
     if status == 0:
         machine.wait_for_unit("onion-addresses")
-    json_info = succeed("sudo -u operator nodeinfo")
+    json_info = succeed("runuser -u operator -- nodeinfo")
     info = json.loads(json_info)
     assert info["bitcoind"]["local_address"]
 
@@ -277,15 +277,16 @@ def assert_unreachable(src, dests):
     if "joinmarket" in enabled_tests:
         # netns-exec should drop capabilities
         assert_full_match(
-            "su operator -c 'netns-exec nb-joinmarket capsh --print | grep Current'", "Current: =\n"
+            "runuser -u operator -- netns-exec nb-joinmarket capsh --print | grep Current",
+            "Current: =\n",
         )
 
     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("sudo -u clightning netns-exec nb-bitcoind ip a")
+        machine.fail("runuser -u clightning -- netns-exec nb-bitcoind ip a")
 
 
 # Impure: stops bitcoind (and dependent services)
@@ -349,17 +350,17 @@ def _():
         assert_full_match(get_block_height_cmd, "10\n")
     if "clightning" in enabled_tests:
         machine.wait_until_succeeds(
-            "[[ $(sudo -u operator lightning-cli getinfo | jq -M .blockheight) == 10 ]]"
+            "[[ $(runuser -u operator -- lightning-cli getinfo | jq -M .blockheight) == 10 ]]"
         )
     if "lnd" in enabled_tests:
         machine.wait_until_succeeds(
-            "[[ $(sudo -u operator lncli getinfo | jq -M .block_height) == 10 ]]"
+            "[[ $(runuser -u operator -- lncli getinfo | jq -M .block_height) == 10 ]]"
         )
     if "lightning-loop" in enabled_tests:
         machine.wait_until_succeeds(
             log_has_string("lightning-loop", "Starting event loop at height 10")
         )
-        succeed("sudo -u operator loop getparams")
+        succeed("runuser -u operator -- loop getparams")
 
 
 if "netns-isolation" in enabled_tests: