Repository: bitcoin/bitcoin
File: src/httprpc.cpp (C++)

Code before:
{
    if (gArgs.GetArg("-rpcpassword", "") == "")
    {
        std::optional<fs::perms> cookie_perms{std::nullopt};
        auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")};
        if (cookie_perms_arg) {
            auto perm_opt = InterpretPermString(*cookie_perms_arg);
            if (!perm_opt) {
                LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg);
                return false;
            }
            cookie_perms = *perm_opt;
        }

        assert(strRPCUserColonPass.empty()); // Only support initializing once
        if (!GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)) {
            return false;
        }
        if (strRPCUserColonPass.empty()) {
            LogInfo("RPC authentication cookie file generation is disabled.");
        } else {
            LogInfo("Using random cookie authentication.");
        }
    } else {
        LogPrintf("Config options rpcuser and rpcpassword will soon be deprecated. Locally-run instances may remove rpcuser to use cookie-based auth, or may be replaced with rpcauth. Please see share/rpcauth for rpcauth auth generation.\n");
        strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", "");
    }

    if (!gArgs.GetArgs("-rpcauth").empty()) {
        LogInfo("Using rpcauth authentication.\n");
        for (const std::string& rpcauth : gArgs.GetArgs("-rpcauth")) {
            std::vector<std::string> fields{SplitString(rpcauth, ':')};
            const std::vector<std::string> salt_hmac{SplitString(fields.back(), '$')};
            if (fields.size() == 2 && salt_hmac.size() == 2) {
                fields.pop_back();
                fields.insert(fields.end(), salt_hmac.begin(), salt_hmac.end());
                g_rpcauth.push_back(fields);
            } else {
                LogPrintf("Invalid -rpcauth argument.\n");
                return false;
            }
        }
    }

    g_rpc_whitelist_default = gArgs.GetBoolArg("-rpcwhitelistdefault", !gArgs.GetArgs("-rpcwhitelist").empty());
    for (const std::string& strRPCWhitelist : gArgs.GetArgs("-rpcwhitelist")) {
        auto pos = strRPCWhitelist.find(':');
        std::string strUser = strRPCWhitelist.substr(0, pos);
        bool intersect = g_rpc_whitelist.count(strUser);
        std::set<std::string>& whitelist = g_rpc_whitelist[strUser];
        if (pos != std::string::npos) {

Review comment (security):
This looks like a new restriction. Could it introduce a breakage for users that have `-rpcpassword=contains:colon`?

It is sub-optimal to join/craft the string "user:pass" and to split/parse it a few lines later. Better store the user and pass in separate variables, like this:

<details>
<summary>[patch] use separate variables for user and pass</summary>

```diff
diff --git i/src/httprpc.cpp w/src/httprpc.cpp
index ed5a0253c6..493f1b0320 100644
--- i/src/httprpc.cpp
+++ w/src/httprpc.cpp
@@ -275,13 +275,14 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
     }
     return true;
 }
 
 static bool InitRPCAuthentication()
 {
-    std::string user_colon_pass;
+    std::string user;
+    std::string pass;
 
     if (gArgs.GetArg("-rpcpassword", "") == "")
     {
         std::optional<fs::perms> cookie_perms{std::nullopt};
         auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")};
         if (cookie_perms_arg) {
@@ -290,36 +291,31 @@ static bool InitRPCAuthentication()
                 LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg);
                 return false;
             }
             cookie_perms = *perm_opt;
         }
 
-        if (!GenerateAuthCookie(&user_colon_pass, cookie_perms)) {
+        switch (GenerateAuthCookie(cookie_perms, user, pass)) {
+        case GenerateAuthCookieResult::ERROR:
             return false;
-        }
-        if (user_colon_pass.empty()) {
+        case GenerateAuthCookieResult::DISABLED:
             LogInfo("RPC authentication cookie file generation is disabled.");
-        } else {
+            break;
+        case GenerateAuthCookieResult::OK:
             LogInfo("Using random cookie authentication.");
+            break;
         }
     } else {
         LogInfo("Using rpcuser/rpcpassword authentication.");
         LogWarning("The use of rpcuser/rpcpassword is less secure, because credentials are configured in plain text. It is recommended that locally-run instances switch to cookie-based auth, or otherwise to use hashed rpcauth credentials. See share/rpcauth for more information.");
-        user_colon_pass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", "");
+        user = gArgs.GetArg("-rpcuser", "");
+        pass = gArgs.GetArg("-rpcpassword", "");
     }
 
     // If there is a plaintext credential, hash it with a random salt before storage.
-    if (!user_colon_pass.empty()) {
-        std::vector<std::string> fields{SplitString(user_colon_pass, ':')};
-        if (fields.size() != 2) {
-            LogError("Unable to parse RPC credentials. The configured rpcuser or rpcpassword cannot contain a \":\".");
-            return false;
-        }
-        const std::string& user = fields[0];
-        const std::string& pass = fields[1];
-
+    if (!user.empty() || !pass.empty()) {
         // Generate a random 16 byte hex salt.
         std::array<unsigned char, 16> raw_salt;
         GetStrongRandBytes(raw_salt);
         std::string salt = HexStr(raw_salt);
 
         // Compute HMAC.
diff --git i/src/rpc/request.cpp w/src/rpc/request.cpp
index 30f4a4f51b..249c6cb621 100644
--- i/src/rpc/request.cpp
+++ w/src/rpc/request.cpp
@@ -94,56 +94,59 @@ static fs::path GetAuthCookieFile(bool temp=false)
     }
     return AbsPathForConfigVal(gArgs, arg);
 }
 
 static bool g_generated_cookie = false;
 
-bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms)
+GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& cookie_perms,
+                                            std::string& user,
+                                            std::string& pass)
 {
     const size_t COOKIE_SIZE = 32;
     unsigned char rand_pwd[COOKIE_SIZE];
     GetRandBytes(rand_pwd);
-    std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd);
+    const std::string rand_pwd_hex{HexStr(rand_pwd)};
+    std::string cookie = COOKIEAUTH_USER + ":" + rand_pwd_hex;
 
     /** the umask determines what permissions are used to create this file -
      * these are set to 0077 in common/system.cpp.
      */
     std::ofstream file;
     fs::path filepath_tmp = GetAuthCookieFile(true);
     if (filepath_tmp.empty()) {
-        return true; // -norpccookiefile
+        return GenerateAuthCookieResult::DISABLED; // -norpccookiefile
     }
     file.open(filepath_tmp);
     if (!file.is_open()) {
         LogWarning("Unable to open cookie authentication file %s for writing", fs::PathToString(filepath_tmp));
-        return false;
+        return GenerateAuthCookieResult::ERROR;
     }
     file << cookie;
     file.close();
 
     fs::path filepath = GetAuthCookieFile(false);
     if (!RenameOver(filepath_tmp, filepath)) {
         LogWarning("Unable to rename cookie authentication file %s to %s", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
-        return false;
+        return GenerateAuthCookieResult::ERROR;
     }
     if (cookie_perms) {
         std::error_code code;
         fs::permissions(filepath, cookie_perms.value(), fs::perm_options::replace, code);
         if (code) {
             LogWarning("Unable to set permissions on cookie authentication file %s", fs::PathToString(filepath));
-            return false;
+            return GenerateAuthCookieResult::ERROR;
         }
     }
 
     g_generated_cookie = true;
     LogInfo("Generated RPC authentication cookie %s\n", fs::PathToString(filepath));
     LogInfo("Permissions used for cookie: %s\n", PermsToSymbolicString(fs::status(filepath).permissions()));
 
-    if (cookie_out)
-        *cookie_out = cookie;
-    return true;
+    user = COOKIEAUTH_USER;
+    pass = rand_pwd_hex;
+    return GenerateAuthCookieResult::OK;
 }
 
 bool GetAuthCookie(std::string *cookie_out)
 {
     std::ifstream file;
     std::string cookie;
diff --git i/src/rpc/request.h w/src/rpc/request.h
index 24887e8691..e93df02ba6 100644
--- i/src/rpc/request.h
+++ w/src/rpc/request.h
@@ -20,14 +20,31 @@ enum class JSONRPCVersion {
 
 /** JSON-RPC 2.0 request, only used in bitcoin-cli **/
 UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id);
 UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version);
 UniValue JSONRPCError(int code, const std::string& message);
 
-/** Generate a new RPC authentication cookie and write it to disk */
-bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms=std::nullopt);
+enum class GenerateAuthCookieResult : uint8_t {
+    DISABLED, // -norpccookiefile
+    ERROR,
+    OK,
+};
+
+/**
+ * Generate a new RPC authentication cookie and write it to disk
+ * @param[in] cookie_perms Filesystem permissions to use for the cookie file.
+ * @param[out] user Generated username, only set if `OK` is returned.
+ * @param[out] pass Generated password, only set if `OK` is returned.
+ * @retval GenerateAuthCookieResult::DISABLED Authentication via cookie is disabled.
+ * @retval GenerateAuthCookieResult::ERROR Error occurred, auth data could not be saved to disk.
+ * @retval GenerateAuthCookieResult::OK Auth data was generated, saved to disk and in `user` and `pass`.
+ */
+GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& cookie_perms,
+                                            std::string& user,
+                                            std::string& pass);
+
 /** Read the RPC authentication cookie from disk */
 bool GetAuthCookie(std::string *cookie_out);
 /** Delete RPC authentication cookie from disk */
 void DeleteAuthCookie();
 /** Parse JSON-RPC batch reply into a vector */
 std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in);
```
</details>

Code after:
            }
            cookie_perms = *perm_opt;
        }

        switch (GenerateAuthCookie(cookie_perms, user, pass)) {
        case GenerateAuthCookieResult::ERR:
            return false;
        case GenerateAuthCookieResult::DISABLED:
            LogInfo("RPC authentication cookie file generation is disabled.");
            break;
        case GenerateAuthCookieResult::OK:
            LogInfo("Using random cookie authentication.");
            break;
        }
    } else {
        LogInfo("Using rpcuser/rpcpassword authentication.");
        LogWarning("The use of rpcuser/rpcpassword is less secure, because credentials are configured in plain text. It is recommended that locally-run instances switch to cookie-based auth, or otherwise to use hashed rpcauth credentials. See share/rpcauth in the source directory for more information.");
        user = gArgs.GetArg("-rpcuser", "");
        pass = gArgs.GetArg("-rpcpassword", "");
    }

    // If there is a plaintext credential, hash it with a random salt before storage.
    if (!user.empty() || !pass.empty()) {
        // Generate a random 16 byte hex salt.
        std::array<unsigned char, 16> raw_salt;
        GetStrongRandBytes(raw_salt);
        std::string salt = HexStr(raw_salt);

        // Compute HMAC.
        std::array<unsigned char, CHMAC_SHA256::OUTPUT_SIZE> out;
        CHMAC_SHA256(UCharCast(salt.data()), salt.size()).Write(UCharCast(pass.data()), pass.size()).Finalize(out.data());
        std::string hash = HexStr(out);

        g_rpcauth.push_back({user, salt, hash});
    }

    if (!gArgs.GetArgs("-rpcauth").empty()) {
        LogInfo("Using rpcauth authentication.\n");
        for (const std::string& rpcauth : gArgs.GetArgs("-rpcauth")) {
            std::vector<std::string> fields{SplitString(rpcauth, ':')};
            const std::vector<std::string> salt_hmac{SplitString(fields.back(), '$')};
            if (fields.size() == 2 && salt_hmac.size() == 2) {
                fields.pop_back();
                fields.insert(fields.end(), salt_hmac.begin(), salt_hmac.end());
                g_rpcauth.push_back(fields);
            } else {
                LogPrintf("Invalid -rpcauth argument.\n");
                return false;
            }
        }
    }