highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-13 06:04:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35592: http: check rpcallowip immediately after accepting connection

Browse Source 
55d3cd51a4 doc: add release note describing change for forbidden clients (Matthew Zipkin)
d1ed2a6e25 http: check rpcallowip immediately after accepting connection (Matthew Zipkin)

Pull request description:

  This is a follow-up to #35182 addressing a review comment from that PR: https://github.com/bitcoin/bitcoin/pull/35182#pullrequestreview-4322490068

  This update to HTTPServer checks the IP subnet allowlist as soon as possible (immediately after receiving a connection from a client) before any data is received. This does not entirely protect the server from the "slow loris" attack or [CWE-400](https://cwe.mitre.org/data/definitions/400.html) but does restrict the attack surface to localhost and clients explicitly allowed by the user.

  If a client is not allowed by the list, we disconnect as soon as possible. This is a behavior change from master branch (and previous release with libevent) where `403 Forbidden` was returned (after a potentially large amount request data was written to memory by the server).

  To facilitate existing unit tests, this commit includes a refactor that moves the subnet allow list and relevant methods into the HTTPServer class instead of static file scope. This is needed because otherwise the allow list would be empty when the unit tests run.

  There is still plenty of refactoring to do in order to modernize `HTTPServer` and de-globalize it, but since this specific issue has a resource allocation guard, I wanted to open it quickly on its own.

ACKs for top commit:
  janb84:
    ACK 55d3cd51a4
  winterrdog:
    ACK 55d3cd51a4
  w0xlt:
    ACK 55d3cd51a4
  fjahr:
    Code review ACK 55d3cd51a4

Tree-SHA512: 545911f2e4d2f97ab8bc854e9e57c39eb896428f8c349d34c8e8025a1f6bfb8cfd436f381e36af8b87592c07df3e16210819f3eef7943e23c6626030e615fdf5
This commit is contained in:
merge-script
2026-08-01 16:43:57 +01:00
parent 67efced1fc 55d3cd51a4
commit 556988790a
8 changed files with 81 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
src/httpserver.cpp
View File

@@ -67,8 +67,6 @@ struct HTTPPathHandler
/** HTTP module state */
static std::unique_ptr<http_bitcoin::HTTPServer> g_http_server{nullptr};
//! List of subnets to allow RPC connections from
static std::vector<CSubNet> rpc_allow_subnets;
//! Handlers for (sub)paths
static GlobalMutex g_httppathhandlers_mutex;
static std::vector<HTTPPathHandler> pathHandlers GUARDED_BY(g_httppathhandlers_mutex);
@@ -77,23 +75,28 @@ static std::vector<HTTPPathHandler> pathHandlers GUARDED_BY(g_httppathhandlers_m
static ThreadPool g_threadpool_http("http");
static int g_max_queue_depth{100};
namespace http_bitcoin {
/** Check if a network address is allowed to access the HTTP server */
static bool ClientAllowed(const CNetAddr& netaddr)
bool HTTPServer::ClientAllowed(const CNetAddr& netaddr) const
{
if (!netaddr.IsValid())
return false;
for(const CSubNet& subnet : rpc_allow_subnets)
for(const CSubNet& subnet : m_allow_subnets)
if (subnet.Match(netaddr))
return true;
return false;
}
/** Initialize ACL list for HTTP server */
static bool InitHTTPAllowList()
bool HTTPServer::InitHTTPAllowList()
{
rpc_allow_subnets.clear();
rpc_allow_subnets.emplace_back(LookupHost("127.0.0.1", false).value(), 8); // always allow IPv4 local subnet
rpc_allow_subnets.emplace_back(LookupHost("::1", false).value()); // always allow IPv6 localhost
// Must be run before StartSocketThreads() because ThreadSocketHandler()
// will check m_allow_subnets from the I/O thread.
Assume(!m_thread_socket_handler.joinable());
m_allow_subnets.clear();
m_allow_subnets.emplace_back(LookupHost("127.0.0.1", false).value(), 8); // always allow IPv4 local subnet
m_allow_subnets.emplace_back(LookupHost("::1", false).value()); // always allow IPv6 localhost
for (const std::string& strAllow : gArgs.GetArgs("-rpcallowip")) {
const CSubNet subnet{LookupSubNet(strAllow)};
if (!subnet.IsValid()) {
@@ -102,14 +105,15 @@ static bool InitHTTPAllowList()
CClientUIInterface::MSG_ERROR);
return false;
}
rpc_allow_subnets.push_back(subnet);
m_allow_subnets.push_back(subnet);
}
std::string strAllowed;
for (const CSubNet& subnet : rpc_allow_subnets)
for (const CSubNet& subnet : m_allow_subnets)
strAllowed += subnet.ToString() + " ";
LogDebug(BCLog::HTTP, "Allowing HTTP connections from: %s\n", strAllowed);
return true;
}
} // namespace http_bitcoin
/** HTTP request method as string - use for logging only */
std::string_view RequestMethodString(HTTPRequestMethod m)
@@ -127,14 +131,6 @@ std::string_view RequestMethodString(HTTPRequestMethod m)
static void MaybeDispatchRequestToWorker(std::shared_ptr<HTTPRequest> hreq)
{
// Early address-based allow check
if (!ClientAllowed(hreq->GetPeer())) {
LogDebug(BCLog::HTTP, "HTTP request from %s rejected: Client network is not allowed RPC access\n",
hreq->GetPeer().ToStringAddrPort());
hreq->WriteReply(HTTP_FORBIDDEN);
return;
}
// Early reject unknown HTTP methods
if (hreq->GetRequestMethod() == HTTPRequestMethod::UNKNOWN) {
LogDebug(BCLog::HTTP, "HTTP request from %s rejected: Unknown HTTP request method\n",
@@ -758,6 +754,11 @@ void HTTPServer::StopListening()
void HTTPServer::StartSocketsThreads()
{
// The socket handler reads m_allow_subnets in ClientAllowed(). InitHTTPAllowList()
// must have populated it first; localhost entries are always added, so an empty
// list means it was never called and every connection is rejected.
Assume(!m_allow_subnets.empty());
m_thread_socket_handler = std::thread(&util::TraceThread,
"http",
[this] { ThreadSocketHandler(); });
@@ -792,13 +793,19 @@ std::unique_ptr<Sock> HTTPServer::AcceptConnection(const Sock& listen_sock, CSer
}
// The OS handed us a valid socket but we can't determine its source address.
// In the unlikely event this occurs, the invalid address will be rejected
// by the downstream ClientAllowed() check.
if (!addr.SetSockAddr(sa, len)) {
LogDebug(BCLog::HTTP,
"Unknown socket family");
}
// Early address-based allow check
if (!ClientAllowed(addr)) {
LogDebug(BCLog::HTTP, "Connection from %s rejected: Client network is not allowed HTTP access\n",
addr.ToStringAddrPort());
// Socket destroyed, connection aborted
return {};
}
return sock;
}
@@ -1212,13 +1219,13 @@ bool HTTPRemoteClient::MaybeSendBytesFromBuffer()
bool InitHTTPServer()
{
if (!InitHTTPAllowList()) {
return false;
}
// Create HTTPServer
g_http_server = std::make_unique<HTTPServer>(MaybeDispatchRequestToWorker);
if (!g_http_server->InitHTTPAllowList()) {
return false;
}
g_http_server->SetServerTimeout(std::chrono::seconds(gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)));
// Bind HTTP server to specified addresses