From 02638582fe737ee8b90dfc605c1e4a5f18dc3491 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sat, 31 May 2025 10:27:59 +1000 Subject: [PATCH] feat(connlib): allow controlling IP stack per DNS resource (#9300) With this patch, `connlib` exposes a new, optional field `ip_stack` within the resource description of each DNS resource that controls the supported IP stack. By default, the IP stack is set to `Dual` to preserve the current behaviour. When set to `IPv4Only` or `IPv6Only`, `connlib` will not assign any IPv4 or IPv6 addresses when receiving DNS queries for such a resource. The DNS query will still respond successfully with NOERROR (and not NXDOMAIN) but the list of IPs will be empty. This is useful to e.g. allow sys-admins to disable IPv6 for resources with buggy clients such as the MongoDB atlas driver. The MongoDB driver does not correctly handle happy-eyeballs and instead fails the connection early on any connection error. Additionally, customers operating in IPv6-exclusive networks can disable IPv4 addresses with this setting. Related: https://jira.mongodb.org/browse/NODE-4678 Related: #9042 Related: #8892 --- rust/connlib/model/src/lib.rs | 25 ++++ .../tunnel/proptest-regressions/tests.txt | 1 + rust/connlib/tunnel/src/client.rs | 5 +- rust/connlib/tunnel/src/client/resource.rs | 85 ++++++++++++- rust/connlib/tunnel/src/dns.rs | 120 ++++++++++++++---- rust/connlib/tunnel/src/messages/client.rs | 6 +- rust/connlib/tunnel/src/p2p_control.rs | 5 +- rust/connlib/tunnel/src/proptest.rs | 14 +- rust/connlib/tunnel/src/tests/reference.rs | 2 +- rust/connlib/tunnel/src/tests/sim_client.rs | 19 ++- 10 files changed, 244 insertions(+), 38 deletions(-) diff --git a/rust/connlib/model/src/lib.rs b/rust/connlib/model/src/lib.rs index 8ae784735..bbbcb16fa 100644 --- a/rust/connlib/model/src/lib.rs +++ b/rust/connlib/model/src/lib.rs @@ -184,3 +184,28 @@ impl fmt::Debug for SiteId { fmt::Display::fmt(&self, f) } } + +/// The IP stack of a DNS resource. +#[derive(Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[serde(rename_all = "snake_case")] +pub enum IpStack { + Dual, + Ipv4Only, + Ipv6Only, +} + +impl IpStack { + pub fn supports_ipv4(&self) -> bool { + match self { + IpStack::Ipv4Only | IpStack::Dual => true, + IpStack::Ipv6Only => false, + } + } + + pub fn supports_ipv6(&self) -> bool { + match self { + IpStack::Ipv4Only => false, + IpStack::Ipv6Only | IpStack::Dual => true, + } + } +} diff --git a/rust/connlib/tunnel/proptest-regressions/tests.txt b/rust/connlib/tunnel/proptest-regressions/tests.txt index e7be3c085..0b6d120c3 100644 --- a/rust/connlib/tunnel/proptest-regressions/tests.txt +++ b/rust/connlib/tunnel/proptest-regressions/tests.txt @@ -166,3 +166,4 @@ cc f2de44e6762e9a681d624467fd19ac9fc00f000dfc1c2a3bda05c905b01674c2 cc 36a7bb4eff285399b9c431675d4337712e7edf016a3a02b05cba5115c8bf8fe4 cc 235333b8c818e464ba339e8c73b2467894d68d594ac896c4f6a36b25ac6b823d cc 436afa9076f65f9abbe801ef2a7f26631e433650a6f717358972f37a1fbf1542 +cc ee518414c1632fb9d49272b985476de0d9de2786cadef997ad7d626e1a4b975a diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index cf4477eac..5056e34e7 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -1567,7 +1567,10 @@ impl ClientState { } let activated = match &new_resource { - Resource::Dns(dns) => self.stub_resolver.add_resource(dns.id, dns.address.clone()), + Resource::Dns(dns) => { + self.stub_resolver + .add_resource(dns.id, dns.address.clone(), dns.ip_stack) + } Resource::Cidr(cidr) => { let existing = self.active_cidr_resources.exact_match(cidr.address); diff --git a/rust/connlib/tunnel/src/client/resource.rs b/rust/connlib/tunnel/src/client/resource.rs index 429a40e37..d6131b471 100644 --- a/rust/connlib/tunnel/src/client/resource.rs +++ b/rust/connlib/tunnel/src/client/resource.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use connlib_model::{ - CidrResourceView, DnsResourceView, InternetResourceView, ResourceId, ResourceStatus, + CidrResourceView, DnsResourceView, InternetResourceView, IpStack, ResourceId, ResourceStatus, ResourceView, Site, }; use ip_network::{IpNetwork, Ipv4Network, Ipv6Network}; @@ -35,6 +35,8 @@ pub struct DnsResource { pub address_description: Option, pub sites: Vec, + + pub ip_stack: IpStack, } /// Description of a resource that maps to a CIDR. @@ -249,6 +251,7 @@ impl DnsResource { name: resource.name, address_description: resource.address_description, sites: resource.sites, + ip_stack: resource.ip_stack.unwrap_or(IpStack::Dual), } } @@ -263,3 +266,83 @@ impl DnsResource { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn can_deserialize_dns_resource_with_ipv4_only_ip_stack() { + let resource = Resource::from_description(ResourceDescription::Dns(serde_json::json!({ + "address": "example.com", + "id": "03000143-e25e-45c7-aafb-144990e57dce", + "name": "example.com", + "gateway_groups": [{"name": "test", "id": "bf56f32d-7b2c-4f5d-a784-788977d014a4"}], + "type": "dns", + "ip_stack": "ipv4_only" + }))) + .unwrap(); + + let Resource::Dns(dns) = resource else { + panic!("Unexpected resource") + }; + + assert_eq!(dns.ip_stack, IpStack::Ipv4Only) + } + + #[test] + fn can_deserialize_dns_resource_with_ipv6_only_ip_stack() { + let resource = Resource::from_description(ResourceDescription::Dns(serde_json::json!({ + "address": "example.com", + "id": "03000143-e25e-45c7-aafb-144990e57dce", + "name": "example.com", + "gateway_groups": [{"name": "test", "id": "bf56f32d-7b2c-4f5d-a784-788977d014a4"}], + "type": "dns", + "ip_stack": "ipv6_only" + }))) + .unwrap(); + + let Resource::Dns(dns) = resource else { + panic!("Unexpected resource") + }; + + assert_eq!(dns.ip_stack, IpStack::Ipv6Only) + } + + #[test] + fn can_deserialize_dns_resource_with_dual_ip_stack() { + let resource = Resource::from_description(ResourceDescription::Dns(serde_json::json!({ + "address": "example.com", + "id": "03000143-e25e-45c7-aafb-144990e57dce", + "name": "example.com", + "gateway_groups": [{"name": "test", "id": "bf56f32d-7b2c-4f5d-a784-788977d014a4"}], + "type": "dns", + "ip_stack": "dual" + }))) + .unwrap(); + + let Resource::Dns(dns) = resource else { + panic!("Unexpected resource") + }; + + assert_eq!(dns.ip_stack, IpStack::Dual) + } + + #[test] + fn can_deserialize_dns_resource_with_no_stack() { + let resource = Resource::from_description(ResourceDescription::Dns(serde_json::json!({ + "address": "example.com", + "id": "03000143-e25e-45c7-aafb-144990e57dce", + "name": "example.com", + "gateway_groups": [{"name": "test", "id": "bf56f32d-7b2c-4f5d-a784-788977d014a4"}], + "type": "dns" + }))) + .unwrap(); + + let Resource::Dns(dns) = resource else { + panic!("Unexpected resource") + }; + + assert_eq!(dns.ip_stack, IpStack::Dual) + } +} diff --git a/rust/connlib/tunnel/src/dns.rs b/rust/connlib/tunnel/src/dns.rs index 95732a08d..f6361b045 100644 --- a/rust/connlib/tunnel/src/dns.rs +++ b/rust/connlib/tunnel/src/dns.rs @@ -1,6 +1,6 @@ use crate::client::IpProvider; use anyhow::Result; -use connlib_model::ResourceId; +use connlib_model::{IpStack, ResourceId}; use dns_types::{ DomainName, DomainNameRef, OwnedRecordData, Query, RecordType, Response, ResponseBuilder, ResponseCode, @@ -38,10 +38,16 @@ pub struct StubResolver { ips_to_fqdn: HashMap, ip_provider: IpProvider, /// All DNS resources we know about, indexed by the glob pattern they match against. - dns_resources: BTreeMap, + dns_resources: BTreeMap, search_domain: Option, } +#[derive(Debug, Clone, Copy)] +struct Resource { + id: ResourceId, + ip_stack: IpStack, +} + /// A query that needs to be forwarded to an upstream DNS server for resolution. #[derive(Debug)] pub(crate) struct RecursiveQuery { @@ -141,7 +147,12 @@ impl StubResolver { .map(|((domain, resource), ips)| (domain, resource, ips)) } - pub(crate) fn add_resource(&mut self, id: ResourceId, pattern: String) -> bool { + pub(crate) fn add_resource( + &mut self, + id: ResourceId, + pattern: String, + ip_stack: IpStack, + ) -> bool { let parsed_pattern = match Pattern::new(&pattern) { Ok(p) => p, Err(e) => { @@ -150,21 +161,23 @@ impl StubResolver { } }; - let existing = self.dns_resources.insert(parsed_pattern, id); + let existing = self + .dns_resources + .insert(parsed_pattern, Resource { id, ip_stack }); existing.is_none() } pub(crate) fn remove_resource(&mut self, id: ResourceId) { - self.dns_resources.retain(|_, r| *r != id); + self.dns_resources.retain(|_, r| r.id != id); } fn get_or_assign_a_records( &mut self, fqdn: dns_types::DomainName, - resource_id: ResourceId, + resource: Resource, ) -> Vec { - self.get_or_assign_ips(fqdn, resource_id) + self.get_or_assign_ips(fqdn, resource) .into_iter() .filter_map(get_v4) .map(dns_types::records::a) @@ -174,9 +187,9 @@ impl StubResolver { fn get_or_assign_aaaa_records( &mut self, fqdn: dns_types::DomainName, - resource_id: ResourceId, + resource: Resource, ) -> Vec { - self.get_or_assign_ips(fqdn, resource_id) + self.get_or_assign_ips(fqdn, resource) .into_iter() .filter_map(get_v6) .map(dns_types::records::aaaa) @@ -186,14 +199,21 @@ impl StubResolver { fn get_or_assign_ips( &mut self, fqdn: dns_types::DomainName, - resource_id: ResourceId, + resource: Resource, ) -> Vec { let ips = self .fqdn_to_ips - .entry((fqdn.clone(), resource_id)) + .entry((fqdn.clone(), resource.id)) .or_insert_with(|| { - let mut ips = self.ip_provider.get_n_ipv4(4); - ips.extend_from_slice(&self.ip_provider.get_n_ipv6(4)); + let mut ips = Vec::with_capacity(8); + + if resource.ip_stack.supports_ipv4() { + ips.extend(self.ip_provider.get_n_ipv4(4)); + } + + if resource.ip_stack.supports_ipv6() { + ips.extend(self.ip_provider.get_n_ipv6(4)); + } tracing::debug!(domain = %fqdn, ?ips, "Assigning proxy IPs"); @@ -201,7 +221,7 @@ impl StubResolver { }) .clone(); for ip in &ips { - self.ips_to_fqdn.insert(*ip, (fqdn.clone(), resource_id)); + self.ips_to_fqdn.insert(*ip, (fqdn.clone(), resource.id)); } ips @@ -210,16 +230,16 @@ impl StubResolver { /// Attempts to match the given domain against our list of possible patterns. /// /// This performs a linear search and is thus O(N) and **must not** be called in the hot-path of packet routing. - fn match_resource_linear(&self, domain: &dns_types::DomainName) -> Option { + fn match_resource_linear(&self, domain: &dns_types::DomainName) -> Option { let _span = telemetry_span!("match_resource_linear").entered(); let name = Candidate::from_domain(domain); - for (pattern, id) in &self.dns_resources { + for (pattern, r) in &self.dns_resources { if pattern.matches(&name) { - tracing::trace!(%id, %pattern, %domain, "Matched resource"); + tracing::trace!(id = %r.id, %pattern, %domain, "Matched resource"); - return Some(*id); + return Some(*r); } } @@ -265,9 +285,9 @@ impl StubResolver { self.get_or_assign_aaaa_records(domain.clone(), resource) } (RecordType::SRV | RecordType::TXT, Some(resource)) => { - tracing::debug!(%qtype, %resource, "Forwarding query for DNS resource to corresponding site"); + tracing::debug!(%qtype, resource = %resource.id, "Forwarding query for DNS resource to corresponding site"); - return ResolveStrategy::RecurseSite(resource); + return ResolveStrategy::RecurseSite(resource.id); } (RecordType::PTR, _) => { let Some(fqdn) = self.resource_address_name_by_reservse_dns(&domain) else { @@ -641,14 +661,14 @@ mod tests { let wc = ResourceId::from_u128(0); let non_wc = ResourceId::from_u128(1); - resolver.add_resource(wc, "**.example.com".to_owned()); - resolver.add_resource(non_wc, "foo.example.com".to_owned()); + resolver.add_resource(wc, "**.example.com".to_owned(), IpStack::Dual); + resolver.add_resource(non_wc, "foo.example.com".to_owned(), IpStack::Dual); - let resource_id = resolver + let resource = resolver .match_resource_linear(&"foo.example.com".parse().unwrap()) .unwrap(); - assert_eq!(resource_id, non_wc); + assert_eq!(resource.id, non_wc); } #[test] @@ -674,6 +694,52 @@ mod tests { assert_eq!(response.response_code(), ResponseCode::NXDOMAIN); assert_eq!(response.records().count(), 0); } + + #[test] + fn a_query_for_ipv6_only_resource_yields_empty_set() { + let mut resolver = StubResolver::default(); + + resolver.add_resource( + ResourceId::from_u128(1), + "example.com".to_owned(), + IpStack::Ipv6Only, + ); + + let query = Query::new( + "example.com".parse::().unwrap(), + RecordType::A, + ); + + let ResolveStrategy::LocalResponse(response) = resolver.handle(&query) else { + panic!("Unexpected result") + }; + + assert_eq!(response.response_code(), ResponseCode::NOERROR); + assert_eq!(response.records().count(), 0); + } + + #[test] + fn aaaa_query_for_ipv4_only_resource_yields_empty_set() { + let mut resolver = StubResolver::default(); + + resolver.add_resource( + ResourceId::from_u128(1), + "example.com".to_owned(), + IpStack::Ipv4Only, + ); + + let query = Query::new( + "example.com".parse::().unwrap(), + RecordType::AAAA, + ); + + let ResolveStrategy::LocalResponse(response) = resolver.handle(&query) else { + panic!("Unexpected result") + }; + + assert_eq!(response.response_code(), ResponseCode::NOERROR); + assert_eq!(response.records().count(), 0); + } } #[cfg(feature = "divan")] @@ -692,7 +758,11 @@ mod benches { let mut rng = rand::thread_rng(); for n in 0..NUM_RES { - resolver.add_resource(ResourceId::from_u128(n), make_domain(&mut rng)); + resolver.add_resource( + ResourceId::from_u128(n), + make_domain(&mut rng), + IpStack::Dual, + ); } let needle = resolver diff --git a/rust/connlib/tunnel/src/messages/client.rs b/rust/connlib/tunnel/src/messages/client.rs index e94898717..9360dff88 100644 --- a/rust/connlib/tunnel/src/messages/client.rs +++ b/rust/connlib/tunnel/src/messages/client.rs @@ -1,7 +1,7 @@ //! Client related messages that are needed within connlib use crate::messages::{IceCredentials, Interface, Key, Relay, RelaysPresence, SecretKey}; -use connlib_model::{GatewayId, ResourceId, Site, SiteId}; +use connlib_model::{GatewayId, IpStack, ResourceId, Site, SiteId}; use ip_network::IpNetwork; use serde::{Deserialize, Serialize}; use std::{ @@ -24,6 +24,10 @@ pub struct ResourceDescriptionDns { pub address_description: Option, #[serde(rename = "gateway_groups")] pub sites: Vec, + + /// The IP stack supported by this resource. + #[serde(default)] + pub ip_stack: Option, } /// Description of a resource that maps to a CIDR. diff --git a/rust/connlib/tunnel/src/p2p_control.rs b/rust/connlib/tunnel/src/p2p_control.rs index 18c99c2ab..7471ed497 100644 --- a/rust/connlib/tunnel/src/p2p_control.rs +++ b/rust/connlib/tunnel/src/p2p_control.rs @@ -29,7 +29,10 @@ pub mod dns_resource_nat { domain: DomainName, proxy_ips: Vec, ) -> Result { - anyhow::ensure!(proxy_ips.len() == 8, "Expected 8 proxy IPs"); + anyhow::ensure!( + proxy_ips.len() == 4 || proxy_ips.len() == 8, + "Expected 4 or 8 proxy IPs" + ); let payload = serde_json::to_vec(&AssignedIps { resource, diff --git a/rust/connlib/tunnel/src/proptest.rs b/rust/connlib/tunnel/src/proptest.rs index ca8db5255..a06f0dd81 100644 --- a/rust/connlib/tunnel/src/proptest.rs +++ b/rust/connlib/tunnel/src/proptest.rs @@ -1,4 +1,4 @@ -use connlib_model::{ClientId, GatewayId, RelayId, ResourceId, Site, SiteId}; +use connlib_model::{ClientId, GatewayId, IpStack, RelayId, ResourceId, Site, SiteId}; use ip_network::{IpNetwork, Ipv4Network, Ipv6Network}; use proptest::{ arbitrary::{any, any_with}, @@ -32,15 +32,17 @@ pub fn dns_resource(sites: impl Strategy>) -> impl Strategy impl Strategy + Clone { (site_name(), site_id()).prop_map(|(name, id)| Site { name, id }) } +pub fn ip_stack() -> impl Strategy + Clone { + prop_oneof![ + Just(IpStack::Dual), + Just(IpStack::Ipv4Only), + Just(IpStack::Ipv6Only) + ] +} + pub fn resource_id() -> impl Strategy + Clone { any::().prop_map(ResourceId::from_u128).no_shrink() } diff --git a/rust/connlib/tunnel/src/tests/reference.rs b/rust/connlib/tunnel/src/tests/reference.rs index 5addbee39..37a3385f0 100644 --- a/rust/connlib/tunnel/src/tests/reference.rs +++ b/rust/connlib/tunnel/src/tests/reference.rs @@ -696,7 +696,7 @@ impl ReferenceState { let Some(resource) = self.client.inner().dns_resource_by_domain(name) else { return false; }; - let Some(gateway) = self.portal.gateway_for_resource(resource) else { + let Some(gateway) = self.portal.gateway_for_resource(resource.id) else { return false; }; diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index 3d6f8a271..f9394e46e 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -825,8 +825,8 @@ impl RefClient { fn resource_by_dst(&self, destination: &Destination) -> Option { match destination { Destination::DomainName { name, .. } => { - if let Some(id) = self.dns_resource_by_domain(name) { - return Some(id); + if let Some(r) = self.dns_resource_by_domain(name) { + return Some(r.id); } } Destination::IpAddr(addr) => { @@ -839,7 +839,7 @@ impl RefClient { self.active_internet_resource() } - pub(crate) fn dns_resource_by_domain(&self, domain: &DomainName) -> Option { + pub(crate) fn dns_resource_by_domain(&self, domain: &DomainName) -> Option { self.resources .iter() .cloned() @@ -847,8 +847,7 @@ impl RefClient { .filter(|r| is_subdomain(&domain.to_string(), &r.address)) .sorted_by_key(|r| r.address.len()) .rev() - .map(|r| r.id) - .find(|id| !self.disabled_resources.contains(id)) + .find(|r| !self.disabled_resources.contains(&r.id)) } fn resolved_domains(&self) -> impl Iterator)> + '_ { @@ -900,6 +899,10 @@ impl RefClient { .any(|r| matches!(r, &RecordType::A)) .then_some(domain) }) + .filter(|d| { + self.dns_resource_by_domain(d) + .is_some_and(|r| r.ip_stack.supports_ipv4()) + }) .collect() } @@ -911,6 +914,10 @@ impl RefClient { .any(|r| matches!(r, &RecordType::AAAA)) .then_some(domain) }) + .filter(|d| { + self.dns_resource_by_domain(d) + .is_some_and(|r| r.ip_stack.supports_ipv6()) + }) .collect() } @@ -1032,7 +1039,7 @@ impl RefClient { return None; } - self.dns_resource_by_domain(&query.domain) + Some(self.dns_resource_by_domain(&query.domain)?.id) } pub(crate) fn all_resource_ids(&self) -> Vec {