From e61023506e52ebd028d41afa4491c56c2e32b6ae Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 13 Jan 2021 15:47:11 +0100 Subject: [PATCH] revert 94d335 net: allow CSubNet of non-IP networks The problem with #20852 is that if a host is banned that cannot be serialized in addrv1 format (e.g. Torv3) it is serialized as a dummy IPv6 (`::`) in `banlist.dat`. So, upon restart, the ban entry is lost (is deserialized as an invalid subnet that matches nothing). This reverts commit 94d335da7f8232bc653c9b08b0a33b517b4c98ad from https://github.com/bitcoin/bitcoin/pull/20852. --- src/netaddress.cpp | 81 +++++++------------------------------- src/netaddress.h | 24 +---------- src/test/netbase_tests.cpp | 21 ++-------- 3 files changed, 19 insertions(+), 107 deletions(-) diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 85e46fd373ef2..b1f9d32d34c0d 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -1068,24 +1068,15 @@ CSubNet::CSubNet(const CNetAddr& addr, const CNetAddr& mask) : CSubNet() CSubNet::CSubNet(const CNetAddr& addr) : CSubNet() { - switch (addr.m_net) { - case NET_IPV4: - case NET_IPV6: - valid = true; - assert(addr.m_addr.size() <= sizeof(netmask)); - memset(netmask, 0xFF, addr.m_addr.size()); - break; - case NET_ONION: - case NET_I2P: - case NET_CJDNS: - valid = true; - break; - case NET_INTERNAL: - case NET_UNROUTABLE: - case NET_MAX: + valid = addr.IsIPv4() || addr.IsIPv6(); + if (!valid) { return; } + assert(addr.m_addr.size() <= sizeof(netmask)); + + memset(netmask, 0xFF, addr.m_addr.size()); + network = addr; } @@ -1097,21 +1088,6 @@ bool CSubNet::Match(const CNetAddr &addr) const { if (!valid || !addr.IsValid() || network.m_net != addr.m_net) return false; - - switch (network.m_net) { - case NET_IPV4: - case NET_IPV6: - break; - case NET_ONION: - case NET_I2P: - case NET_CJDNS: - case NET_INTERNAL: - return addr == network; - case NET_UNROUTABLE: - case NET_MAX: - return false; - } - assert(network.m_addr.size() == addr.m_addr.size()); for (size_t x = 0; x < addr.m_addr.size(); ++x) { if ((addr.m_addr[x] & netmask[x]) != network.m_addr[x]) { @@ -1123,35 +1099,18 @@ bool CSubNet::Match(const CNetAddr &addr) const std::string CSubNet::ToString() const { - std::string suffix; + assert(network.m_addr.size() <= sizeof(netmask)); - switch (network.m_net) { - case NET_IPV4: - case NET_IPV6: { - assert(network.m_addr.size() <= sizeof(netmask)); - - uint8_t cidr = 0; + uint8_t cidr = 0; - for (size_t i = 0; i < network.m_addr.size(); ++i) { - if (netmask[i] == 0x00) { - break; - } - cidr += NetmaskBits(netmask[i]); + for (size_t i = 0; i < network.m_addr.size(); ++i) { + if (netmask[i] == 0x00) { + break; } - - suffix = strprintf("/%u", cidr); - break; - } - case NET_ONION: - case NET_I2P: - case NET_CJDNS: - case NET_INTERNAL: - case NET_UNROUTABLE: - case NET_MAX: - break; + cidr += NetmaskBits(netmask[i]); } - return network.ToString() + suffix; + return network.ToString() + strprintf("/%u", cidr); } bool CSubNet::IsValid() const @@ -1161,19 +1120,7 @@ bool CSubNet::IsValid() const bool CSubNet::SanityCheck() const { - switch (network.m_net) { - case NET_IPV4: - case NET_IPV6: - break; - case NET_ONION: - case NET_I2P: - case NET_CJDNS: - return true; - case NET_INTERNAL: - case NET_UNROUTABLE: - case NET_MAX: - return false; - } + if (!(network.IsIPv4() || network.IsIPv6())) return false; for (size_t x = 0; x < network.m_addr.size(); ++x) { if (network.m_addr[x] & ~netmask[x]) return false; diff --git a/src/netaddress.h b/src/netaddress.h index b9eade7fd526a..cf878fe374898 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -462,33 +462,11 @@ class CSubNet bool SanityCheck() const; public: - /** - * Construct an invalid subnet (empty, `Match()` always returns false). - */ CSubNet(); - - /** - * Construct from a given network start and number of bits (CIDR mask). - * @param[in] addr Network start. Must be IPv4 or IPv6, otherwise an invalid subnet is - * created. - * @param[in] mask CIDR mask, must be in [0, 32] for IPv4 addresses and in [0, 128] for - * IPv6 addresses. Otherwise an invalid subnet is created. - */ CSubNet(const CNetAddr& addr, uint8_t mask); - - /** - * Construct from a given network start and mask. - * @param[in] addr Network start. Must be IPv4 or IPv6, otherwise an invalid subnet is - * created. - * @param[in] mask Network mask, must be of the same type as `addr` and not contain 0-bits - * followed by 1-bits. Otherwise an invalid subnet is created. - */ CSubNet(const CNetAddr& addr, const CNetAddr& mask); - /** - * Construct a single-host subnet. - * @param[in] addr The sole address to be contained in the subnet, can also be non-IPv[46]. - */ + //constructor for single ip subnet (/32 or /128) explicit CSubNet(const CNetAddr& addr); bool Match(const CNetAddr &addr) const; diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 66ad7bb5eaece..ac4db3a5b6850 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -226,22 +226,8 @@ BOOST_AUTO_TEST_CASE(subnet_test) // 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()); - - // Create Non-IP subnets. - - const CNetAddr tor_addr{ - ResolveIP("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion")}; - - subnet = CSubNet(tor_addr); - BOOST_CHECK(subnet.IsValid()); - BOOST_CHECK_EQUAL(subnet.ToString(), tor_addr.ToString()); - BOOST_CHECK(subnet.Match(tor_addr)); - BOOST_CHECK( - !subnet.Match(ResolveIP("kpgvmscirrdqpekbqjsvw5teanhatztpp2gl6eee4zkowvwfxwenqaid.onion"))); - BOOST_CHECK(!subnet.Match(ResolveIP("1.2.3.4"))); - - BOOST_CHECK(!CSubNet(tor_addr, 200).IsValid()); - BOOST_CHECK(!CSubNet(tor_addr, 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"); @@ -456,7 +442,8 @@ BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0"s, ret)); BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com"s, ret)); BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com\0"s, ret)); - BOOST_CHECK(LookupSubNet("5wyqrzbvrdsumnok.onion"s, ret)); + // We only do subnetting for IPv4 and IPv6 + BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion"s, ret)); BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0"s, ret)); BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0example.com"s, ret)); BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0example.com\0"s, ret));