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

chore(connlib): fix proptest failure with TCP DNS (#7093)

Browse Source 
When we query upstream DNS servers through the tunnel via TCP DNS, we
will always be successful in establishing a tunnel, regardless of how
many concurrent queries we send because the TCP stack will keep
re-trying. Thus, tracking, which resources we are connected to after
sending a bunch of DNS queries needs to be split by UDP and TCP.

For UDP, only the "first" resource will be connected, however, with
concurrent TCP and UDP DNS queries, "first" isn't necessarily the order
in which we send the queries because with TCP DNS, one packet doesn't
equate to one query anymore.

This is quite hacky but it will get completely deleted once we buffer
packets during the connection setup.
This commit is contained in:
Thomas Eizinger
2024-10-19 11:02:00 +11:00
committed by GitHub
parent 0452273a16
commit b8f5fb9e25
2 changed files with 26 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -124,3 +124,4 @@ cc 33cd1cba9c6ecf15d6ff86c3114752f2437e432c77f671f67b08116d2b507131
cc d9793b201ec425bd77f9849ea48e63677014aeb4a91a55be9371b81e644b7a24
cc 8fcbd19c41f0483d9b81aac2ab7440bb23d7796ef9f6bf346f73f0d633f65baa
cc 4494e475d22ff9a318d676f10c79f545982b7787d145925c3719fe47e9868acc
cc bafb7db795d394d1771ef07f4dd36db8ac1333dd852653900480d7ed03307853

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

@@ -299,7 +299,10 @@ impl ReferenceState {
}
}),
Transition::SendDnsQueries(queries) => {
let mut new_connections_via_gateways = BTreeMap::<_, BTreeSet<ResourceId>>::new();
let mut new_connections_via_gateways_udp_triggered =
BTreeMap::<_, BTreeSet<ResourceId>>::new();
let mut new_connections_via_gateways_tcp_triggered =
BTreeMap::<_, BTreeSet<ResourceId>>::new();
for query in queries {
// Some queries get answered locally.
@@ -335,19 +338,27 @@ impl ReferenceState {
{
tracing::debug!(%resource, %gateway, "Not connected yet, dropping packet");
let connected_resources =
new_connections_via_gateways.entry(gateway).or_default();
let connected_resources = match query.transport {
DnsTransport::Udp => &mut new_connections_via_gateways_udp_triggered,
DnsTransport::Tcp => &mut new_connections_via_gateways_tcp_triggered,
}
.entry(gateway)
.or_default();
if state.client.inner().is_connected_gateway(gateway) {
connected_resources.insert(resource);
} else {
// As part of batch-processing DNS queries, only the first resource per gateway will be connected / authorized.
if connected_resources.is_empty() {
connected_resources.insert(resource);
}
// TCP has retries so we will also be connected to those for sure.
if query.transport == DnsTransport::Tcp {
connected_resources.insert(resource);
match query.transport {
DnsTransport::Udp => {
// As part of batch-processing DNS queries, only the first resource per gateway will be connected / authorized.
if connected_resources.is_empty() {
connected_resources.insert(resource);
}
}
DnsTransport::Tcp => {
// TCP has retries, so those will always connect.
connected_resources.insert(resource);
}
}
}
@@ -357,7 +368,10 @@ impl ReferenceState {
state.client.exec_mut(|client| client.on_dns_query(query));
}
for (gateway, resources) in new_connections_via_gateways {
for (gateway, resources) in new_connections_via_gateways_udp_triggered
.into_iter()
.chain(new_connections_via_gateways_tcp_triggered)
{
for resource in resources {
state.client.exec_mut(|client| {
client.connect_to_internet_or_cidr_resource(resource, gateway)