Add permission whitelisting system #17498

Similar to #12763.

This adds a system to enable a whitelist and give certain RPC users certain permissions. It works like that:

server=1
rpcauth=emil:9d4e28a0cdbce4e5440a0b26fe9bfbf1$50e12acc8b9ae3e55ad1b7f976b76e2e29c20d312358ce2004da937b8926b964
rpcwhitelistenable=1
rpcwhitelist=emil:getbestblockhash,getblock

With such a bitcoin.conf, emil would only be able to execute getbestblockhash and getblock. This can be repeated as many as you want with as many users as possible. If a rpcwhitelist is malformed it will be skipped. Valid syntax is

rpcwhitelist=[command][,command2]

Commas are only necessary when there are multiple entries.

Requested Reviewers: JeremyRubin

Some related issues: #12248, #16543

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #16673 (Relog configuration args on debug.log rotation by LarryRuane)
• #16115 (On bitcoind startup, write config args to debug.log by LarryRuane)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

for your example to work, the .conf should be:

server=1
rpcauth=emil:9d4e28a0cdbce4e5440a0b26fe9bfbf1$50e12acc8b9ae3e55ad1b7f976b76e2e29c20d312358ce2004da937b8926b964
rpcwhitelistenable=1
rpcwhitelist=emil:getbestblockhash,getblock

Is there a way to set permissions for a RPC user using cookie authentication?

@Thoragh, oh right forgot that when testing invalid args.

Is there a way to set permissions for a RPC user using cookie authentication?

As far as I know rpcauth does not allow cookie authentication. It is currently not possible to restrict the single rpcuser but you shouldn't use rpcauth and rpcuser at the same time as they contradictory.

Hi!

Very cool, glad to see someone picking this feature up! Can you do more to contrast it with #12763 and why this approach is better?

For context, #12763 got a significant amount of review/acks, but lacked the push from anyone (myself included) to get it merged. I believe the core issue was around it being difficult to write tests for this behavior.

You might be better off picking up the patch from #12763, and then figuring out how to write tests for it as that design has already been bikeshedded/reviewed. The PR can be reopened if someone wants to drive it forward.

Unless you have some more specific justifications why the approach you've taken is superior.

@JeremyRubin I already tried to rebased your current code and it didin't worked (probably I missed something) This PR was made more from scratch but offers the same.

Can you clarify what you mean by "didn't work?". The rebase didn't work or the rebased code didn't work?

I pushed up a rebased version here https://github.com/JeremyRubin/bitcoin/commits/whitelistrpc -- does this not work?

I took a cursory look. There's a few behavior changes compared to #12763, you may want to either:

1. pick up those commits and write tests for them
2. Exactly match the behavior there (which has had a decent amount of review) and write tests

I think 1 is the best path, but I don't want to be a cookie-licker if you feel the implementation and design choices you've made are best.

I think I fixed the issue you were seeing, which was because the argsmanage API has changed slightly to require flags.

some more review; IMHO I think it makes sense to just re-pick up #12763 given the behavioral edge conditions which would need to be fixed here.

@JeremyRubin Ok, I will write a test for your code then. Should I do this as a separate PR with your branch as the base or as a PR from master?

Agree, picking up @JeremyRubin's PR seems like a good choice. @emilengler, I would create a branch based on his PR, rebased to master, and add commits with your tests.

@emilengler the current branch is rebased on master so that's all set for you. I created a squashed branch here as the commits are mostly noise: https://github.com/JeremyRubin/bitcoin/tree/whitelistrpc-squashed (the diff between the two versions can be checked to be none).

Here's some code to get you started on tests:

#!/usr/bin/env python3
# Copyright (c) 2015-2017 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test multiple RPC users."""

from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
    assert_equal,
    get_datadir_path,
    str_to_b64str,
)

import os
import http.client
import urllib.parse
import subprocess
from random import SystemRandom
import string
import configparser


class RPCWhitelistTest(BitcoinTestFramework):
    def set_test_params(self):
        self.num_nodes = 1

    def setup_chain(self):
        super().setup_chain()
        #Append rpcauth to bitcoin.conf before initialization
        rpcauth = "rpcauth=rt:93648e835a54c573682c2eb19f882535$7681e9c5b74bdd85e78166031d2058e1069b3ed7ed967c93fc63abba06f31144"
        rpcauth2 = "rpcauth=rt2:f8607b1a88861fac29dfccf9b52ff9f$ff36a0c23c8c62b4846112e50fa888416e94c17bfd4c42f88fd8f55ec6a3137e"
        rpcuser = "rpcuser=rpcuser💻"
        rpcpassword = "rpcpassword=rpcpassword🔑"

        self.user = ''.join(SystemRandom().choice(string.ascii_letters + string.digits) for _ in range(10))
        config = configparser.ConfigParser()
        config.read_file(open(self.options.configfile))
        gen_rpcauth = config['environment']['RPCAUTH']
        p = subprocess.Popen([gen_rpcauth, self.user], stdout=subprocess.PIPE, universal_newlines=True)
        lines = p.stdout.read().splitlines()
        rpcauth3 = lines[1]
        self.password = lines[3]

        with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f:
            f.write("\nrpcwhitelistdefault=1\n")
            f.write(rpcauth+"\n")
            f.write(rpcauth2+"\n")
            f.write(rpcauth3+"\n")

    def run_test(self):

        ##################################################
        # Check correctness of the rpcauth config option #
        ##################################################
        url = urllib.parse.urlparse(self.nodes[0].url)

        #Old authpair
        authpair = url.username + ':' + url.password

        #New authpair generated via share/rpcuser tool
        password = "cA773lm788buwYe4g4WT+05pKyNruVKjQ25x3n0DQcM="

        #Second authpair with different username
        password2 = "8/F3uMDw4KSEbw96U3CA1C4X05dkHDN2BPFjTgZW4KI="
        authpairnew = "rt:"+password

        self.log.info('Correct...')
        headers = {"Authorization": "Basic " + str_to_b64str(authpair)}

        conn = http.client.HTTPConnection(url.hostname, url.port)
        conn.connect()
        conn.request('POST', '/', '{"method": "getbestblockhash"}', headers)
        resp = conn.getresponse()
        assert_equal(resp.status, 403)
        conn.close()



if __name__ == '__main__':
    RPCWhitelistTest().main ()

The core issues with this approach for tests are covered in the original PR; because the testing framework assumes access to RPCs you can't actually test the default all-rpc disabled case.

@JeremyRubin Thanks for the test template! I haven't really ogt much time today, will continue on it tomorrow