Merge #19954: Complete the BIP155 implementation and upgrade to TORv3

dcf0cb4 tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov) 353a3fd net: advertise support for ADDRv2 via new message (Vasil Dimov) 201a459 net: CAddress & CAddrMan: (un)serialize as ADDRv2 (Vasil Dimov) 1d3ec2a Support bypassing range check in ReadCompactSize (Pieter Wuille) Pull request description: This PR contains the two remaining commits from #19031 to complete the [BIP155](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) implementation: `net: CAddress & CAddrMan: (un)serialize as ADDRv2` `net: advertise support for ADDRv2 via new message` plus one more commit: `tor: make a TORv3 hidden service instead of TORv2` ACKs for top commit: jonatack: re-ACK dcf0cb4 per `git diff 9b56a68 dcf0cb4` only change since last review is an update to the release notes which partially picked up the suggested text. Running a node on this branch and addnode-ing to 6 other Tor v3 nodes, I see "addrv2" and "sendaddrv2" messages in getpeerinfo in both the "bytesrecv_per_msg" and "bytessent_per_msg" JSON objects. sipa: ACK dcf0cb4 hebasto: re-ACK dcf0cb4, the node works flawlessly in all of the modes: Tor-only, clearnet-only, mixed. laanwj: Edit: I have to retract this ACK for now, I'm having some problems with this PR on a FreeBSD node. It drops all outgoing connections with this dcf0cb4 merged on master (12a1c3a). ariard: Code Review ACK dcf0cb4 Tree-SHA512: 28d4d0d817b8664d2f4b18c0e0f31579b2f0f2d23310ed213f1f436a4242afea14dfbf99e07e15889bc5c5c71ad50056797e9307ff8a90e96704f588a6171308
bitcoin · Oct 11, 2020 · 0b2abaa · 0b2abaa
2 parents 12a1c3a + dcf0cb4
commit 0b2abaa
Show file tree

Hide file tree

Showing 19 changed files with 596 additions and 48 deletions.
diff --git a/doc/files.md b/doc/files.md
@@ -58,7 +58,7 @@ Subdirectory       | File(s)               | Description
 `./`               | `guisettings.ini.bak` | Backup of former [GUI settings](#gui-settings) after `-resetguisettings` option is used
 `./`               | `ip_asn.map`          | IP addresses to Autonomous System Numbers (ASNs) mapping used for bucketing of the peers; path can be specified with the `-asmap` option
 `./`               | `mempool.dat`         | Dump of the mempool's transactions
-`./`               | `onion_private_key`   | Cached Tor onion service private key for `-listenonion` option
+`./`               | `onion_v3_private_key` | Cached Tor onion service private key for `-listenonion` option
 `./`               | `peers.dat`           | Peer IP address database (custom format)
 `./`               | `settings.json`       | Read-write settings set through GUI or RPC interfaces, augmenting manual settings from [bitcoin.conf](bitcoin-conf.md). File is created automatically if read-write settings storage is not disabled with `-nosettings` option. Path can be specified with `-settings` option
 `./`               | `.cookie`             | Session RPC authentication cookie; if used, created at start and deleted on shutdown; can be specified by `-rpccookiefile` option
@@ -100,6 +100,7 @@ Path           | Description | Repository notes
 `blkindex.dat` | Blockchain index BDB database; replaced by {`chainstate/`, `blocks/index/`, `blocks/revNNNNN.dat`<sup>[\[2\]](#note2)</sup>} in 0.8.0 | [PR #1677](https://github.com/bitcoin/bitcoin/pull/1677)
 `blk000?.dat`  | Block data (custom format, 2 GiB per file); replaced by `blocks/blkNNNNN.dat`<sup>[\[2\]](#note2)</sup> in 0.8.0 | [PR #1677](https://github.com/bitcoin/bitcoin/pull/1677)
 `addr.dat`     | Peer IP address BDB database; replaced by `peers.dat` in [0.7.0](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.7.0.md) | [PR #1198](https://github.com/bitcoin/bitcoin/pull/1198), [`928d3a01`](https://github.com/bitcoin/bitcoin/commit/928d3a011cc66c7f907c4d053f674ea77dc611cc)
+`onion_private_key` | Cached Tor onion service private key for `-listenonion` option. Was used for Tor v2 services; replaced by `onion_v3_private_key` in [0.21.0](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.21.0.md) | [PR #19954](https://github.com/bitcoin/bitcoin/pull/19954)
 
 ## Notes
 

diff --git a/doc/release-notes.md b/doc/release-notes.md
@@ -60,6 +60,14 @@ From Bitcoin Core 0.20.0 onwards, macOS versions earlier than 10.12 are no
 longer supported. Additionally, Bitcoin Core does not yet change appearance
 when macOS "dark mode" is activated.
 
+The node's known peers are persisted to disk in a file called `peers.dat`. The
+format of this file has been changed in a backwards-incompatible way in order to
+accommodate the storage of Tor v3 and other BIP155 addresses. This means that if
+the file is modified by 0.21.0 or newer then older versions will not be able to
+read it. Those old versions, in the event of a downgrade, will log an error
+message that deserialization has failed and will continue normal operation
+as if the file was missing, creating a new empty one. (#19954)
+
 Notable changes
 ===============
 
@@ -74,6 +82,17 @@ P2P and network changes
   node using P2P relay. This version reduces the initial broadcast guarantees
   for wallet transactions submitted via P2P to a node running the wallet. (#18038)
 
+- The Tor onion service that is automatically created by setting the
+  `-listenonion` configuration parameter will now be created as a Tor v3 service
+  instead of Tor v2. The private key that was used for Tor v2 (if any) will be
+  left untouched in the `onion_private_key` file in the data directory (see
+  `-datadir`) and can be removed if not needed. Bitcoin Core will no longer
+  attempt to read it. The private key for the Tor v3 service will be saved in a
+  file named `onion_v3_private_key`. To use the deprecated Tor v2 service (not
+  recommended), then `onion_private_key` can be copied over
+  `onion_v3_private_key`, e.g.
+  `cp -f onion_private_key onion_v3_private_key`. (#19954)
+
 Updated RPCs
 ------------
 

diff --git a/src/addrman.h b/src/addrman.h
@@ -12,6 +12,7 @@
 #include <random.h>
 #include <sync.h>
 #include <timedata.h>
+#include <tinyformat.h>
 #include <util/system.h>
 
 #include <fs.h>
@@ -264,6 +265,14 @@ friend class CAddrManTest;
     void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
 public:
+    //! Serialization versions.
+    enum class Format : uint8_t {
+        V0_HISTORICAL = 0,    //!< historic format, before commit e6b343d88
+        V1_DETERMINISTIC = 1, //!< for pre-asmap files
+        V2_ASMAP = 2,         //!< for files including asmap version
+        V3_BIP155 = 3,        //!< same as V2_ASMAP plus addresses are in BIP155 format
+    };
+
     // Compressed IP->ASN mapping, loaded from a file when a node starts.
     // Should be always empty if no file was provided.
     // This mapping is then used for bucketing nodes in Addrman.
@@ -285,8 +294,8 @@ friend class CAddrManTest;
 
 
     /**
-     * serialized format:
-     * * version byte (1 for pre-asmap files, 2 for files including asmap version)
+     * Serialized format.
+     * * version byte (@see `Format`)
      * * 0x20 + nKey (serialized as if it were a vector, for backward compatibility)
      * * nNew
      * * nTried
@@ -313,13 +322,16 @@ friend class CAddrManTest;
      * We don't use SERIALIZE_METHODS since the serialization and deserialization code has
      * very little in common.
      */
-    template<typename Stream>
-    void Serialize(Stream &s) const
+    template <typename Stream>
+    void Serialize(Stream& s_) const
     {
         LOCK(cs);
 
-        unsigned char nVersion = 2;
-        s << nVersion;
+        // Always serialize in the latest version (currently Format::V3_BIP155).
+
+        OverrideStream<Stream> s(&s_, s_.GetType(), s_.GetVersion() | ADDRV2_FORMAT);
+
+        s << static_cast<uint8_t>(Format::V3_BIP155);
         s << ((unsigned char)32);
         s << nKey;
         s << nNew;
@@ -370,14 +382,34 @@ friend class CAddrManTest;
         s << asmap_version;
     }
 
-    template<typename Stream>
-    void Unserialize(Stream& s)
+    template <typename Stream>
+    void Unserialize(Stream& s_)
     {
         LOCK(cs);
 
         Clear();
-        unsigned char nVersion;
-        s >> nVersion;
+
+        Format format;
+        s_ >> Using<CustomUintFormatter<1>>(format);
+
+        static constexpr Format maximum_supported_format = Format::V3_BIP155;
+        if (format > maximum_supported_format) {
+            throw std::ios_base::failure(strprintf(
+                "Unsupported format of addrman database: %u. Maximum supported is %u. "
+                "Continuing operation without using the saved list of peers.",
+                static_cast<uint8_t>(format),
+                static_cast<uint8_t>(maximum_supported_format)));
+        }
+
+        int stream_version = s_.GetVersion();
+        if (format >= Format::V3_BIP155) {
+            // Add ADDRV2_FORMAT to the version so that the CNetAddr and CAddress
+            // unserialize methods know that an address in addrv2 format is coming.
+            stream_version |= ADDRV2_FORMAT;
+        }
+
+        OverrideStream<Stream> s(&s_, s_.GetType(), stream_version);
+
         unsigned char nKeySize;
         s >> nKeySize;
         if (nKeySize != 32) throw std::ios_base::failure("Incorrect keysize in addrman deserialization");
@@ -386,7 +418,7 @@ friend class CAddrManTest;
         s >> nTried;
         int nUBuckets = 0;
         s >> nUBuckets;
-        if (nVersion != 0) {
+        if (format >= Format::V1_DETERMINISTIC) {
             nUBuckets ^= (1 << 30);
         }
 
@@ -449,21 +481,21 @@ friend class CAddrManTest;
             supplied_asmap_version = SerializeHash(m_asmap);
         }
         uint256 serialized_asmap_version;
-        if (nVersion > 1) {
+        if (format >= Format::V2_ASMAP) {
             s >> serialized_asmap_version;
         }
 
         for (int n = 0; n < nNew; n++) {
             CAddrInfo &info = mapInfo[n];
             int bucket = entryToBucket[n];
             int nUBucketPos = info.GetBucketPosition(nKey, true, bucket);
-            if (nVersion == 2 && nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && vvNew[bucket][nUBucketPos] == -1 &&
+            if (format >= Format::V2_ASMAP && nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && vvNew[bucket][nUBucketPos] == -1 &&
                 info.nRefCount < ADDRMAN_NEW_BUCKETS_PER_ADDRESS && serialized_asmap_version == supplied_asmap_version) {
                 // Bucketing has not changed, using existing bucket positions for the new table
                 vvNew[bucket][nUBucketPos] = n;
                 info.nRefCount++;
             } else {
-                // In case the new table data cannot be used (nVersion unknown, bucket count wrong or new asmap),
+                // In case the new table data cannot be used (format unknown, bucket count wrong or new asmap),
                 // try to give them a reference based on their primary source address.
                 LogPrint(BCLog::ADDRMAN, "Bucketing method was updated, re-bucketing addrman entries from disk\n");
                 bucket = info.GetNewBucket(nKey, m_asmap);

diff --git a/src/net.h b/src/net.h
@@ -867,6 +867,11 @@ class CNode
     bool m_legacyWhitelisted{false};
     bool fClient{false}; // set by version message
     bool m_limited_node{false}; //after BIP159, set by version message
+    /**
+     * Whether the peer has signaled support for receiving ADDRv2 (BIP155)
+     * messages, implying a preference to receive ADDRv2 instead of ADDR ones.
+     */
+    std::atomic_bool m_wants_addrv2{false};
     std::atomic_bool fSuccessfullyConnected{false};
     // Setting fDisconnect to true will cause the node to be disconnected the
     // next time DisconnectNodes() runs
@@ -1114,11 +1119,16 @@ class CNode
 
     void PushAddress(const CAddress& _addr, FastRandomContext &insecure_rand)
     {
+        // Whether the peer supports the address in `_addr`. For example,
+        // nodes that do not implement BIP155 cannot receive Tor v3 addresses
+        // because they require ADDRv2 (BIP155) encoding.
+        const bool addr_format_supported = m_wants_addrv2 || _addr.IsAddrV1Compatible();
+
         // Known checking here is only to save space from duplicates.
         // SendMessages will filter it again for knowns that were added
         // after addresses were pushed.
         assert(m_addr_known);
-        if (_addr.IsValid() && !m_addr_known->contains(_addr.GetKey())) {
+        if (_addr.IsValid() && !m_addr_known->contains(_addr.GetKey()) && addr_format_supported) {
             if (vAddrToSend.size() >= MAX_ADDR_TO_SEND) {
                 vAddrToSend[insecure_rand.randrange(vAddrToSend.size())] = _addr;
             } else {

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -23,6 +23,7 @@
 #include <random.h>
 #include <reverse_iterator.h>
 #include <scheduler.h>
+#include <streams.h>
 #include <tinyformat.h>
 #include <txmempool.h>
 #include <util/check.h> // For NDEBUG compile time check
@@ -2435,11 +2436,16 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
         pfrom.SetCommonVersion(greatest_common_version);
         pfrom.nVersion = nVersion;
 
+        const CNetMsgMaker msg_maker(greatest_common_version);
+
         if (greatest_common_version >= WTXID_RELAY_VERSION) {
-            m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::WTXIDRELAY));
+            m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::WTXIDRELAY));
         }
 
-        m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::VERACK));
+        m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK));
+
+        // Signal ADDRv2 support (BIP155).
+        m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2));
 
         pfrom.nServices = nServices;
         pfrom.SetAddrLocal(addrMe);
@@ -2608,16 +2614,25 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
         return;
     }
 
-    if (msg_type == NetMsgType::ADDR) {
+    if (msg_type == NetMsgType::ADDR || msg_type == NetMsgType::ADDRV2) {
+        int stream_version = vRecv.GetVersion();
+        if (msg_type == NetMsgType::ADDRV2) {
+            // Add ADDRV2_FORMAT to the version so that the CNetAddr and CAddress
+            // unserialize methods know that an address in v2 format is coming.
+            stream_version |= ADDRV2_FORMAT;
+        }
+
+        OverrideStream<CDataStream> s(&vRecv, vRecv.GetType(), stream_version);
         std::vector<CAddress> vAddr;
-        vRecv >> vAddr;
+
+        s >> vAddr;
 
         if (!pfrom.RelayAddrsWithConn()) {
             return;
         }
         if (vAddr.size() > MAX_ADDR_TO_SEND)
         {
-            Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size()));
+            Misbehaving(pfrom.GetId(), 20, strprintf("%s message size = %u", msg_type, vAddr.size()));
             return;
         }
 
@@ -2661,6 +2676,11 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
         return;
     }
 
+    if (msg_type == NetMsgType::SENDADDRV2) {
+        pfrom.m_wants_addrv2 = true;
+        return;
+    }
+
     if (msg_type == NetMsgType::SENDHEADERS) {
         LOCK(cs_main);
         State(pfrom.GetId())->fPreferHeaders = true;
@@ -4117,6 +4137,17 @@ bool PeerManager::SendMessages(CNode* pto)
             std::vector<CAddress> vAddr;
             vAddr.reserve(pto->vAddrToSend.size());
             assert(pto->m_addr_known);
+
+            const char* msg_type;
+            int make_flags;
+            if (pto->m_wants_addrv2) {
+                msg_type = NetMsgType::ADDRV2;
+                make_flags = ADDRV2_FORMAT;
+            } else {
+                msg_type = NetMsgType::ADDR;
+                make_flags = 0;
+            }
+
             for (const CAddress& addr : pto->vAddrToSend)
             {
                 if (!pto->m_addr_known->contains(addr.GetKey()))
@@ -4126,14 +4157,14 @@ bool PeerManager::SendMessages(CNode* pto)
                     // receiver rejects addr messages larger than MAX_ADDR_TO_SEND
                     if (vAddr.size() >= MAX_ADDR_TO_SEND)
                     {
-                        m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr));
+                        m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr));
                         vAddr.clear();
                     }
                 }
             }
             pto->vAddrToSend.clear();
             if (!vAddr.empty())
-                m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr));
+                m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr));
             // we only send the big addr message once
             if (pto->vAddrToSend.capacity() > 40)
                 pto->vAddrToSend.shrink_to_fit();

diff --git a/src/netaddress.cpp b/src/netaddress.cpp
@@ -474,6 +474,26 @@ bool CNetAddr::IsInternal() const
    return m_net == NET_INTERNAL;
 }
 
+bool CNetAddr::IsAddrV1Compatible() const
+{
+    switch (m_net) {
+    case NET_IPV4:
+    case NET_IPV6:
+    case NET_INTERNAL:
+        return true;
+    case NET_ONION:
+        return m_addr.size() == ADDR_TORV2_SIZE;
+    case NET_I2P:
+    case NET_CJDNS:
+        return false;
+    case NET_UNROUTABLE: // m_net is never and should not be set to NET_UNROUTABLE
+    case NET_MAX:        // m_net is never and should not be set to NET_MAX
+        assert(false);
+    } // no default case, so the compiler can warn about missing cases
+
+    assert(false);
+}
+
 enum Network CNetAddr::GetNetwork() const
 {
     if (IsInternal())
@@ -744,9 +764,12 @@ std::vector<unsigned char> CNetAddr::GetGroup(const std::vector<bool> &asmap) co
 
 std::vector<unsigned char> CNetAddr::GetAddrBytes() const
 {
-    uint8_t serialized[V1_SERIALIZATION_SIZE];
-    SerializeV1Array(serialized);
-    return {std::begin(serialized), std::end(serialized)};
+    if (IsAddrV1Compatible()) {
+        uint8_t serialized[V1_SERIALIZATION_SIZE];
+        SerializeV1Array(serialized);
+        return {std::begin(serialized), std::end(serialized)};
+    }
+    return std::vector<unsigned char>(m_addr.begin(), m_addr.end());
 }
 
 uint64_t CNetAddr::GetHash() const

diff --git a/src/netaddress.h b/src/netaddress.h
@@ -177,6 +177,12 @@ class CNetAddr
         bool IsRoutable() const;
         bool IsInternal() const;
         bool IsValid() const;
+
+        /**
+         * Check if the current object can be serialized in pre-ADDRv2/BIP155 format.
+         */
+        bool IsAddrV1Compatible() const;
+
         enum Network GetNetwork() const;
         std::string ToString() const;
         std::string ToStringIP() const;

diff --git a/src/protocol.cpp b/src/protocol.cpp
@@ -14,6 +14,8 @@ namespace NetMsgType {
 const char *VERSION="version";
 const char *VERACK="verack";
 const char *ADDR="addr";
+const char *ADDRV2="addrv2";
+const char *SENDADDRV2="sendaddrv2";
 const char *INV="inv";
 const char *GETDATA="getdata";
 const char *MERKLEBLOCK="merkleblock";
@@ -52,6 +54,8 @@ const static std::string allNetMessageTypes[] = {
     NetMsgType::VERSION,
     NetMsgType::VERACK,
     NetMsgType::ADDR,
+    NetMsgType::ADDRV2,
+    NetMsgType::SENDADDRV2,
     NetMsgType::INV,
     NetMsgType::GETDATA,
     NetMsgType::MERKLEBLOCK,