Merge #19628: net: change CNetAddr::ip to have flexible size

102867c587f5f7954232fb8ed8e85cda78bb4d32 net: change CNetAddr::ip to have flexible size (Vasil Dimov)
1ea57ad67406b3aaaef5254bc2fa7e4134f3a6df net: don't accept non-left-contiguous netmasks (Vasil Dimov)

Pull request description:

  (chopped off from #19031 to ease review)

  Before this change `CNetAddr::ip` was a fixed-size array of 16 bytes,
  not being able to store larger addresses (e.g. TORv3) and encoded
  smaller ones as 16-byte IPv6 addresses.

  Change its type to `prevector`, so that it can hold larger addresses and
  do not disguise non-IPv6 addresses as IPv6. So the IPv4 address
  `1.2.3.4` is now encoded as `01020304` instead of
  `00000000000000000000FFFF01020304`.

  Rename `CNetAddr::ip` to `CNetAddr::m_addr` because it is not an "IP" or
  "IP address" (TOR addresses are not IP addresses).

  In order to preserve backward compatibility with serialization (where
  e.g. `1.2.3.4` is serialized as `00000000000000000000FFFF01020304`)
  introduce `CNetAddr` dedicated legacy serialize/unserialize methods.

  Adjust `CSubNet` accordingly. Still use `CSubNet::netmask[]` of fixed 16
  bytes, but use the first 4 for IPv4 (not the last 4). Do not accept
  invalid netmasks that have 0-bits followed by 1-bits and only allow
  subnetting for IPv4 and IPv6.

  Co-authored-by: Carl Dong <contact@carldong.me>

ACKs for top commit:
  sipa:
    utACK 102867c587f5f7954232fb8ed8e85cda78bb4d32
  MarcoFalke:
    Concept ACK 102867c587f5f7954232fb8ed8e85cda78bb4d32
  ryanofsky:
    Code review ACK 102867c587f5f7954232fb8ed8e85cda78bb4d32. Just many suggested updates since last review. Thanks for following up on everything!
  jonatack:
    re-ACK 102867c587f5f7954232fb8ed8e85cda78bb4d32 diff review, code review, build/tests/running bitcoind with ipv4/ipv6/onion peers
  kallewoof:
    ACK 102867c587f5f7954232fb8ed8e85cda78bb4d32

Tree-SHA512: d60bf716cecf8d3e8146d2f90f897ebe956befb16f711a24cfe680024c5afc758fb9e4a0a22066b42f7630d52cf916318bedbcbc069ae07092d5250a11e8f762

5 files changed, 101 insertions, 11 deletions

diff --git a/src/test/fuzz/asmap.cpp b/src/test/fuzz/asmap.cpp
index 40ca01bd9f..e3aefa18a3 100644
--- a/src/test/fuzz/asmap.cpp
+++ b/src/test/fuzz/asmap.cpp
@@ -33,7 +33,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     if (buffer.size() < 1 + 3 + 4) return;
     int asmap_size = 3 + (buffer[0] & 127);
     bool ipv6 = buffer[0] & 128;
-    int addr_size = ipv6 ? 16 : 4;
+    const size_t addr_size = ipv6 ? ADDR_IPV6_SIZE : ADDR_IPV4_SIZE;
     if (buffer.size() < size_t(1 + asmap_size + addr_size)) return;
     std::vector<bool> asmap = ipv6 ? IPV6_PREFIX_ASMAP : IPV4_PREFIX_ASMAP;
     asmap.reserve(asmap.size() + 8 * asmap_size);
@@ -43,7 +43,17 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         }
     }
     if (!SanityCheckASMap(asmap)) return;
+
+    const uint8_t* addr_data = buffer.data() + 1 + asmap_size;
     CNetAddr net_addr;
-    net_addr.SetRaw(ipv6 ? NET_IPV6 : NET_IPV4, buffer.data() + 1 + asmap_size);
+    if (ipv6) {
+        assert(addr_size == ADDR_IPV6_SIZE);
+        net_addr.SetLegacyIPv6(Span<const uint8_t>(addr_data, addr_size));
+    } else {
+        assert(addr_size == ADDR_IPV4_SIZE);
+        in_addr ipv4;
+        memcpy(&ipv4, addr_data, addr_size);
+        net_addr.SetIP(CNetAddr{ipv4});
+    }
     (void)net_addr.GetMappedAS(asmap);
 }
diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp
index 2901c704f6..8252f38726 100644
--- a/src/test/fuzz/netaddress.cpp
+++ b/src/test/fuzz/netaddress.cpp
@@ -17,9 +17,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
     const CNetAddr net_addr = ConsumeNetAddr(fuzzed_data_provider);
-    for (int i = 0; i < 15; ++i) {
-        (void)net_addr.GetByte(i);
-    }
     (void)net_addr.GetHash();
     (void)net_addr.GetNetClass();
     if (net_addr.GetNetwork() == Network::NET_IPV4) {
@@ -78,7 +75,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     (void)net_addr.ToString();
     (void)net_addr.ToStringIP();
 
-    const CSubNet sub_net{net_addr, fuzzed_data_provider.ConsumeIntegral<int32_t>()};
+    const CSubNet sub_net{net_addr, fuzzed_data_provider.ConsumeIntegral<uint8_t>()};
     (void)sub_net.IsValid();
     (void)sub_net.ToString();
 
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h
index 9f9552edb9..ed6093a8a8 100644
--- a/src/test/fuzz/util.h
+++ b/src/test/fuzz/util.h
@@ -257,7 +257,7 @@ CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept
 
 CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept
 {
-    return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int32_t>()};
+    return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<uint8_t>()};
 }
 
 void InitializeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST)
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 317000c771..917ae571f5 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -13,8 +13,10 @@
 #include <streams.h>
 #include <test/util/setup_common.h>
 #include <util/memory.h>
+#include <util/strencodings.h>
 #include <util/string.h>
 #include <util/system.h>
+#include <version.h>
 
 #include <boost/test/unit_test.hpp>
 
@@ -188,6 +190,78 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode2->IsInboundConn() == true);
 }
 
+BOOST_AUTO_TEST_CASE(cnetaddr_basic)
+{
+    CNetAddr addr;
+
+    // IPv4, INADDR_ANY
+    BOOST_REQUIRE(LookupHost("0.0.0.0", addr, false));
+    BOOST_REQUIRE(!addr.IsValid());
+    BOOST_REQUIRE(addr.IsIPv4());
+
+    BOOST_CHECK(addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "0.0.0.0");
+
+    // IPv4, INADDR_NONE
+    BOOST_REQUIRE(LookupHost("255.255.255.255", addr, false));
+    BOOST_REQUIRE(!addr.IsValid());
+    BOOST_REQUIRE(addr.IsIPv4());
+
+    BOOST_CHECK(!addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "255.255.255.255");
+
+    // IPv4, casual
+    BOOST_REQUIRE(LookupHost("12.34.56.78", addr, false));
+    BOOST_REQUIRE(addr.IsValid());
+    BOOST_REQUIRE(addr.IsIPv4());
+
+    BOOST_CHECK(!addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "12.34.56.78");
+
+    // IPv6, in6addr_any
+    BOOST_REQUIRE(LookupHost("::", addr, false));
+    BOOST_REQUIRE(!addr.IsValid());
+    BOOST_REQUIRE(addr.IsIPv6());
+
+    BOOST_CHECK(addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "::");
+
+    // IPv6, casual
+    BOOST_REQUIRE(LookupHost("1122:3344:5566:7788:9900:aabb:ccdd:eeff", addr, false));
+    BOOST_REQUIRE(addr.IsValid());
+    BOOST_REQUIRE(addr.IsIPv6());
+
+    BOOST_CHECK(!addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "1122:3344:5566:7788:9900:aabb:ccdd:eeff");
+
+    // TORv2
+    addr.SetSpecial("6hzph5hv6337r6p2.onion");
+    BOOST_REQUIRE(addr.IsValid());
+    BOOST_REQUIRE(addr.IsTor());
+
+    BOOST_CHECK(!addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "6hzph5hv6337r6p2.onion");
+
+    // Internal
+    addr.SetInternal("esffpp");
+    BOOST_REQUIRE(!addr.IsValid()); // "internal" is considered invalid
+    BOOST_REQUIRE(addr.IsInternal());
+
+    BOOST_CHECK(!addr.IsBindAny());
+    BOOST_CHECK_EQUAL(addr.ToString(), "esffpvrt3wpeaygy.internal");
+}
+
+BOOST_AUTO_TEST_CASE(cnetaddr_serialize)
+{
+    CNetAddr addr;
+    CDataStream s(SER_NETWORK, PROTOCOL_VERSION);
+
+    addr.SetInternal("a");
+    s << addr;
+    BOOST_CHECK_EQUAL(HexStr(s), "fd6b88c08724ca978112ca1bbdcafac2");
+    s.clear();
+}
+
 // prior to PR #14728, this test triggers an undefined behavior
 BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
 {
diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
index 49073ea657..6681c92bb5 100644
--- a/src/test/netbase_tests.cpp
+++ b/src/test/netbase_tests.cpp
@@ -185,6 +185,7 @@ BOOST_AUTO_TEST_CASE(subnet_test)
     BOOST_CHECK(!ResolveSubNet("1.2.3.0/-1").IsValid());
     BOOST_CHECK(ResolveSubNet("1.2.3.0/32").IsValid());
     BOOST_CHECK(!ResolveSubNet("1.2.3.0/33").IsValid());
+    BOOST_CHECK(!ResolveSubNet("1.2.3.0/300").IsValid());
     BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:8/0").IsValid());
     BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:8/33").IsValid());
     BOOST_CHECK(!ResolveSubNet("1:2:3:4:5:6:7:8/-1").IsValid());
@@ -216,6 +217,11 @@ BOOST_AUTO_TEST_CASE(subnet_test)
     BOOST_CHECK(CSubNet(ResolveIP("1:2:3:4:5:6:7:8")).Match(ResolveIP("1:2:3:4:5:6:7:8")));
     BOOST_CHECK(!CSubNet(ResolveIP("1:2:3:4:5:6:7:8")).Match(ResolveIP("1:2:3:4:5:6:7:9")));
     BOOST_CHECK(CSubNet(ResolveIP("1:2:3:4:5:6:7:8")).ToString() == "1:2:3:4:5:6:7:8/128");
+    // IPv4 address with IPv6 netmask or the other way around.
+    BOOST_CHECK(!CSubNet(ResolveIP("1.1.1.1"), ResolveIP("ffff::")).IsValid());
+    BOOST_CHECK(!CSubNet(ResolveIP("::1"), ResolveIP("255.0.0.0")).IsValid());
+    // Can't subnet TOR (or any other non-IPv4 and non-IPv6 network).
+    BOOST_CHECK(!CSubNet(ResolveIP("5wyqrzbvrdsumnok.onion"), ResolveIP("255.0.0.0")).IsValid());
 
     subnet = ResolveSubNet("1.2.3.4/255.255.255.255");
     BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.4/32");
@@ -290,11 +296,13 @@ BOOST_AUTO_TEST_CASE(subnet_test)
     BOOST_CHECK_EQUAL(subnet.ToString(), "1::/16");
     subnet = ResolveSubNet("1:2:3:4:5:6:7:8/0000:0000:0000:0000:0000:0000:0000:0000");
     BOOST_CHECK_EQUAL(subnet.ToString(), "::/0");
+    // Invalid netmasks (with 1-bits after 0-bits)
     subnet = ResolveSubNet("1.2.3.4/255.255.232.0");
-    BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/255.255.232.0");
+    BOOST_CHECK(!subnet.IsValid());
+    subnet = ResolveSubNet("1.2.3.4/255.0.255.255");
+    BOOST_CHECK(!subnet.IsValid());
     subnet = ResolveSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f");
-    BOOST_CHECK_EQUAL(subnet.ToString(), "1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f");
-
+    BOOST_CHECK(!subnet.IsValid());
 }
 
 BOOST_AUTO_TEST_CASE(netbase_getgroup)
@@ -428,7 +436,8 @@ BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters)
     BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0", 11), ret));
     BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0example.com", 22), ret));
     BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0example.com\0", 23), ret));
-    BOOST_CHECK(LookupSubNet(std::string("5wyqrzbvrdsumnok.onion", 22), ret));
+    // We only do subnetting for IPv4 and IPv6
+    BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion", 22), ret));
     BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0", 23), ret));
     BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0example.com", 34), ret));
     BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0example.com\0", 35), ret));