rpc: Avoid join-split roundtrip for user:pass for auth credentials

Author: vasild

src/httprpc.cpp

@@ -278,7 +278,8 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
 
 static bool InitRPCAuthentication()
 {
-    std::string user_colon_pass;
+    std::string user;
+    std::string pass;
 
     if (gArgs.GetArg("-rpcpassword", "") == "")
     {
@@ -293,30 +294,25 @@ static bool InitRPCAuthentication()
             cookie_perms = *perm_opt;
         }
 
-        if (!GenerateAuthCookie(&user_colon_pass, cookie_perms)) {
+        switch (GenerateAuthCookie(cookie_perms, user, pass)) {
+        case GenerateAuthCookieResult::ERR:
             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 in the source directory 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);

src/rpc/request.cpp

@@ -97,50 +97,52 @@ static fs::path GetAuthCookieFile(bool temp=false)
 
 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)};
 
     /** 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::ERR;
     }
-    file << cookie;
+    file << COOKIEAUTH_USER << ":" << rand_pwd_hex;
     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::ERR;
     }
     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::ERR;
         }
     }
 
     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)

src/rpc/request.h

@@ -23,8 +23,25 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params,
 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
+    ERR,
+    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 */