scottk/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-04-05 07:06:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(connlib): track dedicated TunConfig (#6427)

Browse Source 
Currently, `connlib` tracks the `Interface` as it is given it by the
portal. This includes the tunnel IP addresses plus the upstream servers.

Upstreams servers however only take effect when they are defined.
Without upstream DNS servers, `connlib` uses the system-defined DNS
servers. In that case, the `Interface` no longer accurately represents,
what we actually configure on the TUN device.

To fix this, we introduce a dedicated `TunConfig` struct that tracks,
what is actually set on the interface. This also allows us to track,
whether or not we need to re-emit this configuration after a change.

Related: #6423.

---------

Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
This commit is contained in:
Thomas Eizinger
2024-08-29 03:32:41 +01:00
committed by GitHub
parent fa5da77484
commit 216f6efc5f
11 changed files with 191 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
rust/connlib/clients/shared/src/eventloop.rs
View File

@@ -171,15 +171,11 @@ where
firezone_tunnel::ClientEvent::ResourcesChanged { resources } => {
self.callbacks.on_update_resources(resources)
}
firezone_tunnel::ClientEvent::TunInterfaceUpdated {
ip4,
ip6,
dns_by_sentinel,
} => {
let dns_servers = dns_by_sentinel.left_values().copied().collect();
firezone_tunnel::ClientEvent::TunInterfaceUpdated(config) => {
let dns_servers = config.dns_by_sentinel.left_values().copied().collect();
self.callbacks
.on_set_interface_config(ip4, ip6, dns_servers);
.on_set_interface_config(config.ip4, config.ip6, dns_servers);
}
firezone_tunnel::ClientEvent::TunRoutesUpdated { ip4, ip6 } => {
self.callbacks.on_update_routes(ip4, ip6);

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

@@ -1,6 +1,6 @@
use crate::dns::StubResolver;
use crate::peer_store::PeerStore;
use crate::{dns, BUF_SIZE};
use crate::{dns, TunConfig, BUF_SIZE};
use anyhow::Context;
use bimap::BiMap;
use connlib_shared::callbacks::Status;
@@ -244,6 +244,10 @@ pub struct ClientState {
/// The DNS resolvers configured on the system outside of connlib.
system_resolvers: Vec<IpAddr>,
/// The DNS resolvers configured in the portal.
///
/// Has priority over system-configured DNS servers.
upstream_dns: Vec<DnsServer>,
/// Maps from connlib-assigned IP of a DNS server back to the originally configured system DNS resolver.
dns_mapping: BiMap<IpAddr, DnsServer>,
@@ -267,7 +271,7 @@ pub struct ClientState {
stub_resolver: StubResolver,
/// Configuration of the TUN device, when it is up.
interface_config: Option<InterfaceConfig>,
tun_config: Option<TunConfig>,
/// Resources that have been disabled by the UI
disabled_resources: BTreeSet<ResourceId>,
@@ -302,7 +306,7 @@ impl ClientState {
peers: Default::default(),
dns_mapping: Default::default(),
buffered_events: Default::default(),
interface_config: Default::default(),
tun_config: Default::default(),
buffered_packets: Default::default(),
node: ClientNode::new(private_key.into(), BUF_SIZE, seed),
system_resolvers: Default::default(),
@@ -315,17 +319,18 @@ impl ClientState {
buffered_transmits: Default::default(),
internet_resource: None,
recently_connected_gateways: LruCache::new(MAX_REMEMBERED_GATEWAYS),
upstream_dns: Default::default(),
}
}
#[cfg(all(test, feature = "proptest"))]
pub(crate) fn tunnel_ip4(&self) -> Option<Ipv4Addr> {
Some(self.interface_config.as_ref()?.ipv4)
Some(self.tun_config.as_ref()?.ip4)
}
#[cfg(all(test, feature = "proptest"))]
pub(crate) fn tunnel_ip6(&self) -> Option<Ipv6Addr> {
Some(self.interface_config.as_ref()?.ipv6)
Some(self.tun_config.as_ref()?.ip6)
}
#[cfg(all(test, feature = "proptest"))]
@@ -618,11 +623,7 @@ impl ClientState {
}
fn is_upstream_set_by_the_portal(&self) -> bool {
let Some(interface) = &self.interface_config else {
return false;
};
!interface.upstream_dns.is_empty()
!self.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.
@@ -786,8 +787,6 @@ impl ClientState {
}
fn set_dns_mapping(&mut self, new_mapping: BiMap<IpAddr, DnsServer>) {
tracing::debug!(mapping = ?new_mapping, "Updating DNS servers");
self.dns_mapping = new_mapping;
self.mangled_dns_queries.clear();
}
@@ -828,6 +827,8 @@ impl ClientState {
.map(|(ip, _)| ip)
.chain(iter::once(IPV4_RESOURCES.into()))
.chain(iter::once(IPV6_RESOURCES.into()))
.chain(iter::once(DNS_SENTINELS_V4.into()))
.chain(iter::once(DNS_SENTINELS_V6.into()))
.chain(
self.internet_resource
.map(|_| Ipv4Network::DEFAULT_ROUTE.into()),
@@ -836,7 +837,6 @@ impl ClientState {
self.internet_resource
.map(|_| Ipv6Network::DEFAULT_ROUTE.into()),
)
.chain(self.dns_mapping.left_values().copied().map(Into::into))
}
fn is_resource_enabled(&self, resource: &ResourceId) -> bool {
@@ -862,13 +862,32 @@ impl ClientState {
}
pub(crate) fn update_system_resolvers(&mut self, new_dns: Vec<IpAddr>) {
tracing::debug!(servers = ?new_dns, "Received system-defined DNS servers");
self.system_resolvers = new_dns;
self.update_dns_mapping()
}
pub(crate) fn update_interface_config(&mut self, config: InterfaceConfig) {
self.interface_config = Some(config);
tracing::trace!(upstream_dns = ?config.upstream_dns, 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.ip4 = config.ipv4;
existing.ip6 = config.ipv6;
}
None => {
self.tun_config = Some(TunConfig {
ip4: config.ipv4,
ip6: config.ipv6,
dns_by_sentinel: Default::default(),
})
}
}
self.upstream_dns = config.upstream_dns;
self.update_dns_mapping()
}
@@ -1118,7 +1137,7 @@ impl ClientState {
}
fn update_dns_mapping(&mut self) {
let Some(config) = &self.interface_config else {
let Some(config) = self.tun_config.clone() else {
// For the Tauri clients this can happen because it's called immediately after phoenix_channel's connect, before on_set_interface_config
tracing::debug!("Unable to update DNS servers without interface configuration");
@@ -1126,12 +1145,12 @@ impl ClientState {
};
let effective_dns_servers =
effective_dns_servers(config.upstream_dns.clone(), self.system_resolvers.clone());
effective_dns_servers(self.upstream_dns.clone(), self.system_resolvers.clone());
if HashSet::<&DnsServer>::from_iter(effective_dns_servers.iter())
== HashSet::from_iter(self.dns_mapping.right_values())
{
tracing::debug!("Effective DNS servers are unchanged");
tracing::debug!(servers = ?effective_dns_servers, "Effective DNS servers are unchanged");
return;
}
@@ -1145,26 +1164,28 @@ impl ClientState {
.collect_vec(),
);
let ip4 = config.ipv4;
let ip6 = config.ipv6;
let new_tun_config = TunConfig {
ip4: config.ip4,
ip6: config.ip6,
dns_by_sentinel: dns_mapping
.iter()
.map(|(sentinel_dns, effective_dns)| (*sentinel_dns, effective_dns.address()))
.collect::<BiMap<_, _>>(),
};
self.set_dns_mapping(dns_mapping);
if new_tun_config == config {
tracing::debug!(current = ?config, "TUN device configuration unchanged");
return;
}
tracing::info!(config = ?new_tun_config, "Updating TUN device");
self.tun_config = Some(new_tun_config.clone());
self.buffered_events
.push_back(ClientEvent::TunInterfaceUpdated {
ip4,
ip6,
dns_by_sentinel: self
.dns_mapping
.iter()
.map(|(sentinel_dns, effective_dns)| (*sentinel_dns, effective_dns.address()))
.collect(),
});
self.buffered_events
.push_back(ClientEvent::TunRoutesUpdated {
ip4: self.routes().filter_map(utils::ipv4).collect(),
ip6: self.routes().filter_map(utils::ipv6).collect(),
});
.push_back(ClientEvent::TunInterfaceUpdated(new_tun_config));
}
pub fn update_relays(
@@ -1764,7 +1785,9 @@ mod proptests {
resource_routes
.into_iter()
.chain(iter::once(IPV4_RESOURCES.into()))
.chain(iter::once(IPV6_RESOURCES.into())),
.chain(iter::once(IPV6_RESOURCES.into()))
.chain(iter::once(DNS_SENTINELS_V4.into()))
.chain(iter::once(DNS_SENTINELS_V6.into())),
)
}

25
rust/connlib/tunnel/src/lib.rs
View File

@@ -302,23 +302,26 @@ pub enum ClientEvent {
resources: Vec<callbacks::ResourceDescription>,
},
// TODO: Make this more fine-granular.
TunInterfaceUpdated {
ip4: Ipv4Addr,
ip6: Ipv6Addr,
/// The map of DNS servers that connlib will use.
///
/// - The "left" values are the connlib-assigned, proxy (or "sentinel") IPs.
/// - The "right" values are the effective DNS servers.
/// If upstream DNS servers are configured (in the portal), we will use those.
/// Otherwise, we will use the DNS servers configured on the system.
dns_by_sentinel: BiMap<IpAddr, SocketAddr>,
},
TunInterfaceUpdated(TunConfig),
TunRoutesUpdated {
ip4: Vec<Ipv4Network>,
ip6: Vec<Ipv6Network>,
},
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct TunConfig {
pub ip4: Ipv4Addr,
pub ip6: Ipv6Addr,
/// The map of DNS servers that connlib will use.
///
/// - The "left" values are the connlib-assigned, proxy (or "sentinel") IPs.
/// - The "right" values are the effective DNS servers.
/// If upstream DNS servers are configured (in the portal), we will use those.
/// Otherwise, we will use the DNS servers configured on the system.
pub dns_by_sentinel: BiMap<IpAddr, SocketAddr>,
}
#[derive(Debug, Clone)]
pub enum GatewayEvent {
AddedIceCandidates {

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

@@ -127,6 +127,15 @@ pub(crate) fn assert_known_hosts_are_valid(ref_client: &RefClient, sim_client: &
}
}
pub(crate) fn assert_dns_servers_are_valid(ref_client: &RefClient, sim_client: &SimClient) {
let expected = ref_client.expected_dns_servers();
let actual = sim_client.effective_dns_servers();
if actual != expected {
tracing::error!(target: "assertions", ?actual, ?expected, "❌ Effective DNS servers are incorrect");
}
}
pub(crate) fn assert_dns_packets_properties(ref_client: &RefClient, sim_client: &SimClient) {
let unexpected_dns_replies = find_unexpected_entries(
&ref_client.expected_dns_handshakes,

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

@@ -59,15 +59,17 @@ impl ReferenceStateMachine for ReferenceState {
type Transition = Transition;
fn init_state() -> BoxedStrategy<Self::State> {
stub_portal()
.prop_flat_map(|portal| {
(stub_portal(), dns_servers())
.prop_flat_map(|(portal, dns_servers)| {
let gateways = portal.gateways();
let dns_resource_records = portal.dns_resource_records();
let client = portal.client();
let client = portal.client(
system_dns_servers(dns_servers.values().cloned().collect()),
upstream_dns_servers(dns_servers.values().cloned().collect()),
);
let relays = relays();
let global_dns_records = global_dns_records(); // Start out with a set of global DNS records so we have something to resolve outside of DNS resources.
let drop_direct_client_traffic = any::<bool>();
let dns_servers = dns_servers();
(
client,
@@ -77,7 +79,7 @@ impl ReferenceStateMachine for ReferenceState {
relays,
global_dns_records,
drop_direct_client_traffic,
dns_servers,
Just(dns_servers),
)
})
.prop_filter_map(
@@ -176,11 +178,13 @@ impl ReferenceStateMachine for ReferenceState {
CompositeStrategy::default()
.with(
1,
update_system_dns_servers(state.dns_servers.values().cloned().collect()),
system_dns_servers(state.dns_servers.values().cloned().collect())
.prop_map(Transition::UpdateSystemDnsServers),
)
.with(
1,
update_upstream_dns_servers(state.dns_servers.values().cloned().collect()),
upstream_dns_servers(state.dns_servers.values().cloned().collect())
.prop_map(Transition::UpdateUpstreamDnsServers),
)
.with_if_not_empty(
5,
@@ -410,12 +414,12 @@ impl ReferenceStateMachine for ReferenceState {
Transition::UpdateSystemDnsServers(servers) => {
state
.client
.exec_mut(|client| client.system_dns_resolvers.clone_from(servers));
.exec_mut(|client| client.set_system_dns_resolvers(servers));
}
Transition::UpdateUpstreamDnsServers(servers) => {
state
.client
.exec_mut(|client| client.upstream_dns_resolvers.clone_from(servers));
.exec_mut(|client| client.set_upstream_dns_resolvers(servers));
}
Transition::RoamClient { ip4, ip6, .. } => {
state.network.remove_host(&state.client);

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

@@ -257,10 +257,10 @@ pub struct RefClient {
/// The DNS resolvers configured on the client outside of connlib.
#[derivative(Debug = "ignore")]
pub(crate) system_dns_resolvers: Vec<IpAddr>,
system_dns_resolvers: Vec<IpAddr>,
/// The upstream DNS resolvers configured in the portal.
#[derivative(Debug = "ignore")]
pub(crate) upstream_dns_resolvers: Vec<DnsServer>,
upstream_dns_resolvers: Vec<DnsServer>,
/// Tracks all resources in the order they have been added in.
///
@@ -731,6 +731,18 @@ impl RefClient {
pub(crate) fn all_resources(&self) -> Vec<ResourceDescription> {
self.resources.clone()
}
pub(crate) fn set_system_dns_resolvers(&mut self, servers: &Vec<IpAddr>) {
self.system_dns_resolvers.clone_from(servers);
}
pub(crate) fn set_upstream_dns_resolvers(&mut self, servers: &Vec<DnsServer>) {
self.upstream_dns_resolvers.clone_from(servers);
}
pub(crate) fn upstream_dns_resolvers(&self) -> Vec<DnsServer> {
self.upstream_dns_resolvers.clone()
}
}
// This function only works on the tests because we are limited to resources with a single wildcard at the beginning of the resource.
@@ -757,11 +769,13 @@ fn is_subdomain(name: &str, record: &str) -> bool {
pub(crate) fn ref_client_host(
tunnel_ip4s: impl Strategy<Value = Ipv4Addr>,
tunnel_ip6s: impl Strategy<Value = Ipv6Addr>,
system_dns: impl Strategy<Value = Vec<IpAddr>>,
upstream_dns: impl Strategy<Value = Vec<DnsServer>>,
) -> impl Strategy<Value = Host<RefClient>> {
host(
any_ip_stack(),
any_port(),
ref_client(tunnel_ip4s, tunnel_ip6s),
ref_client(tunnel_ip4s, tunnel_ip6s, system_dns, upstream_dns),
latency(300), // TODO: Increase with #6062.
)
}
@@ -769,41 +783,58 @@ pub(crate) fn ref_client_host(
fn ref_client(
tunnel_ip4s: impl Strategy<Value = Ipv4Addr>,
tunnel_ip6s: impl Strategy<Value = Ipv6Addr>,
system_dns: impl Strategy<Value = Vec<IpAddr>>,
upstream_dns: impl Strategy<Value = Vec<DnsServer>>,
) -> impl Strategy<Value = RefClient> {
(
tunnel_ip4s,
tunnel_ip6s,
system_dns,
upstream_dns,
client_id(),
private_key(),
known_hosts(),
)
.prop_map(
move |(tunnel_ip4, tunnel_ip6, id, key, known_hosts)| RefClient {
move |(
tunnel_ip4,
tunnel_ip6,
system_dns_resolvers,
upstream_dns_resolvers,
id,
key,
known_hosts,
tunnel_ip4,
tunnel_ip6,
system_dns_resolvers: Default::default(),
upstream_dns_resolvers: Default::default(),
internet_resource: Default::default(),
cidr_resources: IpNetworkTable::new(),
dns_records: Default::default(),
connected_cidr_resources: Default::default(),
connected_dns_resources: Default::default(),
connected_internet_resource: Default::default(),
expected_icmp_handshakes: Default::default(),
expected_dns_handshakes: Default::default(),
disabled_resources: Default::default(),
resources: Default::default(),
)| {
RefClient {
id,
key,
known_hosts,
tunnel_ip4,
tunnel_ip6,
system_dns_resolvers,
upstream_dns_resolvers,
internet_resource: Default::default(),
cidr_resources: IpNetworkTable::new(),
dns_records: Default::default(),
connected_cidr_resources: Default::default(),
connected_dns_resources: Default::default(),
connected_internet_resource: Default::default(),
expected_icmp_handshakes: Default::default(),
expected_dns_handshakes: Default::default(),
disabled_resources: Default::default(),
resources: Default::default(),
}
},
)
}
fn known_hosts() -> impl Strategy<Value = BTreeMap<String, Vec<IpAddr>>> {
collection::btree_map(
domain_name(2..4).prop_map(|d| d.parse().unwrap()),
collection::vec(any::<IpAddr>(), 1..6),
0..15,
prop_oneof![
Just("api.firezone.dev".to_owned()),
Just("api.firez.one".to_owned())
],
collection::vec(any::<IpAddr>(), 1..3),
1..=2,
)
}

5
rust/connlib/tunnel/src/tests/sim_dns.rs
View File

@@ -61,6 +61,7 @@ impl SimDns {
let query = query.sole_question().unwrap();
let name = query.qname().to_vec();
let qtype = query.qtype();
let records = global_dns_records
.get(&name)
@@ -68,7 +69,7 @@ impl SimDns {
.flatten()
.filter(|ip| {
#[allow(clippy::wildcard_enum_match_arm)]
match query.qtype() {
match qtype {
Rtype::A => ip.is_ipv4(),
Rtype::AAAA => ip.is_ipv6(),
_ => todo!(),
@@ -87,7 +88,7 @@ impl SimDns {
let payload = answers.finish();
tracing::debug!(%name, "Responding to DNS query");
tracing::debug!(%name, %qtype, "Responding to DNS query");
Some(Transmit {
src: Some(transmit.dst),

20
rust/connlib/tunnel/src/tests/strategies.rs
View File

@@ -11,7 +11,7 @@ use connlib_shared::{
client::{
ResourceDescriptionCidr, ResourceDescriptionDns, ResourceDescriptionInternet, Site,
},
RelayId,
DnsServer, RelayId,
},
DomainName,
};
@@ -260,3 +260,21 @@ pub(crate) fn documentation_ip6s(subnet: u16, num_ips: usize) -> impl Strategy<V
sample::select(ips)
}
pub(crate) fn system_dns_servers(
dns_servers: Vec<Host<RefDns>>,
) -> impl Strategy<Value = Vec<IpAddr>> {
let max = dns_servers.len();
sample::subsequence(dns_servers, ..=max)
.prop_map(|seq| seq.into_iter().map(|h| h.single_socket().ip()).collect())
}
pub(crate) fn upstream_dns_servers(
dns_servers: Vec<Host<RefDns>>,
) -> impl Strategy<Value = Vec<DnsServer>> {
let max = dns_servers.len();
sample::subsequence(dns_servers, ..=max)
.prop_map(|seq| seq.into_iter().map(|h| h.single_socket().into()).collect())
}

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

@@ -6,7 +6,7 @@ use super::{
};
use crate::proptest::*;
use connlib_shared::{
messages::{client, gateway, GatewayId, ResourceId},
messages::{client, gateway, DnsServer, GatewayId, ResourceId},
DomainName,
};
use ip_network::{Ipv4Network, Ipv6Network};
@@ -197,11 +197,20 @@ impl StubPortal {
.prop_map(BTreeMap::from_iter)
}
pub(crate) fn client(&self) -> impl Strategy<Value = Host<RefClient>> {
pub(crate) fn client(
&self,
system_dns: impl Strategy<Value = Vec<IpAddr>>,
upstream_dns: impl Strategy<Value = Vec<DnsServer>>,
) -> impl Strategy<Value = Host<RefClient>> {
let client_tunnel_ip4 = tunnel_ip4s().next().unwrap();
let client_tunnel_ip6 = tunnel_ip6s().next().unwrap();
ref_client_host(Just(client_tunnel_ip4), Just(client_tunnel_ip6))
ref_client_host(
Just(client_tunnel_ip4),
Just(client_tunnel_ip6),
system_dns,
upstream_dns,
)
}
pub(crate) fn dns_resource_records(

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

@@ -257,7 +257,7 @@ impl TunnelTest {
Transition::ReconnectPortal => {
let ipv4 = state.client.inner().sut.tunnel_ip4().unwrap();
let ipv6 = state.client.inner().sut.tunnel_ip6().unwrap();
let upstream_dns = ref_state.client.inner().upstream_dns_resolvers.clone();
let upstream_dns = ref_state.client.inner().upstream_dns_resolvers();
let all_resources = ref_state.client.inner().all_resources();
// Simulate receiving `init`.
@@ -351,11 +351,7 @@ impl TunnelTest {
);
assert_dns_packets_properties(ref_client, sim_client);
assert_known_hosts_are_valid(ref_client, sim_client);
assert_eq!(
sim_client.effective_dns_servers(),
ref_client.expected_dns_servers(),
"Effective DNS servers should match either system or upstream DNS"
);
assert_dns_servers_are_valid(ref_client, sim_client);
}
}
@@ -676,11 +672,13 @@ impl TunnelTest {
ClientEvent::ResourcesChanged { .. } => {
tracing::warn!("Unimplemented");
}
ClientEvent::TunInterfaceUpdated {
dns_by_sentinel, ..
} => {
ClientEvent::TunInterfaceUpdated(config) => {
if self.client.inner().dns_by_sentinel == config.dns_by_sentinel {
tracing::error!("Emitted `TunInterfaceUpdated` without changing DNS servers");
}
self.client
.exec_mut(|c| c.dns_by_sentinel = dns_by_sentinel);
.exec_mut(|c| c.dns_by_sentinel = config.dns_by_sentinel);
}
ClientEvent::TunRoutesUpdated { .. } => {}
ClientEvent::RequestConnection {

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

@@ -1,7 +1,4 @@
use super::{
sim_dns::RefDns,
sim_net::{any_ip_stack, any_port, Host},
};
use super::sim_net::{any_ip_stack, any_port, Host};
use connlib_shared::{
messages::{client::ResourceDescription, DnsServer, RelayId, ResourceId},
DomainName,
@@ -220,27 +217,3 @@ pub(crate) fn roam_client() -> impl Strategy<Value = Transition> {
port,
})
}
pub(crate) fn update_system_dns_servers(
dns_servers: Vec<Host<RefDns>>,
) -> impl Strategy<Value = Transition> {
let max = dns_servers.len();
sample::subsequence(dns_servers, ..=max).prop_map(|seq| {
Transition::UpdateSystemDnsServers(
seq.into_iter().map(|h| h.single_socket().ip()).collect(),
)
})
}
pub(crate) fn update_upstream_dns_servers(
dns_servers: Vec<Host<RefDns>>,
) -> impl Strategy<Value = Transition> {
let max = dns_servers.len();
sample::subsequence(dns_servers, ..=max).prop_map(|seq| {
Transition::UpdateUpstreamDnsServers(
seq.into_iter().map(|h| h.single_socket().into()).collect(),
)
})
}