test(connlib): proactively force connections through relay (#5917)

Currently, the relay path in `tunnel_test` is only hit accidentally
because we don't run the gateways in dual-stack mode and thus, some
testcases have a client and gateways that can't talk to each other (and
thus fall back to the relay).

This requires us to filter out certain resources because we can't route
to an IPv6 CIDR resource from an IPv4-only gateway. This causes quite a
lot of rejections which creates problems when one attempts up the number
of test cases (i.e. 10_000).

To fix this, we run the gateways always in dual-stack mode and introduce
a dedicated flag that sometimes drop all direct traffic between the
client and the gateways.
This commit is contained in:
Thomas Eizinger
2024-07-20 14:38:20 +10:00
committed by GitHub
parent 79c815fbbc
commit 5e021a235c
6 changed files with 39 additions and 30 deletions

View File

@@ -40,6 +40,7 @@ pub(crate) struct ReferenceState {
/// This is used to e.g. mock DNS resolution on the gateway.
pub(crate) global_dns_records: BTreeMap<DomainName, HashSet<IpAddr>>,
pub(crate) drop_direct_client_traffic: bool,
pub(crate) network: RoutingTable,
}
@@ -67,12 +68,21 @@ impl ReferenceStateMachine for ReferenceState {
gateways_and_portal(),
collection::hash_map(relay_id(), relay_prototype(), 1..=2),
global_dns_records(), // Start out with a set of global DNS records so we have something to resolve outside of DNS resources.
any::<bool>(),
Just(Instant::now()),
Just(Utc::now()),
)
.prop_filter_map(
"network IPs must be unique",
|(c, (gateways, portal, records), relays, mut global_dns, now, utc_now)| {
|(
c,
(gateways, portal, records),
relays,
mut global_dns,
drop_direct_client_traffic,
now,
utc_now,
)| {
let mut routing_table = RoutingTable::default();
if !routing_table.add_host(c.inner().id, &c) {
@@ -99,6 +109,7 @@ impl ReferenceStateMachine for ReferenceState {
relays,
portal,
global_dns,
drop_direct_client_traffic,
now,
utc_now,
routing_table,
@@ -107,7 +118,7 @@ impl ReferenceStateMachine for ReferenceState {
)
.prop_filter(
"private keys must be unique",
|(c, gateways, _, _, _, _, _, _)| {
|(c, gateways, _, _, _, _, _, _, _)| {
let different_keys = gateways
.iter()
.map(|(_, g)| g.inner().key)
@@ -118,7 +129,17 @@ impl ReferenceStateMachine for ReferenceState {
},
)
.prop_map(
|(client, gateways, relays, portal, global_dns_records, now, utc_now, network)| {
|(
client,
gateways,
relays,
portal,
global_dns_records,
drop_direct_client_traffic,
now,
utc_now,
network,
)| {
Self {
now,
utc_now,
@@ -128,6 +149,7 @@ impl ReferenceStateMachine for ReferenceState {
portal,
global_dns_records,
network,
drop_direct_client_traffic,
}
},
)

View File

@@ -1,6 +1,6 @@
use super::{
reference::{private_key, PrivateKey},
sim_net::{any_ip_stack, any_port, host, Host},
sim_net::{any_port, dual_ip_stack, host, Host},
};
use crate::{tests::sut::hickory_name_to_domain, GatewayState};
use connlib_shared::DomainName;
@@ -99,7 +99,7 @@ impl RefGateway {
}
pub(crate) fn ref_gateway_host() -> impl Strategy<Value = Host<RefGateway>> {
host(any_ip_stack(), any_port(), ref_gateway())
host(dual_ip_stack(), any_port(), ref_gateway())
}
fn ref_gateway() -> impl Strategy<Value = RefGateway> {

View File

@@ -99,10 +99,10 @@ impl<T> Host<T> {
}
}
pub(crate) fn can_route_to(&self, network: IpNetwork) -> bool {
match network {
IpNetwork::V4(_) => self.ip4.is_some(),
IpNetwork::V6(_) => self.ip6.is_some(),
pub(crate) fn is_sender(&self, src: IpAddr) -> bool {
match src {
IpAddr::V4(src) => self.ip4.is_some_and(|v4| v4 == src),
IpAddr::V6(src) => self.ip6.is_some_and(|v6| v6 == src),
}
}
}

View File

@@ -189,21 +189,6 @@ pub(crate) fn gateways_and_portal() -> impl Strategy<
(Just(gateways), Just(portal), dns_resource_records)
},
)
.prop_filter(
"gateway must be able to access their assigned CIDR resources",
|(gateways, portal, _)| {
portal.cidr_resources().all(|(rid, r)| {
portal
.gateway_for_resource(*rid)
.and_then(|g| gateways.get(g))
.is_some_and(|g| {
// TODO: PRODUCTION CODE DOES NOT HANDLE THIS!
g.can_route_to(r.address)
})
})
},
)
}
fn any_site(sites: HashSet<Site>) -> impl Strategy<Value = Site> {

View File

@@ -78,12 +78,6 @@ impl StubPortal {
.collect()
}
pub(crate) fn cidr_resources(
&self,
) -> impl Iterator<Item = (&ResourceId, &client::ResourceDescriptionCidr)> + '_ {
self.cidr_resources.iter()
}
/// Picks, which gateway and site we should connect to for the given resource.
pub(crate) fn handle_connection_intent(
&self,

View File

@@ -50,6 +50,7 @@ pub(crate) struct TunnelTest {
pub(crate) gateways: HashMap<GatewayId, Host<SimGateway>>,
relays: HashMap<RelayId, Host<SimRelay>>,
drop_direct_client_traffic: bool,
network: RoutingTable,
#[allow(dead_code)]
@@ -130,6 +131,7 @@ impl StateMachineTest for TunnelTest {
now: ref_state.now,
utc_now: ref_state.utc_now,
network: ref_state.network.clone(),
drop_direct_client_traffic: ref_state.drop_direct_client_traffic,
client,
gateways,
logger,
@@ -504,6 +506,12 @@ impl TunnelTest {
.exec_mut(|c| c.handle_packet(payload, src, dst, self.now));
}
HostId::Gateway(id) => {
if self.drop_direct_client_traffic && self.client.is_sender(src.ip()) {
tracing::debug!("Dropping direct traffic from client -> gateway");
return;
}
let gateway = self.gateways.get_mut(&id).expect("unknown gateway");
let Some(transmit) = gateway