Merge bitcoin/bitcoin#33395: net: do not apply whitelist permissions to onion inbounds

fanquake · Tom Trevethan · commit 0f06cebc6101 · 2025-11-25T13:39:28.000Z
f563ce90818d486d2a199439d2f6ba39cd106352 net: Do not apply whitelist permission to onion inbounds (Martin Zumsande)

Pull request description:

  Tor inbound connections do not reveal the peer's actual network address. Do not apply whitelist permissions to them since address-based matching is ineffective.

ACKs for top commit:
  darosior:
    ACK f563ce90818d486d2a199439d2f6ba39cd106352
  furszy:
    ACK f563ce90818d486d2a199439d2f6ba39cd106352
  vasild:
    ACK f563ce90818d486d2a199439d2f6ba39cd106352

Tree-SHA512: 49ae70e382fc2f78b7073553fe649a6843a41214b2986ea7f77e285d02b7bd00fe0320a1b71d1aaca08713808fb14af058f0b1f19f19adb3a77b97cb9d3449ce
diff --git a/src/net.cpp b/src/net.cpp
@@ -569,9 +569,9 @@ void CNode::CloseSocketDisconnect()
     }
 }
 
-void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const {
+void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional<CNetAddr> addr) const {
     for (const auto& subnet : vWhitelistedRange) {
-        if (subnet.m_subnet.Match(addr)) NetPermissions::AddFlag(flags, subnet.m_flags);
+        if (addr.has_value() && subnet.m_subnet.Match(addr.value())) NetPermissions::AddFlag(flags, subnet.m_flags);
     }
 }
 
@@ -1179,7 +1179,10 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
     int nInbound = 0;
     int nMaxInbound = nMaxConnections - m_max_outbound;
 
-    AddWhitelistPermissionFlags(permissionFlags, addr);
+    const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
+    // Tor inbound connections do not reveal the peer's actual network address.
+    // Therefore do not apply address-based whitelist permissions to them.
+    AddWhitelistPermissionFlags(permissionFlags, inbound_onion ? std::optional<CNetAddr>{} : addr);
     if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::Implicit)) {
         NetPermissions::ClearFlag(permissionFlags, NetPermissionFlags::Implicit);
         if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::ForceRelay);
@@ -1243,7 +1246,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
         nodeServices = static_cast<ServiceFlags>(nodeServices | NODE_BLOOM);
     }
 
-    const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
     CNode* pnode = new CNode(id,
                              nodeServices,
                              std::move(sock),
diff --git a/src/net.h b/src/net.h
@@ -1077,7 +1077,7 @@ class CConnman
 
     bool AttemptToEvictConnection();
     CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type);
-    void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const;
+    void AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional<CNetAddr> addr) const;
 
     void DeleteNode(CNode* pnode);
 

Original file line number	Diff line number	Diff line change
`@@ -569,9 +569,9 @@ void CNode::CloseSocketDisconnect()`
`569`	`569`	`}`
`570`	`570`	`}`
`571`	`571`
`572`		`-void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const {`
	`572`	`+void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional<CNetAddr> addr) const {`
`573`	`573`	`for (const auto& subnet : vWhitelistedRange) {`
`574`		`- if (subnet.m_subnet.Match(addr)) NetPermissions::AddFlag(flags, subnet.m_flags);`
	`574`	`+ if (addr.has_value() && subnet.m_subnet.Match(addr.value())) NetPermissions::AddFlag(flags, subnet.m_flags);`
`575`	`575`	`}`
`576`	`576`	`}`
`577`	`577`
`@@ -1179,7 +1179,10 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,`
`1179`	`1179`	`int nInbound = 0;`
`1180`	`1180`	`int nMaxInbound = nMaxConnections - m_max_outbound;`
`1181`	`1181`
`1182`		`- AddWhitelistPermissionFlags(permissionFlags, addr);`
	`1182`	`+ const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();`
	`1183`	`+ // Tor inbound connections do not reveal the peer's actual network address.`
	`1184`	`+ // Therefore do not apply address-based whitelist permissions to them.`
	`1185`	`+ AddWhitelistPermissionFlags(permissionFlags, inbound_onion ? std::optional<CNetAddr>{} : addr);`
`1183`	`1186`	`if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::Implicit)) {`
`1184`	`1187`	`NetPermissions::ClearFlag(permissionFlags, NetPermissionFlags::Implicit);`
`1185`	`1188`	`if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::ForceRelay);`
`@@ -1243,7 +1246,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,`
`1243`	`1246`	`nodeServices = static_cast<ServiceFlags>(nodeServices \| NODE_BLOOM);`
`1244`	`1247`	`}`
`1245`	`1248`
`1246`		`- const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();`
`1247`	`1249`	`CNode* pnode = new CNode(id,`
`1248`	`1250`	`nodeServices,`
`1249`	`1251`	`std::move(sock),`