From 84c3f8d325ec636921286fc9a33eb04a811ac16b Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Tue, 31 Mar 2026 10:08:18 +0200 Subject: [PATCH 1/3] refactor(rpc): GenerateAuthCookieResult -> AuthCookieResult Type will be used for reading the cookie in next commit. Also corrects ERR/ERROR mismatch in docstring in request.h, and changes to CamelCase to avoid potential collision with Windows headers (https://github.com/bitcoin/bitcoin/pull/34965#issuecomment-4161331392). --- src/httprpc.cpp | 6 +++--- src/rpc/request.cpp | 16 ++++++++-------- src/rpc/request.h | 16 ++++++++-------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 2a6751df332..9a076996656 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -264,12 +264,12 @@ static bool InitRPCAuthentication() } switch (GenerateAuthCookie(cookie_perms, user, pass)) { - case GenerateAuthCookieResult::ERR: + case AuthCookieResult::Error: return false; - case GenerateAuthCookieResult::DISABLED: + case AuthCookieResult::Disabled: LogInfo("RPC authentication cookie file generation is disabled."); break; - case GenerateAuthCookieResult::OK: + case AuthCookieResult::Ok: LogInfo("Using random cookie authentication."); break; } diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index 1798cdef235..cf7109a4376 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -97,9 +97,9 @@ static fs::path GetAuthCookieFile(bool temp=false) static bool g_generated_cookie = false; -GenerateAuthCookieResult GenerateAuthCookie(const std::optional& cookie_perms, - std::string& user, - std::string& pass) +AuthCookieResult GenerateAuthCookie(const std::optional& cookie_perms, + std::string& user, + std::string& pass) { const size_t COOKIE_SIZE = 32; unsigned char rand_pwd[COOKIE_SIZE]; @@ -112,12 +112,12 @@ GenerateAuthCookieResult GenerateAuthCookie(const std::optional& cook std::ofstream file; fs::path filepath_tmp = GetAuthCookieFile(true); if (filepath_tmp.empty()) { - return GenerateAuthCookieResult::DISABLED; // -norpccookiefile + return AuthCookieResult::Disabled; // -norpccookiefile } file.open(filepath_tmp.std_path()); if (!file.is_open()) { LogWarning("Unable to open cookie authentication file %s for writing", fs::PathToString(filepath_tmp)); - return GenerateAuthCookieResult::ERR; + return AuthCookieResult::Error; } file << COOKIEAUTH_USER << ":" << rand_pwd_hex; file.close(); @@ -125,14 +125,14 @@ GenerateAuthCookieResult GenerateAuthCookie(const std::optional& cook 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 GenerateAuthCookieResult::ERR; + return AuthCookieResult::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 GenerateAuthCookieResult::ERR; + return AuthCookieResult::Error; } } @@ -142,7 +142,7 @@ GenerateAuthCookieResult GenerateAuthCookie(const std::optional& cook user = COOKIEAUTH_USER; pass = rand_pwd_hex; - return GenerateAuthCookieResult::OK; + return AuthCookieResult::Ok; } bool GetAuthCookie(std::string *cookie_out) diff --git a/src/rpc/request.h b/src/rpc/request.h index 4588db5122d..357f5d7ef0a 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -23,10 +23,10 @@ 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); -enum class GenerateAuthCookieResult : uint8_t { - DISABLED, // -norpccookiefile - ERR, - OK, +enum class AuthCookieResult : uint8_t { + Disabled, // -norpccookiefile + Error, + Ok, }; /** @@ -34,11 +34,11 @@ enum class GenerateAuthCookieResult : uint8_t { * @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`. + * @retval AuthCookieResult::Disabled Authentication via cookie is disabled. + * @retval AuthCookieResult::Error Error occurred, auth data could not be saved to disk. + * @retval AuthCookieResult::Ok Auth data was generated, saved to disk and in `user` and `pass`. */ -GenerateAuthCookieResult GenerateAuthCookie(const std::optional& cookie_perms, +AuthCookieResult GenerateAuthCookie(const std::optional& cookie_perms, std::string& user, std::string& pass); From 20a94c1524cc1e1c275cd15a2bbd6731eb93131d Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:25:05 +0200 Subject: [PATCH 2/3] cli: Clearer error messages on authentication failure Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> --- src/bitcoin-cli.cpp | 33 +++++++++++++++--------- src/rpc/request.cpp | 17 +++++------- src/rpc/request.h | 2 +- test/functional/interface_bitcoin_cli.py | 13 +++++++--- test/functional/interface_ipc_cli.py | 4 +-- 5 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 24cb0344c58..2725842fce4 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -901,15 +901,13 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co evhttp_request_set_error_cb(req.get(), http_error_cb); // Get credentials - std::string strRPCUserColonPass; - bool failedToGetAuthCookie = false; + std::string rpc_credentials; + std::optional auth_cookie_result; if (gArgs.GetArg("-rpcpassword", "") == "") { // Try fall back to cookie-based authentication if no password is provided - if (!GetAuthCookie(&strRPCUserColonPass)) { - failedToGetAuthCookie = true; - } + auth_cookie_result = GetAuthCookie(rpc_credentials); } else { - strRPCUserColonPass = username + ":" + gArgs.GetArg("-rpcpassword", ""); + rpc_credentials = username + ":" + gArgs.GetArg("-rpcpassword", ""); } struct evkeyvalq* output_headers = evhttp_request_get_output_headers(req.get()); @@ -917,7 +915,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co evhttp_add_header(output_headers, "Host", host.c_str()); evhttp_add_header(output_headers, "Connection", "close"); evhttp_add_header(output_headers, "Content-Type", "application/json"); - evhttp_add_header(output_headers, "Authorization", (std::string("Basic ") + EncodeBase64(strRPCUserColonPass)).c_str()); + evhttp_add_header(output_headers, "Authorization", (std::string("Basic ") + EncodeBase64(rpc_credentials)).c_str()); // Attach request data std::string strRequest = rh->PrepareRequest(strMethod, args).write() + "\n"; @@ -942,13 +940,24 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co "Use \"bitcoin-cli -help\" for more info.", host, port, responseErrorMessage)); } else if (response.status == HTTP_UNAUTHORIZED) { - if (failedToGetAuthCookie) { - throw std::runtime_error(strprintf( - "Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (%s)", - fs::PathToString(gArgs.GetConfigFilePath()))); + std::string error{"Authorization failed: "}; + if (auth_cookie_result.has_value()) { + switch (*auth_cookie_result) { + case AuthCookieResult::Error: + error += "Failed to read cookie file and no rpcpassword was specified."; + break; + case AuthCookieResult::Disabled: + error += "Cookie file was disabled via -norpccookiefile and no rpcpassword was specified."; + break; + case AuthCookieResult::Ok: + error += "Cookie file credentials were invalid and no rpcpassword was specified."; + break; + } } else { - throw std::runtime_error("Authorization failed: Incorrect rpcuser or rpcpassword"); + error += "Incorrect rpcuser or rpcpassword were specified."; } + error += strprintf(" Configuration file: (%s)", fs::PathToString(gArgs.GetConfigFilePath())); + throw std::runtime_error(error); } else if (response.status == HTTP_SERVICE_UNAVAILABLE) { throw std::runtime_error(strprintf("Server response: %s", response.body)); } else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR) diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index cf7109a4376..34a9ff75535 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -145,23 +145,20 @@ AuthCookieResult GenerateAuthCookie(const std::optional& cookie_perms return AuthCookieResult::Ok; } -bool GetAuthCookie(std::string *cookie_out) +AuthCookieResult GetAuthCookie(std::string& cookie_out) { std::ifstream file; - std::string cookie; fs::path filepath = GetAuthCookieFile(); if (filepath.empty()) { - return true; // -norpccookiefile + return AuthCookieResult::Disabled; // -norpccookiefile } file.open(filepath.std_path()); - if (!file.is_open()) - return false; - std::getline(file, cookie); + if (!file.is_open()) { + return AuthCookieResult::Error; + } + std::getline(file, cookie_out); file.close(); - - if (cookie_out) - *cookie_out = cookie; - return true; + return AuthCookieResult::Ok; } void DeleteAuthCookie() diff --git a/src/rpc/request.h b/src/rpc/request.h index 357f5d7ef0a..00216562fcb 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -43,7 +43,7 @@ AuthCookieResult GenerateAuthCookie(const std::optional& cookie_perms std::string& pass); /** Read the RPC authentication cookie from disk */ -bool GetAuthCookie(std::string *cookie_out); +AuthCookieResult GetAuthCookie(std::string& cookie_out); /** Delete RPC authentication cookie from disk */ void DeleteAuthCookie(); /** Parse JSON-RPC batch reply into a vector */ diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index fbe753a65fa..953eea3d63c 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -139,11 +139,11 @@ class TestBitcoinCli(BitcoinTestFramework): self.log.info("Test -stdinrpcpass option") assert_equal(BLOCKS, self.nodes[0].cli(f'-rpcuser={user}', '-stdinrpcpass', input=password).getblockcount()) - assert_raises_process_error(1, 'Incorrect rpcuser or rpcpassword', self.nodes[0].cli(f'-rpcuser={user}', '-stdinrpcpass', input='foo').echo) + assert_raises_process_error(1, 'Incorrect rpcuser or rpcpassword were specified', self.nodes[0].cli(f'-rpcuser={user}', '-stdinrpcpass', input='foo').echo) self.log.info("Test -stdin and -stdinrpcpass") assert_equal(['foo', 'bar'], self.nodes[0].cli(f'-rpcuser={user}', '-stdin', '-stdinrpcpass', input=f'{password}\nfoo\nbar').echo()) - assert_raises_process_error(1, 'Incorrect rpcuser or rpcpassword', self.nodes[0].cli(f'-rpcuser={user}', '-stdin', '-stdinrpcpass', input='foo').echo) + assert_raises_process_error(1, 'Incorrect rpcuser or rpcpassword were specified', self.nodes[0].cli(f'-rpcuser={user}', '-stdin', '-stdinrpcpass', input='foo').echo) self.log.info("Test connecting to a non-existing server") assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcport=1').echo) @@ -196,7 +196,14 @@ class TestBitcoinCli(BitcoinTestFramework): self.nodes[0].replace_in_config([("#" + conf_rpcport, conf_rpcport)]) self.log.info("Test connecting with non-existing RPC cookie file") - assert_raises_process_error(1, "Could not locate RPC credentials", self.nodes[0].cli('-rpccookiefile=does-not-exist', '-rpcpassword=').echo) + assert_raises_process_error(1, "Failed to read cookie file and no rpcpassword was specified.", self.nodes[0].cli('-rpccookiefile=does-not-exist', '-rpcpassword=').echo) + + self.log.info("Test connecting with neither cookie file, nor password") + assert_raises_process_error(1, "Cookie file was disabled via -norpccookiefile and no rpcpassword was specified.", self.nodes[0].cli("-norpccookiefile").echo) + assert_raises_process_error(1, "Cookie file was disabled via -norpccookiefile and no rpcpassword was specified.", self.nodes[0].cli("-norpccookiefile", "-rpcpassword=").echo) + + self.log.info("Test connecting with invalid cookie file") + assert_raises_process_error(1, "Cookie file credentials were invalid and no rpcpassword was specified.", self.nodes[0].cli(f"-rpccookiefile={self.nodes[0].datadir_path / 'bitcoin.conf'}").echo) self.log.info("Test connecting without RPC cookie file and with password arg") assert_equal(BLOCKS, self.nodes[0].cli('-norpccookiefile', f'-rpcuser={user}', f'-rpcpassword={password}').getblockcount()) diff --git a/test/functional/interface_ipc_cli.py b/test/functional/interface_ipc_cli.py index 92ccc879fba..9ad704d3f01 100755 --- a/test/functional/interface_ipc_cli.py +++ b/test/functional/interface_ipc_cli.py @@ -31,7 +31,7 @@ class TestBitcoinIpcCli(BitcoinTestFramework): if error is None: assert_equal(result.stdout, '[\n "foo"\n]\n') else: - assert_equal(result.stdout, error) + assert result.stdout.startswith(error), f"Output didn't start with the expected error {error!r}:\n{result.stdout}" assert_equal(result.stderr, None) assert_equal(result.returncode, 0 if error is None else 1) @@ -40,7 +40,7 @@ class TestBitcoinIpcCli(BitcoinTestFramework): if node.ipc_tmp_dir: self.log.info("Skipping a few checks because temporary directory path is too long") - http_auth_error = "error: Authorization failed: Incorrect rpcuser or rpcpassword\n" + http_auth_error = "error: Authorization failed: Incorrect rpcuser or rpcpassword were specified." http_connect_error = f"error: timeout on transient error: Could not connect to the server 127.0.0.1:{rpc_port(node.index)}\n\nMake sure the bitcoind server is running and that you are connecting to the correct RPC port.\nUse \"bitcoin-cli -help\" for more info.\n" ipc_connect_error = "error: timeout on transient error: Connection refused\n\nProbably bitcoin-node is not running or not listening on a unix socket. Can be started with:\n\n bitcoin-node -chain=regtest -ipcbind=unix\n" ipc_http_conflict = "error: -rpcconnect and -ipcconnect options cannot both be enabled\n" From 257769a7ce8c8ca70e705569f29aac244ada45aa Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:16:24 +0200 Subject: [PATCH 3/3] qa: Improve error message --- test/functional/test_framework/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 2eb891cf6c9..8e1e3c716f6 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -136,7 +136,7 @@ def assert_raises_process_error(returncode: int, output: str, fun: Callable, *ar if returncode != e.returncode: raise AssertionError("Unexpected returncode %i" % e.returncode) if output not in e.output: - raise AssertionError("Expected substring not found:" + e.output) + raise AssertionError(f"Expected substring not found in: {e.output!r}") else: raise AssertionError("No exception raised")