diff --git a/rust/connlib/shared/src/messages/client.rs b/rust/connlib/shared/src/messages/client.rs index 9c0de97f7..ae1aba083 100644 --- a/rust/connlib/shared/src/messages/client.rs +++ b/rust/connlib/shared/src/messages/client.rs @@ -195,6 +195,7 @@ impl ResourceDescription { (ResourceDescription::Cidr(cidr_a), ResourceDescription::Cidr(cidr_b)) => { cidr_a.address != cidr_b.address } + (ResourceDescription::Internet(_), ResourceDescription::Internet(_)) => false, _ => true, } } diff --git a/rust/connlib/shared/src/messages/gateway.rs b/rust/connlib/shared/src/messages/gateway.rs index 77e5959a9..18750fbbb 100644 --- a/rust/connlib/shared/src/messages/gateway.rs +++ b/rust/connlib/shared/src/messages/gateway.rs @@ -2,7 +2,7 @@ use std::net::IpAddr; -use ip_network::IpNetwork; +use ip_network::{IpNetwork, Ipv4Network, Ipv6Network}; use itertools::Itertools; use serde::Deserialize; @@ -146,7 +146,10 @@ impl ResourceDescription { match self { ResourceDescription::Dns(r) => r.addresses.iter().copied().map_into().collect_vec(), ResourceDescription::Cidr(r) => vec![r.address], - ResourceDescription::Internet(_) => vec![], + ResourceDescription::Internet(_) => vec![ + Ipv4Network::DEFAULT_ROUTE.into(), + Ipv6Network::DEFAULT_ROUTE.into(), + ], } } diff --git a/rust/connlib/tunnel/proptest-regressions/tests.txt b/rust/connlib/tunnel/proptest-regressions/tests.txt index ed8a22275..822253e08 100644 --- a/rust/connlib/tunnel/proptest-regressions/tests.txt +++ b/rust/connlib/tunnel/proptest-regressions/tests.txt @@ -85,3 +85,8 @@ cc ec2f348067458f6a7d3f2fbd1ab708a53fc27708440a3fcb6ed8557adc6db7d3 cc 2984b737f902f82c96ffec888a624afd7117078c125822b85de908c05f8e0b4c cc 51ad9fe7ef585d42bd1a6369da810a5adb6d756e71aa393362e542f1560d0273 cc b926f32ea3b2a04753bddd37be4804fd38fe35646e08507e68565883bd9fe2ed +cc f2b3aed5ec01491ac9f95c6baaadbf9fd05786940bc906d5a243a318b15907ea +cc 405da6a8d05abdc6fb7dfed17ca87d5f0893b289a277381eac757532992642f2 +cc 79e8ea948dddadd5db8bb7f587f8155490bcf2862c026eaf62c1039e5e1fd73d +cc 2ef061cf2eabbbef7db30f80486ec771e4b0d79ed55215fd86c9d14943e12eb4 +cc fa1ad96fcad83aa29156936f13a7722ccc1b349bc8c2022de4a6a20c3f4e9121 diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index 73f365f5e..cf77113f6 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -248,6 +248,8 @@ pub struct ClientState { /// All CIDR resources we know about, indexed by the IP range they cover (like `1.1.0.0/8`). active_cidr_resources: IpNetworkTable, + /// `Some` if the Internet resource is enabled. + internet_resource: Option, /// All resources indexed by their ID. resources_by_id: HashMap, @@ -318,6 +320,7 @@ impl ClientState { stub_resolver: StubResolver::new(known_hosts), disabled_resources: Default::default(), buffered_transmits: Default::default(), + internet_resource: None, } } @@ -623,6 +626,22 @@ impl ClientState { !interface.upstream_dns.is_empty() } + /// For DNS queries to IPs that are a CIDR resources we want to mangle and forward to the gateway that handles that resource. + /// + /// We only want to do this if the upstream DNS server is set by the portal, otherwise, the server might be a local IP. + fn should_forward_dns_query_to_gateway(&self, dns_server: IpAddr) -> bool { + if !self.is_upstream_set_by_the_portal() { + return false; + } + if self.internet_resource.is_some() { + return true; + } + + self.active_cidr_resources + .longest_match(dns_server) + .is_some() + } + /// Attempt to handle the given packet as a DNS query packet. /// /// Returns `Ok` if the packet is in fact a DNS query with an optional response to send back. @@ -645,10 +664,7 @@ impl ClientState { }) => { let ip = server.ip(); - // In case the DNS server is a CIDR resource, it needs to go through the tunnel. - if self.is_upstream_set_by_the_portal() - && self.active_cidr_resources.longest_match(ip).is_some() - { + if self.should_forward_dns_query_to_gateway(ip) { return Err((packet, ip)); } @@ -812,6 +828,14 @@ impl ClientState { .map(|(ip, _)| ip) .chain(iter::once(IPV4_RESOURCES.into())) .chain(iter::once(IPV6_RESOURCES.into())) + .chain( + self.internet_resource + .map(|_| Ipv4Network::DEFAULT_ROUTE.into()), + ) + .chain( + self.internet_resource + .map(|_| Ipv6Network::DEFAULT_ROUTE.into()), + ) .chain(self.dns_mapping.left_values().copied().map(Into::into)) } @@ -820,17 +844,21 @@ impl ClientState { } fn get_resource_by_destination(&self, destination: IpAddr) -> Option { - let maybe_dns_resource_id = self.stub_resolver.resolve_resource_by_ip(&destination); + // We need to filter disabled resources because we never remove resources from the stub_resolver + let maybe_dns_resource_id = self + .stub_resolver + .resolve_resource_by_ip(&destination) + .filter(|resource| self.is_resource_enabled(resource)); + // We don't need to filter from here because resources are removed from the active_cidr_resources as soon as they are disabled. let maybe_cidr_resource_id = self .active_cidr_resources .longest_match(destination) .map(|(_, res)| res.id); - // We need to filter disabled resources because we never remove resources from the stub_resolver maybe_dns_resource_id .or(maybe_cidr_resource_id) - .filter(|resource| self.is_resource_enabled(resource)) + .or(self.internet_resource) } pub(crate) fn update_system_resolvers(&mut self, new_dns: Vec) { @@ -1006,7 +1034,9 @@ impl ClientState { None => true, } } - ResourceDescription::Internet(_) => false, // FIXME: Update with real check once Internet Resources get added. + ResourceDescription::Internet(resource) => { + self.internet_resource.replace(resource.id) != Some(resource.id) + } }; if !added { @@ -1034,7 +1064,11 @@ impl ClientState { match resource { ResourceDescription::Dns(_) => self.stub_resolver.remove_resource(id), ResourceDescription::Cidr(_) => self.active_cidr_resources.retain(|_, r| r.id != id), - ResourceDescription::Internet(_) => {} + ResourceDescription::Internet(_) => { + if self.internet_resource.is_some_and(|r_id| r_id == id) { + self.internet_resource = None; + } + } } let name = resource.name(); @@ -1161,7 +1195,10 @@ fn get_addresses_for_awaiting_resource( .map_into() .collect_vec(), ResourceDescription::Cidr(r) => vec![r.address], - ResourceDescription::Internet(_) => vec![], + ResourceDescription::Internet(_) => vec![ + Ipv4Network::DEFAULT_ROUTE.into(), + Ipv6Network::DEFAULT_ROUTE.into(), + ], } } diff --git a/rust/connlib/tunnel/src/tests/reference.rs b/rust/connlib/tunnel/src/tests/reference.rs index ac281e237..c5f93212e 100644 --- a/rust/connlib/tunnel/src/tests/reference.rs +++ b/rust/connlib/tunnel/src/tests/reference.rs @@ -305,7 +305,7 @@ impl ReferenceStateMachine for ReferenceState { client.cidr_resources.insert(r.address, r.clone()); } client::ResourceDescription::Internet(r) => { - client.internet_resource = Some(r.id) + client.internet_resource = Some(r.clone()) } }); } @@ -338,10 +338,9 @@ impl ReferenceStateMachine for ReferenceState { continue; } - // Check if we the DNS server is defined as a CIDR resource. - let Some(resource) = state.client.inner().dns_query_via_cidr_resource(query) - else { - // Not a CIDR resource, process normally. + // Check if the DNS server is defined as a resource. + let Some(resource) = state.client.inner().dns_query_via_resource(query) else { + // Not a resource, process normally. state.client.exec_mut(|client| client.on_dns_query(query)); continue; }; @@ -352,10 +351,14 @@ impl ReferenceStateMachine for ReferenceState { continue; } - if !state.client.inner().is_connected_to_cidr(resource) { - state - .client - .exec_mut(|client| client.connected_cidr_resources.insert(resource)); + if !state + .client + .inner() + .is_connected_to_internet_or_cidr(resource) + { + state.client.exec_mut(|client| { + client.connect_to_internet_or_cidr_resource(resource) + }); pending_connections.insert(resource); continue; } @@ -554,7 +557,7 @@ impl ReferenceStateMachine for ReferenceState { .expected_dns_servers() .contains(&query.dns_server); let gateway_is_present_in_case_dns_server_is_cidr_resource = - match state.client.inner().dns_query_via_cidr_resource(query) { + match state.client.inner().dns_query_via_resource(query) { Some(r) => { let Some(gateway) = state.portal.gateway_for_resource(r) else { return false; diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index f7521cbc2..eb928c05d 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -10,7 +10,10 @@ use crate::{proptest::*, ClientState}; use bimap::BiMap; use connlib_shared::{ messages::{ - client::{ResourceDescription, ResourceDescriptionCidr, ResourceDescriptionDns}, + client::{ + ResourceDescription, ResourceDescriptionCidr, ResourceDescriptionDns, + ResourceDescriptionInternet, + }, ClientId, DnsServer, GatewayId, Interface, RelayId, ResourceId, }, DomainName, @@ -256,7 +259,7 @@ pub struct RefClient { /// The upstream DNS resolvers configured in the portal. pub(crate) upstream_dns_resolvers: Vec, - pub(crate) internet_resource: Option, + pub(crate) internet_resource: Option, /// The CIDR resources the client is aware of. #[derivative(Debug = "ignore")] @@ -322,6 +325,7 @@ impl RefClient { pub(crate) fn reset_connections(&mut self) { self.connected_cidr_resources.clear(); self.connected_dns_resources.clear(); + self.connected_internet_resources = false; } pub(crate) fn is_tunnel_ip(&self, ip: IpAddr) -> bool { @@ -350,7 +354,7 @@ impl RefClient { tracing::Span::current().record("dst", tracing::field::display(dst)); // Second, if we are not yet connected, check if we have a resource for this IP. - let Some(rid) = self.internet_resource else { + let Some(rid) = self.internet_resource.as_ref().map(|r| r.id) else { tracing::debug!("No internet resource"); return; }; @@ -402,7 +406,7 @@ impl RefClient { return; }; - if self.is_connected_to_cidr(rid) && self.is_tunnel_ip(src) { + if self.is_connected_to_internet_or_cidr(rid) && self.is_tunnel_ip(src) { tracing::debug!("Connected to CIDR resource, expecting packet to be routed"); self.expected_icmp_handshakes .entry(gateway) @@ -413,7 +417,7 @@ impl RefClient { // If we have a resource, the first packet will initiate a connection to the gateway. tracing::debug!("Not connected to resource, expecting to trigger connection intent"); - self.connected_cidr_resources.insert(rid); + self.connect_to_internet_or_cidr_resource(rid); } #[tracing::instrument(level = "debug", skip_all, fields(dst, resource))] @@ -463,6 +467,25 @@ impl RefClient { } } + pub(crate) fn is_connected_to_internet_or_cidr(&self, resource: ResourceId) -> bool { + self.is_connected_to_cidr(resource) || self.is_connected_to_internet(resource) + } + + pub(crate) fn connect_to_internet_or_cidr_resource(&mut self, resource: ResourceId) { + if self + .internet_resource + .as_ref() + .is_some_and(|r| r.id == resource) + { + self.connected_internet_resources = true; + return; + } + + if self.cidr_resources.iter().any(|(_, r)| r.id == resource) { + self.connected_cidr_resources.insert(resource); + } + } + pub(crate) fn on_dns_query(&mut self, query: &DnsQuery) { self.dns_records .entry(query.domain.clone()) @@ -487,6 +510,11 @@ impl RefClient { .collect_vec() } + fn is_connected_to_internet(&self, id: ResourceId) -> bool { + self.internet_resource.as_ref().map(|r| r.id) == Some(id) + && self.connected_internet_resources + } + pub(crate) fn is_connected_to_cidr(&self, id: ResourceId) -> bool { self.connected_cidr_resources.contains(&id) } @@ -615,10 +643,10 @@ impl RefClient { .copied() } - /// Returns the CIDR resource we will forward the DNS query for the given name to. + /// Returns the resource we will forward the DNS query for the given name to. /// /// DNS servers may be resources, in which case queries that need to be forwarded actually need to be encapsulated. - pub(crate) fn dns_query_via_cidr_resource(&self, query: &DnsQuery) -> Option { + pub(crate) fn dns_query_via_resource(&self, query: &DnsQuery) -> Option { // Unless we are using upstream resolvers, DNS queries are never routed through the tunnel. if self.upstream_dns_resolvers.is_empty() { return None; @@ -630,6 +658,7 @@ impl RefClient { } self.cidr_resource_by_ip(query.dns_server.ip()) + .or(self.internet_resource.as_ref().map(|r| r.id)) } pub(crate) fn all_resource_ids(&self) -> Vec { @@ -648,7 +677,11 @@ impl RefClient { return true; } - if self.internet_resource.is_some_and(|r| r == resource_id) { + if self + .internet_resource + .as_ref() + .is_some_and(|r| r.id == resource_id) + { return true; } @@ -662,13 +695,19 @@ impl RefClient { .map(|(_, r)| r) .cloned() .map(ResourceDescription::Cidr); + let dns_resources = self .dns_resources .values() .cloned() .map(ResourceDescription::Dns); - Vec::from_iter(cidr_resources.chain(dns_resources)) + let internet_resource = self + .internet_resource + .clone() + .map(ResourceDescription::Internet); + + Vec::from_iter(cidr_resources.chain(dns_resources).chain(internet_resource)) } } diff --git a/rust/connlib/tunnel/src/tests/stub_portal.rs b/rust/connlib/tunnel/src/tests/stub_portal.rs index d38e970cb..73b03c424 100644 --- a/rust/connlib/tunnel/src/tests/stub_portal.rs +++ b/rust/connlib/tunnel/src/tests/stub_portal.rs @@ -107,10 +107,9 @@ impl StubPortal { .cloned() .map(client::ResourceDescription::Dns), ) - // TODO: Enable once we actually implement the Internet resource - // .chain(iter::once(client::ResourceDescription::Internet( - // self.internet_resource.clone(), - // ))) + .chain(iter::once(client::ResourceDescription::Internet( + self.internet_resource.clone(), + ))) .collect() }