Merge bitcoin/bitcoin#27411: p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting

e7cf865 test: add unit test for local address advertising (Martin Zumsande)
f4754b9 net: restrict self-advertisements with privacy networks (Martin Zumsande)
e4d541c net, refactor: pass reference for peer address in GetReachabilityFrom (Martin Zumsande)
62d73f5 net, refactor: pass CNode instead of CNetAddr to GetLocalAddress (Martin Zumsande)

Pull request description:

  The current logic for self-advertisements works such that we detect as many local addresses as we can, and then, using the scoring matrix from `CNetAddr::GetReachabilityFrom()`, self-advertise with the address that fits best to our peer.
  It is in general not hard for our peers to distinguish our self-advertisements from other addrs we send them, because we self-advertise every ~24h and because the first addr we send over a connection is likely our self-advertisement.

  `GetReachabilityFrom()` currently only takes into account actual reachability, but not whether we'd _want_ to announce our identity for one network to peers from other networks, which is not straightforward in connection with privacy networks.

  While the general approach is to prefer self-advertising with the address for the network our peer is on, there are several special situations in which we don't have one, and as a result could allow self-advertise other local addresses, for example:

  A) We run i2p and clearnet, use `-i2pacceptincoming=0` (so we have no local i2p address), and we have a local ipv4 address. In this case, we'd advertise the ipv4 address to our outbound i2p peers.

  B) Our `-discover` logic cannot detect any local clearnet addresses in our network environment, but we are actually reachable over clearnet. If we ran bitcoind clearnet-only, we'd always advertise the address our peer sees us with instead, and could get inbound peers this way. Now, if we also have an onion service running (but aren't using tor as a proxy for clearnet connections), we could advertise our onion address to clearnet peers, so that they would be able to connect our clearnet and onion identities.

  This PR tries to avoid these situations by
  1.) never advertising our local Tor or I2P address to peers from other networks.
  2.) never advertising local addresses from non-anonymity networks to peers from Tor or I2P

  Note that this affects only our own self-advertisements, the rules to forward other people's addrs are not changed.

  [Edit] after Initial [discussion](bitcoin/bitcoin#27411 (comment)): CJDNS is not being treated like Tor and I2P at least for now, because it has different privacy properties and for the practical reason that it has still very few bitcoin nodes.

ACKs for top commit:
  achow101:
    ACK e7cf865
  vasild:
    ACK e7cf865
  luke-jr:
    utACK e7cf865

Tree-SHA512: 3db8415dea6f82223d11a23bd6cbb3b8cf68831321280e926034a1f110cbe22562570013925f6fa20d8f08e41d0202fd69c733d9f16217318a660d2a1a21b795

Author: achow101

src/net.cpp

@@ -146,7 +146,7 @@ uint16_t GetListenPort()
 }
 
 // find 'best' local address for a particular peer
-bool GetLocal(CService& addr, const CNetAddr *paddrPeer)
+bool GetLocal(CService& addr, const CNode& peer)
 {
     if (!fListen)
         return false;
@@ -157,8 +157,18 @@ bool GetLocal(CService& addr, const CNetAddr *paddrPeer)
         LOCK(g_maplocalhost_mutex);
         for (const auto& entry : mapLocalHost)
         {
+            // For privacy reasons, don't advertise our privacy-network address
+            // to other networks and don't advertise our other-network address
+            // to privacy networks.
+            const Network our_net{entry.first.GetNetwork()};
+            const Network peers_net{peer.ConnectedThroughNetwork()};
+            if (our_net != peers_net &&
+                (our_net == NET_ONION || our_net == NET_I2P ||
+                 peers_net == NET_ONION || peers_net == NET_I2P)) {
+                continue;
+            }
             int nScore = entry.second.nScore;
-            int nReachability = entry.first.GetReachabilityFrom(paddrPeer);
+            int nReachability = entry.first.GetReachabilityFrom(peer.addr);
             if (nReachability > nBestReachability || (nReachability == nBestReachability && nScore > nBestScore))
             {
                 addr = CService(entry.first, entry.second.nPort);
@@ -196,10 +206,10 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn)
 // Otherwise, return the unroutable 0.0.0.0 but filled in with
 // the normal parameters, since the IP may be changed to a useful
 // one by discovery.
-CService GetLocalAddress(const CNetAddr& addrPeer)
+CService GetLocalAddress(const CNode& peer)
 {
     CService addr;
-    if (GetLocal(addr, &addrPeer)) {
+    if (GetLocal(addr, peer)) {
         return addr;
     }
     return CService{CNetAddr(), GetListenPort()};
@@ -222,7 +232,7 @@ bool IsPeerAddrLocalGood(CNode *pnode)
 
 std::optional<CService> GetLocalAddrForPeer(CNode& node)
 {
-    CService addrLocal{GetLocalAddress(node.addr)};
+    CService addrLocal{GetLocalAddress(node)};
     if (gArgs.GetBoolArg("-addrmantest", false)) {
         // use IPv4 loopback during addrmantest
         addrLocal = CService(LookupNumeric("127.0.0.1", GetListenPort()));

src/net.h

@@ -164,8 +164,8 @@ bool AddLocal(const CNetAddr& addr, int nScore = LOCAL_NONE);
 void RemoveLocal(const CService& addr);
 bool SeenLocal(const CService& addr);
 bool IsLocal(const CService& addr);
-bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr);
-CService GetLocalAddress(const CNetAddr& addrPeer);
+bool GetLocal(CService& addr, const CNode& peer);
+CService GetLocalAddress(const CNode& peer);
 CService MaybeFlipIPv6toCJDNS(const CService& service);
 
 

src/netaddress.cpp

@@ -723,19 +723,16 @@ std::vector<unsigned char> CNetAddr::GetAddrBytes() const
 
 // private extensions to enum Network, only returned by GetExtNetwork,
 // and only used in GetReachabilityFrom
-static const int NET_UNKNOWN = NET_MAX + 0;
-static const int NET_TEREDO  = NET_MAX + 1;
-int static GetExtNetwork(const CNetAddr *addr)
+static const int NET_TEREDO = NET_MAX;
+int static GetExtNetwork(const CNetAddr& addr)
 {
-    if (addr == nullptr)
-        return NET_UNKNOWN;
-    if (addr->IsRFC4380())
+    if (addr.IsRFC4380())
         return NET_TEREDO;
-    return addr->GetNetwork();
+    return addr.GetNetwork();
 }
 
 /** Calculates a metric for how reachable (*this) is from a given partner */
-int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
+int CNetAddr::GetReachabilityFrom(const CNetAddr& paddrPartner) const
 {
     enum Reachability {
         REACH_UNREACHABLE,
@@ -750,7 +747,7 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
     if (!IsRoutable() || IsInternal())
         return REACH_UNREACHABLE;
 
-    int ourNet = GetExtNetwork(this);
+    int ourNet = GetExtNetwork(*this);
     int theirNet = GetExtNetwork(paddrPartner);
     bool fTunnel = IsRFC3964() || IsRFC6052() || IsRFC6145();
 
@@ -790,7 +787,6 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
         case NET_IPV6:    return REACH_IPV6_WEAK;
         case NET_IPV4:    return REACH_IPV4;
         }
-    case NET_UNKNOWN:
     case NET_UNROUTABLE:
     default:
         switch(ourNet) {

src/netaddress.h

@@ -203,7 +203,7 @@ class CNetAddr
     bool HasLinkedIPv4() const;
 
     std::vector<unsigned char> GetAddrBytes() const;
-    int GetReachabilityFrom(const CNetAddr* paddrPartner = nullptr) const;
+    int GetReachabilityFrom(const CNetAddr& paddrPartner) const;
 
     explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0);
     bool GetIn6Addr(struct in6_addr* pipv6Addr) const;

src/test/fuzz/netaddress.cpp

@@ -84,7 +84,7 @@ FUZZ_TARGET(netaddress)
     (void)CServiceHash(0, 0)(service);
 
     const CNetAddr other_net_addr = ConsumeNetAddr(fuzzed_data_provider);
-    (void)net_addr.GetReachabilityFrom(&other_net_addr);
+    (void)net_addr.GetReachabilityFrom(other_net_addr);
     (void)sub_net.Match(other_net_addr);
 
     const CService other_service{net_addr, fuzzed_data_provider.ConsumeIntegral<uint16_t>()};

src/test/net_tests.cpp

@@ -904,4 +904,109 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
     TestOnlyResetTimeData();
 }
 
+
+BOOST_AUTO_TEST_CASE(advertise_local_address)
+{
+    auto CreatePeer = [](const CAddress& addr) {
+        return std::make_unique<CNode>(/*id=*/0,
+                                       /*sock=*/nullptr,
+                                       addr,
+                                       /*nKeyedNetGroupIn=*/0,
+                                       /*nLocalHostNonceIn=*/0,
+                                       CAddress{},
+                                       /*pszDest=*/std::string{},
+                                       ConnectionType::OUTBOUND_FULL_RELAY,
+                                       /*inbound_onion=*/false);
+    };
+    SetReachable(NET_CJDNS, true);
+
+    CAddress addr_ipv4{Lookup("1.2.3.4", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_ipv4.IsValid());
+    BOOST_REQUIRE(addr_ipv4.IsIPv4());
+
+    CAddress addr_ipv6{Lookup("1122:3344:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_ipv6.IsValid());
+    BOOST_REQUIRE(addr_ipv6.IsIPv6());
+
+    CAddress addr_ipv6_tunnel{Lookup("2002:3344:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_ipv6_tunnel.IsValid());
+    BOOST_REQUIRE(addr_ipv6_tunnel.IsIPv6());
+    BOOST_REQUIRE(addr_ipv6_tunnel.IsRFC3964());
+
+    CAddress addr_teredo{Lookup("2001:0000:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_teredo.IsValid());
+    BOOST_REQUIRE(addr_teredo.IsIPv6());
+    BOOST_REQUIRE(addr_teredo.IsRFC4380());
+
+    CAddress addr_onion;
+    BOOST_REQUIRE(addr_onion.SetSpecial("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"));
+    BOOST_REQUIRE(addr_onion.IsValid());
+    BOOST_REQUIRE(addr_onion.IsTor());
+
+    CAddress addr_i2p;
+    BOOST_REQUIRE(addr_i2p.SetSpecial("udhdrtrcetjm5sxzskjyr5ztpeszydbh4dpl3pl4utgqqw2v4jna.b32.i2p"));
+    BOOST_REQUIRE(addr_i2p.IsValid());
+    BOOST_REQUIRE(addr_i2p.IsI2P());
+
+    CService service_cjdns{Lookup("fc00:3344:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    CAddress addr_cjdns{MaybeFlipIPv6toCJDNS(service_cjdns), NODE_NONE};
+    BOOST_REQUIRE(addr_cjdns.IsValid());
+    BOOST_REQUIRE(addr_cjdns.IsCJDNS());
+
+    const auto peer_ipv4{CreatePeer(addr_ipv4)};
+    const auto peer_ipv6{CreatePeer(addr_ipv6)};
+    const auto peer_ipv6_tunnel{CreatePeer(addr_ipv6_tunnel)};
+    const auto peer_teredo{CreatePeer(addr_teredo)};
+    const auto peer_onion{CreatePeer(addr_onion)};
+    const auto peer_i2p{CreatePeer(addr_i2p)};
+    const auto peer_cjdns{CreatePeer(addr_cjdns)};
+
+    // one local clearnet address - advertise to all but privacy peers
+    AddLocal(addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv4) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6_tunnel) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_teredo) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_cjdns) == addr_ipv4);
+    BOOST_CHECK(!GetLocalAddress(*peer_onion).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_i2p).IsValid());
+    RemoveLocal(addr_ipv4);
+
+    // local privacy addresses - don't advertise to clearnet peers
+    AddLocal(addr_onion);
+    AddLocal(addr_i2p);
+    BOOST_CHECK(!GetLocalAddress(*peer_ipv4).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_ipv6).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_ipv6_tunnel).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_teredo).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_cjdns).IsValid());
+    BOOST_CHECK(GetLocalAddress(*peer_onion) == addr_onion);
+    BOOST_CHECK(GetLocalAddress(*peer_i2p) == addr_i2p);
+    RemoveLocal(addr_onion);
+    RemoveLocal(addr_i2p);
+
+    // local addresses from all networks
+    AddLocal(addr_ipv4);
+    AddLocal(addr_ipv6);
+    AddLocal(addr_ipv6_tunnel);
+    AddLocal(addr_teredo);
+    AddLocal(addr_onion);
+    AddLocal(addr_i2p);
+    AddLocal(addr_cjdns);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv4) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6) == addr_ipv6);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6_tunnel) == addr_ipv6);
+    BOOST_CHECK(GetLocalAddress(*peer_teredo) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_onion) == addr_onion);
+    BOOST_CHECK(GetLocalAddress(*peer_i2p) == addr_i2p);
+    BOOST_CHECK(GetLocalAddress(*peer_cjdns) == addr_cjdns);
+    RemoveLocal(addr_ipv4);
+    RemoveLocal(addr_ipv6);
+    RemoveLocal(addr_ipv6_tunnel);
+    RemoveLocal(addr_teredo);
+    RemoveLocal(addr_onion);
+    RemoveLocal(addr_i2p);
+    RemoveLocal(addr_cjdns);
+}
+
 BOOST_AUTO_TEST_SUITE_END()