From d91d0227aeecf620be4a803acc057b61dcb9d37b Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 23 Dec 2023 18:41:15 +0000 Subject: [PATCH 1/3] Bugfix: RPC: Check for blank rpcauth on a per-param basis Without this, a blank rpcauth will error if there is a non-blank following it, and a non-blank rpcauth will get ignored if the last one is blank --- src/httprpc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 4e7e72b037..dd2bf2f2c3 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -250,9 +250,10 @@ 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", "") != "") { + if (!gArgs.GetArgs("-rpcauth").empty()) { LogPrintf("Using rpcauth authentication.\n"); for (const std::string& rpcauth : gArgs.GetArgs("-rpcauth")) { + if (rpcauth.empty()) continue; std::vector fields{SplitString(rpcauth, ':')}; const std::vector salt_hmac{SplitString(fields.back(), '$')}; if (fields.size() == 2 && salt_hmac.size() == 2) { From 415734ebaf9d2279e95943e7cc9273e46537e9a6 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 23 Dec 2023 18:47:32 +0000 Subject: [PATCH 2/3] QA: rpc_users: Test blank rpcauth in combination with non-blank --- test/functional/rpc_users.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 1a35a57802..7055797e68 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -103,7 +103,10 @@ class HTTPBasicsTest(BitcoinTestFramework): self.log.info('Check -rpcauth are validated') # Empty -rpcauth= are ignored + self.restart_node(0, extra_args=['-rpcauth=', '-rpcauth=abc:$2e32c2f20c67e29c328dd64a4214180f18da9e667d67c458070fd856f1e9e5e7']) self.restart_node(0, extra_args=['-rpcauth=']) + # ...without disrupting usage of other -rpcauth tokens + self.test_auth(self.nodes[0], 'rt', self.rtpassword) self.stop_node(0) 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']) From 6b79de4f9aed365adda0a99d6522e83ccb6c593f Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Tue, 26 Dec 2023 21:54:57 +0000 Subject: [PATCH 3/3] QA: rpc_users: Test behaviour of -norpcauth --- test/functional/rpc_users.py | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 7055797e68..0500d0d8f5 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -101,12 +101,35 @@ 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=', '-rpcauth=abc:$2e32c2f20c67e29c328dd64a4214180f18da9e667d67c458070fd856f1e9e5e7']) - self.restart_node(0, extra_args=['-rpcauth=']) + self.log.info('Check blank -rpcauth is ignored') + rpcauth_abc = '-rpcauth=abc:$2e32c2f20c67e29c328dd64a4214180f18da9e667d67c458070fd856f1e9e5e7' + rpcauth_def = '-rpcauth=def:$fd7adb152c05ef80dccf50a1fa4c05d5a3ec6da95575fc312ae7c5d091836351' + self.restart_node(0, extra_args=['-rpcauth']) + self.restart_node(0, extra_args=['-rpcauth=', rpcauth_abc]) + self.restart_node(0, extra_args=[rpcauth_def, '-rpcauth=']) # ...without disrupting usage of other -rpcauth tokens - self.test_auth(self.nodes[0], 'rt', self.rtpassword) + assert_equal(200, call_with_auth(self.nodes[0], 'def', 'abc').status) + assert_equal(200, call_with_auth(self.nodes[0], 'rt', self.rtpassword).status) + + self.log.info('Check -norpcauth disables all previous -rpcauth params') + self.restart_node(0, extra_args=[rpcauth_def, '-norpcauth']) + assert_equal(401, call_with_auth(self.nodes[0], 'def', 'abc').status) + assert_equal(401, call_with_auth(self.nodes[0], 'rt', self.rtpassword).status) + + self.log.info('Check -norpcauth can be reversed with -rpcauth') + self.restart_node(0, extra_args=[rpcauth_def, '-norpcauth', '-rpcauth']) + # FIXME: assert_equal(200, call_with_auth(self.nodes[0], 'def', 'abc').status) + assert_equal(200, call_with_auth(self.nodes[0], 'rt', self.rtpassword).status) + + self.log.info('Check -norpcauth followed by a specific -rpcauth=* restores config file -rpcauth=* values too') + self.restart_node(0, extra_args=[rpcauth_def, '-norpcauth', rpcauth_abc]) + assert_equal(401, call_with_auth(self.nodes[0], 'def', 'abc').status) + assert_equal(200, call_with_auth(self.nodes[0], 'rt', self.rtpassword).status) + self.restart_node(0, extra_args=[rpcauth_def, '-norpcauth', '-rpcauth=']) + assert_equal(401, call_with_auth(self.nodes[0], 'def', 'abc').status) + assert_equal(200, call_with_auth(self.nodes[0], 'rt', self.rtpassword).status) + + self.log.info('Check -rpcauth are validated') self.stop_node(0) 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'])