From 2b781ad66e34000037f589c71366c203255ed058 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 8 Jun 2022 15:27:11 +0200 Subject: [PATCH 1/7] i2p: add support for creating transient sessions Instead of providing our destination (private key) to the I2P proxy when creating the session, ask it to generate one for us and do not save it on disk. --- src/i2p.cpp | 61 +++++++++++++++++++++++++++++++----------- src/i2p.h | 19 +++++++++++++ src/test/i2p_tests.cpp | 2 +- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index c45bcc15d2..9aa1ad0f47 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -12,11 +12,11 @@ #include #include #include -#include #include #include #include #include +#include #include #include @@ -115,8 +115,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} { } @@ -355,29 +366,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); - LogPrintfCategory(BCLog::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() diff --git a/src/i2p.h b/src/i2p.h index eb0a10103d..ebbcb437da 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/test/i2p_tests.cpp b/src/test/i2p_tests.cpp index bd9ba4b8f7..80684ac205 100644 --- a/src/test/i2p_tests.cpp +++ b/src/test/i2p_tests.cpp @@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv) i2p::sam::Session session(gArgs.GetDataDirNet() / "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; From a1580a04f5d7c9ecb30ee0d3bfdae519843a67ac Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 8 Jun 2022 17:26:24 +0200 Subject: [PATCH 2/7] net: store an optional I2P session in CNode and destroy it when `CNode::m_sock` is closed. I2P transient sessions are created per connection (i.e. per `CNode`) and should be destroyed when the connection is closed. Storing the session in `CNode` is a convenient way to destroy it together with the connection socket (`CNode::m_sock`). An alternative approach would be to store a list of all I2P sessions in `CConnman` and from `CNode::CloseSocketDisconnect()` to somehow ask the `CConnman` to destroy the relevant session. --- src/net.cpp | 30 +++++++++++++++++++----------- src/net.h | 26 ++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e87ac079b5..710a7b800e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -564,6 +564,7 @@ void CNode::CloseSocketDisconnect() LogPrint(BCLog::NET, "disconnecting peer=%d\n", id); m_sock.reset(); } + m_i2p_sam_session.reset(); } void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const { @@ -2702,20 +2703,27 @@ ServiceFlags CConnman::GetLocalServices() const unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; } -CNode::CNode(NodeId idIn, std::shared_ptr sock, const CAddress& addrIn, - uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, - const CAddress& addrBindIn, const std::string& addrNameIn, - ConnectionType conn_type_in, bool inbound_onion) +CNode::CNode(NodeId idIn, + std::shared_ptr sock, + 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) : m_sock{sock}, m_connected{GetTime()}, - addr(addrIn), - addrBind(addrBindIn), + 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) + m_inbound_onion{inbound_onion}, + nKeyedNetGroup{nKeyedNetGroupIn}, + id{idIn}, + nLocalHostNonce{nLocalHostNonceIn}, + m_conn_type{conn_type_in}, + m_i2p_sam_session{std::move(i2p_sam_session)} { if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND); diff --git a/src/net.h b/src/net.h index 2036e9078c..0e4afe9fe0 100644 --- a/src/net.h +++ b/src/net.h @@ -513,10 +513,16 @@ public: * criterium in CConnman::AttemptToEvictConnection. */ std::atomic m_min_ping_time{std::chrono::microseconds::max()}; - CNode(NodeId id, std::shared_ptr sock, 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, + std::shared_ptr sock, + 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(const CNode&) = delete; CNode& operator=(const CNode&) = delete; @@ -596,6 +602,18 @@ private: mapMsgTypeSize mapSendBytesPerMsgType GUARDED_BY(cs_vSend); mapMsgTypeSize mapRecvBytesPerMsgType 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(m_sock_mutex); }; /** From ae1e97ce863609e06be44a2632fb9d1fbb8e5698 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 8 Jun 2022 17:59:32 +0200 Subject: [PATCH 3/7] net: use transient I2P session for outbound if -i2pacceptincoming=0 If not accepting I2P connections, then do not create `CConnman::m_i2p_sam_session`. When opening a new outbound I2P connection either use `CConnman::m_i2p_sam_session` like before or create a temporary one and store it in `CNode` for destruction later. --- src/net.cpp | 24 +++++++++++++++++------- src/net.h | 3 ++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 710a7b800e..bbee6c125d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -485,18 +485,27 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo Proxy 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; @@ -547,7 +556,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo addr_bind, pszDest ? pszDest : "", conn_type, - /*inbound_onion=*/false); + /*inbound_onion=*/false, + std::move(i2p_transient_session)); pnode->AddRef(); // We're making a new connection, harvest entropy from the time (and our peer count) @@ -2260,7 +2270,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) } Proxy 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(gArgs.GetDataDirNet() / "i2p_private_key", i2p_sam.proxy, &interruptNet); } @@ -2334,7 +2344,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) // 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] { ThreadI2PAcceptIncoming(); }); } diff --git a/src/net.h b/src/net.h index 0e4afe9fe0..7156a2259f 100644 --- a/src/net.h +++ b/src/net.h @@ -1090,7 +1090,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; From 3914e472f5685c29aa3d1c6dc5af9a758313d6c1 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 9 Jun 2022 12:22:24 +0200 Subject: [PATCH 4/7] test: add a test that -i2pacceptincoming=0 creates a transient session The test is a bit primitive as it checks the Bitcoin Core log and assumes that if it logs that it creates a transient session, then it does that indeed. A more thorough test would be to check that it indeed sends the `SESSION CREATE ... DESTINATION=TRANSIENT` command and that it uses the returned I2P address for connecting, even for repeated connections to the same I2P peer. That would require a mocked SAM server (proxy) implementation in Python. --- test/functional/p2p_i2p_sessions.py | 36 +++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 37 insertions(+) create mode 100755 test/functional/p2p_i2p_sessions.py 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 2b365d8d10..d542286cfb 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -328,6 +328,7 @@ BASE_SCRIPTS = [ 'feature_blocksdir.py', 'wallet_startup.py', 'p2p_i2p_ports.py', + 'p2p_i2p_sessions.py', 'feature_config_args.py', 'feature_presegwit_node_upgrade.py', 'feature_settings.py', From 47c0d02f126c73755288c3084402098567964329 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 9 Jun 2022 12:04:33 +0200 Subject: [PATCH 5/7] doc: document I2P transient addresses usage in doc/i2p.md --- doc/i2p.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/doc/i2p.md b/doc/i2p.md index 39f65c4e5f..1599c2fe0f 100644 --- a/doc/i2p.md +++ b/doc/i2p.md @@ -47,9 +47,26 @@ In a typical situation, this suffices: bitcoind -i2psam=127.0.0.1:7656 ``` -The first time Bitcoin 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 Bitcoin Core data directory. +The first time Bitcoin 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 Bitcoin 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 Bitcoin 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 @@ -85,7 +102,8 @@ one of the networks has issues. ## I2P-related information in Bitcoin Core -There are several ways to see your I2P address in Bitcoin Core: +There are several ways to see your I2P address in Bitcoin Core if accepting +incoming I2P connections (`-i2pacceptincoming`): - in the "Local addresses" output of CLI `-netinfo` - in the "localaddresses" output of RPC `getnetworkinfo` - in the debug log (grep for `AddLocal`; the I2P address ends in `.b32.i2p`) From d7ec30b648721133b5a5ac3f52275f779c54310f Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 9 Jun 2022 12:10:37 +0200 Subject: [PATCH 6/7] doc: add release notes about the I2P transient addresses --- doc/release-notes-25355.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 doc/release-notes-25355.md diff --git a/doc/release-notes-25355.md b/doc/release-notes-25355.md new file mode 100644 index 0000000000..34dd2c3687 --- /dev/null +++ b/doc/release-notes-25355.md @@ -0,0 +1,8 @@ +Notable changes +=============== + +P2P and network changes +----------------------- + +- With I2P connections, a new, transient address is used for each outbound + connection if `-i2pacceptincoming=0`. (#25355) From 59aa54f7312f3441692c89feed86b8756d9d6b7a Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 29 Jul 2022 14:59:25 +0200 Subject: [PATCH 7/7] i2p: log "SAM session" instead of "session" This way the log messages are consistent with "Creating SAM session..." --- src/i2p.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index 9aa1ad0f47..f7d480988b 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -434,9 +434,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 = std::make_unique(INVALID_SOCKET);