diff options
author | Ava Chow <github@achow101.com> | 2024-09-09 12:29:17 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-09-09 12:29:17 -0400 |
commit | 746f88000e8b219b1318839ca1cd7fd9f2057793 (patch) | |
tree | 77193f4ce27b7acd91653dd7eb65d7ee0f77651e /src | |
parent | df86a4f333e13a4799945ba79e572ccfefff2283 (diff) | |
parent | 27c976d11a68d66db97d9a7a30c6a6a71c6ab586 (diff) |
Merge bitcoin/bitcoin#30401: fix: increase consistency of rpcauth parsing
27c976d11a68d66db97d9a7a30c6a6a71c6ab586 fix: increase consistency of rpcauth parsing (tdb3)
2ad3689512a36eaff957df9ac28e65b2fedbc36c test: add norpcauth test (tdb3)
67df0dec1abe547773e532aa60aff0317e018123 test: blank rpcauth CLI interaction (tdb3)
ecc98ccff25b7e758337e764e59d764076772fec 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 [https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586](https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586)
achow101:
ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586
ryanofsky:
Code review ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586. Since last review commit message was just tweaked to clarify previous behavior.
Tree-SHA512: af2e9dd60d1ad030409ae2c3805ab139c7435327823d9f8bbeede815f376cb696a5929b08a6e8c8b5f7278ed49cfb231789f9041bd57f1f03ec96501b669da5b
Diffstat (limited to 'src')
-rw-r--r-- | src/httprpc.cpp | 5 |
1 files changed, 3 insertions, 2 deletions
diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 05767a253f..69dd821dc0 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -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(), '$')}; |