github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(connlib): always update TunConfig on any changes (#8453)

Browse Source 
Currently, we are only emitting updates to the `TunConfig` when the
routes or the DNS servers change. This isn't correct, we should also
emit updates for it when the IPs or the search-domain changes.

In order to achieve that, we create a new `TunConfig` based on the
existing one every time we receive an `InterfaceConfig` update.
Depending on our current state, we may create an entirely new
`TunConfig` or create a new one where we copy the fields in from the new
`InterfaceConfig`. We then unconditionally call
`maybe_update_tun_config` which does the necessary work to only emit
updates when things actually changed.

To ensure this works in all cases and the latest update is always
reflected on the TUN device, we also extend the proptests to assert the
latest search domain.

Fixes: #8451
This commit is contained in:
Thomas Eizinger
2025-03-17 01:59:32 +11:00
committed by GitHub
parent d5fda62036
commit 99624a4302
8 changed files with 91 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
rust/connlib/tunnel/proptest-regressions/tests.txt
View File

@@ -164,3 +164,4 @@ cc 4b8aab1f09422751b66d7e46a968bb29fb9b11c8fff9bceb67cd5c8ddeab0a3d
cc c48e5d18ae2cc7533bbe1d0cd155a1ec7bcaf00e8d029b0345c241ec3371dcca
cc f2de44e6762e9a681d624467fd19ac9fc00f000dfc1c2a3bda05c905b01674c2
cc 36a7bb4eff285399b9c431675d4337712e7edf016a3a02b05cba5115c8bf8fe4
cc 235333b8c818e464ba339e8c73b2467894d68d594ac896c4f6a36b25ac6b823d

43
rust/connlib/tunnel/src/client.rs
View File

@@ -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<IpPacket> {
@@ -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(_)));

9
rust/connlib/tunnel/src/tests/assertions.rs
View File

@@ -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) = (

13
rust/connlib/tunnel/src/tests/reference.rs
View File

@@ -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

16
rust/connlib/tunnel/src/tests/sim_client.rs
View File

@@ -45,6 +45,9 @@ pub(crate) struct SimClient {
pub(crate) ipv4_routes: BTreeSet<Ipv4Network>,
pub(crate) ipv6_routes: BTreeSet<Ipv6Network>,
/// The search-domain emitted by connlib.
pub(crate) search_domain: Option<DomainName>,
pub(crate) resource_status: BTreeMap<ResourceId, ResourceStatus>,
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<DomainName> {
self.search_domain.clone()
}
pub(crate) fn set_new_dns_servers(&mut self, mapping: BiMap<IpAddr, SocketAddr>) {
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<DomainName> {
self.search_domain.clone()
}
pub(crate) fn expected_routes(&self) -> (BTreeSet<Ipv4Network>, BTreeSet<Ipv6Network>) {
(
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<DnsServer> {
self.upstream_dns_resolvers.clone()
}

18
rust/connlib/tunnel/src/tests/stub_portal.rs
View File

@@ -253,6 +253,16 @@ impl StubPortal {
system_dns: impl Strategy<Value = Vec<IpAddr>>,
upstream_dns: impl Strategy<Value = Vec<DnsServer>>,
) -> impl Strategy<Value = Host<RefClient>> {
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<Value = Option<DomainName>> {
let possible_search_domains = self
.dns_resources
.values()
@@ -267,13 +277,7 @@ impl StubPortal {
})
.collect::<Vec<_>>();
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<Value = DnsRecords> {

15
rust/connlib/tunnel/src/tests/sut.rs
View File

@@ -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
});
}
}

2
rust/connlib/tunnel/src/tests/transition.rs
View File

@@ -55,6 +55,8 @@ pub(crate) enum Transition {
UpdateSystemDnsServers(Vec<IpAddr>),
/// The upstream DNS servers changed.
UpdateUpstreamDnsServers(Vec<DnsServer>),
/// The upstream search domain changed.
UpdateUpstreamSearchDomain(Option<DomainName>),
/// Roam the client to a new pair of sockets.
RoamClient {