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:
0diff --git i/src/httprpc.cpp w/src/httprpc.cpp
1index ed5a0253c6..493f1b0320 100644
2--- i/src/httprpc.cpp
3+++ w/src/httprpc.cpp
4@@ -275,13 +275,14 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
5 }
6 return true;
7 }
8
9 static bool InitRPCAuthentication()
10 {
11- std::string user_colon_pass;
12+ std::string user;
13+ std::string pass;
14
15 if (gArgs.GetArg("-rpcpassword", "") == "")
16 {
17 std::optional<fs::perms> cookie_perms{std::nullopt};
18 auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")};
19 if (cookie_perms_arg) {
20@@ -290,36 +291,31 @@ static bool InitRPCAuthentication()
21 LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg);
22 return false;
23 }
24 cookie_perms = *perm_opt;
25 }
26
27- if (!GenerateAuthCookie(&user_colon_pass, cookie_perms)) {
28+ switch (GenerateAuthCookie(cookie_perms, user, pass)) {
29+ case GenerateAuthCookieResult::ERROR:
30 return false;
31- }
32- if (user_colon_pass.empty()) {
33+ case GenerateAuthCookieResult::DISABLED:
34 LogInfo("RPC authentication cookie file generation is disabled.");
35- } else {
36+ break;
37+ case GenerateAuthCookieResult::OK:
38 LogInfo("Using random cookie authentication.");
39+ break;
40 }
41 } else {
42 LogInfo("Using rpcuser/rpcpassword authentication.");
43 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.");
44- user_colon_pass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", "");
45+ user = gArgs.GetArg("-rpcuser", "");
46+ pass = gArgs.GetArg("-rpcpassword", "");
47 }
48
49 // If there is a plaintext credential, hash it with a random salt before storage.
50- if (!user_colon_pass.empty()) {
51- std::vector<std::string> fields{SplitString(user_colon_pass, ':')};
52- if (fields.size() != 2) {
53- LogError("Unable to parse RPC credentials. The configured rpcuser or rpcpassword cannot contain a \":\".");
54- return false;
55- }
56- const std::string& user = fields[0];
57- const std::string& pass = fields[1];
58-
59+ if (!user.empty() || !pass.empty()) {
60 // Generate a random 16 byte hex salt.
61 std::array<unsigned char, 16> raw_salt;
62 GetStrongRandBytes(raw_salt);
63 std::string salt = HexStr(raw_salt);
64
65 // Compute HMAC.
66diff --git i/src/rpc/request.cpp w/src/rpc/request.cpp
67index 30f4a4f51b..249c6cb621 100644
68--- i/src/rpc/request.cpp
69+++ w/src/rpc/request.cpp
70@@ -94,56 +94,59 @@ static fs::path GetAuthCookieFile(bool temp=false)
71 }
72 return AbsPathForConfigVal(gArgs, arg);
73 }
74
75 static bool g_generated_cookie = false;
76
77-bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms)
78+GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& cookie_perms,
79+ std::string& user,
80+ std::string& pass)
81 {
82 const size_t COOKIE_SIZE = 32;
83 unsigned char rand_pwd[COOKIE_SIZE];
84 GetRandBytes(rand_pwd);
85- std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd);
86+ const std::string rand_pwd_hex{HexStr(rand_pwd)};
87+ std::string cookie = COOKIEAUTH_USER + ":" + rand_pwd_hex;
88
89 /** the umask determines what permissions are used to create this file -
90 * these are set to 0077 in common/system.cpp.
91 */
92 std::ofstream file;
93 fs::path filepath_tmp = GetAuthCookieFile(true);
94 if (filepath_tmp.empty()) {
95- return true; // -norpccookiefile
96+ return GenerateAuthCookieResult::DISABLED; // -norpccookiefile
97 }
98 file.open(filepath_tmp);
99 if (!file.is_open()) {
100 LogWarning("Unable to open cookie authentication file %s for writing", fs::PathToString(filepath_tmp));
101- return false;
102+ return GenerateAuthCookieResult::ERROR;
103 }
104 file << cookie;
105 file.close();
106
107 fs::path filepath = GetAuthCookieFile(false);
108 if (!RenameOver(filepath_tmp, filepath)) {
109 LogWarning("Unable to rename cookie authentication file %s to %s", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
110- return false;
111+ return GenerateAuthCookieResult::ERROR;
112 }
113 if (cookie_perms) {
114 std::error_code code;
115 fs::permissions(filepath, cookie_perms.value(), fs::perm_options::replace, code);
116 if (code) {
117 LogWarning("Unable to set permissions on cookie authentication file %s", fs::PathToString(filepath));
118- return false;
119+ return GenerateAuthCookieResult::ERROR;
120 }
121 }
122
123 g_generated_cookie = true;
124 LogInfo("Generated RPC authentication cookie %s\n", fs::PathToString(filepath));
125 LogInfo("Permissions used for cookie: %s\n", PermsToSymbolicString(fs::status(filepath).permissions()));
126
127- if (cookie_out)
128- *cookie_out = cookie;
129- return true;
130+ user = COOKIEAUTH_USER;
131+ pass = rand_pwd_hex;
132+ return GenerateAuthCookieResult::OK;
133 }
134
135 bool GetAuthCookie(std::string *cookie_out)
136 {
137 std::ifstream file;
138 std::string cookie;
139diff --git i/src/rpc/request.h w/src/rpc/request.h
140index 24887e8691..e93df02ba6 100644
141--- i/src/rpc/request.h
142+++ w/src/rpc/request.h
143@@ -20,14 +20,31 @@ enum class JSONRPCVersion {
144
145 /** JSON-RPC 2.0 request, only used in bitcoin-cli **/
146 UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id);
147 UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version);
148 UniValue JSONRPCError(int code, const std::string& message);
149
150-/** Generate a new RPC authentication cookie and write it to disk */
151-bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms=std::nullopt);
152+enum class GenerateAuthCookieResult : uint8_t {
153+ DISABLED, // -norpccookiefile
154+ ERROR,
155+ OK,
156+};
157+
158+/**
159+ * Generate a new RPC authentication cookie and write it to disk
160+ * [@param](/bitcoin-bitcoin/contributor/param/)[in] cookie_perms Filesystem permissions to use for the cookie file.
161+ * [@param](/bitcoin-bitcoin/contributor/param/)[out] user Generated username, only set if `OK` is returned.
162+ * [@param](/bitcoin-bitcoin/contributor/param/)[out] pass Generated password, only set if `OK` is returned.
163+ * [@retval](/bitcoin-bitcoin/contributor/retval/) GenerateAuthCookieResult::DISABLED Authentication via cookie is disabled.
164+ * [@retval](/bitcoin-bitcoin/contributor/retval/) GenerateAuthCookieResult::ERROR Error occurred, auth data could not be saved to disk.
165+ * [@retval](/bitcoin-bitcoin/contributor/retval/) GenerateAuthCookieResult::OK Auth data was generated, saved to disk and in `user` and `pass`.
166+ */
167+GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& cookie_perms,
168+ std::string& user,
169+ std::string& pass);
170+
171 /** Read the RPC authentication cookie from disk */
172 bool GetAuthCookie(std::string *cookie_out);
173 /** Delete RPC authentication cookie from disk */
174 void DeleteAuthCookie();
175 /** Parse JSON-RPC batch reply into a vector */
176 std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in);
