From 4ab9bedee9d86fdecaa4afbbf4214ca6c7d9a94e Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Tue, 6 May 2025 09:19:41 +0200 Subject: [PATCH 1/4] rpc: Undeprecate rpcuser/rpcpassword, change message to security warning Back in 2015, in #7044, we added configuration option `rpcauth` for multiple RPC users. At the same time the old settings for single-user configuration `rpcuser` and `rpcpassword` were "soon" to be deprecated. The main reason for this deprecation is that while `-rpcpassword` stores the password in plain text, `-rpcauth` stores a hash, so it doesn't appear in the configuration in plain text. As the options are still in active use, actually removing them is expected to be a hassle to many, and it's not clear that is worth it. As for the security risk, in many kinds of setups (no wallet, containerized, single-user-single-application, local-only, etc) it is an unlikely point of escalation. In the end, it is good to encourage secure practices, but it is the responsibility of the user. Log a clear warning but remove the deprecation notice. Closes #29240. --- src/httprpc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 6d2dffaf7a..0ec3ea9caa 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -314,7 +314,8 @@ static bool InitRPCAuthentication() 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"); + 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."); strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); } From 879a17bcb1a5eab2ff1841ce5f3762dcccecb0ba Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Tue, 6 May 2025 11:08:30 +0200 Subject: [PATCH 2/4] rpc: Store all credentials hashed in memory This gets rid of the special-casing of `strRPCUserColonPass` by hashing cookies as well as manually provided `-rpcuser`/`-rpcpassword` with a random salt before storing them. Also take the opportunity to modernize the surrounding code a bit. There should be no end-user visible differences in behavior. --- src/httprpc.cpp | 69 +++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 0ec3ea9caa..0c85ba9e83 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -69,8 +69,6 @@ private: }; -/* Pre-base64-encoded authentication token */ -static std::string strRPCUserColonPass; /* Stored RPC timer interface (for unregistration) */ static std::unique_ptr httpRPCTimerInterface; /* List of -rpcauth values */ @@ -101,31 +99,27 @@ static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCReq //This function checks username and password against -rpcauth //entries from config file. -static bool multiUserAuthorized(std::string strUserPass) +static bool CheckUserAuthorized(std::string_view user_pass) { - if (strUserPass.find(':') == std::string::npos) { + if (user_pass.find(':') == std::string::npos) { return false; } - std::string strUser = strUserPass.substr(0, strUserPass.find(':')); - std::string strPass = strUserPass.substr(strUserPass.find(':') + 1); + std::string_view user = user_pass.substr(0, user_pass.find(':')); + std::string_view pass = user_pass.substr(user_pass.find(':') + 1); - for (const auto& vFields : g_rpcauth) { - std::string strName = vFields[0]; - if (!TimingResistantEqual(strName, strUser)) { + for (const auto& fields : g_rpcauth) { + if (!TimingResistantEqual(std::string_view(fields[0]), user)) { continue; } - std::string strSalt = vFields[1]; - std::string strHash = vFields[2]; + const std::string& salt = fields[1]; + const std::string& hash = fields[2]; - static const unsigned int KEY_SIZE = 32; - unsigned char out[KEY_SIZE]; + std::array out; + CHMAC_SHA256(UCharCast(salt.data()), salt.size()).Write(UCharCast(pass.data()), pass.size()).Finalize(out.data()); + std::string hash_from_pass = HexStr(out); - CHMAC_SHA256(reinterpret_cast(strSalt.data()), strSalt.size()).Write(reinterpret_cast(strPass.data()), strPass.size()).Finalize(out); - std::vector hexvec(out, out+KEY_SIZE); - std::string strHashFromPass = HexStr(hexvec); - - if (TimingResistantEqual(strHashFromPass, strHash)) { + if (TimingResistantEqual(hash_from_pass, hash)) { return true; } } @@ -145,12 +139,7 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna if (strUserPass.find(':') != std::string::npos) strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':')); - // Check if authorized under single-user field. - // (strRPCUserColonPass is empty when -norpccookiefile is specified). - if (!strRPCUserColonPass.empty() && TimingResistantEqual(strUserPass, strRPCUserColonPass)) { - return true; - } - return multiUserAuthorized(strUserPass); + return CheckUserAuthorized(strUserPass); } static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) @@ -291,6 +280,8 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) static bool InitRPCAuthentication() { + std::string user_colon_pass; + if (gArgs.GetArg("-rpcpassword", "") == "") { std::optional cookie_perms{std::nullopt}; @@ -304,11 +295,10 @@ static bool InitRPCAuthentication() cookie_perms = *perm_opt; } - assert(strRPCUserColonPass.empty()); // Only support initializing once - if (!GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)) { + if (!GenerateAuthCookie(&user_colon_pass, cookie_perms)) { return false; } - if (strRPCUserColonPass.empty()) { + if (user_colon_pass.empty()) { LogInfo("RPC authentication cookie file generation is disabled."); } else { LogInfo("Using random cookie authentication."); @@ -316,7 +306,30 @@ static bool InitRPCAuthentication() } 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."); - strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); + user_colon_pass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); + } + + // If there is a plaintext credential, hash it with a random salt before storage. + if (!user_colon_pass.empty()) { + std::vector 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]; + + // Generate a random 16 byte hex salt. + std::array raw_salt; + GetStrongRandBytes(raw_salt); + std::string salt = HexStr(raw_salt); + + // Compute HMAC. + std::array 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()) { From 98ff38a6f1a8a1e214bd3905a2dcac31ae6c2f52 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Tue, 6 May 2025 15:42:06 +0200 Subject: [PATCH 3/4] rpc: Perform HTTP user:pass split once in `RPCAuthorized` --- src/httprpc.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 0c85ba9e83..8c7cd74625 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -99,14 +99,8 @@ static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCReq //This function checks username and password against -rpcauth //entries from config file. -static bool CheckUserAuthorized(std::string_view user_pass) +static bool CheckUserAuthorized(std::string_view user, std::string_view pass) { - if (user_pass.find(':') == std::string::npos) { - return false; - } - std::string_view user = user_pass.substr(0, user_pass.find(':')); - std::string_view pass = user_pass.substr(user_pass.find(':') + 1); - for (const auto& fields : g_rpcauth) { if (!TimingResistantEqual(std::string_view(fields[0]), user)) { continue; @@ -136,10 +130,14 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna if (!userpass_data) return false; strUserPass.assign(userpass_data->begin(), userpass_data->end()); - if (strUserPass.find(':') != std::string::npos) - strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':')); - - return CheckUserAuthorized(strUserPass); + size_t colon_pos = strUserPass.find(':'); + if (colon_pos == std::string::npos) { + return false; // Invalid basic auth. + } + std::string user = strUserPass.substr(0, colon_pos); + std::string pass = strUserPass.substr(colon_pos + 1); + strAuthUsernameOut = user; + return CheckUserAuthorized(user, pass); } static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) From e49a7274a2141dcb9e188bc4b45c2d7b928ccecd Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 9 May 2025 16:44:34 +0200 Subject: [PATCH 4/4] rpc: Avoid join-split roundtrip for user:pass for auth credentials --- src/httprpc.cpp | 26 +++++++++++--------------- src/rpc/request.cpp | 22 ++++++++++++---------- src/rpc/request.h | 21 +++++++++++++++++++-- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 8c7cd74625..afdb6784ae 100644 --- a/src/httprpc.cpp +++ b/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 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 raw_salt; GetStrongRandBytes(raw_salt); diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index 30f4a4f51b..79cab2d8d2 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -97,12 +97,14 @@ static fs::path GetAuthCookieFile(bool temp=false) static bool g_generated_cookie = false; -bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie_perms) +GenerateAuthCookieResult GenerateAuthCookie(const std::optional& 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. @@ -110,27 +112,27 @@ bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie 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; } } @@ -138,9 +140,9 @@ bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie 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) diff --git a/src/rpc/request.h b/src/rpc/request.h index 24887e8691..2fb234c197 100644 --- a/src/rpc/request.h +++ b/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 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 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& 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 */