forked from syscoin/syscoin
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge bitcoin#12763: Add RPC Whitelist Feature from bitcoin#12248
2081442 test: Add test for rpc_whitelist (Emil Engler) 7414d38 Add RPC Whitelist Feature from bitcoin#12248 (Jeremy Rubin) Pull request description: Summary ==== This patch adds the RPC whitelisting feature requested in bitcoin#12248. RPC Whitelists help enforce application policies for services being built on top of Bitcoin Core (e.g., your Lightning Node maybe shouldn't be adding new peers). The aim of this PR is not to make it advisable to connect your Bitcoin node to arbitrary services, but to reduce risk and prevent unintended access. Using RPC Whitelists ==== The way it works is you specify (in your bitcoin.conf) configurations such as ``` rpcauth=user1:4cc74397d6e9972e5ee7671fd241$11849357f26a5be7809c68a032bc2b16ab5dcf6348ef3ed1cf30dae47b8bcc71 rpcauth=user2:181b4a25317bff60f3749adee7d6bca0$d9c331474f1322975fa170a2ffbcb176ba11644211746b27c1d317f265dd4ada rpcauth=user3:a6c8a511b53b1edcf69c36984985e$13cfba0e626db19061c9d61fa58e712d0319c11db97ad845fa84517f454f6675 rpcwhitelist=user1:getnetworkinfo rpcwhitelist=user2:getnetworkinfo,getwalletinfo, getbestblockhash rpcwhitelistdefault=0 ``` Now user1 can only call getnetworkinfo, user2 can only call getnetworkinfo or getwalletinfo, while user3 can still call all RPCs. If any rpcwhitelist is set, act as if all users are subject to whitelists unless rpcwhitelistdefault is set to 0. If rpcwhitelistdefault is set to 1 and no rpcwhitelist is set, act as if all users are subject to whitelists. Review Request ===== In addition to normal review, would love specific review from someone working on LN (e.g., @ roasbeef) and someone working on an infrastructure team at an exchange (e.g., @ jimpo) to check that this works well with their system. Notes ===== The rpc list is spelling sensitive -- whitespace is stripped though. Spelling errors fail towards the RPC call being blocked, which is safer. It was unclear to me if HTTPReq_JSONRPC is the best function to patch this functionality into, or if it would be better to place it in exec or somewhere else. It was also unclear to me if it would be preferred to cache the whitelists on startup or parse them on every RPC as is done with multiUserAuthorized. I opted for the cached approach as I thought it was a bit cleaner. Future Work ===== In a future PR, I would like to add an inheritance scheme. This seemed more controversial so I didn't want to include that here. Inheritance semantics are tricky, but it would also make these whitelists easier to read. It also might be good to add a `getrpcwhitelist` command to facilitate permission discovery. Tests ===== Thanks to @ emilengler for adding tests for this feature. The tests cover all cases except for where `rpcwhitelistdefault=1` is used, given difficulties around testing with the current test framework. ACKs for top commit: laanwj: ACK 2081442 Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
- Loading branch information
Showing
4 changed files
with
162 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| #!/usr/bin/env python3 | ||
| # Copyright (c) 2017-2019 The Bitcoin Core developers | ||
| # Distributed under the MIT software license, see the accompanying | ||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
| """ | ||
| A test for RPC users with restricted permissions | ||
| """ | ||
| from test_framework.test_framework import BitcoinTestFramework | ||
| import os | ||
| from test_framework.util import ( | ||
| get_datadir_path, | ||
| assert_equal, | ||
| str_to_b64str | ||
| ) | ||
| import http.client | ||
| import urllib.parse | ||
|
|
||
| def rpccall(node, user, method): | ||
| url = urllib.parse.urlparse(node.url) | ||
| headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(user[0], user[3]))} | ||
| conn = http.client.HTTPConnection(url.hostname, url.port) | ||
| conn.connect() | ||
| conn.request('POST', '/', '{"method": "' + method + '"}', headers) | ||
| resp = conn.getresponse() | ||
| conn.close() | ||
| return resp | ||
|
|
||
|
|
||
| class RPCWhitelistTest(BitcoinTestFramework): | ||
| def set_test_params(self): | ||
| self.num_nodes = 1 | ||
|
|
||
| def setup_chain(self): | ||
| super().setup_chain() | ||
| # 0 => Username | ||
| # 1 => Password (Hashed) | ||
| # 2 => Permissions | ||
| # 3 => Password Plaintext | ||
| self.users = [ | ||
| ["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash,getblockcount,", "12345"], | ||
| ["user2", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount", "54321"] | ||
| ] | ||
| # For exceptions | ||
| self.strange_users = [ | ||
| # Test empty | ||
| ["strangedude", "62d67dffec03836edd698314f1b2be62$c2fb4be29bb0e3646298661123cf2d8629640979cabc268ef05ea613ab54068d", ":", "s7R4nG3R7H1nGZ"], | ||
| ["strangedude2", "575c012c7fe4b1e83b9d809412da3ef7$09f448d0acfc19924dd62ecb96004d3c2d4b91f471030dfe43c6ea64a8f658c1", "", "s7R4nG3R7H1nGZ"], | ||
| # Test trailing comma | ||
| ["strangedude3", "23189c561b5975a56f4cf94030495d61$3a2f6aac26351e2257428550a553c4c1979594e36675bbd3db692442387728c0", ":getblockcount,", "s7R4nG3R7H1nGZ"], | ||
| # Test overwrite | ||
| ["strangedude4", "990c895760a70df83949e8278665e19a$8f0906f20431ff24cb9e7f5b5041e4943bdf2a5c02a19ef4960dcf45e72cde1c", ":getblockcount, getbestblockhash", "s7R4nG3R7H1nGZ"], | ||
| ["strangedude4", "990c895760a70df83949e8278665e19a$8f0906f20431ff24cb9e7f5b5041e4943bdf2a5c02a19ef4960dcf45e72cde1c", ":getblockcount", "s7R4nG3R7H1nGZ"], | ||
| # Testing the same permission twice | ||
| ["strangedude5", "d12c6e962d47a454f962eb41225e6ec8$2dd39635b155536d3c1a2e95d05feff87d5ba55f2d5ff975e6e997a836b717c9", ":getblockcount,getblockcount", "s7R4nG3R7H1nGZ"] | ||
| ] | ||
| # These commands shouldn't be allowed for any user to test failures | ||
| self.never_allowed = ["getnetworkinfo"] | ||
| with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f: | ||
| f.write("\nrpcwhitelistdefault=0\n") | ||
| for user in self.users: | ||
| f.write("rpcauth=" + user[0] + ":" + user[1] + "\n") | ||
| f.write("rpcwhitelist=" + user[0] + ":" + user[2] + "\n") | ||
| # Special cases | ||
| for strangedude in self.strange_users: | ||
| f.write("rpcauth=" + strangedude[0] + ":" + strangedude[1] + "\n") | ||
| f.write("rpcwhitelist=" + strangedude[0] + strangedude[2] + "\n") | ||
|
|
||
|
|
||
| def run_test(self): | ||
| for user in self.users: | ||
| permissions = user[2].replace(" ", "").split(",") | ||
| # Pop all empty items | ||
| i = 0 | ||
| while i < len(permissions): | ||
| if permissions[i] == '': | ||
| permissions.pop(i) | ||
|
|
||
| i += 1 | ||
| for permission in permissions: | ||
| self.log.info("[" + user[0] + "]: Testing a permitted permission (" + permission + ")") | ||
| assert_equal(200, rpccall(self.nodes[0], user, permission).status) | ||
| for permission in self.never_allowed: | ||
| self.log.info("[" + user[0] + "]: Testing a non permitted permission (" + permission + ")") | ||
| assert_equal(403, rpccall(self.nodes[0], user, permission).status) | ||
| # Now test the strange users | ||
| for permission in self.never_allowed: | ||
| self.log.info("Strange test 1") | ||
| assert_equal(403, rpccall(self.nodes[0], self.strange_users[0], permission).status) | ||
| for permission in self.never_allowed: | ||
| self.log.info("Strange test 2") | ||
| assert_equal(403, rpccall(self.nodes[0], self.strange_users[1], permission).status) | ||
| self.log.info("Strange test 3") | ||
| assert_equal(200, rpccall(self.nodes[0], self.strange_users[2], "getblockcount").status) | ||
| self.log.info("Strange test 4") | ||
| assert_equal(403, rpccall(self.nodes[0], self.strange_users[3], "getbestblockhash").status) | ||
| self.log.info("Strange test 5") | ||
| assert_equal(200, rpccall(self.nodes[0], self.strange_users[4], "getblockcount").status) | ||
|
|
||
| if __name__ == "__main__": | ||
| RPCWhitelistTest().main() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters