mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-21 14:34:49 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#30401: fix: increase consistency of rpcauth parsing

Browse source 
27c976d11a fix: increase consistency of rpcauth parsing (tdb3)
2ad3689512 test: add norpcauth test (tdb3)
67df0dec1a test: blank rpcauth CLI interaction (tdb3)
ecc98ccff2 test: add cases for blank rpcauth (tdb3)

Pull request description:

  The current `rpcauth` parsing behavior is inconsistent and unintuitive (see https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251 and additional details below).
  The current behavior inconsistently treats empty `rpcauth` as an error (or not) depending on the location within CLI/bitcoin.conf and the location of adjacent valid `rpcauth` params.

  Empty `rpcauth` is now consistently treated as an error and prevents bitcoind from starting.
  Continuation of the upforgrabs PR #29141.

  ### Additional details:
  Current `rpcauth` behavior is nonsensical:

   - If an empty `rpcauth` argument was specified as the last command line argument, it would cause all other `rpcauth` arguments to be ignored.
   - If an empty `rpcauth` argument was specified on the command line followed by any nonempty `rpcauth` argument, it would cause an error.
   - If an empty `rpcauth=` line was specified after non-empty rpcauth line in the config file it would cause an error.
   - If an empty `rpcauth=` line in a config file was first it would cause other rpcauth entries in the config file to be ignored, unless there were `-rpcauth` command line arguments and the last one was nonempty, in which case it would cause an error.

  New behavior is simple:
   - If an empty rpcauth config line or command line argument is used it will cause an error

ACKs for top commit:
  naiyoma:
    Tested ACK  [27c976d11a)
  achow101:
    ACK 27c976d11a
  ryanofsky:
    Code review ACK 27c976d11a. Since last review commit message was just tweaked to clarify previous behavior.

Tree-SHA512: af2e9dd60d1ad030409ae2c3805ab139c7435327823d9f8bbeede815f376cb696a5929b08a6e8c8b5f7278ed49cfb231789f9041bd57f1f03ec96501b669da5b
This commit is contained in:
Ava Chow 2024-09-09 12:29:17 -04:00
commit 746f88000e
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
2 changed files with 22 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/httprpc.cpp
View file

@ -314,8 +314,9 @@ static bool InitRPCAuthentication()
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");
strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", "");
}
if (gArgs.GetArg("-rpcauth", "") != "") {
LogPrintf("Using rpcauth authentication.\n");
if (!gArgs.GetArgs("-rpcauth").empty()) {
LogInfo("Using rpcauth authentication.\n");
for (const std::string& rpcauth : gArgs.GetArgs("-rpcauth")) {
std::vector<std::string> fields{SplitString(rpcauth, ':')};
const std::vector<std::string> salt_hmac{SplitString(fields.back(), '$')};

21
test/functional/rpc_users.py
View file

@ -139,15 +139,32 @@ class HTTPBasicsTest(BitcoinTestFramework):
init_error = 'Error: Unable to start HTTP server. See debug log for details.'
self.log.info('Check -rpcauth are validated')
# Empty -rpcauth= are ignored
self.restart_node(0, extra_args=['-rpcauth='])
self.log.info('Empty -rpcauth are treated as error')
self.stop_node(0)
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth'])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth='])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=""'])
self.log.info('Check malformed -rpcauth')
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo'])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo:bar'])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo:bar:baz'])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo$bar:baz'])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo$bar$baz'])
self.log.info('Check interactions between blank and non-blank rpcauth')
# pw = bitcoin
rpcauth_user1 = '-rpcauth=user1:6dd184e5e69271fdd69103464630014f$eb3d7ce67c4d1ff3564270519b03b636c0291012692a5fa3dd1d2075daedd07b'
rpcauth_user2 = '-rpcauth=user2:57b2f77c919eece63cfa46c2f06e46ae$266b63902f99f97eeaab882d4a87f8667ab84435c3799f2ce042ef5a994d620b'
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=[rpcauth_user1, rpcauth_user2, '-rpcauth='])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=[rpcauth_user1, '-rpcauth=', rpcauth_user2])
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=', rpcauth_user1, rpcauth_user2])
self.log.info('Check -norpcauth disables previous -rpcauth params')
self.restart_node(0, extra_args=[rpcauth_user1, rpcauth_user2, '-norpcauth'])
assert_equal(401, call_with_auth(self.nodes[0], 'user1', 'bitcoin').status)
assert_equal(401, call_with_auth(self.nodes[0], 'rt', self.rtpassword).status)
self.stop_node(0)
self.log.info('Check that failure to write cookie file will abort the node gracefully')
(self.nodes[0].chain_path / ".cookie.tmp").mkdir()
self.nodes[0].assert_start_raises_init_error(expected_msg=init_error)