From ce2b44577728d437b3df05be88be32420a0d9d46 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Sat, 30 Jan 2021 23:08:42 +0100
Subject: [PATCH 1/4] treewide: use runuser for dropping privileges

When running as root, use runuser instead of sudo.
As opposed to sudo or doas, runuser is a standalone
binary that needs no external configuration.
Also, it's a bit faster.
---
 modules/joinmarket.nix |  4 ++--
 test/tests.py          | 31 ++++++++++++++++---------------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/modules/joinmarket.nix b/modules/joinmarket.nix
index f3cbf5d..5263097 100644
--- a/modules/joinmarket.nix
+++ b/modules/joinmarket.nix
@@ -166,7 +166,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 +182,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
diff --git a/test/tests.py b/test/tests.py
index 7832433..178706f 100644
--- a/test/tests.py
+++ b/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,7 +277,8 @@ def _():
     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:
@@ -285,7 +286,7 @@ def _():
         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:

From 2ca92a34a5a48b75a6c79d659086664749d04581 Mon Sep 17 00:00:00 2001
From: nixbitcoin <nixbitcoin@i2pmail.org>
Date: Sat, 30 Jan 2021 23:08:43 +0100
Subject: [PATCH 2/4] services: use doas if enabled

- Remove sudo from recurring-donations path because it's not used by
  the service

- Use doas instead of sudo in secure-node.nix
---
 modules/joinmarket.nix             |  5 +++--
 modules/lnd-rest-onion-service.nix |  3 ++-
 modules/lnd.nix                    |  5 +++--
 modules/modules.nix                |  8 ++++++++
 modules/operator.nix               | 16 ++++++++++------
 modules/presets/secure-node.nix    |  4 ++++
 modules/recurring-donations.nix    |  2 +-
 7 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/modules/joinmarket.nix b/modules/joinmarket.nix
index 5263097..80da753 100644
--- a/modules/joinmarket.nix
+++ b/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
@@ -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;
diff --git a/modules/lnd-rest-onion-service.nix b/modules/lnd-rest-onion-service.nix
index 8e182f1..9af27c0 100644
--- a/modules/lnd-rest-onion-service.nix
+++ b/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) \
diff --git a/modules/lnd.nix b/modules/lnd.nix
index 930d7d4..e051a29 100644
--- a/modules/lnd.nix
+++ b/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 = {
diff --git a/modules/modules.nix b/modules/modules.nix
index 548bc37..b409f2e 100644
--- a/modules/modules.nix
+++ b/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";
+      };
     };
   };
 
diff --git a/modules/operator.nix b/modules/operator.nix
index a7a7361..4be5eb9 100644
--- a/modules/operator.nix
+++ b/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
+      '';
+    };
   };
 }
diff --git a/modules/presets/secure-node.nix b/modules/presets/secure-node.nix
index cf09b60..1c631da 100644
--- a/modules/presets/secure-node.nix
+++ b/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
     ];
diff --git a/modules/recurring-donations.nix b/modules/recurring-donations.nix
index f1fa533..d4351d2 100644
--- a/modules/recurring-donations.nix
+++ b/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";

From b0039d68a076c9d954faedb798f142694133b660 Mon Sep 17 00:00:00 2001
From: nixbitcoin <nixbitcoin@i2pmail.org>
Date: Sun, 31 Jan 2021 17:51:12 +0000
Subject: [PATCH 3/4] docs: discourage users from ssh'ing into the root user

Instead recommend using the operator user for all normal system
management tasks.
---
 docs/install.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/install.md b/docs/install.md
index 39a8cc2..a7881e5 100644
--- a/docs/install.md
+++ b/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.

From 47d257ad3a68dd2f6b0e53f5088f9ef33f5bb8fa Mon Sep 17 00:00:00 2001
From: nixbitcoin <nixbitcoin@i2pmail.org>
Date: Thu, 4 Feb 2021 12:26:33 +0000
Subject: [PATCH 4/4] docs: add rationale for doas to README and FAQ

---
 README.md   | 2 +-
 docs/faq.md | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index c6e658b..3e8a754 100644
--- a/README.md
+++ b/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.
diff --git a/docs/faq.md b/docs/faq.md
index 5f6252b..62b8986 100644
--- a/docs/faq.md
+++ b/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.