http: check rpcallowip immediately after accepting connection

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index a05d19db..ad8b5b2a 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -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
diff --git a/src/httpserver.h b/src/httpserver.h
index 9031eb61..295f193f 100644
--- a/src/httpserver.h
+++ b/src/httpserver.h
@@ -215,6 +215,11 @@ public:
         Assume(m_listen.empty()); // Missing call to StopListening()
     }
 
+    /**
+     * Parse the user's -rpcallowip settings and populate m_allow_subnets
+     */
+    bool InitHTTPAllowList();
+
     /**
      * Bind to a new address:port, start listening and add the listen socket to `m_listen`.
      * @param[in] to Where to bind.
@@ -376,6 +381,16 @@ private:
      */
     std::chrono::seconds m_rpcservertimeout{DEFAULT_HTTP_SERVER_TIMEOUT};
 
+    /**
+     * List of subnets to allow HTTP connections from
+     */
+    std::vector<CSubNet> m_allow_subnets;
+
+    /**
+     * Check an incoming connection's source IP against the allow list
+     */
+    bool ClientAllowed(const CNetAddr& netaddr) const;
+
     /**
      * Accept a connection.
      * @param[in] listen_sock Socket on which to accept the connection.
diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp
index aae89920..cd020fc6 100644
--- a/src/test/httpserver_tests.cpp
+++ b/src/test/httpserver_tests.cpp
@@ -522,6 +522,7 @@ BOOST_AUTO_TEST_CASE(http_server_socket_tests)
     };
 
     HTTPServer server{StoreRequest};
+    server.InitHTTPAllowList();
 
     {
         // We can only bind to NET_IPV4 and NET_IPV6
@@ -647,6 +648,7 @@ BOOST_AUTO_TEST_CASE(http_socket_error_tests)
         // Can't call BOOST_REQUIRE from worker thread
         Assert(workers.Submit(std::move(item)));
     }};
+    server.InitHTTPAllowList();
 
     // All replies will be the same size
     static constexpr std::size_t reply_length = std::string_view{
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index bcf63778..051035e8 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -640,6 +640,10 @@ std::vector<CTransactionRef> TestChain100Setup::PopulateMempool(FastRandomContex
 
 SocketTestingSetup::SocketTestingSetup()
 {
+    // HTTPServer is not integrated into NodeContext yet and still pulls global args.
+    // This is the IP address DynSock claims to be from when connecting.
+    gArgs.ForceSetArg("-rpcallowip", "5.5.5.5");
+
     // "back up" the current CreateSock() so we can restore it after the test
     m_create_sock_orig = CreateSock;
 
diff --git a/test/functional/interface_http.py b/test/functional/interface_http.py
index a71fc474..b0c5243f 100755
--- a/test/functional/interface_http.py
+++ b/test/functional/interface_http.py
@@ -5,6 +5,7 @@
 """Test the HTTP server basics."""
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.netutil import NETWORK_ERRORS
 from test_framework.util import assert_equal, str_to_b64str
 
 import http.client
@@ -17,15 +18,6 @@ RPCSERVERTIMEOUT = 2
 MAX_HEADERS_SIZE = 8192
 MAX_BODY_SIZE = 32 * 1024 * 1024
 
-# When a test expects a server disconnection, any of these errors are
-# acceptable. The specific event is determined by race condition and platform OS.
-NETWORK_ERRORS = (
-    BrokenPipeError,                 # write to a closed socket/pipe
-    ConnectionResetError,            # connection forcibly closed by peer
-    ConnectionAbortedError,          # connection aborted locally or by network stack
-    http.client.ResponseNotReady,    # server response not ready or connection out of sync
-)
-
 class BitcoinHTTPConnection:
     def __init__(self, node):
         self.url = urllib.parse.urlparse(node.url)
diff --git a/test/functional/rpc_bind.py b/test/functional/rpc_bind.py
index 517df5d9..4494dde5 100755
--- a/test/functional/rpc_bind.py
+++ b/test/functional/rpc_bind.py
@@ -4,10 +4,10 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test running bitcoind with the -rpcbind and -rpcallowip options."""
 
-from test_framework.netutil import all_interfaces, addr_to_hex, get_bind_addrs, test_ipv6_local
+from test_framework.netutil import NETWORK_ERRORS, all_interfaces, addr_to_hex, get_bind_addrs, test_ipv6_local
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
 from test_framework.test_node import ErrorMatch
-from test_framework.util import assert_equal, assert_raises_rpc_error, rpc_port
+from test_framework.util import assert_equal, rpc_port
 
 class RPCBindTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -62,6 +62,7 @@ class RPCBindTest(BitcoinTestFramework):
         Start a node with rpcallow IP, and request getnetworkinfo
         at a non-localhost IP.
         '''
+        success = True
         self.log.info("Allow IP test for %s:%d" % (rpchost, rpcport))
         node_args = \
             ['-disablewallet', '-nolisten'] + \
@@ -72,8 +73,12 @@ class RPCBindTest(BitcoinTestFramework):
         self.nodes[0].rpchost = f"{rpchost}:{rpcport}"
         # connect to node through non-loopback interface
         node = self.nodes[0].create_new_rpc_connection()
-        node.getnetworkinfo()
+        try:
+            node.getnetworkinfo()
+        except NETWORK_ERRORS:
+            success = False
         self.stop_nodes()
+        return success
 
     def run_invalid_allowip_test(self):
         '''
@@ -162,12 +167,13 @@ class RPCBindTest(BitcoinTestFramework):
         self.run_bind_test([self.non_loopback_ip], self.non_loopback_ip, [self.non_loopback_ip],
             [(self.non_loopback_ip, self.defaultport)])
 
-        # Check that with invalid rpcallowip, we are denied
-        self.run_allowip_test([self.non_loopback_ip], self.non_loopback_ip, self.defaultport)
+        # Check that connections from allowed IPs are allowed
+        assert self.run_allowip_test([self.non_loopback_ip], self.non_loopback_ip, self.defaultport)
+        # Otherwise we are denied
         if self.options.usecli:
             self.log.info("Skip negative IP test with CLI, because the CLI can not throw the tested exception type")
             return
-        assert_raises_rpc_error(-342, "non-JSON HTTP response with '403 Forbidden' from server", self.run_allowip_test, ['1.1.1.1'], self.non_loopback_ip, self.defaultport)
+        assert not self.run_allowip_test(['1.1.1.1'], self.non_loopback_ip, self.defaultport)
 
 if __name__ == '__main__':
     RPCBindTest(__file__).main()
diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py
index 17043541..a13ee61e 100644
--- a/test/functional/test_framework/netutil.py
+++ b/test/functional/test_framework/netutil.py
@@ -7,6 +7,7 @@
 Roughly based on https://web.archive.org/web/20190424172231/http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ by Ricardo Pascal
 """
 
+import http.client
 import sys
 import socket
 import struct
@@ -34,6 +35,15 @@ ADDRMAN_NEW_BUCKET_COUNT = 1 << 10
 ADDRMAN_TRIED_BUCKET_COUNT = 1 << 8
 ADDRMAN_BUCKET_SIZE = 1 << 6
 
+# When a test expects a server disconnection, any of these errors are
+# acceptable. The specific event is determined by race condition and platform OS.
+NETWORK_ERRORS = (
+    BrokenPipeError,                 # write to a closed socket/pipe
+    ConnectionResetError,            # connection forcibly closed by peer
+    ConnectionAbortedError,          # connection aborted locally or by network stack
+    http.client.ResponseNotReady,    # server response not ready or connection out of sync
+)
+
 def get_socket_inodes(pid):
     '''
     Get list of socket inodes for process pid.