[netgroupman] Move GetMappedAS() and GetGroup() logic to NetGroupManager

Reviewer hint: use:

`git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`

src/addrman.cpp

@@ -610,7 +610,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_
             pinfo->nRefCount++;
             vvNew[nUBucket][nUBucketPos] = nId;
             LogPrint(BCLog::ADDRMAN, "Added %s mapped to AS%i to new[%i][%i]\n",
-                     addr.ToString(), addr.GetMappedAS(m_netgroupman.GetAsmap()), nUBucket, nUBucketPos);
+                     addr.ToString(), m_netgroupman.GetMappedAS(addr), nUBucket, nUBucketPos);
         } else {
             if (pinfo->nRefCount == 0) {
                 Delete(nId);
@@ -669,7 +669,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT
         // move nId to the tried tables
         MakeTried(info, nId);
         LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n",
-                 addr.ToString(), addr.GetMappedAS(m_netgroupman.GetAsmap()), tried_bucket, tried_bucket_pos);
+                 addr.ToString(), m_netgroupman.GetMappedAS(addr), tried_bucket, tried_bucket_pos);
         return true;
     }
 }

src/netaddress.cpp

@@ -10,7 +10,6 @@
 #include <hash.h>
 #include <prevector.h>
 #include <tinyformat.h>
-#include <util/asmap.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 
@@ -722,113 +721,6 @@ Network CNetAddr::GetNetClass() const
     return m_net;
 }
 
-uint32_t CNetAddr::GetMappedAS(const std::vector<bool> &m_asmap) const {
-    const CNetAddr& address = *this;
-    uint32_t net_class = address.GetNetClass();
-    if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) {
-        return 0; // Indicates not found, safe because AS0 is reserved per RFC7607.
-    }
-    std::vector<bool> ip_bits(128);
-    if (address.HasLinkedIPv4()) {
-        // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits)
-        for (int8_t byte_i = 0; byte_i < 12; ++byte_i) {
-            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
-                ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1;
-            }
-        }
-        uint32_t ipv4 = address.GetLinkedIPv4();
-        for (int i = 0; i < 32; ++i) {
-            ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1;
-        }
-    } else {
-        // Use all 128 bits of the IPv6 address otherwise
-        assert(address.IsIPv6());
-        auto addr_bytes = address.GetAddrBytes();
-        for (int8_t byte_i = 0; byte_i < 16; ++byte_i) {
-            uint8_t cur_byte = addr_bytes[byte_i];
-            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
-                ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1;
-            }
-        }
-    }
-    uint32_t mapped_as = Interpret(m_asmap, ip_bits);
-    return mapped_as;
-}
-
-/**
- * Get the canonical identifier of our network group
- *
- * The groups are assigned in a way where it should be costly for an attacker to
- * obtain addresses with many different group identifiers, even if it is cheap
- * to obtain addresses with the same identifier.
- *
- * @note No two connections will be attempted to addresses with the same network
- *       group.
- */
-std::vector<unsigned char> CNetAddr::GetGroup(const std::vector<bool> &m_asmap) const
-{
-    const CNetAddr& address = *this;
-    std::vector<unsigned char> vchRet;
-    // If non-empty asmap is supplied and the address is IPv4/IPv6,
-    // return ASN to be used for bucketing.
-    uint32_t asn = GetMappedAS(m_asmap);
-    if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR).
-        vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket
-        for (int i = 0; i < 4; i++) {
-            vchRet.push_back((asn >> (8 * i)) & 0xFF);
-        }
-        return vchRet;
-    }
-
-    vchRet.push_back(address.GetNetClass());
-    int nStartByte{0};
-    int nBits{0};
-
-    if (address.IsLocal()) {
-        // all local addresses belong to the same group
-    } else if (address.IsInternal()) {
-        // All internal-usage addresses get their own group.
-        // Skip over the INTERNAL_IN_IPV6_PREFIX returned by CAddress::GetAddrBytes().
-        nStartByte = INTERNAL_IN_IPV6_PREFIX.size();
-        nBits = ADDR_INTERNAL_SIZE * 8;
-    } else if (!address.IsRoutable()) {
-        // all other unroutable addresses belong to the same group
-    } else if (address.HasLinkedIPv4()) {
-        // IPv4 addresses (and mapped IPv4 addresses) use /16 groups
-        uint32_t ipv4 = address.GetLinkedIPv4();
-        vchRet.push_back((ipv4 >> 24) & 0xFF);
-        vchRet.push_back((ipv4 >> 16) & 0xFF);
-        return vchRet;
-    } else if (address.IsTor() || address.IsI2P()) {
-        nBits = 4;
-    } else if (address.IsCJDNS()) {
-        // Treat in the same way as Tor and I2P because the address in all of
-        // them is "random" bytes (derived from a public key). However in CJDNS
-        // the first byte is a constant 0xfc, so the random bytes come after it.
-        // Thus skip the constant 8 bits at the start.
-        nBits = 12;
-    } else if (address.IsHeNet()) {
-        // for he.net, use /36 groups
-        nBits = 36;
-    } else {
-        // for the rest of the IPv6 network, use /32 groups
-        nBits = 32;
-    }
-
-    // Push our address onto vchRet.
-    auto addr_bytes = address.GetAddrBytes();
-    const size_t num_bytes = nBits / 8;
-    vchRet.insert(vchRet.end(), addr_bytes.begin() + nStartByte, addr_bytes.begin() + nStartByte + num_bytes);
-    nBits %= 8;
-    // ...for the last byte, push nBits and for the rest of the byte push 1's
-    if (nBits > 0) {
-        assert(num_bytes < addr_bytes.size());
-        vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1));
-    }
-
-    return vchRet;
-}
-
 std::vector<unsigned char> CNetAddr::GetAddrBytes() const
 {
     if (IsAddrV1Compatible()) {

src/netaddress.h

@@ -202,12 +202,6 @@ public:
     //! Whether this address has a linked IPv4 address (see GetLinkedIPv4()).
     bool HasLinkedIPv4() const;
 
-    // The AS on the BGP path to the node we use to diversify
-    // peers in AddrMan bucketing based on the AS infrastructure.
-    // The ip->AS mapping depends on how asmap is constructed.
-    uint32_t GetMappedAS(const std::vector<bool>& asmap) const;
-
-    std::vector<unsigned char> GetGroup(const std::vector<bool>& asmap) const;
     std::vector<unsigned char> GetAddrBytes() const;
     int GetReachabilityFrom(const CNetAddr* paddrPartner = nullptr) const;
 

src/netgroup.cpp

@@ -4,12 +4,100 @@
 
 #include <netgroup.h>
 
+#include <util/asmap.h>
+
 std::vector<unsigned char> NetGroupManager::GetGroup(const CNetAddr& address) const
 {
-    return address.GetGroup(m_asmap);
+    std::vector<unsigned char> vchRet;
+    // If non-empty asmap is supplied and the address is IPv4/IPv6,
+    // return ASN to be used for bucketing.
+    uint32_t asn = GetMappedAS(address);
+    if (asn != 0) { // Either asmap was empty, or address has non-asmappable net class (e.g. TOR).
+        vchRet.push_back(NET_IPV6); // IPv4 and IPv6 with same ASN should be in the same bucket
+        for (int i = 0; i < 4; i++) {
+            vchRet.push_back((asn >> (8 * i)) & 0xFF);
+        }
+        return vchRet;
+    }
+
+    vchRet.push_back(address.GetNetClass());
+    int nStartByte{0};
+    int nBits{0};
+
+    if (address.IsLocal()) {
+        // all local addresses belong to the same group
+    } else if (address.IsInternal()) {
+        // All internal-usage addresses get their own group.
+        // Skip over the INTERNAL_IN_IPV6_PREFIX returned by CAddress::GetAddrBytes().
+        nStartByte = INTERNAL_IN_IPV6_PREFIX.size();
+        nBits = ADDR_INTERNAL_SIZE * 8;
+    } else if (!address.IsRoutable()) {
+        // all other unroutable addresses belong to the same group
+    } else if (address.HasLinkedIPv4()) {
+        // IPv4 addresses (and mapped IPv4 addresses) use /16 groups
+        uint32_t ipv4 = address.GetLinkedIPv4();
+        vchRet.push_back((ipv4 >> 24) & 0xFF);
+        vchRet.push_back((ipv4 >> 16) & 0xFF);
+        return vchRet;
+    } else if (address.IsTor() || address.IsI2P()) {
+        nBits = 4;
+    } else if (address.IsCJDNS()) {
+        // Treat in the same way as Tor and I2P because the address in all of
+        // them is "random" bytes (derived from a public key). However in CJDNS
+        // the first byte is a constant 0xfc, so the random bytes come after it.
+        // Thus skip the constant 8 bits at the start.
+        nBits = 12;
+    } else if (address.IsHeNet()) {
+        // for he.net, use /36 groups
+        nBits = 36;
+    } else {
+        // for the rest of the IPv6 network, use /32 groups
+        nBits = 32;
+    }
+
+    // Push our address onto vchRet.
+    auto addr_bytes = address.GetAddrBytes();
+    const size_t num_bytes = nBits / 8;
+    vchRet.insert(vchRet.end(), addr_bytes.begin() + nStartByte, addr_bytes.begin() + nStartByte + num_bytes);
+    nBits %= 8;
+    // ...for the last byte, push nBits and for the rest of the byte push 1's
+    if (nBits > 0) {
+        assert(num_bytes < addr_bytes.size());
+        vchRet.push_back(addr_bytes[num_bytes] | ((1 << (8 - nBits)) - 1));
+    }
+
+    return vchRet;
 }
 
 uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const
 {
-    return address.GetMappedAS(m_asmap);
+    uint32_t net_class = address.GetNetClass();
+    if (m_asmap.size() == 0 || (net_class != NET_IPV4 && net_class != NET_IPV6)) {
+        return 0; // Indicates not found, safe because AS0 is reserved per RFC7607.
+    }
+    std::vector<bool> ip_bits(128);
+    if (address.HasLinkedIPv4()) {
+        // For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits)
+        for (int8_t byte_i = 0; byte_i < 12; ++byte_i) {
+            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
+                ip_bits[byte_i * 8 + bit_i] = (IPV4_IN_IPV6_PREFIX[byte_i] >> (7 - bit_i)) & 1;
+            }
+        }
+        uint32_t ipv4 = address.GetLinkedIPv4();
+        for (int i = 0; i < 32; ++i) {
+            ip_bits[96 + i] = (ipv4 >> (31 - i)) & 1;
+        }
+    } else {
+        // Use all 128 bits of the IPv6 address otherwise
+        assert(address.IsIPv6());
+        auto addr_bytes = address.GetAddrBytes();
+        for (int8_t byte_i = 0; byte_i < 16; ++byte_i) {
+            uint8_t cur_byte = addr_bytes[byte_i];
+            for (uint8_t bit_i = 0; bit_i < 8; ++bit_i) {
+                ip_bits[byte_i * 8 + bit_i] = (cur_byte >> (7 - bit_i)) & 1;
+            }
+        }
+    }
+    uint32_t mapped_as = Interpret(m_asmap, ip_bits);
+    return mapped_as;
 }

src/netgroup.h

@@ -22,8 +22,23 @@ public:
      * exists, since the data is const. */
     const std::vector<bool>& GetAsmap() const { return m_asmap; }
 
+    /**
+     * Get the canonical identifier of the network group for address.
+     *
+     * The groups are assigned in a way where it should be costly for an attacker to
+     * obtain addresses with many different group identifiers, even if it is cheap
+     * to obtain addresses with the same identifier.
+     *
+     * @note No two connections will be attempted to addresses with the same network
+     *       group.
+     */
     std::vector<unsigned char> GetGroup(const CNetAddr& address) const;
 
+    /**
+     *  Get the autonomous system on the BGP path to address.
+     *
+     *  The ip->AS mapping depends on how asmap is constructed.
+     */
     uint32_t GetMappedAS(const CNetAddr& address) const;
 
 private: