From ecc98ccff25b7e758337e764e59d764076772fec Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:14:08 -0400 Subject: [PATCH 1/4] test: add cases for blank rpcauth --- test/functional/rpc_users.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 44187ce7905..49148b34ef2 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -139,9 +139,11 @@ 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.log.info('Empty -rpcauth are ignored') + self.restart_node(0, extra_args=['-rpcauth']) self.restart_node(0, extra_args=['-rpcauth=']) 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=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']) From 67df0dec1abe547773e532aa60aff0317e018123 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:27:55 -0400 Subject: [PATCH 2/4] test: blank rpcauth CLI interaction Tests interactions between blank and non-blank rpcauth args. --- test/functional/rpc_users.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 49148b34ef2..3fe4b7c5183 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -150,6 +150,15 @@ class HTTPBasicsTest(BitcoinTestFramework): 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.restart_node(0, extra_args=[rpcauth_user1, rpcauth_user2, '-rpcauth=']) # fixed in subsequent commit + self.stop_node(0) + 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 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) From 2ad3689512a36eaff957df9ac28e65b2fedbc36c Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:57:27 -0400 Subject: [PATCH 3/4] test: add norpcauth test Adds test for disabling rpcauth args. Co-Authored-By: Luke Dashjr --- test/functional/rpc_users.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 3fe4b7c5183..c5c9d3c86c4 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -159,6 +159,12 @@ class HTTPBasicsTest(BitcoinTestFramework): 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) From 27c976d11a68d66db97d9a7a30c6a6a71c6ab586 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Tue, 6 Aug 2024 20:54:59 -0400 Subject: [PATCH 4/4] fix: increase consistency of rpcauth parsing Previous rpcauth behavior was to sometimes ignore empty -rpcauth= settings, and other times treat them as errors. Empty rpcauth is now consistently treated as an error and prevents bitcoind from starting. Updates associated test cases. Also updates to non-deprecated logging macro. Co-Authored-By: Luke Dashjr Co-Authored-By: Ryan Ofsky --- src/httprpc.cpp | 5 +++-- test/functional/rpc_users.py | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index af809eaf384..1457b0cea33 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 fields{SplitString(rpcauth, ':')}; const std::vector salt_hmac{SplitString(fields.back(), '$')}; diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index c5c9d3c86c4..49eb64abada 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -139,11 +139,12 @@ class HTTPBasicsTest(BitcoinTestFramework): init_error = 'Error: Unable to start HTTP server. See debug log for details.' self.log.info('Check -rpcauth are validated') - self.log.info('Empty -rpcauth are ignored') - self.restart_node(0, extra_args=['-rpcauth']) - 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']) @@ -154,8 +155,7 @@ class HTTPBasicsTest(BitcoinTestFramework): # pw = bitcoin rpcauth_user1 = '-rpcauth=user1:6dd184e5e69271fdd69103464630014f$eb3d7ce67c4d1ff3564270519b03b636c0291012692a5fa3dd1d2075daedd07b' rpcauth_user2 = '-rpcauth=user2:57b2f77c919eece63cfa46c2f06e46ae$266b63902f99f97eeaab882d4a87f8667ab84435c3799f2ce042ef5a994d620b' - self.restart_node(0, extra_args=[rpcauth_user1, rpcauth_user2, '-rpcauth=']) # fixed in subsequent commit - self.stop_node(0) + 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])