Merge #18023: Fix some asmap issues

c86bc14 Make asmap Interpret tolerant of malicious map data (Pieter Wuille)
38c2395 Use ASNs for mapped IPv4 addresses correctly (Pieter Wuille)
6f8c937 Mark asmap const in statistics code (Pieter Wuille)
d58bcdc Avoid asmap copies in initialization (Pieter Wuille)

Pull request description:

  Here are a few things to improve in the asmap implementation. The first two commits are just code improvements. The last one is a bugfix (the exsting code wouldn't correctly apply ASN lookups to mapped/embedded IPv4 addresses).

ACKs for top commit:
  practicalswift:
    ACK c86bc14 -- patch looks correct
  naumenkogs:
    utACK c86bc14
  laanwj:
    ACK c86bc14
  jonatack:
    ACK c86bc14 code looks correct, built/ran tests, bitcoind with -asmap pointed to asmap/demo.map

Tree-SHA512: 1036f43152754d621bfbecfd3b7c7276e4670598fcaed42a3d275e51fa2cf3653e2c9e9cfa714f6c7719362541510e92171e076ac4169b55a0cc8908b2d514c0

Author: laanwj

src/init.cpp

@@ -1835,8 +1835,8 @@ bool AppInitMain(NodeContext& node)
             InitError(strprintf(_("Could not find or parse specified asmap: '%s'").translated, asmap_path));
             return false;
         }
-        node.connman->SetAsmap(asmap);
         const uint256 asmap_version = SerializeHash(asmap);
+        node.connman->SetAsmap(std::move(asmap));
         LogPrintf("Using asmap version %s for IP bucketing.\n", asmap_version.ToString());
     } else {
         LogPrintf("Using /16 prefix for IP bucketing.\n");

src/net.cpp

@@ -498,7 +498,7 @@ void CNode::SetAddrLocal(const CService& addrLocalIn) {
 
 #undef X
 #define X(name) stats.name = name
-void CNode::copyStats(CNodeStats &stats, std::vector<bool> &m_asmap)
+void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
 {
     stats.nodeid = this->GetId();
     X(nServices);

src/net.h

@@ -331,7 +331,7 @@ class CConnman
     */
     int64_t PoissonNextSendInbound(int64_t now, int average_interval_seconds);
 
-    void SetAsmap(std::vector<bool> asmap) { addrman.m_asmap = asmap; }
+    void SetAsmap(std::vector<bool> asmap) { addrman.m_asmap = std::move(asmap); }
 
 private:
     struct ListenSocket {
@@ -983,7 +983,7 @@ class CNode
 
     void CloseSocketDisconnect();
 
-    void copyStats(CNodeStats &stats, std::vector<bool> &m_asmap);
+    void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap);
 
     ServiceFlags GetLocalServices() const
     {

src/netaddress.cpp

@@ -401,6 +401,26 @@ bool CNetAddr::GetIn6Addr(struct in6_addr* pipv6Addr) const
     return true;
 }
 
+bool CNetAddr::HasLinkedIPv4() const
+{
+    return IsRoutable() && (IsIPv4() || IsRFC6145() || IsRFC6052() || IsRFC3964() || IsRFC4380());
+}
+
+uint32_t CNetAddr::GetLinkedIPv4() const
+{
+    if (IsIPv4() || IsRFC6145() || IsRFC6052()) {
+        // IPv4, mapped IPv4, SIIT translated IPv4: the IPv4 address is the last 4 bytes of the address
+        return ReadBE32(ip + 12);
+    } else if (IsRFC3964()) {
+        // 6to4 tunneled IPv4: the IPv4 address is in bytes 2-6
+        return ReadBE32(ip + 2);
+    } else if (IsRFC4380()) {
+        // Teredo tunneled IPv4: the IPv4 address is in the last 4 bytes of the address, but bitflipped
+        return ~ReadBE32(ip + 12);
+    }
+    assert(false);
+}
+
 uint32_t CNetAddr::GetNetClass() const {
     uint32_t net_class = NET_IPV6;
     if (IsLocal()) {
@@ -410,7 +430,7 @@ uint32_t CNetAddr::GetNetClass() const {
         net_class = NET_INTERNAL;
     } else if (!IsRoutable()) {
         net_class = NET_UNROUTABLE;
-    } else if (IsIPv4() || IsRFC6145() || IsRFC6052() || IsRFC3964() || IsRFC4380()) {
+    } else if (HasLinkedIPv4()) {
         net_class = NET_IPV4;
     } else if (IsTor()) {
         net_class = NET_ONION;
@@ -424,10 +444,24 @@ uint32_t CNetAddr::GetMappedAS(const std::vector<bool> &asmap) const {
         return 0; // Indicates not found, safe because AS0 is reserved per RFC7607.
     }
     std::vector<bool> ip_bits(128);
-    for (int8_t byte_i = 0; byte_i < 16; ++byte_i) {
-        uint8_t cur_byte = GetByte(15 - 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;
+    if (HasLinkedIPv4()) {
+        // For lookup, treat as if it was just an IPv4 address (pchIPv4 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] = (pchIPv4[byte_i] >> (7 - bit_i)) & 1;
+            }
+        }
+        uint32_t ipv4 = 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
+        for (int8_t byte_i = 0; byte_i < 16; ++byte_i) {
+            uint8_t cur_byte = GetByte(15 - 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(asmap, ip_bits);
@@ -463,51 +497,32 @@ std::vector<unsigned char> CNetAddr::GetGroup(const std::vector<bool> &asmap) co
     int nStartByte = 0;
     int nBits = 16;
 
-    // all local addresses belong to the same group
-    if (IsLocal())
-    {
+    if (IsLocal()) {
+        // all local addresses belong to the same group
         nBits = 0;
-    }
-    // all internal-usage addresses get their own group
-    if (IsInternal())
-    {
+    } else if (IsInternal()) {
+        // all internal-usage addresses get their own group
         nStartByte = sizeof(g_internal_prefix);
         nBits = (sizeof(ip) - sizeof(g_internal_prefix)) * 8;
-    }
-    // all other unroutable addresses belong to the same group
-    else if (!IsRoutable())
-    {
+    } else if (!IsRoutable()) {
+        // all other unroutable addresses belong to the same group
         nBits = 0;
-    }
-    // for IPv4 addresses, '1' + the 16 higher-order bits of the IP
-    // includes mapped IPv4, SIIT translated IPv4, and the well-known prefix
-    else if (IsIPv4() || IsRFC6145() || IsRFC6052())
-    {
-        nStartByte = 12;
-    }
-    // for 6to4 tunnelled addresses, use the encapsulated IPv4 address
-    else if (IsRFC3964())
-    {
-        nStartByte = 2;
-    }
-    // for Teredo-tunnelled IPv6 addresses, use the encapsulated IPv4 address
-    else if (IsRFC4380())
-    {
-        vchRet.push_back(GetByte(3) ^ 0xFF);
-        vchRet.push_back(GetByte(2) ^ 0xFF);
+    } else if (HasLinkedIPv4()) {
+        // IPv4 addresses (and mapped IPv4 addresses) use /16 groups
+        uint32_t ipv4 = GetLinkedIPv4();
+        vchRet.push_back((ipv4 >> 24) & 0xFF);
+        vchRet.push_back((ipv4 >> 16) & 0xFF);
         return vchRet;
-    }
-    else if (IsTor())
-    {
+    } else if (IsTor()) {
         nStartByte = 6;
         nBits = 4;
-    }
-    // for he.net, use /36 groups
-    else if (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x04 && GetByte(12) == 0x70)
+    } else if (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x04 && GetByte(12) == 0x70) {
+        // for he.net, use /36 groups
         nBits = 36;
-    // for the rest of the IPv6 network, use /32 groups
-    else
+    } else {
+        // for the rest of the IPv6 network, use /32 groups
         nBits = 32;
+    }
 
     // push our ip onto vchRet byte by byte...
     while (nBits >= 8)

src/netaddress.h

@@ -79,6 +79,11 @@ class CNetAddr
         bool GetInAddr(struct in_addr* pipv4Addr) const;
         uint32_t GetNetClass() const;
 
+        //! For IPv4, mapped IPv4, SIIT translated IPv4, Teredo, 6to4 tunneled addresses, return the relevant IPv4 address as a uint32.
+        uint32_t GetLinkedIPv4() const;
+        //! 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.

src/util/asmap.cpp

@@ -8,13 +8,14 @@
 
 namespace {
 
-uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, uint8_t minval, const std::vector<uint8_t> &bit_sizes)
+uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos, uint8_t minval, const std::vector<uint8_t> &bit_sizes)
 {
     uint32_t val = minval;
     bool bit;
     for (std::vector<uint8_t>::const_iterator bit_sizes_it = bit_sizes.begin();
         bit_sizes_it != bit_sizes.end(); ++bit_sizes_it) {
         if (bit_sizes_it + 1 != bit_sizes.end()) {
+            if (bitpos == endpos) break;
             bit = *bitpos;
             bitpos++;
         } else {
@@ -24,6 +25,7 @@ uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, uint8_t minval, c
             val += (1 << *bit_sizes_it);
         } else {
             for (int b = 0; b < *bit_sizes_it; b++) {
+                if (bitpos == endpos) break;
                 bit = *bitpos;
                 bitpos++;
                 val += bit << (*bit_sizes_it - 1 - b);
@@ -35,63 +37,67 @@ uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, uint8_t minval, c
 }
 
 const std::vector<uint8_t> TYPE_BIT_SIZES{0, 0, 1};
-uint32_t DecodeType(std::vector<bool>::const_iterator& bitpos)
+uint32_t DecodeType(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
 {
-    return DecodeBits(bitpos, 0, TYPE_BIT_SIZES);
+    return DecodeBits(bitpos, endpos, 0, TYPE_BIT_SIZES);
 }
 
 const std::vector<uint8_t> ASN_BIT_SIZES{15, 16, 17, 18, 19, 20, 21, 22, 23, 24};
-uint32_t DecodeASN(std::vector<bool>::const_iterator& bitpos)
+uint32_t DecodeASN(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
 {
-    return DecodeBits(bitpos, 1, ASN_BIT_SIZES);
+    return DecodeBits(bitpos, endpos, 1, ASN_BIT_SIZES);
 }
 
 
 const std::vector<uint8_t> MATCH_BIT_SIZES{1, 2, 3, 4, 5, 6, 7, 8};
-uint32_t DecodeMatch(std::vector<bool>::const_iterator& bitpos)
+uint32_t DecodeMatch(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
 {
-    return DecodeBits(bitpos, 2, MATCH_BIT_SIZES);
+    return DecodeBits(bitpos, endpos, 2, MATCH_BIT_SIZES);
 }
 
 
 const std::vector<uint8_t> JUMP_BIT_SIZES{5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30};
-uint32_t DecodeJump(std::vector<bool>::const_iterator& bitpos)
+uint32_t DecodeJump(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
 {
-    return DecodeBits(bitpos, 17, JUMP_BIT_SIZES);
+    return DecodeBits(bitpos, endpos, 17, JUMP_BIT_SIZES);
 }
 
 }
 
 uint32_t Interpret(const std::vector<bool> &asmap, const std::vector<bool> &ip)
 {
     std::vector<bool>::const_iterator pos = asmap.begin();
+    const std::vector<bool>::const_iterator endpos = asmap.end();
     uint8_t bits = ip.size();
-    uint8_t default_asn = 0;
+    uint32_t default_asn = 0;
     uint32_t opcode, jump, match, matchlen;
-    while (1) {
-        assert(pos != asmap.end());
-        opcode = DecodeType(pos);
+    while (pos != endpos) {
+        opcode = DecodeType(pos, endpos);
         if (opcode == 0) {
-            return DecodeASN(pos);
+            return DecodeASN(pos, endpos);
         } else if (opcode == 1) {
-            jump = DecodeJump(pos);
+            jump = DecodeJump(pos, endpos);
+            if (bits == 0) break;
             if (ip[ip.size() - bits]) {
+                if (jump >= endpos - pos) break;
                 pos += jump;
             }
             bits--;
         } else if (opcode == 2) {
-            match = DecodeMatch(pos);
+            match = DecodeMatch(pos, endpos);
             matchlen = CountBits(match) - 1;
             for (uint32_t bit = 0; bit < matchlen; bit++) {
+                if (bits == 0) break;
                 if ((ip[ip.size() - bits]) != ((match >> (matchlen - 1 - bit)) & 1)) {
                     return default_asn;
                 }
                 bits--;
             }
         } else if (opcode == 3) {
-            default_asn = DecodeASN(pos);
+            default_asn = DecodeASN(pos, endpos);
         } else {
-            assert(0);
+            break;
         }
     }
+    return 0; // 0 is not a valid ASN
 }