Merge #229: Improve bitcoind RPC user config

9b6a3ec generate-secrets: extract fn 'makeHMAC' (Erik Arvstedt) ca18ffb generate-secrets: fetch rpcauth.py from github (Erik Arvstedt) 4d6127b bitcoind: clarify RPC whitelist test (Erik Arvstedt) 9d61099 bitcoind: remove custom rpc user names (Erik Arvstedt) 1408403 bitcoind: clarify how bitcoin-cli RPC access is enabled (Erik Arvstedt) 4790c60 bitcoind: move rpc user config to bitcoind (Erik Arvstedt) 876cfad bitcoind: add rpc user option 'passwordHMACFromFile' (Erik Arvstedt) 59434e7 bitcoind: simplify default rpc user name config (Erik Arvstedt) 205829b bitcoind: remove whitespace (Erik Arvstedt) Pull request description: ACKs for top commit: nixbitcoin: ACK 9b6a3ec jonasnick: concept ACK 9b6a3ec Tree-SHA512: ccb9a8d2dc1f360cc1f0bd77535fa8edfd9afec0a519719103fd059d5912a1ed4960c22ef14df616a731f6a88861fecb8d1653fb71c2288b851e4a02f9f49cb2
fort-nix · Aug 29, 2020 · 1c31208 · 1c31208
2 parents 4d19fb7 + 9b6a3ec
commit 1c31208
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 148 deletions.
diff --git a/modules/bitcoind-rpc-public-whitelist.nix b/modules/bitcoind-rpc-public-whitelist.nix
@@ -0,0 +1,61 @@
+# RPC calls that are safe for public use
+[
+  "echo"
+  "getinfo"
+  # Blockchain
+  "getbestblockhash"
+  "getblock"
+  "getblockchaininfo"
+  "getblockcount"
+  "getblockfilter"
+  "getblockhash"
+  "getblockheader"
+  "getblockstats"
+  "getchaintips"
+  "getchaintxstats"
+  "getdifficulty"
+  "getmempoolancestors"
+  "getmempooldescendants"
+  "getmempoolentry"
+  "getmempoolinfo"
+  "getrawmempool"
+  "gettxout"
+  "gettxoutproof"
+  "gettxoutsetinfo"
+  "scantxoutset"
+  "verifytxoutproof"
+  # Mining
+  "getblocktemplate"
+  "getmininginfo"
+  "getnetworkhashps"
+  # Network
+  "getnetworkinfo"
+  # Rawtransactions
+  "analyzepsbt"
+  "combinepsbt"
+  "combinerawtransaction"
+  "converttopsbt"
+  "createpsbt"
+  "createrawtransaction"
+  "decodepsbt"
+  "decoderawtransaction"
+  "decodescript"
+  "finalizepsbt"
+  "fundrawtransaction"
+  "getrawtransaction"
+  "joinpsbts"
+  "sendrawtransaction"
+  "signrawtransactionwithkey"
+  "testmempoolaccept"
+  "utxoupdatepsbt"
+  # Util
+  "createmultisig"
+  "deriveaddresses"
+  "estimatesmartfee"
+  "getdescriptorinfo"
+  "signmessagewithprivkey"
+  "validateaddress"
+  "verifymessage"
+  # Zmq
+ "getzmqnotifications"
+]
diff --git a/modules/bitcoind.nix b/modules/bitcoind.nix
@@ -30,17 +30,14 @@ let
     ${optionalString (cfg.rpcthreads != null) "rpcthreads=${toString cfg.rpcthreads}"}
     rpcport=${toString cfg.rpc.port}
     rpcwhitelistdefault=0
-    ${concatMapStringsSep  "\n"
-      (rpcUser: ''
-        rpcauth=${rpcUser.name}:${rpcUser.passwordHMAC}
-        ${optionalString (rpcUser.rpcwhitelist != []) "rpcwhitelist=${rpcUser.name}:${lib.strings.concatStringsSep "," rpcUser.rpcwhitelist}"}
-      '')
-      (attrValues cfg.rpc.users)
+    ${concatMapStrings (user: ''
+        ${optionalString (!user.passwordHMACFromFile) "rpcauth=${user.name}:${passwordHMAC}"}
+        ${optionalString (user.rpcwhitelist != [])
+          "rpcwhitelist=${user.name}:${lib.strings.concatStringsSep "," user.rpcwhitelist}"}
+      '') (builtins.attrValues cfg.rpc.users)
     }
     ${lib.concatMapStrings (rpcbind: "rpcbind=${rpcbind}\n") cfg.rpcbind}
     ${lib.concatMapStrings (rpcallowip: "rpcallowip=${rpcallowip}\n") cfg.rpcallowip}
-    # Credentials for bitcoin-cli
-    rpcuser=${cfg.rpc.users.privileged.name}
 
     # Wallet options
     ${optionalString (cfg.addresstype != null) "addresstype=${cfg.addresstype}"}
@@ -109,6 +106,7 @@ in {
             options = {
               name = mkOption {
                 type = types.str;
+                default = name;
                 example = "alice";
                 description = ''
                   Username for JSON-RPC connections.
@@ -122,6 +120,11 @@ in {
                   format <SALT-HEX>$<HMAC-HEX>.
                 '';
               };
+              passwordHMACFromFile = mkOption {
+                type = lib.types.bool;
+                internal = true;
+                default = false;
+              };
               rpcwhitelist = mkOption {
                 type = types.listOf types.str;
                 default = [];
@@ -131,9 +134,6 @@ in {
                 '';
               };
             };
-            config = {
-              name = mkDefault name;
-            };
           }));
           description = ''
             RPC user information for JSON-RPC connections.
@@ -283,10 +283,21 @@ in {
   config = mkIf cfg.enable {
     environment.systemPackages = [ cfg.package (hiPrio cfg.cli) ];
 
-    services.bitcoind = mkIf cfg.dataDirReadableByGroup {
-      disablewallet = true;
-      sysperms = true;
-    };
+    services.bitcoind = mkMerge [
+      (mkIf cfg.dataDirReadableByGroup {
+        disablewallet = true;
+        sysperms = true;
+      })
+      {
+        rpc.users.privileged = {
+          passwordHMACFromFile = true;
+        };
+        rpc.users.public = {
+          passwordHMACFromFile = true;
+          rpcwhitelist = import ./bitcoind-rpc-public-whitelist.nix;
+        };
+      }
+    ];
 
     systemd.tmpfiles.rules = [
       "d '${cfg.dataDir}' 0770 ${cfg.user} ${cfg.group} - -"
@@ -298,16 +309,24 @@ in {
       requires = [ "nix-bitcoin-secrets.target" ];
       after = [ "network.target" "nix-bitcoin-secrets.target" ];
       wantedBy = [ "multi-user.target" ];
-      preStart = ''
-        ${optionalString cfg.dataDirReadableByGroup  "chmod -R g+rX '${cfg.dataDir}/blocks'"}
-
-        cfgpre=$(cat ${configFile}; printf "rpcpassword="; cat "${secretsDir}/bitcoin-rpcpassword-privileged")
-        cfg=$(echo "$cfgpre" | \
-        sed "s/bitcoin-HMAC-privileged/$(cat ${secretsDir}/bitcoin-HMAC-privileged)/g" | \
-        sed "s/bitcoin-HMAC-public/$(cat ${secretsDir}/bitcoin-HMAC-public)/g")
+      preStart = let
+        extraRpcauth = concatMapStrings (name: let
+          user = cfg.rpc.users.${name};
+        in optionalString user.passwordHMACFromFile ''
+            echo "rpcauth=${user.name}:$(cat ${secretsDir}/bitcoin-HMAC-${name})"
+          ''
+        ) (builtins.attrNames cfg.rpc.users);
+      in ''
+        ${optionalString cfg.dataDirReadableByGroup "chmod -R g+rX '${cfg.dataDir}/blocks'"}
+        cfg=$(
+          cat ${configFile};
+          ${extraRpcauth}
+          ${/* Enable bitcoin-cli for group 'bitcoin' */ ""}
+          printf "rpcuser=${cfg.rpc.users.privileged.name}\nrpcpassword="; cat "${secretsDir}/bitcoin-rpcpassword-privileged";
+        )
         confFile='${cfg.dataDir}/bitcoin.conf'
         if [[ ! -e $confFile || $cfg != $(cat $confFile) ]]; then
-          install -o '${cfg.user}' -g '${cfg.group}' -m 640  <(echo "$cfg") $confFile
+          install -o '${cfg.user}' -g '${cfg.group}' -m 640 <(echo "$cfg") $confFile
         fi
       '';
       postStart = ''

diff --git a/modules/presets/secure-node.nix b/modules/presets/secure-node.nix
@@ -75,76 +75,6 @@ in {
       # higher rpcthread count due to reports that lightning implementations fail
       # under high bitcoind rpc load
       rpcthreads = 16;
-      rpc.users.privileged = {
-        name = "bitcoinrpc";
-        # Placeholder to be sed'd out by bitcoind preStart
-        passwordHMAC = "bitcoin-HMAC-privileged";
-      };
-      rpc.users.public = {
-        name = "publicrpc";
-        # Placeholder to be sed'd out by bitcoind preStart
-        passwordHMAC = "bitcoin-HMAC-public";
-        rpcwhitelist = [
-          "echo"
-          "getinfo"
-          # Blockchain
-          "getbestblockhash"
-          "getblock"
-          "getblockchaininfo"
-          "getblockcount"
-          "getblockfilter"
-          "getblockhash"
-          "getblockheader"
-          "getblockstats"
-          "getchaintips"
-          "getchaintxstats"
-          "getdifficulty"
-          "getmempoolancestors"
-          "getmempooldescendants"
-          "getmempoolentry"
-          "getmempoolinfo"
-          "getrawmempool"
-          "gettxout"
-          "gettxoutproof"
-          "gettxoutsetinfo"
-          "scantxoutset"
-          "verifytxoutproof"
-          # Mining
-          "getblocktemplate"
-          "getmininginfo"
-          "getnetworkhashps"
-          # Network
-          "getnetworkinfo"
-          # Rawtransactions
-          "analyzepsbt"
-          "combinepsbt"
-          "combinerawtransaction"
-          "converttopsbt"
-          "createpsbt"
-          "createrawtransaction"
-          "decodepsbt"
-          "decoderawtransaction"
-          "decodescript"
-          "finalizepsbt"
-          "fundrawtransaction"
-          "getrawtransaction"
-          "joinpsbts"
-          "sendrawtransaction"
-          "signrawtransactionwithkey"
-          "testmempoolaccept"
-          "utxoupdatepsbt"
-          # Util
-          "createmultisig"
-          "deriveaddresses"
-          "estimatesmartfee"
-          "getdescriptorinfo"
-          "signmessagewithprivkey"
-          "validateaddress"
-          "verifymessage"
-          # Zmq
-          "getzmqnotifications"
-        ];
-      };
     };
     services.tor.hiddenServices.bitcoind = mkHiddenService { port = cfg.bitcoind.port; toHost = cfg.bitcoind.bind; };
 

diff --git a/pkgs/generate-secrets/default.nix b/pkgs/generate-secrets/default.nix
@@ -1,9 +1,15 @@
 { pkgs }: with pkgs;
 
 let
-  rpcauth = pkgs.writeScriptBin "rpcauth" (builtins.readFile ./rpcauth/rpcauth.py);
+  rpcauthSrc = builtins.fetchurl {
+    url = "https://raw.githubusercontent.com/bitcoin/bitcoin/d6cde007db9d3e6ee93bd98a9bbfdce9bfa9b15b/share/rpcauth/rpcauth.py";
+    sha256 = "189mpplam6yzizssrgiyv70c9899ggh8cac76j4n7v0xqzfip07n";
+  };
+  rpcauth = pkgs.writeScriptBin "rpcauth" ''
+    exec ${pkgs.python35}/bin/python ${rpcauthSrc} "$@"
+  '';
 in
 writeScript "generate-secrets" ''
-  export PATH=${lib.makeBinPath [ coreutils apg openssl gnugrep rpcauth python35 ]}
+  export PATH=${lib.makeBinPath [ coreutils apg openssl gnugrep rpcauth ]}
   . ${./generate-secrets.sh} ${./openssl.cnf}
 ''
diff --git a/pkgs/generate-secrets/generate-secrets.sh b/pkgs/generate-secrets/generate-secrets.sh
@@ -5,6 +5,10 @@ opensslConf=${1:-openssl.cnf}
 makePasswordSecret() {
     [[ -e $1 ]] || apg -m 20 -x 20 -M Ncl -n 1 > "$1"
 }
+makeHMAC() {
+    user=$1
+    rpcauth $user $(cat bitcoin-rpcpassword-$user) | grep rpcauth | cut -d ':' -f 2 > bitcoin-HMAC-$user
+}
 
 makePasswordSecret bitcoin-rpcpassword-privileged
 makePasswordSecret bitcoin-rpcpassword-public
@@ -14,8 +18,8 @@ makePasswordSecret lightning-charge-token
 makePasswordSecret spark-wallet-password
 makePasswordSecret backup-encryption-password
 
-[[ -e bitcoin-HMAC-privileged ]] || rpcauth privileged $(cat bitcoin-rpcpassword-privileged) | grep rpcauth | cut -d ':' -f 2 > bitcoin-HMAC-privileged
-[[ -e bitcoin-HMAC-public ]] || rpcauth public $(cat bitcoin-rpcpassword-public) | grep rpcauth | cut -d ':' -f 2 > bitcoin-HMAC-public
+[[ -e bitcoin-HMAC-privileged ]] || makeHMAC privileged
+[[ -e bitcoin-HMAC-public ]] || makeHMAC public
 [[ -e lightning-charge-env ]] || echo "API_TOKEN=$(cat lightning-charge-token)" > lightning-charge-env
 [[ -e nanopos-env          ]] || echo "CHARGE_TOKEN=$(cat lightning-charge-token)" > nanopos-env
 [[ -e spark-wallet-login   ]] || echo "login=spark-wallet:$(cat spark-wallet-password)" > spark-wallet-login

diff --git a/pkgs/generate-secrets/rpcauth/rpcauth.py b/pkgs/generate-secrets/rpcauth/rpcauth.py
diff --git a/test/base.py b/test/base.py
@@ -46,14 +46,12 @@ def run_tests(extra_tests):
     assert_running("bitcoind")
     machine.wait_until_succeeds("bitcoin-cli getnetworkinfo")
     assert_matches("su operator -c 'bitcoin-cli getnetworkinfo' | jq", '"version"')
-    # Test RPC Whitelist
-    machine.wait_until_succeeds("su operator -c 'bitcoin-cli help'")
-    # Restating rpcuser & rpcpassword overrides privileged credentials
+    # RPC access for user 'public' should be restricted
     machine.fail(
-        "bitcoin-cli -rpcuser=publicrpc -rpcpassword=$(cat /secrets/bitcoin-rpcpassword-public) help"
+        "bitcoin-cli -rpcuser=public -rpcpassword=$(cat /secrets/bitcoin-rpcpassword-public) stop"
     )
     machine.wait_until_succeeds(
-        log_has_string("bitcoind", "RPC User publicrpc not allowed to call method help")
+        log_has_string("bitcoind", "RPC User public not allowed to call method stop")
     )
 
     assert_running("electrs")