Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(connlib): deterministically route packets in case of overlap #5082

Merged
merged 12 commits into from
May 25, 2024
Merged

fix(connlib): deterministically route packets in case of overlap #5082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7445524
Check for definite non-resources before looking up peer
thomaseizinger May 22, 2024
11955e0
De-duplicate code
thomaseizinger May 22, 2024
2905ae7
Always resolve gateway to use by checking longest_match of resources
thomaseizinger May 22, 2024
b81f95e
Generalize connection intents
thomaseizinger May 22, 2024
a83b80e
Remove stray newline
thomaseizinger May 22, 2024
a06de19
Pass on domain from deferred DNS response
thomaseizinger May 22, 2024
65d52a1
Reduce code duplication and simplify call sites
thomaseizinger May 22, 2024
ff0a54a
Merge branch 'main' into fix/connlib/pick-gateway-based-on-resource-ip
thomaseizinger May 24, 2024
3fabaca
Merge branch 'main' into fix/connlib/pick-gateway-based-on-resource-ip
thomaseizinger May 24, 2024
ad4df1c
Fix compile error
thomaseizinger May 24, 2024
d0b3579
Merge branch 'main' into fix/connlib/pick-gateway-based-on-resource-ip
thomaseizinger May 24, 2024
ba25239
Optimise mapping of assigned IPs to go from IP to `DnsResource`
thomaseizinger May 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions rust/connlib/tunnel/proptest-regressions/tests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ cc a0b8c281bd6af69051cbbd75b7b8b4536e34bc653c93cff023d7389b04a93ac1 # shrinks to
cc 1691bc65aeb05519f0b8260d401fd679a1d4d17e33b34b9fdd2b5b46e0b7085f # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 28695, tv_nsec: 35917448 }, utc_now: 2024-05-21T01:16:24.705071359Z, client: SimNode { id: ClientId(370da223-692a-9056-a112-89ebbb2dce03), state: [110, 16, 21, 47, 10, 169, 122, 21, 171, 78, 228, 176, 10, 219, 13, 227, 33, 27, 218, 49, 19, 174, 93, 113, 127, 105, 160, 28, 196, 217, 219, 110], ip4_socket: None, ip6_socket: Some([::ffff:127.0.0.1]:32927), tunnel_ip4: 100.70.5.224, tunnel_ip6: ::ffff:143.211.81.183, span: Span { name: "client", level: Level(Error), target: "firezone_tunnel::tests", id: Id(57005), module_path: "firezone_tunnel::tests", line: 1054, file: "connlib/tunnel/src/tests.rs" } }, gateway: SimNode { id: GatewayId(432fc066-88ba-6d6b-c66f-edf484f0cedd), state: [27, 255, 104, 33, 198, 118, 202, 78, 135, 92, 163, 134, 244, 39, 41, 133, 96, 10, 106, 15, 100, 244, 123, 241, 244, 67, 4, 168, 199, 185, 138, 149], ip4_socket: Some(197.157.239.81:1428), ip6_socket: Some([4179:ad2c:abd:cbb4:3520:1b25:47d9:4b0c]:56849), tunnel_ip4: 100.91.72.11, tunnel_ip6: ::ffff:0.0.0.0, span: Span { name: "gateway", level: Level(Error), target: "firezone_tunnel::tests", id: Id(57005), module_path: "firezone_tunnel::tests", line: 1055, file: "connlib/tunnel/src/tests.rs" } }, relay: SimRelay { id: RelayId(533d43e8-650b-64d0-be61-f496af032fbe), state: 10959706197027164454, ip_stack: Dual { ip4: 182.38.26.219, ip6: fc1:f22f:37c6:61fc:45e3:a9ce:7b2e:ee90 }, allocations: {}, span: Span { name: "relay", level: Level(Error), target: "firezone_tunnel::tests", id: Id(57005), module_path: "firezone_tunnel::tests", line: 1037, file: "connlib/tunnel/src/tests.rs" }, .. }, client_cidr_resources: {}, connected_resources: {}, gateway_received_icmp_packets: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 32 }), name: "aaaa", address_description: "sodhxc", sites: [Site { name: "zbcrxpj", id: SiteId(73d9c9bd-76fc-976b-3406-79978de91c27) }, Site { name: "ngcarw", id: SiteId(6dd336e2-4da4-9420-2830-030d4001f6f8) }, Site { name: "sokappy", id: SiteId(1c66a22d-4ec3-3af6-042b-8deffcc96a7a) }, Site { name: "ddeckw", id: SiteId(05877cc0-abce-5b88-4a8b-cec56677b75e) }, Site { name: "dgxjp", id: SiteId(49fcf247-0717-c475-9528-4239296f40e4) }, Site { name: "tgcvhldh", id: SiteId(f355b3f1-6985-d4d4-c1bf-5cc8f1e2d7c3) }] }), AddCidrResource(ResourceDescriptionCidr { id: ResourceId(f121092f-0c97-c814-dbfc-73a2705a777a), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 32 }), name: "fqhkj", address_description: "fsgcu", sites: [Site { name: "shnps", id: SiteId(d03d3893-f788-bcb2-65d7-91dd13a6052e) }, Site { name: "ghhk", id: SiteId(b21516a7-b15e-48cb-0178-e768224a5d46) }, Site { name: "xwwk", id: SiteId(2b309495-0bb0-5dcb-4ba3-9697c19e4d94) }, Site { name: "lhgzqanaqc", id: SiteId(d903ce14-3485-2c2e-dba7-3d783fe5568e) }, Site { name: "dtalsidlh", id: SiteId(8dfcce4e-6496-3408-2e6e-02c91b64bf05) }, Site { name: "nzpurg", id: SiteId(d4382a12-0853-2a6a-e53f-9d15fff31b58) }, Site { name: "tjltskkgf", id: SiteId(c425b8b0-a4e9-5e45-4b1e-102200a02851) }, Site { name: "sbsvmcbktg", id: SiteId(9024bb4f-1edb-8cbd-ecc0-49f1df0cc0e2) }, Site { name: "ugvid", id: SiteId(d3bd6108-fafd-fe4a-4aa4-c8c068cbcbb8) }, Site { name: "hylnofbb", id: SiteId(6af88c22-637e-46ab-47b4-1b88657cf8b2) }] })], None)
cc 4855625684cfe850e8b965097038ac8fed979cd21abd5284eb25e48136fcbb3e
cc bbc56c960f18a6115b3c114f2c5280a813fa2a12f21af383ad4154863fb944bc
cc 71165314a457a9f39928d882ffcc2ae02aedc2a35ce55b387c990b1bc5022a38 # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 18548, tv_nsec: 387490499 }, utc_now: 2024-05-22T05:00:47.375132041Z, client: SimNode { id: ClientId(a89380c1-5379-5a2b-c5da-47fdeb956f75), state: PrivateKey("d348d4c5fa2ad05dfc686f289fb7084a16a095e208000844bc0ef7566b25addb"), ip4_socket: Some(127.0.0.1:41241), ip6_socket: Some([6e67:9b39:4a3f:a896:9e02:dc56:353e:65b3]:58740), tunnel_ip4: 100.111.145.90, tunnel_ip6: ::ffff:63.59.65.126 }, gateway: SimNode { id: GatewayId(d680e99a-31a7-9d8b-1052-6b85edfe42bc), state: PrivateKey("d39211badb6305b5b583574ee303b6a112b66311c38b4c22d68d69cb8d917bae"), ip4_socket: Some(82.215.208.98:18290), ip6_socket: Some([f122:b9a6:b3fb:bf35:319d:f921:d911:d012]:57327), tunnel_ip4: 100.81.19.26, tunnel_ip6: 3bfa:312f:c388:91b9:f6ce:95a6:a07:8784 }, relay: SimRelay { id: RelayId(a908052e-8823-7892-b1e2-e2c64deab06a), ip_stack: Dual { ip4: 75.190.170.204, ip6: ::ffff:0.235.25.126 }, allocations: {} }, client_cidr_resources: {}, connected_resources: {}, gateway_received_icmp_packets: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 28 }), name: "aaaa", address_description: "bmuf", sites: [Site { name: "zvkyrxzs", id: SiteId(67566938-ef2b-b442-64e0-f0f6fd05634d) }, Site { name: "hvzhiban", id: SiteId(4b7a5400-0fb2-2e42-444a-bc94fbaeed2e) }, Site { name: "fjmyjoh", id: SiteId(13340020-2717-880f-ba45-c823725c2234) }, Site { name: "fgrhxuvi", id: SiteId(8c260658-d7fb-c49c-c9f0-769cd8ced648) }] }), SendICMPPacketToIp4Resource { r_idx: Index(0) }, AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000001), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 28 }), name: "aaaa", address_description: "clkriftb", sites: [Site { name: "fgfg", id: SiteId(6b8d5470-3636-4879-2a32-94790962e6fd) }] }), SendICMPPacketToIp4Resource { r_idx: Index(11791212112709260679) }], None)
cc c9b812a8526dad4b00e366ea6b6d60054a4dd64c58849f1244f6fb2ba9d3686b # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 20170, tv_nsec: 301610515 }, utc_now: 2024-05-22T05:27:49.998877399Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: Some(127.0.0.1:1), ip6_socket: None, tunnel_ip4: 100.64.0.1, tunnel_ip6: ::ffff:0.0.0.0 }, gateway: SimNode { id: GatewayId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000001"), ip4_socket: None, ip6_socket: Some([::ffff:0.0.0.0]:12607), tunnel_ip4: 100.66.167.161, tunnel_ip6: ::ffff:155.93.248.155 }, relay: SimRelay { id: RelayId(c4e3332a-6f75-3a59-3ed8-15fbf26de134), ip_stack: Dual { ip4: 115.115.97.13, ip6: ::ffff:196.17.152.128 }, allocations: {} }, client_cidr_resources: {}, connected_resources: {}, gateway_received_icmp_packets: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 127.0.0.1, netmask: 32 }), name: "aaaa", address_description: "mgnx", sites: [Site { name: "njmblcn", id: SiteId(b6f995cd-2c8e-a35d-117f-1e7affe3bc7b) }, Site { name: "angxwaw", id: SiteId(c1772dc0-60c6-ebfc-bcb5-978b288bbabf) }, Site { name: "vblitlg", id: SiteId(f41e2663-0254-e054-2353-0855dabea706) }, Site { name: "plcfnotvz", id: SiteId(72f3a8cc-8015-bf96-adcd-b40df5f25383) }, Site { name: "lflx", id: SiteId(214a47c8-5ce9-3e33-4738-da1a36d79d68) }, Site { name: "shfamjx", id: SiteId(1f2c00bf-3437-9ac9-c9d3-e66ebf3a1558) }, Site { name: "offgdto", id: SiteId(edb8c103-1dda-01f7-d648-47ccfad1eb97) }, Site { name: "gzss", id: SiteId(e2058be3-2fe7-5459-b8b3-eaee2c434828) }, Site { name: "ovougquk", id: SiteId(1410820a-7236-ac4d-e649-f276f3418e51) }] }), SendICMPPacketToIp4Resource { r_idx: Index(0) }, AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000001), address: V4(Ipv4Network { network_address: 127.0.0.1, netmask: 32 }), name: "aaaa", address_description: "llgt", sites: [Site { name: "jmcxcphun", id: SiteId(190023c3-f08f-0b3c-ca68-ac5d45127341) }, Site { name: "mxofshkeo", id: SiteId(c39f0553-11d6-ddb0-1bc2-57e841e038fc) }, Site { name: "kaasxisn", id: SiteId(7c30df2b-95f9-0a6c-ebc7-92fd0a131376) }, Site { name: "hhitinvik", id: SiteId(39c252f6-4848-e5b7-1fe3-a4b886258e9a) }] }), SendICMPPacketToIp4Resource { r_idx: Index(0) }, SendICMPPacketToRandomIp { dst: 127.0.0.1 }], None)
160 changes: 74 additions & 86 deletions rust/connlib/tunnel/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,7 @@ pub struct ClientState {
buffered_dns_queries: VecDeque<DnsQuery<'static>>,

/// The (internal) IPs we have assigned for a certain DNS resource.
///
/// We assign one internal IP per externally resolved IP.
dns_resources_internal_ips: HashMap<DnsResource, HashSet<IpAddr>>,
dns_resources_internal_ips: HashMap<IpAddr, DnsResource>,
/// DNS queries we can only answer once we have connected to the resource.
///
/// See [`dns::ResolveStrategy`] for details.
Expand Down Expand Up @@ -417,6 +415,7 @@ impl ClientState {
self.node.public_key()
}

#[tracing::instrument(level = "trace", skip_all, fields(dest))]
pub(crate) fn encapsulate<'s>(
&'s mut self,
packet: MutableIpPacket<'_>,
Expand All @@ -430,19 +429,39 @@ impl ClientState {
Err(non_dns_packet) => non_dns_packet,
};

let Some(peer) = self.peers.peer_by_ip_mut(dest) else {
self.on_connection_intent_ip(dest, now);
tracing::Span::current().record("dest", tracing::field::display(dest));

if is_definitely_not_a_resource(dest) {
return None;
}

let Some(resource) = self.get_resource_by_destination(dest) else {
tracing::trace!("Unknown resource");
return None;
};

let Some(peer) = self.peer_by_resource_mut(resource) else {
// If the resource are intending to is a DNS resource (i.e. we resolved an IP for it), look up the original name.
let domain = self
.dns_resources_internal_ips
.values()
.find_map(|r| (r.id == resource).then_some(r.address.clone()));

self.on_connection_intent_to_resource(resource, domain, now);
return None;
};

let packet = peer.transform_tun_to_network(packet);
let gateway_id = peer.id();

let transmit = self
.node
.encapsulate(peer.id(), packet.as_immutable(), now)
.encapsulate(gateway_id, packet.as_immutable(), now)
.inspect_err(|e| tracing::debug!("Failed to encapsulate: {e}"))
.ok()??;

tracing::trace!("Encapsulated packet");

Some(transmit)
}

Expand Down Expand Up @@ -550,17 +569,8 @@ impl ClientState {
.get(&resource_id)
.ok_or(Error::UnknownResource)?;

let domain = self.get_awaiting_connection(&resource_id)?.domain.clone();

if self.is_connected_to(resource_id, &domain) {
return Err(Error::UnexpectedConnectionDetails);
}

let awaiting_connection = self
.awaiting_connection
.get(&resource_id)
.ok_or(Error::UnexpectedConnectionDetails)?
.clone();
let awaiting_connection = self.get_awaiting_connection(&resource_id)?.clone();
let domain = awaiting_connection.domain.clone();

self.resources_gateways.insert(resource_id, gateway_id);

Expand Down Expand Up @@ -601,7 +611,7 @@ impl ClientState {
username: offer.credentials.username,
password: offer.credentials.password,
},
domain: awaiting_connection.domain,
domain,
},
}));
}
Expand Down Expand Up @@ -656,8 +666,10 @@ impl ClientState {
})
.collect();

self.dns_resources_internal_ips
.insert(resource_description.clone(), addrs.clone());
for addr in addrs.clone() {
self.dns_resources_internal_ips
.insert(addr, resource_description.clone());
}

send_dns_answer(self, Rtype::AAAA, &resource_description, &addrs);
send_dns_answer(self, Rtype::A, &resource_description, &addrs);
Expand Down Expand Up @@ -700,10 +712,14 @@ impl ClientState {

Ok(None)
}
Some(dns::ResolveStrategy::DeferredResponse(resource)) => {
self.on_connection_intent_dns(&resource.0, now);
Some(dns::ResolveStrategy::DeferredResponse((resource, r_type))) => {
self.on_connection_intent_to_resource(
resource.id,
Some(resource.address.clone()),
now,
);
self.deferred_dns_queries
.insert(resource, packet.as_immutable().to_owned());
.insert((resource, r_type), packet.as_immutable().to_owned());

Ok(None)
}
Expand All @@ -728,37 +744,7 @@ impl ClientState {
self.resources_gateways.remove(&resource);
}

#[tracing::instrument(level = "debug", skip_all, fields(resource_address = %resource.address, resource_id = %resource.id))]
fn on_connection_intent_dns(&mut self, resource: &DnsResource, now: Instant) {
self.on_connection_intent_to_resource(resource.id, Some(resource.address.clone()), now)
}

#[tracing::instrument(level = "debug", skip_all, fields(resource_ip = %destination, resource_id))]
fn on_connection_intent_ip(&mut self, destination: IpAddr, now: Instant) {
if is_definitely_not_a_resource(destination) {
return;
}

let Some(resource_id) = self.get_cidr_resource_by_destination(destination) else {
if let Some(resource) = self
.dns_resources_internal_ips
.iter()
.find_map(|(r, i)| i.contains(&destination).then_some(r))
.cloned()
{
self.on_connection_intent_dns(&resource, now);
}

tracing::trace!("Unknown resource");

return;
};

tracing::Span::current().record("resource_id", tracing::field::display(&resource_id));

self.on_connection_intent_to_resource(resource_id, None, now)
}

#[tracing::instrument(level = "debug", skip_all, fields(%resource, ?domain))]
fn on_connection_intent_to_resource(
&mut self,
resource: ResourceId,
Expand Down Expand Up @@ -818,15 +804,6 @@ impl ClientState {
self.dns_mapping.clone()
}

fn is_connected_to(&self, resource: ResourceId, domain: &Option<DomainName>) -> bool {
let Some(resource) = self.resource_ids.get(&resource) else {
return false;
};

let ips = self.get_resource_ip(resource, domain);
ips.iter().any(|ip| self.peers.exact_match(*ip).is_some())
}

fn get_resource_ip(
&self,
resource: &ResourceDescription,
Expand All @@ -839,11 +816,7 @@ impl ClientState {
};

let description = DnsResource::from_description(dns_resource, domain.clone());
self.dns_resources_internal_ips
.get(&description)
.cloned()
.unwrap_or_default()
.into_iter()
dns::ips_of_resource(&self.dns_resources_internal_ips, &description)
.map(Into::into)
.collect()
}
Expand All @@ -854,7 +827,7 @@ impl ClientState {
pub fn cleanup_connected_gateway(&mut self, gateway_id: &GatewayId) {
self.update_site_status_by_gateway(gateway_id, Status::Unknown);
self.peers.remove(gateway_id);
self.dns_resources_internal_ips.retain(|resource, _| {
self.dns_resources_internal_ips.retain(|_, resource| {
!self
.resources_gateways
.get(&resource.id)
Expand All @@ -871,10 +844,18 @@ impl ClientState {
.chain(self.dns_mapping.left_values().copied().map(Into::into))
}

fn get_cidr_resource_by_destination(&self, destination: IpAddr) -> Option<ResourceId> {
self.cidr_resources
fn get_resource_by_destination(&self, destination: IpAddr) -> Option<ResourceId> {
let maybe_cidr_resource_id = self
.cidr_resources
.longest_match(destination)
.map(|(_, res)| res.id)
.map(|(_, r)| r.id);

let maybe_dns_resource_id = self
.dns_resources_internal_ips
.get(&destination)
.map(|r| r.id);

maybe_cidr_resource_id.or(maybe_dns_resource_id)
}

#[must_use]
Expand Down Expand Up @@ -912,18 +893,15 @@ impl ClientState {

self.peers.iter_mut().for_each(|p| p.expire_dns_track());

for resource in self.dns_resources_internal_ips.keys() {
let Some(gateway_id) = self.resources_gateways.get(&resource.id) else {
for resource in self.dns_resources_internal_ips.values() {
let Some(peer) = self.peer_by_resource(resource.id) else {
// filter inactive connections
continue;
};
// filter inactive connections
if self.peers.get(gateway_id).is_none() {
continue;
}

connections.push(ReuseConnection {
resource_id: resource.id,
gateway_id: *gateway_id,
gateway_id: peer.id(),
payload: Some(resource.address.clone()),
});
}
Expand Down Expand Up @@ -1054,21 +1032,17 @@ impl ClientState {
fn remove_resources(&mut self, ids: &[ResourceId]) {
for id in ids {
self.awaiting_connection.remove(id);
self.dns_resources_internal_ips.retain(|r, _| r.id != *id);
self.dns_resources_internal_ips.retain(|_, r| r.id != *id);
self.dns_resources.retain(|_, r| r.id != *id);
self.cidr_resources.retain(|_, r| r.id != *id);
self.deferred_dns_queries.retain(|(r, _), _| r.id != *id);

self.resource_ids.remove(id);

let Some(gateway_id) = self.resources_gateways.remove(id) else {
tracing::debug!("No gateway associated with resource");
continue;
};

let Some(peer) = self.peers.get_mut(&gateway_id) else {
let Some(peer) = self.peer_by_resource_mut(*id) else {
continue;
};
let gateway_id = peer.id();

// First we remove the id from all allowed ips
for (network, resources) in peer
Expand Down Expand Up @@ -1146,6 +1120,20 @@ impl ClientState {
) {
self.node.update_relays(to_remove, &to_add, now);
}

fn peer_by_resource(&self, resource: ResourceId) -> Option<&GatewayOnClient> {
let gateway_id = self.resources_gateways.get(&resource)?;
let peer = self.peers.get(gateway_id)?;

Some(peer)
}

fn peer_by_resource_mut(&mut self, resource: ResourceId) -> Option<&mut GatewayOnClient> {
let gateway_id = self.resources_gateways.get(&resource)?;
let peer = self.peers.get_mut(gateway_id)?;

Some(peer)
}
}

fn effective_dns_servers(
Expand Down