From 12513976516d2d0fdc37c05e23eea694ee52ea85 Mon Sep 17 00:00:00 2001 From: Jamil Date: Wed, 3 Jan 2024 12:08:33 -0800 Subject: [PATCH] fix(ios/android): Pass device name and os version as overrides over connect (#3036) Fixes #3035 Fixes #3037 # Before Screenshot 2023-12-28 at 8 05 31 AM Screenshot 2023-12-28 at 6 12 30 PM # After Screenshot 2023-12-28 at 10 48 31 AM Screenshot 2023-12-28 at 6 29 37 PM --- .../firezone/android/tunnel/TunnelService.kt | 3 ++ .../firezone/android/tunnel/TunnelSession.kt | 2 + rust/connlib/clients/android/src/lib.rs | 13 ++++++ rust/connlib/clients/apple/src/lib.rs | 9 ++++ rust/connlib/clients/shared/src/lib.rs | 13 +++++- rust/connlib/shared/src/control.rs | 21 +++++++-- rust/connlib/shared/src/lib.rs | 10 ++++- rust/gateway/src/main.rs | 2 +- rust/linux-client/src/main.rs | 2 + .../src-tauri/src/client/gui.rs | 2 + .../FirezoneNetworkExtension/Adapter.swift | 45 ++++++++++++++++--- 11 files changed, 108 insertions(+), 14 deletions(-) diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt index d3250aa16..976b39cb0 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt @@ -9,6 +9,7 @@ import android.app.PendingIntent import android.content.Context import android.content.Intent import android.net.VpnService +import android.os.Build import android.system.OsConstants import android.util.Log import androidx.core.app.NotificationCompat @@ -185,6 +186,8 @@ class TunnelService : VpnService() { apiUrl = config.apiUrl, token = config.token, deviceId = deviceId(), + deviceName = Build.MODEL, + osVersion = Build.VERSION.RELEASE, logDir = getLogDir(), logFilter = config.logFilter, callback = callback, diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelSession.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelSession.kt index 3aa20008d..695ee9493 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelSession.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelSession.kt @@ -6,6 +6,8 @@ object TunnelSession { apiUrl: String, token: String, deviceId: String, + deviceName: String, + osVersion: String, logDir: String, logFilter: String, callback: Any, diff --git a/rust/connlib/clients/android/src/lib.rs b/rust/connlib/clients/android/src/lib.rs index aca468d9a..ac6734a13 100644 --- a/rust/connlib/clients/android/src/lib.rs +++ b/rust/connlib/clients/android/src/lib.rs @@ -382,11 +382,16 @@ macro_rules! string_from_jstring { }; } +// TODO: Refactor this when we refactor PhoenixChannel. +// See https://github.com/firezone/firezone/issues/2158 +#[allow(clippy::too_many_arguments)] fn connect( env: &mut JNIEnv, api_url: JString, token: JString, device_id: JString, + device_name: JString, + os_version: JString, log_dir: JString, log_filter: JString, callback_handler: GlobalRef, @@ -394,6 +399,8 @@ fn connect( let api_url = string_from_jstring!(env, api_url); let secret = SecretString::from(string_from_jstring!(env, token)); let device_id = string_from_jstring!(env, device_id); + let device_name = string_from_jstring!(env, device_name); + let os_version = string_from_jstring!(env, os_version); let log_dir = string_from_jstring!(env, log_dir); let log_filter = string_from_jstring!(env, log_filter); @@ -409,6 +416,8 @@ fn connect( api_url.as_str(), secret, device_id, + Some(device_name), + Some(os_version), callback_handler, Duration::from_secs(5 * 60), )?; @@ -427,6 +436,8 @@ pub unsafe extern "system" fn Java_dev_firezone_android_tunnel_TunnelSession_con api_url: JString, token: JString, device_id: JString, + device_name: JString, + os_version: JString, log_dir: JString, log_filter: JString, callback_handler: JObject, @@ -441,6 +452,8 @@ pub unsafe extern "system" fn Java_dev_firezone_android_tunnel_TunnelSession_con api_url, token, device_id, + device_name, + os_version, log_dir, log_filter, callback_handler, diff --git a/rust/connlib/clients/apple/src/lib.rs b/rust/connlib/clients/apple/src/lib.rs index 8511494e5..7bcf91fc3 100644 --- a/rust/connlib/clients/apple/src/lib.rs +++ b/rust/connlib/clients/apple/src/lib.rs @@ -24,6 +24,8 @@ mod ffi { api_url: String, token: String, device_id: String, + device_name_override: Option, + os_version_override: Option, log_dir: String, log_filter: String, callback_handler: CallbackHandler, @@ -174,10 +176,15 @@ fn init_logging(log_dir: PathBuf, log_filter: String) -> file_logger::Handle { } impl WrappedSession { + // TODO: Refactor this when we refactor PhoenixChannel. + // See https://github.com/firezone/firezone/issues/2158 + #[allow(clippy::too_many_arguments)] fn connect( api_url: String, token: String, device_id: String, + device_name_override: Option, + os_version_override: Option, log_dir: String, log_filter: String, callback_handler: ffi::CallbackHandler, @@ -188,6 +195,8 @@ impl WrappedSession { api_url.as_str(), secret, device_id, + device_name_override, + os_version_override, CallbackHandler { inner: Arc::new(callback_handler), handle: init_logging(log_dir.into(), log_filter), diff --git a/rust/connlib/clients/shared/src/lib.rs b/rust/connlib/clients/shared/src/lib.rs index df746ae2b..744536725 100644 --- a/rust/connlib/clients/shared/src/lib.rs +++ b/rust/connlib/clients/shared/src/lib.rs @@ -65,6 +65,8 @@ where api_url: impl TryInto, token: SecretString, device_id: String, + device_name_override: Option, + os_version_override: Option, callbacks: CB, max_partition_time: Duration, ) -> Result { @@ -111,6 +113,8 @@ where api_url.try_into().map_err(|_| Error::UriError)?, token, device_id, + device_name_override, + os_version_override, this.callbacks.clone(), max_partition_time, ); @@ -122,18 +126,23 @@ where Ok(this) } + // TODO: Refactor this when we refactor PhoenixChannel. + // See https://github.com/firezone/firezone/issues/2158 + #[allow(clippy::too_many_arguments)] fn connect_inner( runtime: &Runtime, runtime_stopper: tokio::sync::mpsc::Sender, api_url: Url, token: SecretString, device_id: String, + device_name_override: Option, + os_version_override: Option, callbacks: CallbackErrorFacade, max_partition_time: Duration, ) { runtime.spawn(async move { let (connect_url, private_key) = fatal_error!( - login_url(Mode::Client, api_url, token, device_id, None), + login_url(Mode::Client, api_url, token, device_id, device_name_override), runtime_stopper, &callbacks ); @@ -143,7 +152,7 @@ where // to force queue ordering. let (control_plane_sender, mut control_plane_receiver) = tokio::sync::mpsc::channel(1); - let mut connection = PhoenixChannel::<_, IngressMessages, ReplyMessages, Messages>::new(Secret::new(SecureUrl::from_url(connect_url)), move |msg, reference, topic| { + let mut connection = PhoenixChannel::<_, IngressMessages, ReplyMessages, Messages>::new(Secret::new(SecureUrl::from_url(connect_url)), os_version_override, move |msg, reference, topic| { let control_plane_sender = control_plane_sender.clone(); async move { tracing::trace!(?msg); diff --git a/rust/connlib/shared/src/control.rs b/rust/connlib/shared/src/control.rs index 9885dd15b..e4f4d640e 100644 --- a/rust/connlib/shared/src/control.rs +++ b/rust/connlib/shared/src/control.rs @@ -64,6 +64,7 @@ impl secrecy::Zeroize for SecureUrl { /// `await` it, it will block until you use `close` in a [PhoenixSender], the portal close the connection or something goes wrong. pub struct PhoenixChannel { secret_url: Secret, + os_version_override: Option, handler: F, sender: Sender, receiver: Receiver, @@ -71,7 +72,10 @@ pub struct PhoenixChannel { } // This is basically the same as tungstenite does but we add some new headers (namely user-agent) -fn make_request(secret_url: &Secret) -> Result { +fn make_request( + secret_url: &Secret, + os_version_override: Option, +) -> Result { use secrecy::ExposeSecret; let host = secret_url @@ -96,7 +100,7 @@ fn make_request(secret_url: &Secret) -> Result { .header("Upgrade", "websocket") .header("Sec-WebSocket-Version", "13") .header("Sec-WebSocket-Key", key) - .header("User-Agent", get_user_agent()) + .header("User-Agent", get_user_agent(os_version_override)) .uri(secret_url.expose_secret().inner.as_str()) .body(())?; Ok(req) @@ -127,7 +131,11 @@ where ) -> Result<()> { tracing::trace!("Trying to connect to portal..."); - let (ws_stream, _) = connect_async(make_request(&self.secret_url)?).await?; + let (ws_stream, _) = connect_async(make_request( + &self.secret_url, + self.os_version_override.clone(), + )?) + .await?; tracing::trace!("Successfully connected to portal"); @@ -269,13 +277,18 @@ where /// - `handler`: The handle that will be called for each received message. /// /// For more info see [struct-level docs][PhoenixChannel]. - pub fn new(secret_url: Secret, handler: F) -> Self { + pub fn new( + secret_url: Secret, + os_version_override: Option, + handler: F, + ) -> Self { let (sender, receiver) = channel(CHANNEL_SIZE); Self { sender, receiver, secret_url, + os_version_override, handler, _phantom: PhantomData, } diff --git a/rust/connlib/shared/src/lib.rs b/rust/connlib/shared/src/lib.rs index 9dc3678b4..27af9dd1a 100644 --- a/rust/connlib/shared/src/lib.rs +++ b/rust/connlib/shared/src/lib.rs @@ -75,15 +75,21 @@ pub enum Mode { Gateway, } -pub fn get_user_agent() -> String { +pub fn get_user_agent(os_version_override: Option) -> String { // Note: we could switch to sys-info and get the hostname // but we lose the arch // and neither of the libraries provide the kernel version. // so I rather keep os_info which seems like the most popular // and keep implementing things that we are missing on top let info = os_info::get(); + + // iOS returns "Unknown", but we already know we're on iOS here + #[cfg(target_os = "ios")] + let os_type = "iOS"; + #[cfg(not(target_os = "ios"))] let os_type = info.os_type(); - let os_version = info.version(); + + let os_version = os_version_override.unwrap_or(info.version().to_string()); let additional_info = additional_info(); let lib_version = VERSION; let lib_name = LIB_NAME; diff --git a/rust/gateway/src/main.rs b/rust/gateway/src/main.rs index f94927e28..ae7f491b6 100644 --- a/rust/gateway/src/main.rs +++ b/rust/gateway/src/main.rs @@ -173,7 +173,7 @@ async fn connect_to_portal( loop { let result = phoenix_channel::init::( Secret::new(SecureUrl::from_url(connect_url.clone())), - get_user_agent(), + get_user_agent(None), PHOENIX_TOPIC, (), ) diff --git a/rust/linux-client/src/main.rs b/rust/linux-client/src/main.rs index 5534b5659..e5057b715 100644 --- a/rust/linux-client/src/main.rs +++ b/rust/linux-client/src/main.rs @@ -15,6 +15,8 @@ fn main() -> Result<()> { cli.common.api_url, SecretString::from(cli.common.token), cli.firezone_id, + None, + None, CallbackHandler { handle }, cli.max_partition_time.into(), ) diff --git a/rust/windows-client/src-tauri/src/client/gui.rs b/rust/windows-client/src-tauri/src/client/gui.rs index 3e353d300..51eb1a469 100755 --- a/rust/windows-client/src-tauri/src/client/gui.rs +++ b/rust/windows-client/src-tauri/src/client/gui.rs @@ -395,6 +395,8 @@ impl Controller { self.advanced_settings.api_url.clone(), auth_info.token.clone(), self.device_id.clone(), + None, // TODO: Send device name here (windows computer name) + None, callback_handler.clone(), Duration::from_secs(5 * 60), )?; diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index 3dd6effc2..e775385a0 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -143,8 +143,15 @@ class Adapter { do { self.state = .startingTunnel( session: try WrappedSession.connect( - self.controlPlaneURLString, self.token, self.getDeviceId(), self.connlibLogFolderPath, - self.logFilter, self.callbackHandler), + self.controlPlaneURLString, + self.token, + self.getDeviceId(), + self.getDeviceName(), + self.getOSVersion(), + self.connlibLogFolderPath, + self.logFilter, + self.callbackHandler + ), onStarted: completionHandler ) } catch let error { @@ -219,9 +226,31 @@ class Adapter { } } -// MARK: Device unique identifiers +// MARK: Device metadata extension Adapter { + func getDeviceName() -> String? { + // Returns a generic device name on iOS 16 and higher + // See https://github.com/firezone/firezone/issues/3034 + #if os(iOS) + return UIDevice.current.name + #else + // Fallback to connlib's gethostname() + return nil + #endif + } + + func getOSVersion() -> String? { + // Returns the OS version + // See https://github.com/firezone/firezone/issues/3034 + #if os(iOS) + return UIDevice.current.systemVersion + #else + // Fallback to connlib's osinfo + return nil + #endif + } + // uuidString and copyMACAddress() *should* reliably return valid Strings, but if // for whatever reason they're nil, return a random UUID instead to prevent // upsert collisions in the portal. @@ -305,9 +334,15 @@ extension Adapter { do { self.state = .startingTunnel( session: try WrappedSession.connect( - controlPlaneURLString, token, self.getDeviceId(), self.connlibLogFolderPath, + controlPlaneURLString, + token, + self.getDeviceId(), + self.getDeviceName(), + self.getOSVersion(), + self.connlibLogFolderPath, self.logFilter, - self.callbackHandler), + self.callbackHandler + ), onStarted: { error in if let error = error { self.logger.error(