Following up from Discord with @matheus23.
Context: We run a VPN overlay (rayfish) where the endpoint binds 0.0.0.0 and a TUN device carries overlay IPs. We use an address filter to keep those overlay IPs out of iroh, so peers never try to reach them (otherwise underlay traffic routes back into the tunnel).
Problem: AddrFilter (via Builder::addr_filter) only filters the publish path. It is applied in AddressLookupServices::publish before publishing to pkarr/DNS:
|
pub(crate) fn publish(&self, data: &EndpointData) { |
|
let data = match &*self.addr_filter.read().expect("poisoned") { |
|
Some(filter) => data.apply_filter(filter), |
|
None => Cow::Borrowed(data), |
But the in-band QUIC NAT-traversal candidates come from a different place. update_qnt_candidates reads the unfiltered direct_addrs set and registers each address via conn.add_nat_traversal_address(...):
|
fn update_qnt_candidates(conn: &noq::Connection, direct_addrs: &BTreeSet<SocketAddr>) { |
|
let noq_candidates = match conn.get_local_nat_traversal_addresses() { |
|
Ok(addrs) => BTreeSet::from_iter(addrs), |
|
Err(err) => { |
|
warn!("failed to get local nat candidates: {err:#}"); |
|
return; |
|
} |
|
}; |
|
for addr in direct_addrs.difference(&noq_candidates) { |
|
if let Err(err) = conn.add_nat_traversal_address(*addr) { |
|
warn!("failed adding local addr: {err:#}",); |
|
} |
|
} |
|
for addr in noq_candidates.difference(direct_addrs) { |
|
if let Err(err) = conn.remove_nat_traversal_address(*addr) { |
|
warn!("failed removing local addr: {err:#}"); |
|
} |
|
} |
|
trace!(?direct_addrs, "updated local QNT addresses"); |
|
} |
- candidates sourced unfiltered from
direct_addrs:
|
fn local_candidates(&mut self) -> BTreeSet<SocketAddr> { |
|
self.local_direct_addrs |
|
.get() |
|
.iter() |
|
.map(|d| d.addr) |
|
.collect() |
|
} |
So a filtered-out address is still advertised to a connected peer as a holepunch candidate, and iroh still probes it.
Both sinks derive from the same direct_addrs set, written once in store_direct_addresses:
|
fn store_direct_addresses(&self, addrs: BTreeSet<DirectAddr>) { |
|
let updated = self.direct_addrs.update(addrs); |
|
if updated { |
|
self.publish_my_addr(); |
|
} |
|
} |
|
|
|
/// Get a reference to the DNS resolver used in this [`Socket`]. |
|
#[cfg(not(wasm_browser))] |
|
pub(crate) fn dns_resolver(&self) -> &DnsResolver { |
|
&self.dns_resolver |
- publish sink:
|
fn publish_my_addr(&self) { |
Proposal: Filter at that shared source, in update_direct_addresses before store_direct_addresses, so one filter covers both publish and QNT:
|
fn update_direct_addresses(&mut self, net_report_report: Option<&net_report::Report>) { |
Concretely, a per-IP keeps(IpAddr) -> bool filter applied in the filter_map that builds the address set right before store_direct_addresses. That is the single write into the direct_addrs Watchable, and both readers (publish_my_addr and local_candidates() -> update_qnt_candidates) observe that watched set. Dropping the address there means it is never published to discovery and never registered as a QNT NAT-traversal candidate, closing the gap addr_filter (publish-only) leaves open.
Reusing AddrFilter directly at the QNT site is awkward since it is Vec<TransportAddr>-shaped and presets like relay_only() would strip every QNT candidate. A per-IP predicate at the source avoids that. Happy to open a PR.
Following up from Discord with @matheus23.
Context: We run a VPN overlay (rayfish) where the endpoint binds
0.0.0.0and a TUN device carries overlay IPs. We use an address filter to keep those overlay IPs out of iroh, so peers never try to reach them (otherwise underlay traffic routes back into the tunnel).Problem:
AddrFilter(viaBuilder::addr_filter) only filters the publish path. It is applied inAddressLookupServices::publishbefore publishing to pkarr/DNS:iroh/iroh/src/address_lookup.rs
Lines 517 to 520 in c3ccf50
But the in-band QUIC NAT-traversal candidates come from a different place.
update_qnt_candidatesreads the unfiltereddirect_addrsset and registers each address viaconn.add_nat_traversal_address(...):iroh/iroh/src/socket/remote_map/remote_state.rs
Lines 1107 to 1126 in c3ccf50
direct_addrs:iroh/iroh/src/socket/remote_map/remote_state.rs
Lines 1094 to 1100 in c3ccf50
So a filtered-out address is still advertised to a connected peer as a holepunch candidate, and iroh still probes it.
Both sinks derive from the same
direct_addrsset, written once instore_direct_addresses:iroh/iroh/src/socket.rs
Lines 511 to 521 in c3ccf50
iroh/iroh/src/socket.rs
Line 672 in c3ccf50
Proposal: Filter at that shared source, in
update_direct_addressesbeforestore_direct_addresses, so one filter covers both publish and QNT:iroh/iroh/src/socket.rs
Line 1814 in c3ccf50
Concretely, a per-IP
keeps(IpAddr) -> boolfilter applied in thefilter_mapthat builds the address set right beforestore_direct_addresses. That is the single write into thedirect_addrsWatchable, and both readers (publish_my_addrandlocal_candidates()->update_qnt_candidates) observe that watched set. Dropping the address there means it is never published to discovery and never registered as a QNT NAT-traversal candidate, closing the gapaddr_filter(publish-only) leaves open.Reusing
AddrFilterdirectly at the QNT site is awkward since it isVec<TransportAddr>-shaped and presets likerelay_only()would strip every QNT candidate. A per-IP predicate at the source avoids that. Happy to open a PR.