Merge bitcoin/bitcoin#32176: net: Prevent accidental circuit sharing when using Tor stream isolation

ec81a72b36 net: Add randomized prefix to Tor stream isolation credentials (laanwj) c47f81e8ac net: Rename `_randomize_credentials` Proxy parameter to `tor_stream_isolation` (laanwj) Pull request description: Add a class TorsStreamIsolationCredentialsGenerator that generates unique credentials based on a randomly generated session prefix and an atomic counter. Use this in `ConnectThroughProxy` instead of a simple atomic int counter. This makes sure that different launches of the application won't share the same credentials, and thus circuits, even in edge cases. Example with `-debug=proxy`: ``` 2025-03-31T16:30:27Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-0:0afb2da441f5c105-0 2025-03-31T16:30:31Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-1:0afb2da441f5c105-1 ``` Thanks to hodlinator in https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2020973352 for the idea. ACKs for top commit: hodlinator: re-ACK ec81a72b36 jonatack: ACK ec81a72b36 danielabrozzoni: tACK ec81a72b36 Tree-SHA512: 195f5885fade77545977b91bdc41394234ae575679cb61631341df443fd8482cd74650104e323c7dbfff7826b10ad61692cca1284d6810f84500a3488f46597a
2026-04-20 11:49:07 +02:00 · 2025-04-10 12:42:34 -04:00
parent c58ae197a3 ec81a72b36
commit e364e6b509
8 changed files with 55 additions and 19 deletions
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1578,14 +1578,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
    if (proxyArg != "" && proxyArg != "0") {
        Proxy addrProxy;
        if (IsUnixSocketPath(proxyArg)) {
-            addrProxy = Proxy(proxyArg, proxyRandomize);
+            addrProxy = Proxy(proxyArg, /*tor_stream_isolation=*/proxyRandomize);
        } else {
            const std::optional<CService> proxyAddr{Lookup(proxyArg, 9050, fNameLookup)};
            if (!proxyAddr.has_value()) {
                return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg));
            }

-            addrProxy = Proxy(proxyAddr.value(), proxyRandomize);
+            addrProxy = Proxy(proxyAddr.value(), /*tor_stream_isolation=*/proxyRandomize);
        }

        if (!addrProxy.IsValid())
@@ -1614,14 +1614,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
            }
        } else {
            if (IsUnixSocketPath(onionArg)) {
-                onion_proxy = Proxy(onionArg, proxyRandomize);
+                onion_proxy = Proxy(onionArg, /*tor_stream_isolation=*/proxyRandomize);
            } else {
                const std::optional<CService> addr{Lookup(onionArg, 9050, fNameLookup)};
                if (!addr.has_value() || !addr->IsValid()) {
                    return InitError(strprintf(_("Invalid -onion address or hostname: '%s'"), onionArg));
                }

-                onion_proxy = Proxy(addr.value(), proxyRandomize);
+                onion_proxy = Proxy(addr.value(), /*tor_stream_isolation=*/proxyRandomize);
            }
        }
    }
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -749,6 +749,43 @@ bool IsProxy(const CNetAddr &addr) {
    return false;
 }

+/**
+ * Generate unique credentials for Tor stream isolation. Tor will create
+ * separate circuits for SOCKS5 proxy connections with different credentials, which
+ * makes it harder to correlate the connections.
+ */
+class TorStreamIsolationCredentialsGenerator
+{
+public:
+    TorStreamIsolationCredentialsGenerator():
+        m_prefix(GenerateUniquePrefix()) {
+    }
+
+    /** Return the next unique proxy credentials. */
+    ProxyCredentials Generate() {
+        ProxyCredentials auth;
+        auth.username = auth.password = strprintf("%s%i", m_prefix, m_counter);
+        ++m_counter;
+        return auth;
+    }
+
+    /** Size of session prefix in bytes. */
+    static constexpr size_t PREFIX_BYTE_LENGTH = 8;
+private:
+    const std::string m_prefix;
+    std::atomic<uint64_t> m_counter;
+
+    /** Generate a random prefix for each of the credentials returned by this generator.
+     * This makes sure that different launches of the application (either successively or in parallel)
+     * will not share the same circuits, as would be the case with a bare counter.
+     */
+    static std::string GenerateUniquePrefix() {
+        std::array<uint8_t, PREFIX_BYTE_LENGTH> prefix_bytes;
+        GetRandBytes(prefix_bytes);
+        return HexStr(prefix_bytes) + "-";
+    }
+};
+
 std::unique_ptr<Sock> ConnectThroughProxy(const Proxy& proxy,
                                          const std::string& dest,
                                          uint16_t port,
@@ -762,10 +799,9 @@ std::unique_ptr<Sock> ConnectThroughProxy(const Proxy& proxy,
    }

    // do socks negotiation
-    if (proxy.m_randomize_credentials) {
-        ProxyCredentials random_auth;
-        static std::atomic_int counter(0);
-        random_auth.username = random_auth.password = strprintf("%i", counter++);
+    if (proxy.m_tor_stream_isolation) {
+        static TorStreamIsolationCredentialsGenerator generator;
+        ProxyCredentials random_auth{generator.Generate()};
        if (!Socks5(dest, port, &random_auth, *sock)) {
            return {};
        }
--- a/src/netbase.h
+++ b/src/netbase.h
@@ -58,14 +58,14 @@ bool IsUnixSocketPath(const std::string& name);
 class Proxy
 {
 public:
-    Proxy() : m_is_unix_socket(false), m_randomize_credentials(false) {}
-    explicit Proxy(const CService& _proxy, bool _randomize_credentials = false) : proxy(_proxy), m_is_unix_socket(false), m_randomize_credentials(_randomize_credentials) {}
-    explicit Proxy(const std::string path, bool _randomize_credentials = false) : m_unix_socket_path(path), m_is_unix_socket(true), m_randomize_credentials(_randomize_credentials) {}
+    Proxy() : m_is_unix_socket(false), m_tor_stream_isolation(false) {}
+    explicit Proxy(const CService& _proxy, bool tor_stream_isolation = false) : proxy(_proxy), m_is_unix_socket(false), m_tor_stream_isolation(tor_stream_isolation) {}
+    explicit Proxy(const std::string path, bool tor_stream_isolation = false) : m_unix_socket_path(path), m_is_unix_socket(true), m_tor_stream_isolation(tor_stream_isolation) {}

    CService proxy;
    std::string m_unix_socket_path;
    bool m_is_unix_socket;
-    bool m_randomize_credentials;
+    bool m_tor_stream_isolation;

    bool IsValid() const
    {
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsdialog.cpp
@@ -482,7 +482,7 @@ QValidator::State ProxyAddressValidator::validate(QString &input, int &pos) cons
    if (!SplitHostPort(input.toStdString(), port, hostname) || port != 0) return QValidator::Invalid;

    CService serv(LookupNumeric(input.toStdString(), DEFAULT_GUI_PROXY_PORT));
-    Proxy addrProxy = Proxy(serv, true);
+    Proxy addrProxy = Proxy(serv, /*tor_stream_isolation=*/true);
    if (addrProxy.IsValid())
        return QValidator::Acceptable;

--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -609,7 +609,7 @@ static UniValue GetNetworksInfo()
        obj.pushKV("limited", !g_reachable_nets.Contains(network));
        obj.pushKV("reachable", g_reachable_nets.Contains(network));
        obj.pushKV("proxy", proxy.IsValid() ? proxy.ToString() : std::string());
-        obj.pushKV("proxy_randomize_credentials", proxy.m_randomize_credentials);
+        obj.pushKV("proxy_randomize_credentials", proxy.m_tor_stream_isolation);
        networks.push_back(std::move(obj));
    }
    return networks;
--- a/src/test/fuzz/i2p.cpp
+++ b/src/test/fuzz/i2p.cpp
@@ -34,7 +34,7 @@ FUZZ_TARGET(i2p, .init = initialize_i2p)

    const fs::path private_key_path = gArgs.GetDataDirNet() / "fuzzed_i2p_private_key";
    const CService addr{in6_addr(IN6ADDR_LOOPBACK_INIT), 7656};
-    const Proxy sam_proxy{addr, false};
+    const Proxy sam_proxy{addr, /*tor_stream_isolation=*/false};
    CThreadInterrupt interrupt;

    i2p::sam::Session session{private_key_path, sam_proxy, &interrupt};
--- a/src/test/i2p_tests.cpp
+++ b/src/test/i2p_tests.cpp
@@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv)

    CThreadInterrupt interrupt;
    const std::optional<CService> addr{Lookup("127.0.0.1", 9000, false)};
-    const Proxy sam_proxy(addr.value(), false);
+    const Proxy sam_proxy(addr.value(), /*tor_stream_isolation=*/false);
    i2p::sam::Session session(gArgs.GetDataDirNet() / "test_i2p_private_key", sam_proxy, &interrupt);

    {
@@ -114,7 +114,7 @@ BOOST_AUTO_TEST_CASE(listen_ok_accept_fail)

    CThreadInterrupt interrupt;
    const CService addr{in6_addr(IN6ADDR_LOOPBACK_INIT), /*port=*/7656};
-    const Proxy sam_proxy(addr, false);
+    const Proxy sam_proxy(addr, /*tor_stream_isolation=*/false);
    i2p::sam::Session session(gArgs.GetDataDirNet() / "test_i2p_private_key",
                              sam_proxy,
                              &interrupt);
@@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE(damaged_private_key)

        CThreadInterrupt interrupt;
        const CService addr{in6_addr(IN6ADDR_LOOPBACK_INIT), /*port=*/7656};
-        const Proxy sam_proxy{addr, false};
+        const Proxy sam_proxy{addr, /*tor_stream_isolation=*/false};
        i2p::sam::Session session(i2p_private_key_file, sam_proxy, &interrupt);

        {
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -407,7 +407,7 @@ void TorController::get_socks_cb(TorControlConnection& _conn, const TorControlRe
    // With m_randomize_credentials = true, generates unique SOCKS credentials per proxy connection (e.g., Tor).
    // Prevents connection correlation and enhances privacy by forcing different Tor circuits.
    // Requires Tor's IsolateSOCKSAuth (default enabled) for effective isolation (see IsolateSOCKSAuth section in https://2019.www.torproject.org/docs/tor-manual.html.en).
-    Proxy addrOnion = Proxy(resolved, /*_randomize_credentials=*/ true);
+    Proxy addrOnion = Proxy(resolved, /*tor_stream_isolation=*/ true);
    SetProxy(NET_ONION, addrOnion);

    const auto onlynets = gArgs.GetArgs("-onlynet");