From 5ae06a7b8c8be8bcd9870f3b2114ba6d767cca16 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 18 Sep 2024 10:12:46 -0400 Subject: [PATCH] chore(gateway): remove domain response (breaks < 1.1.0 clients) (#6708) Prior to version 1.1.0, clients did not have an embedded DNS resolver and relied on the gateway for DNS resolution. In that design, the gateway responded with the IPs that the domain resolved to. Our next iteration of the control protocol (#6461) will decouple the details of how DNS works from the flow-authorization. As a result, we will need to be able to establish a flow for a DNS resource without knowing which concrete domain the client is going to access. Without a concrete domain, we cannot send anything back to these old clients, meaning we unfortunately have to break compatibility with < 1.1.0 clients as part of implementing the new control protocol. --- rust/connlib/clients/shared/src/eventloop.rs | 8 +- rust/connlib/shared/src/messages.rs | 7 -- rust/gateway/src/eventloop.rs | 80 +++++--------- rust/gateway/src/messages.rs | 105 +------------------ 4 files changed, 33 insertions(+), 167 deletions(-) diff --git a/rust/connlib/clients/shared/src/eventloop.rs b/rust/connlib/clients/shared/src/eventloop.rs index e514e2e33..8a4ded486 100644 --- a/rust/connlib/clients/shared/src/eventloop.rs +++ b/rust/connlib/clients/shared/src/eventloop.rs @@ -9,7 +9,7 @@ use crate::{ use anyhow::Result; use connlib_shared::messages::{ ClientPayload, ConnectionAccepted, GatewayResponse, RelaysPresence, RequestConnection, - ResourceAccepted, ResourceId, ReuseConnection, + ResourceId, ReuseConnection, }; use firezone_tunnel::ClientTunnel; use phoenix_channel::{ErrorReply, OutboundRequestId, PhoenixChannel}; @@ -294,12 +294,6 @@ where tracing::warn!("Failed to accept connection: {e}"); } } - ReplyMessages::Connect(Connect { - gateway_payload: GatewayResponse::ResourceAccepted(ResourceAccepted { .. }), - .. - }) => { - tracing::trace!("Connection response received, ignored as it's deprecated") - } ReplyMessages::ConnectionDetails(ConnectionDetails { gateway_id, resource_id, diff --git a/rust/connlib/shared/src/messages.rs b/rust/connlib/shared/src/messages.rs index b5bc26113..9d30587de 100644 --- a/rust/connlib/shared/src/messages.rs +++ b/rust/connlib/shared/src/messages.rs @@ -236,18 +236,11 @@ pub struct DomainResponse { #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] pub struct ConnectionAccepted { pub ice_parameters: Answer, - pub domain_response: Option, -} - -#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] -pub struct ResourceAccepted { - pub domain_response: DomainResponse, } #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] pub enum GatewayResponse { ConnectionAccepted(ConnectionAccepted), - ResourceAccepted(ResourceAccepted), } #[derive(Deserialize, Serialize, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/rust/gateway/src/eventloop.rs b/rust/gateway/src/eventloop.rs index 32362344d..ab08266a0 100644 --- a/rust/gateway/src/eventloop.rs +++ b/rust/gateway/src/eventloop.rs @@ -5,7 +5,7 @@ use crate::messages::{ use anyhow::Result; use boringtun::x25519::PublicKey; use connlib_shared::messages::{ - ClientId, ConnectionAccepted, Interface, RelaysPresence, ResourceAccepted, ResourceId, + ClientId, ConnectionAccepted, Interface, RelaysPresence, ResourceId, }; use connlib_shared::{messages::GatewayResponse, DomainName}; #[cfg(not(target_os = "windows"))] @@ -158,7 +158,7 @@ impl Eventloop { if self .resolve_tasks .try_push( - resolve(req.client.payload.domain.as_ref().map(|r| r.name())), + resolve(req.client.payload.domain.as_ref().map(|r| r.name.clone())), ResolveTrigger::RequestConnection(req), ) .is_err() @@ -173,7 +173,7 @@ impl Eventloop { if self .resolve_tasks .try_push( - resolve(req.payload.as_ref().map(|r| r.name())), + resolve(req.payload.as_ref().map(|r| r.name.clone())), ResolveTrigger::AllowAccess(req), ) .is_err() @@ -272,40 +272,30 @@ impl Eventloop { PublicKey::from(req.client.peer.public_key.0), ); - match self.tunnel.allow_access( + if let Err(e) = self.tunnel.allow_access( req.client.id, req.client.peer.ipv4, req.client.peer.ipv6, req.client.payload.domain.as_ref().map(|r| r.as_tuple()), req.expires_at, - req.resource.into_resolved(addresses.clone()), + req.resource.into_resolved(addresses), ) { - Ok(()) => { - self.portal.send( - PHOENIX_TOPIC, - EgressMessages::ConnectionReady(ConnectionReady { - reference: req.reference, - gateway_payload: GatewayResponse::ConnectionAccepted(ConnectionAccepted { - ice_parameters: answer, - domain_response: req.client.payload.domain.map(|r| { - connlib_shared::messages::DomainResponse { - domain: r.name(), - address: addresses, - } - }), - }), - }), - ); + let client = req.client.id; - // TODO: If outbound request fails, cleanup connection. - } - Err(e) => { - let client = req.client.id; - - self.tunnel.cleanup_connection(&client); - tracing::debug!(%client, "Connection request failed: {e:#}"); - } + self.tunnel.cleanup_connection(&client); + tracing::debug!(%client, "Connection request failed: {e:#}"); + return; } + + self.portal.send( + PHOENIX_TOPIC, + EgressMessages::ConnectionReady(ConnectionReady { + reference: req.reference, + gateway_payload: GatewayResponse::ConnectionAccepted(ConnectionAccepted { + ice_parameters: answer, + }), + }), + ); } pub fn allow_access(&mut self, result: Result, Timeout>, req: AllowAccess) { @@ -313,30 +303,16 @@ impl Eventloop { .inspect_err(|e| tracing::debug!(client = %req.client_id, reference = %req.reference, "DNS resolution timed out as part of allow access request: {e}")) .unwrap_or_default(); - if let (Ok(()), Some(resolve_request)) = ( - self.tunnel.allow_access( - req.client_id, - req.client_ipv4, - req.client_ipv6, - req.payload.as_ref().map(|r| r.as_tuple()), - req.expires_at, - req.resource.into_resolved(addresses.clone()), - ), - req.payload, + if let Err(e) = self.tunnel.allow_access( + req.client_id, + req.client_ipv4, + req.client_ipv6, + req.payload.as_ref().map(|r| r.as_tuple()), + req.expires_at, + req.resource.into_resolved(addresses), ) { - self.portal.send( - PHOENIX_TOPIC, - EgressMessages::ConnectionReady(ConnectionReady { - reference: req.reference, - gateway_payload: GatewayResponse::ResourceAccepted(ResourceAccepted { - domain_response: connlib_shared::messages::DomainResponse { - domain: resolve_request.name(), - address: addresses, - }, - }), - }), - ); - } + tracing::warn!(client = %req.client_id, "Allow access request failed: {e:#}"); + }; } pub fn refresh_translation( diff --git a/rust/gateway/src/messages.rs b/rust/gateway/src/messages.rs index f42d605cc..68266c5ba 100644 --- a/rust/gateway/src/messages.rs +++ b/rust/gateway/src/messages.rs @@ -57,32 +57,14 @@ pub struct RemoveResource { } #[derive(Debug, Deserialize, Clone, PartialEq, Eq)] -#[serde(untagged)] -pub enum ResolveRequest { - ReturnResponse(DomainName), - MapResponse { - name: DomainName, - proxy_ips: Vec, - }, +pub struct ResolveRequest { + pub name: DomainName, + pub proxy_ips: Vec, } impl ResolveRequest { - pub fn name(&self) -> DomainName { - match self { - ResolveRequest::ReturnResponse(name) => name.clone(), - ResolveRequest::MapResponse { name, .. } => name.clone(), - } - } - - // TODO: remove me pub fn as_tuple(&self) -> (DomainName, Vec) { - match self { - ResolveRequest::ReturnResponse(name) => (name.clone(), Vec::new()), - ResolveRequest::MapResponse { - name, - proxy_ips: proxy, - } => (name.clone(), proxy.clone()), - } + (self.name.clone(), self.proxy_ips.clone()) } } @@ -313,85 +295,6 @@ mod test { let _: PhoenixMessage = serde_json::from_str(message).unwrap(); } - #[test] - fn request_connection_with_payload_old_format() { - let message = r#"{ - "event": "request_connection", - "ref": null, - "topic": "gateway", - "payload": { - "client": { - "id": "2e5c0210-3ac0-49cd-bfc9-8005046291de", - "peer": { - "ipv6": "fd00:2021:1111::4:4616", - "public_key": "zHtdIFPDm8QQkqjbmAc1r8O1WegviA6UeUTP6rpminA=", - "ipv4": "100.87.247.184", - "persistent_keepalive": 25, - "preshared_key": "BzPiNE9qszKczZcZzGsyieLYeJ2EQfkfdibls/l3beM=" - }, - "payload": { - "domain": "download.httpbin", - "ice_parameters": { - "password": "MMceouYA5jGIPkxbvIiLvD", - "username": "aYaH" - } - } - }, - "resource": { - "id": "619fbe83-bc95-4635-9a08-68da9a944c88", - "name": "?.httpbin", - "type": "dns", - "address": "?.httpbin", - "filters": [ - { - "protocol": "tcp", - "port_range_end": 80, - "port_range_start": 80 - }, - { - "protocol": "tcp", - "port_range_end": 433, - "port_range_start": 433 - }, - { - "protocol": "udp", - "port_range_end": 53, - "port_range_start": 53 - }, - { - "protocol": "icmp" - } - ] - }, - "ref": "SFMyNTY.g2gDbQAAAVhnMmdFV0hjVllYQnBRR0Z3YVM1amJIVnpkR1Z5TG14dlkyRnNBQUFENndBQUFBQm1hakdpYUFWWWR4VmhjR2xBWVhCcExtTnNkWE4wWlhJdWJHOWpZV3dBQUFQZ0FBQUFBR1pxTWFKM0owVnNhWGhwY2k1UWFHOWxibWw0TGxOdlkydGxkQzVXTVM1S1UwOU9VMlZ5YVdGc2FYcGxjbTBBQUFBR1kyeHBaVzUwWVFSaEFHMEFBQUFrTmpFNVptSmxPRE10WW1NNU5TMDBOak0xTFRsaE1EZ3ROamhrWVRsaE9UUTBZemc0YkFBQUFBRm9BbTBBQUFBTGRISmhZMlZ3WVhKbGJuUnRBQUFBTnpBd0xUZzROMlptTUdKaU1EZGhOakU1TkdOa01tTTRNamsxWkRGaE1tSXlabU15TFRnMU9USXpZV1kzTVRZNVlUQmlPR1F0TURGcW4GAKnP0g6QAWIAAVGA.MBccK7A6wR4EA1ZkKnGlHzAnh-tRitZ2d97q_IvzoD8", - "expires_at": 1718663242, - "actor": { - "id": "0f4a5f16-3c59-47c9-a7c1-31de7a51b26c" - }, - "relays": [ - { - "id": "75542064-e5f7-491b-b054-5350b4f34963", - "type": "turn", - "addr": "172.28.0.101:3478", - "username": "1719445215:D8RIljXHIIYGn88dBjUIFw", - "password": "AipiIetIXo33EnV36U+nET8lAtJXr+iSwwFU5VOfF5k", - "expires_at": 1719445215 - }, - { - "id": "649cd79f-2536-4f9d-906b-d20b1c5d3ac3", - "type": "turn", - "addr": "172.28.0.201:3478", - "username": "1719445215:hpx751e3Wt-Mg9kv7yv0mg", - "password": "D6A4ytn/U8xAvfOE1l3G/rNlduZD1gq21BFXhITkjpA", - "expires_at": 1719445215 - } - ], - "flow_id": "b944e68a-c936-4a81-bd8d-88c45efdcb2c" - } -}"#; - let _: PhoenixMessage = serde_json::from_str(message).unwrap(); - } - #[test] fn invalidate_ice_candidates_message() { let msg = r#"{"event":"invalidate_ice_candidates","ref":null,"topic":"gateway","payload":{"candidates":["candidate:7854631899965427361 1 udp 1694498559 172.28.0.100 47717 typ srflx"],"client_id":"2b1524e6-239e-4570-bc73-70a188e12101"}}"#;