merge bitcoin#22112: Force port 0 in I2P

doc/i2p.md

@@ -70,3 +70,18 @@ RPC.
 
 Dash Core uses the [SAM v3.1](https://geti2p.net/en/docs/api/samv3) protocol
 to connect to the I2P network. Any I2P router that supports it can be used.
+
+## Ports in I2P and Dash Core
+
+Dash Core uses the [SAM v3.1](https://geti2p.net/en/docs/api/samv3)
+protocol. One particularity of SAM v3.1 is that it does not support ports,
+unlike newer versions of SAM (v3.2 and up) that do support them and default the
+port numbers to 0. From the point of view of peers that use newer versions of
+SAM or other protocols that support ports, a SAM v3.1 peer is connecting to them
+on port 0, from source port 0.
+
+To allow future upgrades to newer versions of SAM, Dash Core sets its
+listening port to 0 when listening for incoming I2P connections and advertises
+its own I2P address with port 0. Furthermore, it will not attempt to connect to
+I2P addresses with a non-zero port number because with SAM v3.1 the destination
+port (`TO_PORT`) is always set to 0 and is not in the control of Dash Core.

src/addrman.cpp

@@ -6,6 +6,7 @@
 #include <addrman.h>
 
 #include <hash.h>
+#include <i2p.h>
 #include <logging.h>
 #include <netaddress.h>
 #include <serialize.h>
@@ -733,3 +734,100 @@ std::vector<bool> CAddrMan::DecodeAsmap(fs::path path)
     }
     return bits;
 }
+
+void CAddrMan::ResetI2PPorts()
+{
+    for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; ++bucket) {
+        for (int i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
+            const auto id = vvNew[bucket][i];
+            if (id == -1) {
+                continue;
+            }
+            auto it = mapInfo.find(id);
+            if (it == mapInfo.end()) {
+                return;
+            }
+            auto& addr_info = it->second;
+            if (!addr_info.IsI2P() || addr_info.GetPort() == I2P_SAM31_PORT) {
+                continue;
+            }
+
+            auto addr_info_newport = addr_info;
+            // The below changes addr_info_newport.GetKey(), which is used in finding a
+            // bucket and a position within that bucket. So a re-bucketing may be necessary.
+            addr_info_newport.port = I2P_SAM31_PORT;
+
+            // Reposition entries of vvNew within the same bucket because we don't know the source
+            // address which led to the decision to store the entry in vvNew[bucket] so we can't
+            // re-evaluate that decision, but even if we could, CAddrInfo::GetNewBucket() does not
+            // use CAddrInfo::GetKey() so it would end up in the same bucket as before the port
+            // change.
+            const auto i_target = addr_info_newport.GetBucketPosition(nKey, true, bucket);
+
+            if (i_target == i) { // No need to re-position.
+                addr_info = addr_info_newport;
+                continue;
+            }
+
+            // Reposition from i to i_target, removing the entry from i_target (if any).
+            ClearNew(bucket, i_target);
+            vvNew[bucket][i_target] = id;
+            vvNew[bucket][i] = -1;
+            addr_info = addr_info_newport;
+        }
+    }
+
+    for (int bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; ++bucket) {
+        for (int i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
+            const auto id = vvTried[bucket][i];
+            if (id == -1) {
+                continue;
+            }
+            auto it = mapInfo.find(id);
+            if (it == mapInfo.end()) {
+                return;
+            }
+            auto& addr_info = it->second;
+            if (!addr_info.IsI2P() || addr_info.GetPort() == I2P_SAM31_PORT) {
+                continue;
+            }
+
+            auto addr_info_newport = addr_info;
+            // The below changes addr_info_newport.GetKey(), which is used in finding a
+            // bucket and a position within that bucket. So a re-bucketing may be necessary.
+            addr_info_newport.port = I2P_SAM31_PORT;
+
+            const auto bucket_target = addr_info_newport.GetTriedBucket(nKey, m_asmap);
+            const auto i_target = addr_info_newport.GetBucketPosition(nKey, false, bucket_target);
+
+            if (bucket_target == bucket && i_target == i) { // No need to re-position.
+                addr_info = addr_info_newport;
+                continue;
+            }
+
+            // Reposition from (bucket, i) to (bucket_target, i_target). If the latter is
+            // occupied, then move the entry from there to vvNew.
+
+            const auto old_target_id = vvTried[bucket_target][i_target];
+            if (old_target_id != -1) {
+                CAddrInfo& old_target_info = mapInfo[old_target_id];
+
+                old_target_info.fInTried = false;
+                vvTried[bucket_target][i_target] = -1;
+                --nTried;
+
+                const auto new_bucket = old_target_info.GetNewBucket(nKey, m_asmap);
+                const auto new_bucket_i = old_target_info.GetBucketPosition(nKey, true, new_bucket);
+                ClearNew(new_bucket, new_bucket_i);
+
+                old_target_info.nRefCount = 1;
+                vvNew[new_bucket][new_bucket_i] = old_target_id;
+                ++nNew;
+            }
+
+            vvTried[bucket_target][i_target] = id;
+            vvTried[bucket][i] = -1;
+            addr_info = addr_info_newport;
+        }
+    }
+}

src/addrman.h

@@ -461,6 +461,8 @@ public:
 
         RemoveInvalid();
 
+        ResetI2PPorts();
+
         Check();
     }
 
@@ -789,6 +791,14 @@ private:
     //! Remove invalid addresses.
     void RemoveInvalid() EXCLUSIVE_LOCKS_REQUIRED(cs);
 
+    /**
+     * Reset the ports of I2P peers to 0.
+     * This is needed as a temporary measure because now we enforce port 0 and
+     * only connect to I2P hosts if the port is 0, but in the early days some
+     * I2P addresses with port 8333 were rumoured and persisted into addrmans.
+     */
+    void ResetI2PPorts() EXCLUSIVE_LOCKS_REQUIRED(cs);
+
     friend class CAddrManTest;
 };
 

src/chainparams.h

@@ -9,11 +9,13 @@
 #include <chainparamsbase.h>
 #include <consensus/params.h>
 #include <llmq/params.h>
+#include <netaddress.h>
 #include <primitives/block.h>
 #include <protocol.h>
 #include <util/hash_type.h>
 
 #include <memory>
+#include <string>
 #include <vector>
 
 struct SeedSpec6 {
@@ -84,6 +86,15 @@ public:
     const Consensus::Params& GetConsensus() const { return consensus; }
     const CMessageHeader::MessageStartChars& MessageStart() const { return pchMessageStart; }
     uint16_t GetDefaultPort() const { return nDefaultPort; }
+    uint16_t GetDefaultPort(Network net) const
+    {
+        return net == NET_I2P ? I2P_SAM31_PORT : GetDefaultPort();
+    }
+    uint16_t GetDefaultPort(const std::string& addr) const
+    {
+        CNetAddr a;
+        return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : GetDefaultPort();
+    }
     uint16_t GetDefaultPlatformP2PPort() const { return nDefaultPlatformP2PPort; }
     uint16_t GetDefaultPlatformHTTPPort() const { return nDefaultPlatformHTTPPort; }
 

src/i2p.cpp

@@ -159,7 +159,7 @@ bool Session::Accept(Connection& conn)
             const std::string& peer_dest =
                 conn.sock->RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE);
 
-            conn.peer = CService(DestB64ToAddr(peer_dest), Params().GetDefaultPort());
+            conn.peer = CService(DestB64ToAddr(peer_dest), I2P_SAM31_PORT);
 
             return true;
         }
@@ -172,6 +172,13 @@ bool Session::Accept(Connection& conn)
 
 bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
 {
+    // Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy
+    // when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT.
+    if (to.GetPort() != I2P_SAM31_PORT) {
+        proxy_error = false;
+        return false;
+    }
+
     proxy_error = true;
 
     std::string session_id;
@@ -366,7 +373,7 @@ void Session::CreateIfNotCreatedAlready()
     SendRequestAndGetReply(*sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s",
                                             session_id, private_key_b64));
 
-    m_my_addr = CService(DestBinToAddr(MyDestination()), Params().GetDefaultPort());
+    m_my_addr = CService(DestBinToAddr(MyDestination()), I2P_SAM31_PORT);
     m_session_id = session_id;
     m_control_sock = std::move(sock);
 

src/init.cpp

@@ -589,7 +589,7 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-peertimeout=<n>", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet: %u, regtest: %u)", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

src/net.cpp

@@ -430,7 +430,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
     }
 
     // Resolve
-    const uint16_t default_port{Params().GetDefaultPort()};
+    const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) :
+                                                     Params().GetDefaultPort()};
     if (pszDest) {
         std::vector<CService> resolved;
         if (Lookup(pszDest, resolved,  default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) {
@@ -2398,8 +2399,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
             }
 
             // do not allow non-default ports, unless after 50 invalid addresses selected already
-            if ((!isMasternode || !Params().AllowMultiplePorts()) && addr.GetPort() != Params().GetDefaultPort() && addr.GetPort() != GetListenPort() && nTries < 50)
+            if ((!isMasternode || !Params().AllowMultiplePorts()) && addr.GetPort() != Params().GetDefaultPort(addr.GetNetwork()) && addr.GetPort() != GetListenPort() && nTries < 50) {
                 continue;
+            }
 
             addrConnect = addr;
             break;
@@ -2460,7 +2462,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo()
     }
 
     for (const std::string& strAddNode : lAddresses) {
-        CService service(LookupNumeric(strAddNode, Params().GetDefaultPort()));
+        CService service(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode)));
         AddedNodeInfo addedNode{strAddNode, CService(), false, false};
         if (service.IsValid()) {
             // strAddNode is an IP:port

src/netaddress.h

@@ -114,6 +114,9 @@ static constexpr size_t ADDR_CJDNS_SIZE = 16;
 /// Size of "internal" (NET_INTERNAL) address (in bytes).
 static constexpr size_t ADDR_INTERNAL_SIZE = 10;
 
+/// SAM 3.1 and earlier do not support specifying ports and force the port to 0.
+static constexpr uint16_t I2P_SAM31_PORT{0};
+
 /**
  * Network address.
  */

src/test/addrman_tests.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <addrman.h>
+#include <i2p.h>
 #include <hash.h>
 #include <netbase.h>
 #include <random.h>
@@ -967,5 +968,121 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
     BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
 }
 
+BOOST_AUTO_TEST_CASE(reset_i2p_ports)
+{
+    CAddrManTest addrman1;
+    CAddrManTest addrman2;
+    const uint32_t good_time{static_cast<uint32_t>(GetAdjustedTime())};
+    constexpr uint16_t port = 8333;
+
+    // Has its port changed, will be re-positioned within the same bucket in vvNew.
+    const CAddress i2p_new1{
+        ResolveService("72l3ucjkuscrbiiepoehuwqgknyzgo7zuix5ty4puwrkyhtmnsga.b32.i2p", port),
+        NODE_NONE,
+        good_time};
+
+    // Has its port changed, will not be re-positioned in vvNew because ports 0 and 10075 result in
+    // the same bucket position.
+    const CAddress i2p_new2{
+        ResolveService("gehtac45oaghz54ypyopim64mql7oad2bqclla74l6tfeolzmodq.b32.i2p", 10075),
+        NODE_NONE,
+        good_time};
+
+    // Remains unchanged, port is already as it should be.
+    const CAddress i2p_new3{
+        ResolveService("c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p",
+                       I2P_SAM31_PORT),
+        NODE_NONE,
+        good_time};
+
+    // Has its port changed, re-positioning in vvNew will cause i2p_new3 to be evicted.
+    const CAddress i2p_new4{
+        ResolveService("c4cbbkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p", port),
+        NODE_NONE,
+        good_time};
+
+    // Remains unchanged.
+    const CAddress ipv4_new{ResolveService("1.2.3.4", port), NODE_NONE, good_time};
+
+    // Has its port changed, will be re-positioned in vvTried.
+    const CAddress i2p_tried1{
+        ResolveService("h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p", port),
+        NODE_NONE,
+        good_time};
+
+    // Has its port changed, will not be re-positioned in vvTried because ports 0 and 10537
+    // result in the same position (bucket, i).
+    const CAddress i2p_tried2{
+        ResolveService("pjs7or2ctvteeo5tu4bwyrtydeuhqhvdprtujn4daxr75jpebjxa.b32.i2p", 10537),
+        NODE_NONE,
+        good_time};
+
+    // Remains unchanged, port is already as it should be.
+    const CAddress i2p_tried3{
+        ResolveService("hnbbyjpxx54623l555sta7pocy3se4sdgmuebi5k6reesz5rjp6q.b32.i2p",
+                       I2P_SAM31_PORT),
+        NODE_NONE,
+        good_time};
+
+    // Has its port changed, re-positioning in vvTried will cause i2p_tried3 to be moved to vvNew.
+    const CAddress i2p_tried4{
+        ResolveService("hna37nqr3ahkqv62cuqfwgtneekvvpnuc4i4f6yo7tpoqjswvcwa.b32.i2p", port),
+        NODE_NONE,
+        good_time};
+
+    // Remains unchanged.
+    const CAddress ipv4_tried{ResolveService("2.3.4.5", port), NODE_NONE, good_time};
+
+    const CNetAddr source;
+
+    CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
+
+    addrman1.Add(i2p_new1, source);
+    addrman1.Add(i2p_new2, source);
+    addrman1.Add(i2p_new3, source);
+    addrman1.Add(i2p_new4, source);
+    addrman1.Add(ipv4_new, source);
+
+    addrman1.Add(i2p_tried1, source);
+    addrman1.Good(i2p_tried1);
+    addrman1.Add(i2p_tried2, source);
+    addrman1.Good(i2p_tried2);
+    addrman1.Add(i2p_tried3, source);
+    addrman1.Good(i2p_tried3);
+    addrman1.Add(i2p_tried4, source);
+    addrman1.Good(i2p_tried4);
+    addrman1.Add(ipv4_tried, source);
+    addrman1.Good(ipv4_tried);
+
+    stream << addrman1;
+    stream >> addrman2;
+
+    const size_t max_addresses{0};
+    const size_t max_pct{0};
+
+    auto addresses = addrman2.GetAddr(max_addresses, max_pct, NET_I2P);
+    BOOST_REQUIRE_EQUAL(addresses.size(), 7UL);
+    std::sort(addresses.begin(), addresses.end()); // Just some deterministic order.
+    BOOST_CHECK_EQUAL(addresses[0].ToStringIP(), i2p_new4.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[0].GetPort(), I2P_SAM31_PORT);
+    BOOST_CHECK_EQUAL(addresses[1].ToStringIP(), i2p_new2.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[1].GetPort(), I2P_SAM31_PORT);
+    BOOST_CHECK_EQUAL(addresses[2].ToStringIP(), i2p_tried4.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[2].GetPort(), I2P_SAM31_PORT);
+    BOOST_CHECK_EQUAL(addresses[3].ToStringIP(), i2p_tried3.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[3].GetPort(), I2P_SAM31_PORT);
+    BOOST_CHECK_EQUAL(addresses[4].ToStringIP(), i2p_tried1.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[4].GetPort(), I2P_SAM31_PORT);
+    BOOST_CHECK_EQUAL(addresses[5].ToStringIP(), i2p_tried2.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[5].GetPort(), I2P_SAM31_PORT);
+    BOOST_CHECK_EQUAL(addresses[6].ToStringIP(), i2p_new1.ToStringIP());
+    BOOST_CHECK_EQUAL(addresses[6].GetPort(), I2P_SAM31_PORT);
+
+    addresses = addrman2.GetAddr(max_addresses, max_pct, NET_IPV4);
+    BOOST_REQUIRE_EQUAL(addresses.size(), 2UL);
+    std::sort(addresses.begin(), addresses.end()); // Just some deterministic order.
+    BOOST_CHECK_EQUAL(addresses[0].ToStringIPPort(), ipv4_new.ToStringIPPort());
+    BOOST_CHECK_EQUAL(addresses[1].ToStringIPPort(), ipv4_tried.ToStringIPPort());
+}
 
 BOOST_AUTO_TEST_SUITE_END()

test/functional/p2p_i2p_ports.py

@@ -0,0 +1,43 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021-2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test ports handling for I2P hosts
+"""
+
+import re
+
+from test_framework.test_framework import BitcoinTestFramework
+
+
+class I2PPorts(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        # The test assumes that an I2P SAM proxy is not listening here.
+        self.extra_args = [["-i2psam=127.0.0.1:60000"]]
+
+    def run_test(self):
+        node = self.nodes[0]
+
+        self.log.info("Ensure we don't try to connect if port!=0")
+        addr = "zsxwyo6qcn3chqzwxnseusqgsnuw3maqnztkiypyfxtya4snkoka.b32.i2p:8333"
+        raised = False
+        try:
+            with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]):
+                node.addnode(node=addr, command="onetry")
+        except AssertionError as e:
+            raised = True
+            if not re.search(r"Expected messages .* does not partially match log", str(e)):
+                raise AssertionError(f"Assertion raised as expected, but with an unexpected message: {str(e)}")
+        if not raised:
+            raise AssertionError("Assertion should have been raised")
+
+        self.log.info("Ensure we try to connect if port=0 and get an error due to missing I2P proxy")
+        addr = "h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0"
+        with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]):
+            node.addnode(node=addr, command="onetry")
+
+
+if __name__ == '__main__':
+    I2PPorts().main()

test/functional/test_runner.py

@@ -266,6 +266,7 @@ BASE_SCRIPTS = [
     'p2p_permissions.py',
     'feature_blocksdir.py',
     'wallet_startup.py',
+    'p2p_i2p_ports.py',
     'feature_config_args.py',
     'feature_settings.py',
     'rpc_getdescriptorinfo.py',