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

test(connlib): drop DNS queries to CIDR resources for pending connections (#6273)

Browse Source 
In #6259, we added a regression test for concurrent DNS queries. A case
that we overlooked is that when DNS servers are defined as CIDR
resources, the queries themselves will act as connection intents and
thus dropped until we have a connection.

In the tests, the connection is only established as part of `advance`.
Thus, if we get multiple concurrent DNS queries to the same server that
is defined as a CIDR resource, we need to drop all future queries.

Fixes: #6283.

---------

Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
This commit is contained in:
Thomas Eizinger
2024-08-13 22:22:46 +01:00
committed by GitHub
parent 0b55087eff
commit 585e2146ba
3 changed files with 72 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

File diff suppressed because one or more lines are too long

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

@@ -320,42 +320,42 @@ impl ReferenceStateMachine for ReferenceState {
}
}),
Transition::SendDnsQueries(queries) => {
for DnsQuery {
domain,
r_type,
dns_server,
query_id,
} in queries
{
match state
let mut pending_connections = HashSet::new();
for query in queries {
// Queries to known hosts are always successful.
if state
.client
.inner()
.dns_query_via_cidr_resource(dns_server.ip(), domain)
.is_known_host(&query.domain.to_string())
{
Some(resource)
if !state.client.inner().is_connected_to_cidr(resource)
&& !state.client.inner().upstream_dns_resolvers.is_empty()
&& !state.client.inner().is_known_host(&domain.to_string()) =>
{
state.client.exec_mut(|client| {
client.connected_cidr_resources.insert(resource)
});
}
Some(_) | None => {
state.client.exec_mut(|client| {
client
.dns_records
.entry(domain.clone())
.or_default()
.insert(*r_type)
});
state.client.exec_mut(|client| {
client
.expected_dns_handshakes
.push_back((*dns_server, *query_id))
});
}
state.client.exec_mut(|client| client.on_dns_query(query));
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.
state.client.exec_mut(|client| client.on_dns_query(query));
continue;
};
if pending_connections.contains(&resource) {
// DNS server is a CIDR resource and a previous query of this batch is already triggering a connection.
// That connection isn't ready yet so further queries to the same resource are dropped until then.
continue;
}
if !state.client.inner().is_connected_to_cidr(resource) {
state
.client
.exec_mut(|client| client.connected_cidr_resources.insert(resource));
pending_connections.insert(resource);
continue;
}
state.client.exec_mut(|client| client.on_dns_query(query));
}
}
Transition::SendICMPPacketToNonResourceIp {
@@ -537,23 +537,19 @@ impl ReferenceStateMachine for ReferenceState {
.iter()
.any(|dns_server| state.client.sending_socket_for(dns_server.ip()).is_some())
}
Transition::SendDnsQueries(queries) => queries.iter().all(
|DnsQuery {
domain, dns_server, ..
}| {
let has_socket_for_server =
state.client.sending_socket_for(dns_server.ip()).is_some();
let is_known_domain = state.global_dns_records.contains_key(domain);
let has_dns_server = state
.client
.inner()
.expected_dns_servers()
.contains(dns_server);
let gateway_is_present_in_case_dns_server_is_cidr_resource = match state
.client
.inner()
.dns_query_via_cidr_resource(dns_server.ip(), domain)
{
Transition::SendDnsQueries(queries) => queries.iter().all(|query| {
let has_socket_for_server = state
.client
.sending_socket_for(query.dns_server.ip())
.is_some();
let is_known_domain = state.global_dns_records.contains_key(&query.domain);
let has_dns_server = state
.client
.inner()
.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) {
Some(r) => {
let Some(gateway) = state.portal.gateway_for_resource(r) else {
return false;
@@ -564,12 +560,11 @@ impl ReferenceStateMachine for ReferenceState {
None => true,
};
has_socket_for_server
&& is_known_domain
&& has_dns_server
&& gateway_is_present_in_case_dns_server_is_cidr_resource
},
),
has_socket_for_server
&& is_known_domain
&& has_dns_server
&& gateway_is_present_in_case_dns_server_is_cidr_resource
}),
Transition::RoamClient { ip4, ip6, port } => {
// In production, we always rebind to a new port so we never roam to our old existing IP / port combination.

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

@@ -3,6 +3,7 @@ use super::{
sim_net::{any_ip_stack, any_port, host, Host},
sim_relay::{map_explode, SimRelay},
strategies::latency,
transition::DnsQuery,
IcmpIdentifier, IcmpSeq, QueryId,
};
use crate::{proptest::*, ClientState};
@@ -462,6 +463,16 @@ impl RefClient {
}
}
pub(crate) fn on_dns_query(&mut self, query: &DnsQuery) {
self.dns_records
.entry(query.domain.clone())
.or_default()
.insert(query.r_type);
self.expected_dns_handshakes
.push_back((query.dns_server, query.query_id));
}
pub(crate) fn ipv4_cidr_resource_dsts(&self) -> Vec<Ipv4Network> {
self.cidr_resources
.iter_ipv4()
@@ -601,17 +612,18 @@ impl RefClient {
/// Returns the CIDR 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,
dns_server: IpAddr,
domain: &DomainName,
) -> Option<ResourceId> {
// If we are querying a DNS resource, we will issue a connection intent to the DNS resource, not the CIDR resource.
if self.dns_resource_by_domain(domain).is_some() {
pub(crate) fn dns_query_via_cidr_resource(&self, query: &DnsQuery) -> Option<ResourceId> {
// Unless we are using upstream resolvers, DNS queries are never routed through the tunnel.
if self.upstream_dns_resolvers.is_empty() {
return None;
}
self.cidr_resource_by_ip(dns_server)
// If we are querying a DNS resource, we will issue a connection intent to the DNS resource, not the CIDR resource.
if self.dns_resource_by_domain(&query.domain).is_some() {
return None;
}
self.cidr_resource_by_ip(query.dns_server.ip())
}
pub(crate) fn all_resource_ids(&self) -> Vec<ResourceId> {