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 --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](/bitcoin-bitcoin/contributor/param/)[in] cookie_perms Filesystem permissions to use for the cookie file.
+ * [@param](/bitcoin-bitcoin/contributor/param/)[out] user Generated username, only set if `OK` is returned.
+ * [@param](/bitcoin-bitcoin/contributor/param/)[out] pass Generated password, only set if `OK` is returned.
+ * [@retval](/bitcoin-bitcoin/contributor/retval/) GenerateAuthCookieResult::DISABLED Authentication via cookie is disabled.
+ * [@retval](/bitcoin-bitcoin/contributor/retval/) GenerateAuthCookieResult::ERROR Error occurred, auth data could not be saved to disk.
+ * [@retval](/bitcoin-bitcoin/contributor/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>
