rpc: invalid credentials can occupy every HTTP worker #36200

pull hodlinator wants to merge 1 commits into bitcoin:master from hodlinator:2026/09/http_auth_sleep changing 2 files +36 −6

hodlinator commented at 8:32 AM on September 9, 2026: contributor

Problem

RPC clients are able to easily DoS the HTTP server due to an obligatory worker thread sleep upon authentication failure.

Solution

Remove the sleep.

While this allows the guess rate to increase, it also fixes the DoS vector and reduces production code.

History

fe98cf8dc5066368b78e9ce208118c3532598dd2 from 2010 introduced RPC authentication along with the sleep. At that point the password authentication was timing-sensitive depending on the length, so the credentials could have been brute-forced. A sleep was added to help mitigate these timing attacks (and also reduce the guess rate).

The same author then introduced TimingResistantEqual() in 42656ea2e552b027e174fdceab7348ffcb8245c4 / #2886 in 2013 to solve the timing attack once and for all (https://github.com/bitcoin/bitcoin/issues/2838 / CVE-2013-4165). That made the sleep only serve to reduce the guess rate.

Severity

Low since the attack requires RPC clients to be granted network access to a node.

rpc: invalid credentials can occupy every HTTP worker

A low-medium service-availability issue lets an admitted HTTP client hold each shared worker for 250 ms per invalid Basic credential, filling the waiting queue and delaying or rejecting authenticated RPC and REST work.

Remove the worker-thread sleep while preserving authentication, warning logs, challenge responses, and timing-resistant comparison.

15ced063da

DrahtBot added the label RPC/REST/ZMQ on Sep 9, 2026
DrahtBot commented at 8:32 AM on September 9, 2026: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36200.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.
in src/httprpc.cpp:221 in 15ced063da
```
 213 | @@ -214,12 +214,6 @@ static void HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
 214 |      jreq.URI = req->GetURI();
 215 |      if (!RPCAuthorized(*auth_header, jreq.authUser)) {
 216 |          LogWarning("ThreadRPCServer incorrect password attempt from %s", jreq.peerAddr);
 217 | -
 218 | -        /* Deter brute-forcing
 219 | -           If this results in a DoS the user really
 220 | -           shouldn't have their RPC port exposed. */
 221 | -        UninterruptibleSleep(std::chrono::milliseconds{250});
```
willcl-ark commented at 11:05 AM on September 9, 2026:

This sleep also slows online password guessing, so I'm not sure I agree that it was made redundant by the addition of TimingResistantEqual().

Removing it might be reasonable to avoid tying up shared HTTP workers, but I think the description should acknowledge the increased guessing rate.

Severity

Low since the attack requires RPC clients to be granted access to a node.

This attack requires network access to the RPC listener, not valid RPC credentials (if that's what you meant by "grated access to a node").

hodlinator commented at 11:29 AM on September 9, 2026:

Good points!

Improved the PR desc.

It's not obvious to me that removing this DoS vector is more important than reducing the guess rate you point out. Both of these only occur when we have a malicious attacker who already has network access (on localhost or via -rpcallowip).

Moved to draft.

Another approach that has been mentioned out-of-band is rate-limiting connection attempts from the same IPs. IP source addresses can be spoofed though, so the attacker could make it look as if it was a well-behaving client which failed authentication and induce DoS that way.

Maybe a better approach would be to just expand the existing comment with these trade-offs. What do you think?
willcl-ark commented at 7:36 AM on September 10, 2026:

Yeah I think the DoS vector identified here is cheaper/easier to hit (and more annoying), so I'd be in favour of patching that up as you propose. I mainly wanted to point out that I didn't think the sleep was totally superfluous.

Perhaps we can leave repeated guessing up to external tools like fail2ban or similar (and document, if wanted)? i think it could be handled pretty trivially with something like:

# /etc/fail2ban/filter.d/bitcoind-rpc.conf # ~ Bitcoin Core debug.log format [Definition] failregex = ^\s*\[warning\] ThreadRPCServer incorrect password attempt from <ADDR>:\d+\s*$ ignoreregex =

and

# /etc/fail2ban/jail.d/bitcoind-rpc.local [bitcoind-rpc] enabled = true filter = bitcoind-rpc backend = polling logpath = /var/lib/bitcoind/debug.log usedns = no port = 8332 protocol = tcp maxretry = 5 findtime = 10m bantime = 1h ignoreip = 127.0.0.0/8 ::1 action = nftables[type=multiport, name=bitcoind-rpc, port="8332", protocol=tcp]

Then no action required on our side :)
hodlinator commented at 11:16 AM on September 10, 2026:

Hm.. wondering if we should add fail2ban docs or not. Maybe it just makes the RPC/HTTP ports seem more like something which we expect to be open to the public internet.

An alternative approach to removing the 250ms sleep is to spawn a dedicated "Slow RPC reply" thread which we could hand off the socket and work to upon authentication failure. That way we don't block any HTTP workers but still keep the guess-rate down. Not sure the added complexity would justify itself though.

Edit: Addendum - the source IP (range?) we hand off the sleep for should not be allowed to attempt another authentication before the 250ms timeout though... otherwise we don't keep the guess-rate down.
hodlinator marked this as a draft on Sep 9, 2026

Contributors

Labels

Linked (view graph)

#2838 Timing leak in RPC authentication #2886 Make RPC password resistant to timing attacks

rpc: invalid credentials can occupy every HTTP worker #36200

Problem

Solution

History

Severity

Code Coverage & Benchmarks

Reviews