highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-04-07 14:18:18 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#34965: cli: Return more helpful authentication errors

Browse Source 
257769a7ce qa: Improve error message (Hodlinator)
20a94c1524 cli: Clearer error messages on authentication failure (Hodlinator)
84c3f8d325 refactor(rpc): GenerateAuthCookieResult -> AuthCookieResult (Hodlinator)

Pull request description:

  Increases precision of error messages to help the user correct authentication issues.

  Inspired by #34935.

ACKs for top commit:
  davidgumberg:
    utACK 257769a7ce
  maflcko:
    review ACK 257769a7ce 🦇
  achow101:
    ACK 257769a7ce
  janb84:
    concept ACK 257769a7ce

Tree-SHA512: 1799db4b2c0ab3b67ed3d768da08c6be4f4beaad91a77406884b73950b420c8264c70b8e60a26a9e6fac058370f6accdb73c821d19bebb6edfbc8d7b84d01232
This commit is contained in:
Ava Chow
2026-04-02 15:55:28 -07:00
parent 4b98962731 257769a7ce
commit fa1f4feac4
7 changed files with 61 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
src/bitcoin-cli.cpp
View File

@@ -902,15 +902,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<AuthCookieResult> 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());
@@ -918,7 +916,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";
@@ -943,13 +941,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)

6
src/httprpc.cpp
View File

@@ -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;
}

33
src/rpc/request.cpp
View File

@@ -97,9 +97,9 @@ static fs::path GetAuthCookieFile(bool temp=false)
static bool g_generated_cookie = false;
GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& cookie_perms,
std::string& user,
std::string& pass)
AuthCookieResult 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];
@@ -112,12 +112,12 @@ GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& 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<fs::perms>& 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,26 +142,23 @@ GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms>& cook
user = COOKIEAUTH_USER;
pass = rand_pwd_hex;
return GenerateAuthCookieResult::OK;
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()

18
src/rpc/request.h
View File

@@ -23,10 +23,10 @@ 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);
enum class GenerateAuthCookieResult : uint8_t {
DISABLED, // -norpccookiefile
ERR,
OK,
enum class AuthCookieResult : uint8_t {
Disabled, // -norpccookiefile
Error,
Ok,
};
/**
@@ -34,16 +34,16 @@ 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<fs::perms>& cookie_perms,
AuthCookieResult 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);
AuthCookieResult GetAuthCookie(std::string& cookie_out);
/** Delete RPC authentication cookie from disk */
void DeleteAuthCookie();
/** Parse JSON-RPC batch reply into a vector */

13
test/functional/interface_bitcoin_cli.py
View File

@@ -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())

4
test/functional/interface_ipc_cli.py
View File

@@ -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"

2
test/functional/test_framework/util.py
View File

@@ -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")