RPC idempotency and RPC request replay protection

kanzure commented at 9:43 pm on November 21, 2016: contributor

I noticed pull request 128 to python-bitcoinlib where an RPC client retry wrapper is proposed. The retry wrapper would retry in the event of an IOError exception. However, this does not seem particularly safe for RPC calls like sendmany because it could be possible to experience a socket or connection timeout even though bitcoind satisfied the original RPC request. In some cases, auto retrying could result in duplicate spending.

RPC requests should have at least a nonce in every request. It should maybe be a nonce and sequence pair, per @gmaxwell’s suggestion in below chat logs. It would be good to have RPC request replay protection in bitcoind.

Concurrent writes should require locks. Applications should have some sense of guarantees around isolation of competing concurrent and also replayed RPC calls. It’s not enough to “check state” over RPC with listtransactions because you can check state as much as you like, the state still might change between the time you check state and the time you resend the original RPC command, especially in distributed systems.

https://botbot.me/freenode/bitcoin-core-dev/2016-11-21/?msg=76819688&page=2

maflcko added the label RPC/REST/ZMQ on Nov 21, 2016

brakmic commented at 4:27 pm on April 13, 2020: contributor

Hi,

I’ve experimented a bit and implemented something that could maybe offer a solution to the problem with sending of identical requests.

The changes are located in:

httprpc.cpp - where we check incoming nonces and keep track of previous ones. There are two ways to get a nonce: the client generates X-Nonce header, or we create one on-the-fly based on the JSON-request object content (a Hash160 will be generated)
bitcoin-cli.cpp - here we generate nonces by using the std::random_device+ std::mt19937
rpc/server.cpp - here we use a new method isDangerousCommand which tells if a particular RPC-command belongs to dangerous ones. A command is dangerous when it can change the wallet, manipulate UTXOs, change wallet state etc.
rpcwallet.cpp - the RPC commands[] table was updated to contain the boolean flag dangerous that indicates if a command is destructive.

A client request will be rejected when either of the following problems occur:

Client sends a request containing a dangerous command together with X-Nonce that is already known. That is: the server has already seen and (maybe) executed this command. A client that keeps on sending the same request without changing X-Nonce is not functioning properly, regardless how meaningful the command is.
Client sends a request containing a dangerous command without X-Nonce but with identical JSON content. Here the client is repeating the same command again and again.

In the first case the nonce will be used to reject the request immediately. In the second case the server would first generate a nonce based on the JSON data from the request and then check if this once is already known.

There is another, harmless case, when a client is sending requests containing non-dangerous commands. In such cases the server would continue answering requests as those commands aren’t changing anything on the server. However, they too could be handled more strictly, for example to throttle requests or similar tasks.

When a requests is blocked the following entry will be logged:

reject

Although very interesting to implement and also a nice way to learn more about Bitcoin internals, I am not sure if this approach is really useful and resilient enough to be used in real environments.

The changes are located in my repo. I don’t think that at this stage a PR should be opened.

Regards,

kanzure commented at 4:50 pm on April 13, 2020: contributor

My first concern is that it will be difficult (and perhaps even non-intuitive) which commands are considered “dangerous” and “harmless”. Are there any harmless commands at all?

I would also like to bring up that there are many databases out there in the world that deal with this problem all the time, using transaction isolation: https://www.postgresql.org/docs/current/transaction-iso.html

brakmic commented at 5:19 pm on April 13, 2020: contributor

Maybe we could also give users an option to define which methods they consider dangerous. Via config option.

Like:

dangerous=sendmany,sendtoaddress And maybe also: dangerous=all to declare them all dangerous.

But if we go the postgres way, it’d mean implementing a much more sophisticated structure that’s way beyond my knowledge. But it would be a good way to learn something new. :)

willcl-ark added the label Up for grabs on Oct 14, 2024

willcl-ark commented at 3:46 pm on October 14, 2024: member

This feature request does not seem to have attracted much attention from other contributors. As such, it does not seem important enough to keep it sitting around idle in the list of open issues. Closing due to lack of interest.

Pull requests with improvements are always welcome.

willcl-ark closed this on Oct 14, 2024

RPC idempotency and RPC request replay protection #9197