rpc: restrict bitcoinproxy to a read-only method allowlist#496
Open
Atishyy27 wants to merge 1 commit into
Open
rpc: restrict bitcoinproxy to a read-only method allowlist#496Atishyy27 wants to merge 1 commit into
Atishyy27 wants to merge 1 commit into
Conversation
bitcoin_proxy forwarded any method straight to Bitcoin Core using the node's stored credentials, with only a logging middleware in front. A local caller could therefore invoke wallet/control methods such as dumpprivkey (exfiltrate private keys) or sendtoaddress (drain funds). This becomes critical once the RPC bind address is made configurable and someone exposes it beyond localhost. Gate bitcoin_proxy behind BITCOIN_PROXY_ALLOWED_METHODS, an explicit allowlist of read-only informational methods, and reject anything else before it reaches Bitcoin Core. Matching is exact/case-sensitive so casing cannot bypass the filter. Adds unit tests covering allowed, wallet/control, unknown, empty and mis-cased methods. Authentication on the RPC server (the other half of the issue) is left as a follow-up. Part of braidpool#417
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
bitcoin_proxyinnode/src/rpc_server.rsforwards any method straight to Bitcoin Core using the node's stored RPC credentials, with only a logging middleware in front — no method filtering. Any process that can reach the RPC server can therefore run:{"jsonrpc":"2.0","method":"bitcoinproxy","params":["dumpprivkey",["<address>"]],"id":1}to pull private keys out of Bitcoin Core, or
sendtoaddressto move funds. It is bound to127.0.0.1today, but there is already a path toward making the bind address configurable, at which point this becomes a critical remote vulnerability.Part of #417.
Change
BITCOIN_PROXY_ALLOWED_METHODS— an explicit allowlist of read-only, informational methods (chain/mempool/mining/network info,getblocktemplate,estimatesmartfee,uptime, …).bitcoin_proxybefore it reaches Bitcoin Core, returning a clear JSON-RPC error and awarn!log.dumpprivkey,sendtoaddress,importprivkey,walletpassphrase,stop), and unknown/empty/mis-cased input, plus an allowlist-dedup check.Scope / follow-up
This closes the concrete key-exfiltration / fund-draining vector by adding the method filtering the issue asks for. Adding authentication to the RPC server (the other half of #417) is intentionally left as a separate follow-up so this stays atomic.
Notes
cargo fmtclean. A full localcargo checkisn't possible on Windows (async_zmq/mio::unixis Unix-only), so I verified types by hand and via the new unit tests; CI will compile-check.