merge bitcoin#25355: add support for transient addresses for outbound connections

kwvg · PastaPastaPasta · commit 9bf3829558e3 · 2024-05-29T11:48:37.000-05:00
diff --git a/doc/i2p.md b/doc/i2p.md
@@ -47,9 +47,26 @@ In a typical situation, this suffices:
 dashd -i2psam=127.0.0.1:7656
 ```
 
-The first time Dash Core connects to the I2P router, its I2P address (and
-corresponding private key) will be automatically generated and saved in a file
-named `i2p_private_key` in the Dash Core data directory.
+The first time Dash Core connects to the I2P router, if
+`-i2pacceptincoming=1`, then it will automatically generate a persistent I2P
+address and its corresponding private key. The private key will be saved in a
+file named `i2p_private_key` in the Dash Core data directory. The persistent
+I2P address is used for accepting incoming connections and for making outgoing
+connections if `-i2pacceptincoming=1`. If `-i2pacceptincoming=0` then only
+outbound I2P connections are made and a different transient I2P address is used
+for each connection to improve privacy.
+
+## Persistent vs transient I2P addresses
+
+In I2P connections, the connection receiver sees the I2P address of the
+connection initiator. This is unlike the Tor network where the recipient does
+not know who is connecting to them and can't tell if two connections are from
+the same peer or not.
+
+If an I2P node is not accepting incoming connections, then Dash Core uses
+random, one-time, transient I2P addresses for itself for outbound connections
+to make it harder to discriminate, fingerprint or analyze it based on its I2P
+address.
 
 ## Additional configuration options related to I2P
 
@@ -89,7 +106,8 @@ of the networks has issues.
 
 ## I2P-related information in Dash Core
 
-There are several ways to see your I2P address in Dash Core:
+There are several ways to see your I2P address in Dash Core if accepting
+incoming I2P connections (`-i2pacceptincoming`):
 - in the debug log (grep for `AddLocal`, the I2P address ends in `.b32.i2p`)
 - in the output of the `getnetworkinfo` RPC in the "localaddresses" section
 - in the output of `dash-cli -netinfo` peer connections dashboard
diff --git a/doc/release-notes-25355.md b/doc/release-notes-25355.md
@@ -0,0 +1,5 @@
+P2P and network changes
+-----------------------
+
+- With I2P connections, a new, transient address is used for each outbound
+  connection if `-i2pacceptincoming=0`.
diff --git a/src/i2p.cpp b/src/i2p.cpp
@@ -12,11 +12,11 @@
 #include <netaddress.h>
 #include <netbase.h>
 #include <random.h>
-#include <util/strencodings.h>
 #include <tinyformat.h>
 #include <util/readwritefile.h>
 #include <util/sock.h>
 #include <util/spanparsing.h>
+#include <util/strencodings.h>
 #include <util/system.h>
 
 #include <chrono>
@@ -116,8 +116,19 @@ namespace sam {
 Session::Session(const fs::path& private_key_file,
                  const CService& control_host,
                  CThreadInterrupt* interrupt)
-    : m_private_key_file(private_key_file), m_control_host(control_host), m_interrupt(interrupt),
-      m_control_sock(std::make_unique<Sock>(INVALID_SOCKET))
+    : m_private_key_file{private_key_file},
+      m_control_host{control_host},
+      m_interrupt{interrupt},
+      m_control_sock{std::make_unique<Sock>(INVALID_SOCKET)},
+      m_transient{false}
+{
+}
+
+Session::Session(const CService& control_host, CThreadInterrupt* interrupt)
+    : m_control_host{control_host},
+      m_interrupt{interrupt},
+      m_control_sock{std::make_unique<Sock>(INVALID_SOCKET)},
+      m_transient{true}
 {
 }
 
@@ -356,29 +367,47 @@ void Session::CreateIfNotCreatedAlready()
         return;
     }
 
-    Log("Creating SAM session with %s", m_control_host.ToString());
+    const auto session_type = m_transient ? "transient" : "persistent";
+    const auto session_id = GetRandHash().GetHex().substr(0, 10); // full is overkill, too verbose in the logs
+
+    Log("Creating %s SAM session %s with %s", session_type, session_id, m_control_host.ToString());
 
     auto sock = Hello();
 
-    const auto& [read_ok, data] = ReadBinaryFile(m_private_key_file);
-    if (read_ok) {
-        m_private_key.assign(data.begin(), data.end());
+    if (m_transient) {
+        // The destination (private key) is generated upon session creation and returned
+        // in the reply in DESTINATION=.
+        const Reply& reply = SendRequestAndGetReply(
+            *sock,
+            strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=TRANSIENT", session_id));
+
+        m_private_key = DecodeI2PBase64(reply.Get("DESTINATION"));
     } else {
-        GenerateAndSavePrivateKey(*sock);
-    }
+        // Read our persistent destination (private key) from disk or generate
+        // one and save it to disk. Then use it when creating the session.
+        const auto& [read_ok, data] = ReadBinaryFile(m_private_key_file);
+        if (read_ok) {
+            m_private_key.assign(data.begin(), data.end());
+        } else {
+            GenerateAndSavePrivateKey(*sock);
+        }
 
-    const std::string& session_id = GetRandHash().GetHex().substr(0, 10); // full is an overkill, too verbose in the logs
-    const std::string& private_key_b64 = SwapBase64(EncodeBase64(m_private_key));
+        const std::string& private_key_b64 = SwapBase64(EncodeBase64(m_private_key));
 
-    SendRequestAndGetReply(*sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s",
-                                            session_id, private_key_b64));
+        SendRequestAndGetReply(*sock,
+                               strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s",
+                                         session_id,
+                                         private_key_b64));
+    }
 
     m_my_addr = CService(DestBinToAddr(MyDestination()), I2P_SAM31_PORT);
     m_session_id = session_id;
     m_control_sock = std::move(sock);
 
-    LogPrintf("I2P: SAM session created: session id=%s, my address=%s\n", m_session_id,
-              m_my_addr.ToString());
+    Log("%s SAM session %s created, my address=%s",
+        Capitalize(session_type),
+        m_session_id,
+        m_my_addr.ToString());
 }
 
 std::unique_ptr<Sock> Session::StreamAccept()
@@ -406,9 +435,9 @@ void Session::Disconnect()
 {
     if (m_control_sock->Get() != INVALID_SOCKET) {
         if (m_session_id.empty()) {
-            Log("Destroying incomplete session");
+            Log("Destroying incomplete SAM session");
         } else {
-            Log("Destroying session %s", m_session_id);
+            Log("Destroying SAM session %s", m_session_id);
         }
     }
     m_control_sock->Reset();
diff --git a/src/i2p.h b/src/i2p.h
@@ -70,6 +70,19 @@ class Session
             const CService& control_host,
             CThreadInterrupt* interrupt);
 
+    /**
+     * Construct a transient session which will generate its own I2P private key
+     * rather than read the one from disk (it will not be saved on disk either and
+     * will be lost once this object is destroyed). This will not initiate any IO,
+     * the session will be lazily created later when first used.
+     * @param[in] control_host Location of the SAM proxy.
+     * @param[in,out] interrupt If this is signaled then all operations are canceled as soon as
+     * possible and executing methods throw an exception. Notice: only a pointer to the
+     * `CThreadInterrupt` object is saved, so it must not be destroyed earlier than this
+     * `Session` object.
+     */
+    Session(const CService& control_host, CThreadInterrupt* interrupt);
+
     /**
      * Destroy the session, closing the internally used sockets. The sockets that have been
      * returned by `Accept()` or `Connect()` will not be closed, but they will be closed by
@@ -262,6 +275,12 @@ class Session
      * SAM session id.
      */
     std::string m_session_id GUARDED_BY(m_mutex);
+
+    /**
+     * Whether this is a transient session (the I2P private key will not be
+     * read or written to disk).
+     */
+    const bool m_transient;
 };
 
 } // namespace sam
diff --git a/src/net.cpp b/src/net.cpp
@@ -475,18 +475,27 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
     proxyType proxy;
     CAddress addr_bind;
     assert(!addr_bind.IsValid());
+    std::unique_ptr<i2p::sam::Session> i2p_transient_session;
 
     if (addrConnect.IsValid()) {
+        const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)};
         bool proxyConnectionFailed = false;
 
-        if (addrConnect.GetNetwork() == NET_I2P && m_i2p_sam_session.get() != nullptr) {
+        if (addrConnect.GetNetwork() == NET_I2P && use_proxy) {
             i2p::Connection conn;
-            if (m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed)) {
-                connected = true;
+
+            if (m_i2p_sam_session) {
+                connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed);
+            } else {
+                i2p_transient_session = std::make_unique<i2p::sam::Session>(proxy.proxy, &interruptNet);
+                connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed);
+            }
+
+            if (connected) {
                 sock = std::move(conn.sock);
                 addr_bind = CAddress{conn.me, NODE_NONE};
             }
-        } else if (GetProxy(addrConnect.GetNetwork(), proxy)) {
+        } else if (use_proxy) {
             sock = CreateSock(proxy.proxy);
             if (!sock) {
                 return nullptr;
@@ -528,7 +537,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
     if (!addr_bind.IsValid()) {
         addr_bind = GetBindAddress(sock->Get());
     }
-    CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type, /* inbound_onion */ false);
+    CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type, /* inbound_onion */ false, std::move(i2p_transient_session));
     pnode->AddRef();
     statsClient.inc("peers.connect", 1.0f);
 
@@ -571,6 +580,8 @@ void CNode::CloseSocketDisconnect(CConnman* connman)
 
     LogPrint(BCLog::NET, "disconnecting peer=%d\n", id);
     CloseSocket(hSocket);
+    m_i2p_sam_session.reset();
+
     statsClient.inc("peers.disconnect", 1.0f);
 }
 
@@ -3342,7 +3353,7 @@ bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_met
     }
 
     proxyType i2p_sam;
-    if (GetProxy(NET_I2P, i2p_sam)) {
+    if (GetProxy(NET_I2P, i2p_sam) && connOptions.m_i2p_accept_incoming) {
         m_i2p_sam_session = std::make_unique<i2p::sam::Session>(GetDataDir() / "i2p_private_key",
                                                                 i2p_sam.proxy, &interruptNet);
     }
@@ -3444,7 +3455,7 @@ bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_met
     // Process messages
     threadMessageHandler = std::thread(&util::TraceThread, "msghand", [this] { ThreadMessageHandler(); });
 
-    if (connOptions.m_i2p_accept_incoming && m_i2p_sam_session.get() != nullptr) {
+    if (m_i2p_sam_session) {
         threadI2PAcceptIncoming =
             std::thread(&util::TraceThread, "i2paccept", [this, &mn_sync] { ThreadI2PAcceptIncoming(mn_sync); });
     }
@@ -4012,17 +4023,18 @@ ServiceFlags CConnman::GetLocalServices() const
 
 unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }
 
-CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion)
-    : nTimeConnected(GetTimeSeconds()),
-      addr(addrIn),
-      addrBind(addrBindIn),
+CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion, std::unique_ptr<i2p::sam::Session>&& i2p_sam_session)
+    : nTimeConnected{GetTimeSeconds()},
+      addr{addrIn},
+      addrBind{addrBindIn},
       m_addr_name{addrNameIn.empty() ? addr.ToStringIPPort() : addrNameIn},
-      m_inbound_onion(inbound_onion),
-      nKeyedNetGroup(nKeyedNetGroupIn),
-      id(idIn),
-      nLocalHostNonce(nLocalHostNonceIn),
-      m_conn_type(conn_type_in),
-      nLocalServices(nLocalServicesIn)
+      m_inbound_onion{inbound_onion},
+      nKeyedNetGroup{nKeyedNetGroupIn},
+      id{idIn},
+      nLocalHostNonce{nLocalHostNonceIn},
+      m_conn_type{conn_type_in},
+      nLocalServices{nLocalServicesIn},
+      m_i2p_sam_session{std::move(i2p_sam_session)}
 {
     if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
     hSocket = hSocketIn;
diff --git a/src/net.h b/src/net.h
@@ -622,7 +622,7 @@ class CNode
 
     bool IsBlockRelayOnly() const;
 
-    CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
+    CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn, ConnectionType conn_type_in, bool inbound_onion, std::unique_ptr<i2p::sam::Session>&& i2p_sam_session = nullptr);
     ~CNode();
     CNode(const CNode&) = delete;
     CNode& operator=(const CNode&) = delete;
@@ -776,6 +776,18 @@ class CNode
 
     mapMsgCmdSize mapSendBytesPerMsgCmd GUARDED_BY(cs_vSend);
     mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);
+
+    /**
+     * If an I2P session is created per connection (for outbound transient I2P
+     * connections) then it is stored here so that it can be destroyed when the
+     * socket is closed. I2P sessions involve a data/transport socket (in `m_sock`)
+     * and a control socket (in `m_i2p_sam_session`). For transient sessions, once
+     * the data socket is closed, the control socket is not going to be used anymore
+     * and is just taking up resources. So better close it as soon as `m_sock` is
+     * closed.
+     * Otherwise this unique_ptr is empty.
+     */
+    std::unique_ptr<i2p::sam::Session> m_i2p_sam_session GUARDED_BY(cs_hSocket);
 };
 
 /**
@@ -1498,7 +1510,8 @@ friend class CNode;
 
     /**
      * I2P SAM session.
-     * Used to accept incoming and make outgoing I2P connections.
+     * Used to accept incoming and make outgoing I2P connections from a persistent
+     * address.
      */
     std::unique_ptr<i2p::sam::Session> m_i2p_sam_session;
 
diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp
@@ -31,7 +31,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv)
     i2p::sam::Session session(GetDataDir() / "test_i2p_private_key", CService{}, &interrupt);
 
     {
-        ASSERT_DEBUG_LOG("Creating SAM session");
+        ASSERT_DEBUG_LOG("Creating persistent SAM session");
         ASSERT_DEBUG_LOG("too many bytes without a terminator");
 
         i2p::Connection conn;
diff --git a/test/functional/p2p_i2p_sessions.py b/test/functional/p2p_i2p_sessions.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python3
+# Copyright (c) 2022-2022 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 whether persistent or transient I2P sessions are being used, based on `-i2pacceptincoming`.
+"""
+
+from test_framework.test_framework import BitcoinTestFramework
+
+
+class I2PSessions(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 2
+        # The test assumes that an I2P SAM proxy is not listening here.
+        self.extra_args = [
+            ["-i2psam=127.0.0.1:60000", "-i2pacceptincoming=1"],
+            ["-i2psam=127.0.0.1:60000", "-i2pacceptincoming=0"],
+        ]
+
+    def run_test(self):
+        addr = "zsxwyo6qcn3chqzwxnseusqgsnuw3maqnztkiypyfxtya4snkoka.b32.i2p"
+
+        self.log.info("Ensure we create a persistent session when -i2pacceptincoming=1")
+        node0 = self.nodes[0]
+        with node0.assert_debug_log(expected_msgs=[f"Creating persistent SAM session"]):
+            node0.addnode(node=addr, command="onetry")
+
+        self.log.info("Ensure we create a transient session when -i2pacceptincoming=0")
+        node1 = self.nodes[1]
+        with node1.assert_debug_log(expected_msgs=[f"Creating transient SAM session"]):
+            node1.addnode(node=addr, command="onetry")
+
+
+if __name__ == '__main__':
+    I2PSessions().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
@@ -330,6 +330,7 @@
     'feature_blocksdir.py',
     'wallet_startup.py',
     'p2p_i2p_ports.py',
+    'p2p_i2p_sessions.py',
     'feature_config_args.py',
     'feature_settings.py',
     'rpc_getdescriptorinfo.py',

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv)`
`31`	`31`	`i2p::sam::Session session(GetDataDir() / "test_i2p_private_key", CService{}, &interrupt);`
`32`	`32`
`33`	`33`	`{`
`34`		`- ASSERT_DEBUG_LOG("Creating SAM session");`
	`34`	`+ ASSERT_DEBUG_LOG("Creating persistent SAM session");`
`35`	`35`	`ASSERT_DEBUG_LOG("too many bytes without a terminator");`
`36`	`36`
`37`	`37`	`i2p::Connection conn;`