Merge #20461: rpc: Validate -rpcauth arguments

053b4fbad8 doc: Release note regarding -rpcauth validation (João Barbosa)
46001323b1 rpc: Validate -rpcauth arguments (João Barbosa)
d37c813a43 rpc: Refactor to process -rpcauth once (João Barbosa)

Pull request description:

  Invalid `-rpcauth` arguments are currently silently ignored. This make server initialization fail if any `-rpcauth` is invalid.

ACKs for top commit:
  MarcoFalke:
    review ACK 053b4fbad8
  jonatack:
    ACK 053b4fbad8
  ryanofsky:
    Code review ACK 053b4fbad8. Only changes since last review are moving a variable declaration and adding a comment, release notes, and a `const`.

Tree-SHA512: c99923d4a121f0c9f882b07f5402ea53e9b2d9455ad34468a094ffab1d64df26c82e1279734c0d42bc2e113eae7b581fbc3be52f3ed4a2d7450d11793afcf406

doc/release-notes.md

@@ -80,6 +80,8 @@ Updated settings
 
 Changes to Wallet or GUI related settings can be found in the GUI or Wallet section below.
 
+- Passing an invalid `-rpcauth` argument now cause bitcoind to fail to start.  (#20461)
+
 Tools and Utilities
 -------------------
 

src/httprpc.cpp

@@ -68,6 +68,8 @@ private:
 static std::string strRPCUserColonPass;
 /* Stored RPC timer interface (for unregistration) */
 static std::unique_ptr<HTTPRPCTimerInterface> httpRPCTimerInterface;
+/* List of -rpcauth values */
+static std::vector<std::vector<std::string>> g_rpcauth;
 /* RPC Auth Whitelist */
 static std::map<std::string, std::set<std::string>> g_rpc_whitelist;
 static bool g_rpc_whitelist_default = false;
@@ -99,15 +101,7 @@ static bool multiUserAuthorized(std::string strUserPass)
     std::string strUser = strUserPass.substr(0, strUserPass.find(':'));
     std::string strPass = strUserPass.substr(strUserPass.find(':') + 1);
 
-    for (const std::string& strRPCAuth : gArgs.GetArgs("-rpcauth")) {
+    for (const auto& vFields : g_rpcauth) {
-        //Search for multi-user login/pass "rpcauth" from config
-        std::vector<std::string> vFields;
-        boost::split(vFields, strRPCAuth, boost::is_any_of(":$"));
-        if (vFields.size() != 3) {
-            //Incorrect formatting in config file
-            continue;
-        }
-
         std::string strName = vFields[0];
         if (!TimingResistantEqual(strName, strUser)) {
             continue;
@@ -259,6 +253,16 @@ static bool InitRPCAuthentication()
     if (gArgs.GetArg("-rpcauth","") != "")
     {
         LogPrintf("Using rpcauth authentication.\n");
+        for (const std::string& rpcauth : gArgs.GetArgs("-rpcauth")) {
+            std::vector<std::string> fields;
+            boost::split(fields, rpcauth, boost::is_any_of(":$"));
+            if (fields.size() == 3) {
+                g_rpcauth.push_back(fields);
+            } else {
+                LogPrintf("Invalid -rpcauth argument.\n");
+                return false;
+            }
+        }
     }
 
     g_rpc_whitelist_default = gArgs.GetBoolArg("-rpcwhitelistdefault", gArgs.IsArgSet("-rpcwhitelist"));

test/functional/rpc_users.py

@@ -99,11 +99,18 @@ class HTTPBasicsTest(BitcoinTestFramework):
 
         self.test_auth(self.nodes[1], self.rpcuser, self.rpcpassword)
 
-        self.log.info('Check that failure to write cookie file will abort the node gracefully')
+        init_error = 'Error: Unable to start HTTP server. See debug log for details.'
+
+        self.log.info('Check -rpcauth are validated')
+        # Empty -rpcauth= are ignored
+        self.restart_node(0, extra_args=['-rpcauth='])
         self.stop_node(0)
+        self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo'])
+        self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo:bar'])
+
+        self.log.info('Check that failure to write cookie file will abort the node gracefully')
         cookie_file = os.path.join(get_datadir_path(self.options.tmpdir, 0), self.chain, '.cookie.tmp')
         os.mkdir(cookie_file)
-        init_error = 'Error: Unable to start HTTP server. See debug log for details.'
         self.nodes[0].assert_start_raises_init_error(expected_msg=init_error)