fix(connlib): send dns proxy ips even with overlapping internet resource (#5902)

To determine whether we send proxy IPs we depend on the `allowed_ips`,
since that's where we track what resources we have sent to a given
gateway.

However, the way we were matching if a given resource destination was
sent was using `longest_match` and with overlapping DNS this no longer
works, since this will match for internet resources even if the proxy IP
wasn't sent.

So we check that it's a DNS resource and if it's we exactly match on the
allowed ip table.

Alternatively, we could keep track of `sent_ips` for a gateway, though
this is a bit of a redundant state that we need to keep in sync but has
the benefit of being more explicit, so I'm open to do that in a follow
up PR. But I'd like to merge this to get ready for internet resources.
This commit is contained in:
Gabi
2024-07-19 21:26:36 -03:00
committed by GitHub
parent 5db0424032
commit 18394e3dcb

View File

@@ -384,6 +384,13 @@ impl ClientState {
})
}
fn is_dns_resource(&self, resource: &ResourceId) -> bool {
matches!(
self.resources_by_id.get(resource),
Some(ResourceDescription::Dns(_))
)
}
pub(crate) fn encapsulate<'s>(
&'s mut self,
packet: MutableIpPacket<'_>,
@@ -406,6 +413,9 @@ impl ClientState {
return None;
};
// We read this here to prevent problems with the borrow checker
let is_dns_resource = self.is_dns_resource(&resource);
let Some(peer) = peer_by_resource_mut(&self.resources_gateways, &mut self.peers, resource)
else {
self.on_not_connected_resource(resource, &dst, now);
@@ -419,7 +429,9 @@ impl ClientState {
now,
);
if peer.allowed_ips.longest_match(dst).is_none() {
// Allowed IPs will track the IPs that we have sent to the gateway along with a list of ResourceIds
// for DNS resource we will send the IP one at a time.
if is_dns_resource && peer.allowed_ips.exact_match(dst).is_none() {
let gateway_id = peer.id();
self.send_proxy_ips(&dst, resource, gateway_id);
return None;