Merge bitcoin/bitcoin#32539: init: Configure reachable networks before we start the RPC server

12ff4be test: ensure -rpcallowip is compatible with RFC4193 (Matthew Zipkin)
c02bd3c config: Explain RFC4193 and CJDNS interaction in help and init error (Matthew Zipkin)
f728b6b init: Configure reachable networks before we start the RPC server (Matthew Zipkin)

Pull request description:

  Closes bitcoin/bitcoin#32433

  `MaybeFlipIPv6toCJDNS()` relies on `g_reachable_nets` to distinguish between CJDNS addresses and other IPv6 addresses. In particular, [RFC4193](https://www.rfc-editor.org/rfc/rfc4193#section-3.1) address or "Unique Local Address" with the L-bit unset also begins with the `fc` prefix. #32433 highlights a use case for these addresses that have nothing to do with CJDNS.

  On master we don't parse init flags like `-cjdnsreachable` until *after* the HTTP server has started, causing conflicts with `-rpcallowip` because CJDNS doesn't support subnets.

  This PR ensures that `NET_CJDNS` is only present in the reachable networks list if set by `-cjdnsreachable` *before* `-rpcallowip` is checked. If it is set all `fc` addresses are assumed to be CJDNS, can not have subnets, and can't be set for `-rpcallowip`.

  I also noted this specific parameter interaction in the init help as well as the error message if configured incorrectly.

  This can be tested locally:

  `bitcoind -regtest -rpcallowip=fc00:dead:beef::/64 -rpcuser=u -rpcpassword=p`

  On master this will just throw an error that doesn't even mention IPv6 at all.

  On the branch, this will succeed and can be tested by adding the ULA to a local interface.

  On linux: `sudo ip -6 addr add fc00:dead:beef::1/64 dev lo`

  On macos: `sudo ifconfig lo0 inet6 fc00:dead:beef::1/128 add`

  then: `curl -v -g -6 --interface fc00:dead:beef::1 u:p@[::1]:18443 --data '{"method":"getblockcount"}'`

  If the `rpcallowip` option is removed, the RPC request will fail to authorize.

  Finally, adding `-cjdnsreachable` to the start up command will throw an error and specify the incompatibility:

  > RFC4193 is allowed only if -cjdnsreachable=0.

ACKs for top commit:
  achow101:
    ACK 12ff4be
  tapcrafter:
    tACK 12ff4be
  ryanofsky:
    Code review ACK 12ff4be
  willcl-ark:
    ACK 12ff4be

Tree-SHA512: a4dd70ca2bb9f6ec2c0a9463fd73985d1ed80552c674a9067ac9a86662d1c018cc275ba757cebb2993c5f3971ecf4778b95d35fe7a7178fb41b1d18b601c9960

Author: achow101

src/httpserver.cpp

@@ -229,7 +229,7 @@ static bool InitHTTPAllowList()
         const CSubNet subnet{LookupSubNet(strAllow)};
         if (!subnet.IsValid()) {
             uiInterface.ThreadSafeMessageBox(
-                Untranslated(strprintf("Invalid -rpcallowip subnet specification: %s. Valid are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24).", strAllow)),
+                Untranslated(strprintf("Invalid -rpcallowip subnet specification: %s. Valid values are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). RFC4193 is allowed only if -cjdnsreachable=0.", strAllow)),
                 "", CClientUIInterface::MSG_ERROR);
             return false;
         }

src/init.cpp

@@ -650,7 +650,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-blockversion=<n>", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION);
 
     argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
-    argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid values for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+    argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid values for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). RFC4193 is allowed only if -cjdnsreachable=0. This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
     argsman.AddArg("-rpcauth=<userpw>", "Username and HMAC-SHA-256 hashed password for JSON-RPC connections. The field <userpw> comes in the format: <USERNAME>:<SALT>$<HASH>. A canonical python script is included in share/rpcauth. The client then connects normally using the rpcuser=<USERNAME>/rpcpassword=<PASSWORD> pair of arguments. This option can be specified multiple times", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC);
     argsman.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC);
     argsman.AddArg("-rpcdoccheck", strprintf("Throw a non-fatal error at runtime if the documentation for an RPC is incorrect (default: %u)", DEFAULT_RPC_DOC_CHECK), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
@@ -1392,6 +1392,32 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // Check port numbers
     if (!CheckHostPortOptions(args)) return false;
 
+    // Configure reachable networks before we start the RPC server.
+    // This is necessary for -rpcallowip to distinguish CJDNS from other RFC4193
+    const auto onlynets = args.GetArgs("-onlynet");
+    if (!onlynets.empty()) {
+        g_reachable_nets.RemoveAll();
+        for (const std::string& snet : onlynets) {
+            enum Network net = ParseNetwork(snet);
+            if (net == NET_UNROUTABLE)
+                return InitError(strprintf(_("Unknown network specified in -onlynet: '%s'"), snet));
+            g_reachable_nets.Add(net);
+        }
+    }
+
+    if (!args.IsArgSet("-cjdnsreachable")) {
+        if (!onlynets.empty() && g_reachable_nets.Contains(NET_CJDNS)) {
+            return InitError(
+                _("Outbound connections restricted to CJDNS (-onlynet=cjdns) but "
+                  "-cjdnsreachable is not provided"));
+        }
+        g_reachable_nets.Remove(NET_CJDNS);
+    }
+    // Now g_reachable_nets.Contains(NET_CJDNS) is true if:
+    // 1. -cjdnsreachable is given and
+    // 2.1. -onlynet is not given or
+    // 2.2. -onlynet=cjdns is given
+
     /* Start the RPC server already.  It will be started in "warmup" mode
      * and not really process calls already (but it will signify connections
      * that the server is there and will be ready later).  Warmup mode will
@@ -1504,30 +1530,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             strSubVersion.size(), MAX_SUBVERSION_LENGTH));
     }
 
-    const auto onlynets = args.GetArgs("-onlynet");
-    if (!onlynets.empty()) {
-        g_reachable_nets.RemoveAll();
-        for (const std::string& snet : onlynets) {
-            enum Network net = ParseNetwork(snet);
-            if (net == NET_UNROUTABLE)
-                return InitError(strprintf(_("Unknown network specified in -onlynet: '%s'"), snet));
-            g_reachable_nets.Add(net);
-        }
-    }
-
-    if (!args.IsArgSet("-cjdnsreachable")) {
-        if (!onlynets.empty() && g_reachable_nets.Contains(NET_CJDNS)) {
-            return InitError(
-                _("Outbound connections restricted to CJDNS (-onlynet=cjdns) but "
-                  "-cjdnsreachable is not provided"));
-        }
-        g_reachable_nets.Remove(NET_CJDNS);
-    }
-    // Now g_reachable_nets.Contains(NET_CJDNS) is true if:
-    // 1. -cjdnsreachable is given and
-    // 2.1. -onlynet is not given or
-    // 2.2. -onlynet=cjdns is given
-
     // Requesting DNS seeds entails connecting to IPv4/IPv6, which -onlynet options may prohibit:
     // If -dnsseed=1 is explicitly specified, abort. If it's left unspecified by the user, we skip
     // the DNS seeds by adjusting -dnsseed in InitParameterInteraction.

test/functional/rpc_bind.py

@@ -6,6 +6,7 @@
 
 from test_framework.netutil import 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, get_rpc_proxy, rpc_port, rpc_url
 
 class RPCBindTest(BitcoinTestFramework):
@@ -75,6 +76,25 @@ def run_allowip_test(self, allow_ips, rpchost, rpcport):
         node.getnetworkinfo()
         self.stop_nodes()
 
+    def run_invalid_allowip_test(self):
+        '''
+        Check parameter interaction with -rpcallowip and -cjdnsreachable.
+        RFC4193 addresses are fc00::/7 like CJDNS but have an optional
+        "local" L bit making them fd00:: which should always be OK.
+        '''
+        self.log.info("Allow RFC4193 when compatible with CJDNS options")
+        # Don't rpcallow RFC4193 with L-bit=0 if CJDNS is enabled
+        self.nodes[0].assert_start_raises_init_error(
+            ["-rpcallowip=fc00:db8:c0:ff:ee::/80","-cjdnsreachable"],
+            "Invalid -rpcallowip subnet specification",
+            match=ErrorMatch.PARTIAL_REGEX)
+        # OK to rpcallow RFC4193 with L-bit=1 if CJDNS is enabled
+        self.start_node(0, ["-rpcallowip=fd00:db8:c0:ff:ee::/80","-cjdnsreachable"])
+        self.stop_nodes()
+        # OK to rpcallow RFC4193 with L-bit=0 if CJDNS is not enabled
+        self.start_node(0, ["-rpcallowip=fc00:db8:c0:ff:ee::/80"])
+        self.stop_nodes()
+
     def run_test(self):
         if sum([self.options.run_ipv4, self.options.run_ipv6, self.options.run_nonloopback]) > 1:
             raise AssertionError("Only one of --ipv4, --ipv6 and --nonloopback can be set")
@@ -101,6 +121,7 @@ def run_test(self):
                 self.run_invalid_bind_test(['127.0.0.1'], ['127.0.0.1:notaport', '127.0.0.1:-18443', '127.0.0.1:0', '127.0.0.1:65536'])
             if self.options.run_ipv6:
                 self.run_invalid_bind_test(['[::1]'], ['[::1]:notaport', '[::1]:-18443', '[::1]:0', '[::1]:65536'])
+                self.run_invalid_allowip_test()
         if not self.options.run_ipv4 and not self.options.run_ipv6:
             self._run_nonloopback_tests()