Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
#!/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
|
|
|
|
)
|
Merge #6100: feat: make whitelist works with composite commands for platform needs
85abbb97b4cbc292d81a29e94f75ee20028e8186 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0ef88c6d24505699b0dfdea9891aa83 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a5978718f399aa71c22afad720f2f686eac6 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/66
https://github.com/dashpay/dash-issues/issues/65
## What was done?
Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs
This PR makes whitelist feature to be compatible with composite commands.
Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.
Platform at their side can use config such as this one (created based on shumkov's example):
```
rpc: {
host: '127.0.0.1',
port: 9998,
users: [
{
user: 'dashmate',
password: 'rpcpassword',
whitelist: null,
lowPriority: false,
},
{
username: 'platform-dapi',
password: 'rpcpassword',
whitelist: [],
lowPriority: true,
},
{
username: 'platform-drive-consensus',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
lowPriority: false,
},
{
username: 'platform-drive-other',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
],
lowPriority: true,
},
],
allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
},
```
## How Has This Been Tested?
Updated functional tests, see commits
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
LGTM, utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
PastaPastaPasta:
utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
2024-07-15 18:43:38 +02:00
|
|
|
import json
|
Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
import http.client
|
Merge #6100: feat: make whitelist works with composite commands for platform needs
85abbb97b4cbc292d81a29e94f75ee20028e8186 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0ef88c6d24505699b0dfdea9891aa83 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a5978718f399aa71c22afad720f2f686eac6 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/66
https://github.com/dashpay/dash-issues/issues/65
## What was done?
Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs
This PR makes whitelist feature to be compatible with composite commands.
Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.
Platform at their side can use config such as this one (created based on shumkov's example):
```
rpc: {
host: '127.0.0.1',
port: 9998,
users: [
{
user: 'dashmate',
password: 'rpcpassword',
whitelist: null,
lowPriority: false,
},
{
username: 'platform-dapi',
password: 'rpcpassword',
whitelist: [],
lowPriority: true,
},
{
username: 'platform-drive-consensus',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
lowPriority: false,
},
{
username: 'platform-drive-other',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
],
lowPriority: true,
},
],
allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
},
```
## How Has This Been Tested?
Updated functional tests, see commits
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
LGTM, utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
PastaPastaPasta:
utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
2024-07-15 18:43:38 +02:00
|
|
|
import re
|
Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
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()
|
Merge #6100: feat: make whitelist works with composite commands for platform needs
85abbb97b4cbc292d81a29e94f75ee20028e8186 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0ef88c6d24505699b0dfdea9891aa83 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a5978718f399aa71c22afad720f2f686eac6 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/66
https://github.com/dashpay/dash-issues/issues/65
## What was done?
Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs
This PR makes whitelist feature to be compatible with composite commands.
Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.
Platform at their side can use config such as this one (created based on shumkov's example):
```
rpc: {
host: '127.0.0.1',
port: 9998,
users: [
{
user: 'dashmate',
password: 'rpcpassword',
whitelist: null,
lowPriority: false,
},
{
username: 'platform-dapi',
password: 'rpcpassword',
whitelist: [],
lowPriority: true,
},
{
username: 'platform-drive-consensus',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
lowPriority: false,
},
{
username: 'platform-drive-other',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
],
lowPriority: true,
},
],
allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
},
```
## How Has This Been Tested?
Updated functional tests, see commits
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
LGTM, utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
PastaPastaPasta:
utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
2024-07-15 18:43:38 +02:00
|
|
|
|
|
|
|
# composite commands are presented without space in whitelist
|
|
|
|
# but space can't be ommitted when using CLI/http rpc
|
|
|
|
# for sack of test, substitute missing space for quorum composite command
|
|
|
|
params = []
|
|
|
|
if re.match(r"^quorum[^ ]", method):
|
|
|
|
params = [method[6:]]
|
|
|
|
method = "quorum"
|
|
|
|
query = {"method" : method, "params" : params}
|
|
|
|
|
|
|
|
conn.request('POST', '/', json.dumps(query), headers)
|
Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
resp = conn.getresponse()
|
|
|
|
conn.close()
|
|
|
|
return resp
|
|
|
|
|
|
|
|
|
|
|
|
class RPCWhitelistTest(BitcoinTestFramework):
|
|
|
|
def set_test_params(self):
|
|
|
|
self.num_nodes = 1
|
2022-06-06 20:49:25 +02:00
|
|
|
self.supports_cli = False
|
Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
|
|
|
|
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"],
|
Merge #6100: feat: make whitelist works with composite commands for platform needs
85abbb97b4cbc292d81a29e94f75ee20028e8186 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0ef88c6d24505699b0dfdea9891aa83 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a5978718f399aa71c22afad720f2f686eac6 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/66
https://github.com/dashpay/dash-issues/issues/65
## What was done?
Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs
This PR makes whitelist feature to be compatible with composite commands.
Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.
Platform at their side can use config such as this one (created based on shumkov's example):
```
rpc: {
host: '127.0.0.1',
port: 9998,
users: [
{
user: 'dashmate',
password: 'rpcpassword',
whitelist: null,
lowPriority: false,
},
{
username: 'platform-dapi',
password: 'rpcpassword',
whitelist: [],
lowPriority: true,
},
{
username: 'platform-drive-consensus',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
lowPriority: false,
},
{
username: 'platform-drive-other',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
],
lowPriority: true,
},
],
allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
},
```
## How Has This Been Tested?
Updated functional tests, see commits
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
LGTM, utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
PastaPastaPasta:
utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
2024-07-15 18:43:38 +02:00
|
|
|
["user2", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount", "54321"],
|
|
|
|
["platform-user", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount,quorumlist", "54321"],
|
Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
]
|
|
|
|
# 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
|
Merge #6100: feat: make whitelist works with composite commands for platform needs
85abbb97b4cbc292d81a29e94f75ee20028e8186 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0ef88c6d24505699b0dfdea9891aa83 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a5978718f399aa71c22afad720f2f686eac6 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/66
https://github.com/dashpay/dash-issues/issues/65
## What was done?
Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs
This PR makes whitelist feature to be compatible with composite commands.
Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.
Platform at their side can use config such as this one (created based on shumkov's example):
```
rpc: {
host: '127.0.0.1',
port: 9998,
users: [
{
user: 'dashmate',
password: 'rpcpassword',
whitelist: null,
lowPriority: false,
},
{
username: 'platform-dapi',
password: 'rpcpassword',
whitelist: [],
lowPriority: true,
},
{
username: 'platform-drive-consensus',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
lowPriority: false,
},
{
username: 'platform-drive-other',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
],
lowPriority: true,
},
],
allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
},
```
## How Has This Been Tested?
Updated functional tests, see commits
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
LGTM, utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
PastaPastaPasta:
utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186
Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
2024-07-15 18:43:38 +02:00
|
|
|
self.never_allowed = ["getnetworkinfo", "quorum sign"]
|
Merge #12763: Add RPC Whitelist Feature from #12248
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #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 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2019-12-13 11:25:39 +01:00
|
|
|
with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "dash.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()
|