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

test(connlib): add coverage for the Internet Resource (#6089)

Browse Source 
With the upcoming feature of full-route tunneling aka an "Internet
Resource", we need to expand the reference state machine in
`tunnel_test`. In particular, packets to non-resources will now be
routed the gateway if we have previously activated the Internet
resource.

This is reasonably easy to model as we can see from the small diff.

Because `connlib` doesn't actually support the Internet resource yet,
the code snippet for where it is added to the list of all possible
resources to sample from is commented out.
This commit is contained in:
Thomas Eizinger
2024-07-30 23:04:38 +01:00
committed by GitHub
parent a25e1d10f0
commit 64d2d89542
9 changed files with 160 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
rust/connlib/shared/src/callbacks.rs
View File

@@ -19,7 +19,7 @@ pub enum Status {
pub enum ResourceDescription {
Dns(ResourceDescriptionDns),
Cidr(ResourceDescriptionCidr),
Internet(ResourceDescriptionCidr),
Internet(ResourceDescriptionInternet),
}
impl ResourceDescription {

14
rust/connlib/shared/src/messages/client.rs
View File

@@ -80,6 +80,16 @@ pub struct ResourceDescriptionInternet {
pub sites: Vec<Site>,
}
impl ResourceDescriptionInternet {
pub fn with_status(self, status: Status) -> crate::callbacks::ResourceDescriptionInternet {
crate::callbacks::ResourceDescriptionInternet {
id: self.id,
sites: self.sites,
status,
}
}
}
#[derive(Debug, Deserialize, Serialize, Clone, Eq, PartialOrd, Ord)]
pub struct Site {
pub id: SiteId,
@@ -193,7 +203,9 @@ impl ResourceDescription {
ResourceDescription::Cidr(r) => {
crate::callbacks::ResourceDescription::Cidr(r.with_status(status))
}
ResourceDescription::Internet(_) => todo!(),
ResourceDescription::Internet(r) => {
crate::callbacks::ResourceDescription::Internet(r.with_status(status))
}
}
}
}

11
rust/connlib/shared/src/proptest.rs
View File

@@ -1,5 +1,8 @@
use crate::messages::{
client::{ResourceDescription, ResourceDescriptionCidr, ResourceDescriptionDns, Site, SiteId},
client::{
ResourceDescription, ResourceDescriptionCidr, ResourceDescriptionDns,
ResourceDescriptionInternet, Site, SiteId,
},
ClientId, GatewayId, RelayId, ResourceId,
};
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
@@ -72,6 +75,12 @@ pub fn cidr_resource(
})
}
pub fn internet_resource(
sites: impl Strategy<Value = Vec<Site>>,
) -> impl Strategy<Value = ResourceDescriptionInternet> {
(resource_id(), sites).prop_map(move |(id, sites)| ResourceDescriptionInternet { id, sites })
}
pub fn cidr_v4_resource(host_mask_bits: usize) -> impl Strategy<Value = ResourceDescriptionCidr> {
cidr_resource(
any_ip4_network(host_mask_bits),

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

File diff suppressed because one or more lines are too long

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

@@ -107,6 +107,9 @@ pub(crate) fn assert_icmp_packets_properties(
&mut mapping,
)
}
ResourceDst::Internet(resource_dst) => {
assert_destination_is_cdir_resource(gateway_received_request, resource_dst)
}
}
}
}

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

@@ -41,6 +41,7 @@ pub(crate) struct ReferenceState {
#[derive(Debug, Clone)]
pub(crate) enum ResourceDst {
Internet(IpAddr),
Cidr(IpAddr),
Dns(DomainName),
}
@@ -288,7 +289,9 @@ impl ReferenceStateMachine for ReferenceState {
client::ResourceDescription::Cidr(r) => {
client.cidr_resources.insert(r.address, r.clone());
}
client::ResourceDescription::Internet(_) => todo!("Unsupported"),
client::ResourceDescription::Internet(r) => {
client.internet_resource = Some(r.id)
}
});
}
Transition::DeactivateResource(id) => {
@@ -333,8 +336,20 @@ impl ReferenceStateMachine for ReferenceState {
.exec_mut(|client| client.expected_dns_handshakes.push_back(*query_id));
}
},
Transition::SendICMPPacketToNonResourceIp { .. } => {
// Packets to non-resources are dropped, no state change required.
Transition::SendICMPPacketToNonResourceIp {
src,
dst,
seq,
identifier,
} => {
state.client.exec_mut(|client| {
// If the Internet Resource is active, all packets are expected to be routed.
if client.internet_resource.is_some() {
client.on_icmp_packet_to_internet(*src, *dst, *seq, *identifier, |r| {
state.portal.gateway_for_resource(r).copied()
})
}
});
}
Transition::SendICMPPacketToCidrResource {
src,
@@ -447,10 +462,10 @@ impl ReferenceStateMachine for ReferenceState {
..
} => {
let ref_client = state.client.inner();
let Some(resource) = ref_client.cidr_resource_by_ip(*dst) else {
let Some(rid) = ref_client.cidr_resource_by_ip(*dst) else {
return false;
};
let Some(gateway) = state.portal.gateway_for_resource(resource.id) else {
let Some(gateway) = state.portal.gateway_for_resource(rid) else {
return false;
};

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

@@ -225,6 +225,8 @@ pub struct RefClient {
/// The upstream DNS resolvers configured in the portal.
pub(crate) upstream_dns_resolvers: Vec<DnsServer>,
pub(crate) internet_resource: Option<ResourceId>,
/// The CIDR resources the client is aware of.
#[derivative(Debug = "ignore")]
pub(crate) cidr_resources: IpNetworkTable<ResourceDescriptionCidr>,
@@ -239,6 +241,9 @@ pub struct RefClient {
#[derivative(Debug = "ignore")]
pub(crate) dns_records: BTreeMap<DomainName, HashSet<RecordType>>,
/// Whether we are connected to the gateway serving the Internet resource.
pub(crate) connected_internet_resources: bool,
/// The CIDR resources the client is connected to.
#[derivative(Debug = "ignore")]
pub(crate) connected_cidr_resources: HashSet<ResourceId>,
@@ -293,6 +298,43 @@ impl RefClient {
}
}
#[tracing::instrument(level = "debug", skip_all, fields(dst, resource))]
pub(crate) fn on_icmp_packet_to_internet(
&mut self,
src: IpAddr,
dst: IpAddr,
seq: u16,
identifier: u16,
gateway_by_resource: impl Fn(ResourceId) -> Option<GatewayId>,
) {
tracing::Span::current().record("dst", tracing::field::display(dst));
// Second, if we are not yet connected, check if we have a resource for this IP.
let Some(rid) = self.internet_resource else {
tracing::debug!("No internet resource");
return;
};
tracing::Span::current().record("resource", tracing::field::display(rid));
let Some(gateway) = gateway_by_resource(rid) else {
tracing::error!("No gateway for resource");
return;
};
if self.connected_internet_resources && self.is_tunnel_ip(src) {
tracing::debug!("Connected to Internet resource, expecting packet to be routed");
self.expected_icmp_handshakes
.entry(gateway)
.or_default()
.push_back((ResourceDst::Internet(dst), seq, identifier));
return;
}
// If we have a resource, the first packet will initiate a connection to the gateway.
tracing::debug!("Not connected to resource, expecting to trigger connection intent");
self.connected_internet_resources = true;
}
#[tracing::instrument(level = "debug", skip_all, fields(dst, resource))]
pub(crate) fn on_icmp_packet_to_cidr(
&mut self,
@@ -305,18 +347,18 @@ impl RefClient {
tracing::Span::current().record("dst", tracing::field::display(dst));
// Second, if we are not yet connected, check if we have a resource for this IP.
let Some(resource) = self.cidr_resource_by_ip(dst) else {
let Some(rid) = self.cidr_resource_by_ip(dst) else {
tracing::debug!("No resource corresponds to IP");
return;
};
tracing::Span::current().record("resource", tracing::field::display(resource.id));
tracing::Span::current().record("resource", tracing::field::display(rid));
let Some(gateway) = gateway_by_resource(resource.id) else {
let Some(gateway) = gateway_by_resource(rid) else {
tracing::error!("No gateway for resource");
return;
};
if self.is_connected_to_cidr(resource.id) && self.is_tunnel_ip(src) {
if self.is_connected_to_cidr(rid) && self.is_tunnel_ip(src) {
tracing::debug!("Connected to CIDR resource, expecting packet to be routed");
self.expected_icmp_handshakes
.entry(gateway)
@@ -327,7 +369,7 @@ impl RefClient {
// If we have a resource, the first packet will initiate a connection to the gateway.
tracing::debug!("Not connected to resource, expecting to trigger connection intent");
self.connected_cidr_resources.insert(resource.id);
self.connected_cidr_resources.insert(rid);
}
#[tracing::instrument(level = "debug", skip_all, fields(dst, resource))]
@@ -484,8 +526,8 @@ impl RefClient {
.collect()
}
pub(crate) fn cidr_resource_by_ip(&self, ip: IpAddr) -> Option<&ResourceDescriptionCidr> {
self.cidr_resources.longest_match(ip).map(|(_, r)| r)
pub(crate) fn cidr_resource_by_ip(&self, ip: IpAddr) -> Option<ResourceId> {
self.cidr_resources.longest_match(ip).map(|(_, r)| r.id)
}
pub(crate) fn resolved_ip4_for_non_resources(
@@ -541,7 +583,7 @@ impl RefClient {
return None;
}
self.cidr_resource_by_ip(dns_server).map(|r| r.id)
self.cidr_resource_by_ip(dns_server)
}
pub(crate) fn all_resource_ids(&self) -> Vec<ResourceId> {
@@ -556,7 +598,15 @@ impl RefClient {
return true;
}
self.cidr_resources.iter().any(|(_, r)| r.id == resource_id)
if self.cidr_resources.iter().any(|(_, r)| r.id == resource_id) {
return true;
}
if self.internet_resource.is_some_and(|r| r == resource_id) {
return true;
}
false
}
pub(crate) fn all_resources(&self) -> Vec<ResourceDescription> {
@@ -663,11 +713,13 @@ fn ref_client(
tunnel_ip6,
system_dns_resolvers,
upstream_dns_resolvers,
internet_resource: Default::default(),
cidr_resources: IpNetworkTable::new(),
dns_resources: Default::default(),
dns_records: Default::default(),
connected_cidr_resources: Default::default(),
connected_dns_resources: Default::default(),
connected_internet_resources: Default::default(),
expected_icmp_handshakes: Default::default(),
expected_dns_handshakes: Default::default(),
},

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

@@ -7,7 +7,10 @@ use super::{
use crate::client::{IPV4_RESOURCES, IPV6_RESOURCES};
use connlib_shared::{
messages::{
client::{ResourceDescriptionCidr, ResourceDescriptionDns, Site, SiteId},
client::{
ResourceDescriptionCidr, ResourceDescriptionDns, ResourceDescriptionInternet, Site,
SiteId,
},
DnsServer, GatewayId, RelayId,
},
proptest::{
@@ -123,19 +126,26 @@ pub(crate) fn gateways_and_portal() -> impl Strategy<
prop_oneof![
non_wildcard_dns_resource(any_site(sites.clone())),
star_wildcard_dns_resource(any_site(sites.clone())),
question_mark_wildcard_dns_resource(any_site(sites)),
question_mark_wildcard_dns_resource(any_site(sites.clone())),
],
1..5,
);
let internet_resource = internet_resource(any_site(sites));
let gateways =
collection::hash_map(gateway_id(), (ref_gateway_host(), gateway_site), 1..=3);
let gateway_selector = any::<sample::Selector>();
(gateways, cidr_resources, dns_resources, gateway_selector)
(
gateways,
cidr_resources,
dns_resources,
internet_resource,
gateway_selector,
)
})
.prop_flat_map(
|(gateways, cidr_resources, dns_resources, gateway_selector)| {
|(gateways, cidr_resources, dns_resources, internet_resource, gateway_selector)| {
let (gateways, gateways_by_site) = gateways.into_iter().fold(
(
BTreeMap::<GatewayId, _>::default(),
@@ -190,6 +200,7 @@ pub(crate) fn gateways_and_portal() -> impl Strategy<
gateway_selector,
cidr_resources,
dns_resources,
internet_resource,
);
(Just(gateways), Just(portal), dns_resource_records)
@@ -224,6 +235,12 @@ fn cidr_resource_outside_reserved_ranges(
.prop_filter("resource must not be in the documentation range because we use those for host addresses and DNS IPs", |r| !r.address.is_documentation())
}
fn internet_resource(
site: impl Strategy<Value = Site>,
) -> impl Strategy<Value = ResourceDescriptionInternet> {
connlib_shared::proptest::internet_resource(site.prop_map(|s| vec![s]))
}
fn non_wildcard_dns_resource(
site: impl Strategy<Value = Site>,
) -> impl Strategy<Value = ResourceDescriptionDns> {

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

@@ -3,6 +3,7 @@ use itertools::Itertools;
use proptest::sample::Selector;
use std::{
collections::{HashMap, HashSet},
iter,
net::IpAddr,
};
@@ -16,6 +17,7 @@ pub(crate) struct StubPortal {
sites_by_resource: HashMap<ResourceId, client::SiteId>,
cidr_resources: HashMap<ResourceId, client::ResourceDescriptionCidr>,
dns_resources: HashMap<ResourceId, client::ResourceDescriptionDns>,
internet_resource: client::ResourceDescriptionInternet,
#[derivative(Debug = "ignore")]
gateway_selector: Selector,
@@ -27,6 +29,7 @@ impl StubPortal {
gateway_selector: Selector,
cidr_resources: HashSet<client::ResourceDescriptionCidr>,
dns_resources: HashSet<client::ResourceDescriptionDns>,
internet_resource: client::ResourceDescriptionInternet,
) -> Self {
let cidr_resources = cidr_resources
.into_iter()
@@ -57,13 +60,23 @@ impl StubPortal {
.id,
)
});
let internet_site = iter::once((
internet_resource.id,
internet_resource
.sites
.iter()
.exactly_one()
.expect("only single-site resources")
.id,
));
Self {
gateways_by_site,
gateway_selector,
sites_by_resource: HashMap::from_iter(cidr_sites.chain(dns_sites)),
sites_by_resource: HashMap::from_iter(cidr_sites.chain(dns_sites).chain(internet_site)),
cidr_resources,
dns_resources,
internet_resource,
}
}
@@ -78,6 +91,10 @@ impl StubPortal {
.cloned()
.map(client::ResourceDescription::Dns),
)
// TODO: Enable once we actually implement the Internet resource
// .chain(iter::once(client::ResourceDescription::Internet(
// self.internet_resource.clone(),
// )))
.collect()
}
@@ -122,10 +139,16 @@ impl StubPortal {
addresses: resolved_ips.clone(),
})
});
let internet_resource = Some(gateway::ResourceDescription::Internet(
gateway::ResourceDescriptionInternet {
id: self.internet_resource.id,
},
));
cidr_resource
.or(dns_resource)
.expect("resource to be a known CIDR or DNS resource")
.or(internet_resource)
.expect("resource to be a known CIDR, DNS or Internet resource")
}
pub(crate) fn gateway_for_resource(&self, rid: ResourceId) -> Option<&GatewayId> {
@@ -139,7 +162,11 @@ impl StubPortal {
.get(&rid)
.and_then(|r| Some(r.sites.first()?.id));
let sid = cidr_site.or(dns_site)?;
let internet_site = (self.internet_resource.id == rid)
.then(|| Some(self.internet_resource.sites.first()?.id))
.flatten();
let sid = cidr_site.or(dns_site).or(internet_site)?;
let gateways = self.gateways_by_site.get(&sid)?;
let gid = self.gateway_selector.try_select(gateways)?;