From 9bf3829558e393afd52f4e6f2fdb29b455f4f05f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 26 May 2024 20:08:32 +0000 Subject: [PATCH] merge bitcoin#25355: add support for transient addresses for outbound connections --- doc/i2p.md | 26 ++++++++++-- doc/release-notes-25355.md | 5 +++ src/i2p.cpp | 65 +++++++++++++++++++++-------- src/i2p.h | 19 +++++++++ src/net.cpp | 46 ++++++++++++-------- src/net.h | 17 +++++++- src/test/i2p_tests.cpp | 2 +- test/functional/p2p_i2p_sessions.py | 36 ++++++++++++++++ test/functional/test_runner.py | 1 + 9 files changed, 175 insertions(+), 42 deletions(-) create mode 100644 doc/release-notes-25355.md create mode 100755 test/functional/p2p_i2p_sessions.py diff --git a/doc/i2p.md b/doc/i2p.md index 3bc90259b0..cf36ac137c 100644 --- 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 new file mode 100644 index 0000000000..b539d6bdb1 --- /dev/null +++ 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 index 5e7e42fb77..30468e4511 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -12,11 +12,11 @@ #include #include #include -#include #include #include #include #include +#include #include #include @@ -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(INVALID_SOCKET)) + : m_private_key_file{private_key_file}, + m_control_host{control_host}, + m_interrupt{interrupt}, + m_control_sock{std::make_unique(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(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& private_key_b64 = SwapBase64(EncodeBase64(m_private_key)); + + SendRequestAndGetReply(*sock, + strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s", + session_id, + private_key_b64)); } - 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)); - - 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 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 index 3899dbf81f..6183885435 100644 --- a/src/i2p.h +++ b/src/i2p.h @@ -70,6 +70,19 @@ public: 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 @@ private: * 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 index e8175240f5..df3afbc046 100644 --- 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_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(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(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) + : 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 index d7cbecd779..c38d577b35 100644 --- a/src/net.h +++ b/src/net.h @@ -622,7 +622,7 @@ public: 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 = nullptr); ~CNode(); CNode(const CNode&) = delete; CNode& operator=(const CNode&) = delete; @@ -776,6 +776,18 @@ private: 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 m_i2p_sam_session GUARDED_BY(cs_hSocket); }; /** @@ -1498,7 +1510,8 @@ private: /** * 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 m_i2p_sam_session; diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp index 1c2a6a433d..2f36afaab8 100644 --- 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 new file mode 100755 index 0000000000..4e52522b81 --- /dev/null +++ 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 index d789f9839f..07c6efd4ab 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -330,6 +330,7 @@ BASE_SCRIPTS = [ 'feature_blocksdir.py', 'wallet_startup.py', 'p2p_i2p_ports.py', + 'p2p_i2p_sessions.py', 'feature_config_args.py', 'feature_settings.py', 'rpc_getdescriptorinfo.py',