diff --git a/rust/connlib/tunnel/proptest-regressions/tests.txt b/rust/connlib/tunnel/proptest-regressions/tests.txt index 179b13f60..9c13d8f65 100644 --- a/rust/connlib/tunnel/proptest-regressions/tests.txt +++ b/rust/connlib/tunnel/proptest-regressions/tests.txt @@ -164,3 +164,4 @@ cc 4b8aab1f09422751b66d7e46a968bb29fb9b11c8fff9bceb67cd5c8ddeab0a3d cc c48e5d18ae2cc7533bbe1d0cd155a1ec7bcaf00e8d029b0345c241ec3371dcca cc f2de44e6762e9a681d624467fd19ac9fc00f000dfc1c2a3bda05c905b01674c2 cc 36a7bb4eff285399b9c431675d4337712e7edf016a3a02b05cba5115c8bf8fe4 +cc 235333b8c818e464ba339e8c73b2467894d68d594ac896c4f6a36b25ac6b823d diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index 32fffa058..8380d67e4 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -1045,21 +1045,27 @@ impl ClientState { pub fn update_interface_config(&mut self, config: InterfaceConfig) { tracing::trace!(upstream_dns = ?config.upstream_dns, search_domain = ?config.search_domain, ipv4 = %config.ipv4, ipv6 = %config.ipv6, "Received interface configuration from portal"); - match self.tun_config.as_mut() { - Some(existing) => { - // We don't really expect these to change but let's update them anyway. - existing.ip.v4 = config.ipv4; - existing.ip.v6 = config.ipv6; - - // These are safe to always update - existing.search_domain = config.search_domain.clone(); - } - None => { + // Create a new `TunConfig` by patching the corresponding fields of the existing one. + let new_tun_config = self + .tun_config + .as_ref() + .map(|existing| TunConfig { + ip: IpConfig { + v4: config.ipv4, + v6: config.ipv6, + }, + dns_by_sentinel: existing.dns_by_sentinel.clone(), + search_domain: config.search_domain.clone(), + ipv4_routes: existing.ipv4_routes.clone(), + ipv6_routes: existing.ipv6_routes.clone(), + }) + .unwrap_or_else(|| { let (ipv4_routes, ipv6_routes) = self.routes().partition_map(|route| match route { IpNetwork::V4(v4) => itertools::Either::Left(v4), IpNetwork::V6(v6) => itertools::Either::Right(v6), }); - let new_tun_config = TunConfig { + + TunConfig { ip: IpConfig { v4: config.ipv4, v6: config.ipv6, @@ -1068,16 +1074,14 @@ impl ClientState { search_domain: config.search_domain.clone(), ipv4_routes, ipv6_routes, - }; + } + }); - self.maybe_update_tun_config(new_tun_config); - } - } + // Apply the new `TunConfig` if it differs from the existing one. + self.maybe_update_tun_config(new_tun_config); - self.stub_resolver.set_search_domain(config.search_domain); self.upstream_dns = config.upstream_dns; - - self.update_dns_mapping() + self.update_dns_mapping(); } pub fn poll_packets(&mut self) -> Option { @@ -1487,6 +1491,9 @@ impl ClientState { tracing::info!(config = ?new_tun_config, "Updating TUN device"); + self.stub_resolver + .set_search_domain(new_tun_config.search_domain.clone()); + // Ensure we don't emit multiple interface updates in a row. self.buffered_events .retain(|e| !matches!(e, ClientEvent::TunInterfaceUpdated(_))); diff --git a/rust/connlib/tunnel/src/tests/assertions.rs b/rust/connlib/tunnel/src/tests/assertions.rs index 33382e466..aa86f4944 100644 --- a/rust/connlib/tunnel/src/tests/assertions.rs +++ b/rust/connlib/tunnel/src/tests/assertions.rs @@ -232,6 +232,15 @@ pub(crate) fn assert_dns_servers_are_valid(ref_client: &RefClient, sim_client: & } } +pub(crate) fn assert_search_domain_is_valid(ref_client: &RefClient, sim_client: &SimClient) { + let expected = ref_client.expected_search_domain(); + let actual = sim_client.effective_search_domain(); + + if actual != expected { + tracing::error!(target: "assertions", ?actual, ?expected, "❌ Search domain is incorrect"); + } +} + pub(crate) fn assert_routes_are_valid(ref_client: &RefClient, sim_client: &SimClient) { let (expected_ipv4, expected_ipv6) = ref_client.expected_routes(); let (actual_ipv4, actual_ipv6) = ( diff --git a/rust/connlib/tunnel/src/tests/reference.rs b/rust/connlib/tunnel/src/tests/reference.rs index 1458d5c15..5caa85d7b 100644 --- a/rust/connlib/tunnel/src/tests/reference.rs +++ b/rust/connlib/tunnel/src/tests/reference.rs @@ -187,6 +187,13 @@ impl ReferenceState { 1, upstream_dns_servers().prop_map(Transition::UpdateUpstreamDnsServers), ) + .with( + 1, + state + .portal + .search_domain() + .prop_map(Transition::UpdateUpstreamSearchDomain), + ) .with_if_not_empty( 5, state.all_resources_not_known_to_client(), @@ -451,6 +458,11 @@ impl ReferenceState { .client .exec_mut(|client| client.set_upstream_dns_resolvers(servers)); } + Transition::UpdateUpstreamSearchDomain(domain) => { + state + .client + .exec_mut(|client| client.set_upstream_search_domain(domain.as_ref())); + } Transition::RoamClient { ip4, ip6, .. } => { state.network.remove_host(&state.client); state.client.ip4.clone_from(ip4); @@ -591,6 +603,7 @@ impl ReferenceState { .iter() .any(|dns_server| state.client.sending_socket_for(dns_server.ip()).is_some()) } + Transition::UpdateUpstreamSearchDomain(_) => true, Transition::SendDnsQueries(queries) => queries.iter().all(|query| { let has_socket_for_server = state .client diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index 1075f71ba..b30edb847 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -45,6 +45,9 @@ pub(crate) struct SimClient { pub(crate) ipv4_routes: BTreeSet, pub(crate) ipv6_routes: BTreeSet, + /// The search-domain emitted by connlib. + pub(crate) search_domain: Option, + pub(crate) resource_status: BTreeMap, pub(crate) sent_udp_dns_queries: HashMap<(SocketAddr, QueryId), IpPacket>, @@ -87,6 +90,7 @@ impl SimClient { received_udp_replies: Default::default(), ipv4_routes: Default::default(), ipv6_routes: Default::default(), + search_domain: Default::default(), resource_status: Default::default(), tcp_dns_client, } @@ -97,6 +101,10 @@ impl SimClient { self.dns_by_sentinel.right_values().copied().collect() } + pub(crate) fn effective_search_domain(&self) -> Option { + self.search_domain.clone() + } + pub(crate) fn set_new_dns_servers(&mut self, mapping: BiMap) { self.dns_by_sentinel = mapping; self.tcp_dns_client.reset(); @@ -925,6 +933,10 @@ impl RefClient { .collect() } + pub(crate) fn expected_search_domain(&self) -> Option { + self.search_domain.clone() + } + pub(crate) fn expected_routes(&self) -> (BTreeSet, BTreeSet) { ( self.ipv4_routes @@ -1043,6 +1055,10 @@ impl RefClient { self.upstream_dns_resolvers.clone_from(servers); } + pub(crate) fn set_upstream_search_domain(&mut self, domain: Option<&DomainName>) { + self.search_domain = domain.cloned() + } + pub(crate) fn upstream_dns_resolvers(&self) -> Vec { self.upstream_dns_resolvers.clone() } diff --git a/rust/connlib/tunnel/src/tests/stub_portal.rs b/rust/connlib/tunnel/src/tests/stub_portal.rs index 4829d7bf2..1a6013a48 100644 --- a/rust/connlib/tunnel/src/tests/stub_portal.rs +++ b/rust/connlib/tunnel/src/tests/stub_portal.rs @@ -253,6 +253,16 @@ impl StubPortal { system_dns: impl Strategy>, upstream_dns: impl Strategy>, ) -> impl Strategy> { + ref_client_host( + Just(self.client_tunnel_ipv4), + Just(self.client_tunnel_ipv6), + system_dns, + upstream_dns, + self.search_domain(), + ) + } + + pub(crate) fn search_domain(&self) -> impl Strategy> { let possible_search_domains = self .dns_resources .values() @@ -267,13 +277,7 @@ impl StubPortal { }) .collect::>(); - ref_client_host( - Just(self.client_tunnel_ipv4), - Just(self.client_tunnel_ipv6), - system_dns, - upstream_dns, - proptest::option::of(sample::select(possible_search_domains)), - ) + proptest::option::of(sample::select(possible_search_domains)) } pub(crate) fn dns_resource_records(&self) -> impl Strategy { diff --git a/rust/connlib/tunnel/src/tests/sut.rs b/rust/connlib/tunnel/src/tests/sut.rs index 5be744712..f83028875 100644 --- a/rust/connlib/tunnel/src/tests/sut.rs +++ b/rust/connlib/tunnel/src/tests/sut.rs @@ -240,6 +240,16 @@ impl TunnelTest { }) }); } + Transition::UpdateUpstreamSearchDomain(search_domain) => { + state.client.exec_mut(|c| { + c.sut.update_interface_config(Interface { + ipv4: c.sut.tunnel_ip_config().unwrap().v4, + ipv6: c.sut.tunnel_ip_config().unwrap().v6, + upstream_dns: ref_state.client.inner().upstream_dns_resolvers(), + search_domain, + }) + }); + } Transition::RoamClient { ip4, ip6, port } => { state.network.remove_host(&state.client); state.client.update_interface(ip4, ip6, port); @@ -357,6 +367,7 @@ impl TunnelTest { assert_udp_dns_packets_properties(ref_client, sim_client); assert_tcp_dns(ref_client, sim_client); assert_dns_servers_are_valid(ref_client, sim_client); + assert_search_domain_is_valid(ref_client, sim_client); assert_routes_are_valid(ref_client, sim_client); assert_resource_status(ref_client, sim_client); } @@ -751,9 +762,10 @@ impl TunnelTest { if self.client.inner().dns_mapping() == &config.dns_by_sentinel && self.client.inner().ipv4_routes == config.ipv4_routes && self.client.inner().ipv6_routes == config.ipv6_routes + && self.client.inner().search_domain == config.search_domain { tracing::error!( - "Emitted `TunInterfaceUpdated` without changing DNS servers or routes" + "Emitted `TunInterfaceUpdated` without changing DNS servers, routes or search domain" ); } @@ -770,6 +782,7 @@ impl TunnelTest { c.set_new_dns_servers(config.dns_by_sentinel); c.ipv4_routes = config.ipv4_routes; c.ipv6_routes = config.ipv6_routes; + c.search_domain = config.search_domain }); } } diff --git a/rust/connlib/tunnel/src/tests/transition.rs b/rust/connlib/tunnel/src/tests/transition.rs index d52fa52a6..41e174b69 100644 --- a/rust/connlib/tunnel/src/tests/transition.rs +++ b/rust/connlib/tunnel/src/tests/transition.rs @@ -55,6 +55,8 @@ pub(crate) enum Transition { UpdateSystemDnsServers(Vec), /// The upstream DNS servers changed. UpdateUpstreamDnsServers(Vec), + /// The upstream search domain changed. + UpdateUpstreamSearchDomain(Option), /// Roam the client to a new pair of sockets. RoamClient {