From bc3a5d9e54a848185318e374a1981d42937451e6 Mon Sep 17 00:00:00 2001 From: Francesca Lovebloom Date: Mon, 7 Aug 2023 17:03:28 -0700 Subject: [PATCH] connlib: JNI bridge (#1848) The biggest internal change is that all the methods on `Callbacks` (on the Rust side!) return a `Result` now, so errors from the bridge or even the client callbacks will be handled. @roop there's nothing for you to review here, but note: - the `bool` return values you've asked about in the past are gone now - the route string for `onAddRoute`/`onRemoveRoute` no longer has the extra quotes (it's no longer JSON) --------- Signed-off-by: Francesca Lovebloom Co-authored-by: Jamil --- rust/Cargo.lock | 15 +- rust/connlib/clients/android/Cargo.toml | 21 +- rust/connlib/clients/android/src/lib.rs | 329 +++++++++++++++--- rust/connlib/clients/apple/Cargo.toml | 1 + rust/connlib/clients/apple/src/lib.rs | 60 ++-- rust/connlib/clients/headless/Cargo.toml | 1 + rust/connlib/clients/headless/src/main.rs | 29 +- rust/connlib/gateway/Cargo.toml | 1 + rust/connlib/gateway/src/main.rs | 29 +- rust/connlib/libs/client/src/control.rs | 15 +- rust/connlib/libs/common/src/error.rs | 10 + rust/connlib/libs/common/src/lib.rs | 2 +- rust/connlib/libs/common/src/session.rs | 215 +++++++++--- rust/connlib/libs/gateway/src/control.rs | 4 +- rust/connlib/libs/tunnel/Cargo.toml | 1 - .../libs/tunnel/src/control_protocol.rs | 12 +- rust/connlib/libs/tunnel/src/lib.rs | 36 +- rust/connlib/libs/tunnel/src/peer.rs | 2 +- .../libs/tunnel/src/resource_sender.rs | 2 +- rust/connlib/libs/tunnel/src/tun_android.rs | 13 +- rust/connlib/libs/tunnel/src/tun_darwin.rs | 16 +- rust/connlib/libs/tunnel/src/tun_linux.rs | 8 +- rust/connlib/libs/tunnel/src/tun_win.rs | 8 +- 23 files changed, 618 insertions(+), 212 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index bad3901c4..96901ca3f 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -752,8 +752,11 @@ version = "0.1.6" dependencies = [ "android_logger", "firezone-client-connlib", + "ip_network", "jni 0.21.1", "log", + "serde_json", + "thiserror", ] [[package]] @@ -763,6 +766,7 @@ dependencies = [ "anyhow", "diva", "firezone-client-connlib", + "ip_network", "libc", "serde_json", "swift-bridge", @@ -1161,7 +1165,6 @@ dependencies = [ "rand_core", "rtnetlink", "serde", - "serde_json", "thiserror", "tokio", "tracing", @@ -1279,6 +1282,7 @@ version = "0.1.0" dependencies = [ "anyhow", "firezone-gateway-connlib", + "ip_network", "tracing", "tracing-subscriber", "url", @@ -1345,6 +1349,7 @@ dependencies = [ "anyhow", "clap", "firezone-client-connlib", + "ip_network", "tracing", "tracing-subscriber", "url", @@ -3150,18 +3155,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.40" +version = "1.0.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "978c9a314bd8dc99be594bc3c175faaa9794be04a5a5e153caba6915336cebac" +checksum = "611040a08a0439f8248d1990b111c95baa9c704c805fa1f62104b39655fd7f90" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.40" +version = "1.0.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9456a42c5b0d803c8cd86e73dd7cc9edd429499f37a3550d286d5e86720569f" +checksum = "090198534930841fab3a5d1bb637cde49e339654e606195f8d9c76eeb081dc96" dependencies = [ "proc-macro2", "quote", diff --git a/rust/connlib/clients/android/Cargo.toml b/rust/connlib/clients/android/Cargo.toml index 09619287f..536c46562 100644 --- a/rust/connlib/clients/android/Cargo.toml +++ b/rust/connlib/clients/android/Cargo.toml @@ -3,16 +3,19 @@ name = "connlib-android" version = "0.1.6" edition = "2021" -[features] -mock = ["firezone-client-connlib/mock"] - -[dependencies] -jni = { version = "0.21.1", features = ["invocation"] } -firezone-client-connlib = { path = "../../libs/client" } -log = "0.4" -android_logger = "0.13" - [lib] name = "connlib" crate-type = ["cdylib"] doc = false + +[features] +mock = ["firezone-client-connlib/mock"] + +[dependencies] +android_logger = "0.13" +firezone-client-connlib = { path = "../../libs/client" } +jni = { version = "0.21.1", features = ["invocation"] } +ip_network = "0.4" +log = "0.4" +serde_json = "1" +thiserror = "1" diff --git a/rust/connlib/clients/android/src/lib.rs b/rust/connlib/clients/android/src/lib.rs index 80e4093cc..6e563834c 100644 --- a/rust/connlib/clients/android/src/lib.rs +++ b/rust/connlib/clients/android/src/lib.rs @@ -4,11 +4,14 @@ // ecosystem, so it's used here for consistency. use firezone_client_connlib::{Callbacks, Error, ResourceDescription, Session}; +use ip_network::IpNetwork; use jni::{ objects::{JClass, JObject, JString, JValue}, - JNIEnv, + strings::JNIString, + JNIEnv, JavaVM, }; use std::net::{Ipv4Addr, Ipv6Addr}; +use thiserror::Error; /// This should be called once after the library is loaded by the system. #[allow(non_snake_case)] @@ -25,76 +28,297 @@ pub extern "system" fn Java_dev_firezone_connlib_Logger_init(_: JNIEnv, _: JClas ) } -#[derive(Clone)] -pub struct CallbackHandler; +pub struct CallbackHandler { + vm: JavaVM, + callback_handler: JObject<'static>, +} + +impl Clone for CallbackHandler { + fn clone(&self) -> Self { + // This is essentially a `memcpy` to bypass redundant checks from + // doing `as_raw` -> `from_raw`/etc; both of these fields are just + // dumb pointers but the wrappers don't implement `Clone`. + // + // SAFETY: `self` is guaranteed to be valid and `Self` is POD. + unsafe { std::ptr::read(self) } + } +} + +#[derive(Debug, Error)] +pub enum CallbackError { + #[error("Failed to attach current thread as daemon: {0}")] + AttachCurrentThreadFailed(#[source] jni::errors::Error), + #[error("Failed to serialize JSON: {0}")] + SerializeFailed(#[from] serde_json::Error), + #[error("Failed to create string `{name}`: {source}")] + NewStringFailed { + name: &'static str, + source: jni::errors::Error, + }, + #[error("Failed to call method `{name}`: {source}")] + CallMethodFailed { + name: &'static str, + source: jni::errors::Error, + }, +} + +impl CallbackHandler { + fn env( + &self, + f: impl FnOnce(JNIEnv) -> Result, + ) -> Result { + self.vm + .attach_current_thread_as_daemon() + .map_err(CallbackError::AttachCurrentThreadFailed) + .and_then(f) + } +} + +fn call_method( + env: &mut JNIEnv, + this: &JObject, + name: &'static str, + sig: &str, + args: &[JValue], +) -> Result<(), CallbackError> { + env.call_method(this, name, sig, args) + .map(|val| log::trace!("`{name}` returned `{val:?}`")) + .map_err(|source| CallbackError::CallMethodFailed { name, source }) +} impl Callbacks for CallbackHandler { + type Error = CallbackError; + fn on_set_interface_config( &self, - _tunnel_address_v4: Ipv4Addr, - _tunnel_address_v6: Ipv6Addr, - _dns_address: Ipv4Addr, - ) { - todo!() + tunnel_address_v4: Ipv4Addr, + tunnel_address_v6: Ipv6Addr, + dns_address: Ipv4Addr, + ) -> Result<(), Self::Error> { + self.env(|mut env| { + let tunnel_address_v4 = + env.new_string(tunnel_address_v4.to_string()) + .map_err(|source| CallbackError::NewStringFailed { + name: "tunnel_address_v4", + source, + })?; + let tunnel_address_v6 = + env.new_string(tunnel_address_v6.to_string()) + .map_err(|source| CallbackError::NewStringFailed { + name: "tunnel_address_v6", + source, + })?; + let dns_address = env.new_string(dns_address.to_string()).map_err(|source| { + CallbackError::NewStringFailed { + name: "dns_address", + source, + } + })?; + // TODO: Don't hardcode this string here! + let dns_fallback_strategy = env.new_string("upstream_resolver").map_err(|source| { + CallbackError::NewStringFailed { + name: "dns_fallback_strategy", + source, + } + })?; + call_method( + &mut env, + &self.callback_handler, + "onSetInterfaceConfig", + "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)", + &[ + JValue::from(&tunnel_address_v4), + JValue::from(&tunnel_address_v6), + JValue::from(&dns_address), + JValue::from(&dns_fallback_strategy), + ], + ) + }) } - fn on_tunnel_ready(&self) { - todo!() + fn on_tunnel_ready(&self) -> Result<(), Self::Error> { + self.env(|mut env| { + call_method(&mut env, &self.callback_handler, "onTunnelReady", "()", &[]) + }) } - fn on_add_route(&self, _route: String) { - todo!() + fn on_add_route(&self, route: IpNetwork) -> Result<(), Self::Error> { + self.env(|mut env| { + let route = env.new_string(route.to_string()).map_err(|source| { + CallbackError::NewStringFailed { + name: "route", + source, + } + })?; + call_method( + &mut env, + &self.callback_handler, + "onAddRoute", + "(Ljava/lang/String;)", + &[JValue::from(&route)], + ) + }) } - fn on_remove_route(&self, _route: String) { - todo!() + fn on_remove_route(&self, route: IpNetwork) -> Result<(), Self::Error> { + self.env(|mut env| { + let route = env.new_string(route.to_string()).map_err(|source| { + CallbackError::NewStringFailed { + name: "route", + source, + } + })?; + call_method( + &mut env, + &self.callback_handler, + "onRemoveRoute", + "(Ljava/lang/String;)", + &[JValue::from(&route)], + ) + }) } - fn on_update_resources(&self, _resource_list: Vec) { - todo!() + fn on_update_resources( + &self, + resource_list: Vec, + ) -> Result<(), Self::Error> { + self.env(|mut env| { + let resource_list = env + .new_string(serde_json::to_string(&resource_list)?) + .map_err(|source| CallbackError::NewStringFailed { + name: "resource_list", + source, + })?; + call_method( + &mut env, + &self.callback_handler, + "onUpdateResources", + "(Ljava/lang/String;)", + &[JValue::from(&resource_list)], + ) + }) } - fn on_disconnect(&self, _error: Option<&Error>) { - todo!() + fn on_disconnect(&self, error: Option<&Error>) -> Result<(), Self::Error> { + self.env(|mut env| { + let error = env + .new_string(serde_json::to_string(&error.map(ToString::to_string))?) + .map_err(|source| CallbackError::NewStringFailed { + name: "error", + source, + })?; + call_method( + &mut env, + &self.callback_handler, + "onDisconnect", + "(Ljava/lang/String;)", + &[JValue::from(&error)], + ) + }) } - fn on_error(&self, _error: &Error) { - todo!() + fn on_error(&self, error: &Error) -> Result<(), Self::Error> { + self.env(|mut env| { + let error = env.new_string(error.to_string()).map_err(|source| { + CallbackError::NewStringFailed { + name: "error", + source, + } + })?; + call_method( + &mut env, + &self.callback_handler, + "onError", + "(Ljava/lang/String;)", + &[JValue::from(&error)], + ) + }) } } +fn throw(env: &mut JNIEnv, class: &str, msg: impl Into) { + if let Err(err) = env.throw_new(class, msg) { + // We can't panic, since unwinding across the FFI boundary is UB... + log::error!("failed to throw Java exception: {err}"); + } +} + +fn catch_and_throw R, R>(env: &mut JNIEnv, f: F) -> Option { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(env))) + .map_err(|info| { + log::error!("catching Rust panic"); + throw( + env, + "java/lang/Exception", + match info.downcast_ref::<&str>() { + Some(msg) => format!("Rust panicked: {msg}"), + None => "Rust panicked with no message".to_owned(), + }, + ); + }) + .ok() +} + +#[derive(Debug, Error)] +enum ConnectError { + #[error("Failed to access {name:?}: {source}")] + StringInvalid { + name: &'static str, + source: jni::errors::Error, + }, + #[error("Failed to get Java VM: {0}")] + GetJavaVmFailed(#[source] jni::errors::Error), + #[error(transparent)] + ConnectFailed(#[from] Error), +} + +fn connect( + env: &mut JNIEnv, + portal_url: JString, + portal_token: JString, + callback_handler: JObject<'static>, +) -> Result, ConnectError> { + let portal_url = String::from(env.get_string(&portal_url).map_err(|source| { + ConnectError::StringInvalid { + name: "portal_url", + source, + } + })?); + let portal_token = env + .get_string(&portal_token) + .map_err(|source| ConnectError::StringInvalid { + name: "portal_token", + source, + })? + .into(); + let callback_handler = CallbackHandler { + vm: env.get_java_vm().map_err(ConnectError::GetJavaVmFailed)?, + callback_handler, + }; + Session::connect(portal_url.as_str(), portal_token, callback_handler.clone()) + .map_err(Into::into) +} + /// # Safety /// Pointers must be valid #[allow(non_snake_case)] #[no_mangle] pub unsafe extern "system" fn Java_dev_firezone_connlib_Session_connect( - mut env: JNIEnv, + mut env: JNIEnv<'static>, _class: JClass, portal_url: JString, portal_token: JString, - callback: JObject, + callback_handler: JObject<'static>, ) -> *const Session { - let portal_url: String = env.get_string(&portal_url).unwrap().into(); - let portal_token: String = env.get_string(&portal_token).unwrap().into(); - - let session = Box::new( - Session::connect(portal_url.as_str(), portal_token, CallbackHandler).expect("TODO!"), - ); - - // TODO: Get actual IPs returned from portal based on this device - let tunnelAddressesJSON = "[{\"tunnel_ipv4\": \"100.100.1.1\", \"tunnel_ipv6\": \"fd00:0222:2011:1111:6def:1001:fe67:0012\"}]"; - let tunnel_addresses = env.new_string(tunnelAddressesJSON).unwrap(); - match env.call_method( - callback, - "onTunnelReady", - "(Ljava/lang/String;)Z", - &[JValue::from(&tunnel_addresses)], - ) { - Ok(res) => log::trace!("`onTunnelReady` returned `{res:?}`"), - Err(err) => log::error!("Failed to call `onTunnelReady`: {err}"), + if let Some(result) = catch_and_throw(&mut env, |env| { + connect(env, portal_url, portal_token, callback_handler) + }) { + match result { + Ok(session) => return Box::into_raw(Box::new(session)), + Err(err) => throw(&mut env, "java/lang/Exception", err.to_string()), + } } - - Box::into_raw(session) + std::ptr::null() } /// # Safety @@ -102,14 +326,19 @@ pub unsafe extern "system" fn Java_dev_firezone_connlib_Session_connect( #[allow(non_snake_case)] #[no_mangle] pub unsafe extern "system" fn Java_dev_firezone_connlib_Session_disconnect( - _env: JNIEnv, + mut env: JNIEnv, _: JClass, - session_ptr: *mut Session, -) -> bool { - if session_ptr.is_null() { - return false; + session: *mut Session, +) { + if let Some(session) = session.as_mut() { + catch_and_throw(&mut env, |_| { + session.disconnect(None); + }); + } else { + throw( + &mut env, + "java/lang/NullPointerException", + "Cannot disconnect because \"session\" is null", + ); } - - let session = unsafe { &mut *session_ptr }; - session.disconnect(None) } diff --git a/rust/connlib/clients/apple/Cargo.toml b/rust/connlib/clients/apple/Cargo.toml index 77a0a8fbb..f11e9cb09 100644 --- a/rust/connlib/clients/apple/Cargo.toml +++ b/rust/connlib/clients/apple/Cargo.toml @@ -13,6 +13,7 @@ swift-bridge-build = "0.1.52" walkdir = "2.3.3" [dependencies] +ip_network = "0.4" libc = "0.2" swift-bridge = { workspace = true } firezone-client-connlib = { path = "../../libs/client" } diff --git a/rust/connlib/clients/apple/src/lib.rs b/rust/connlib/clients/apple/src/lib.rs index f2735114b..570c37884 100644 --- a/rust/connlib/clients/apple/src/lib.rs +++ b/rust/connlib/clients/apple/src/lib.rs @@ -3,6 +3,7 @@ #![allow(improper_ctypes, non_camel_case_types)] use firezone_client_connlib::{Callbacks, Error, ResourceDescription, Session}; +use ip_network::IpNetwork; use std::{ net::{Ipv4Addr, Ipv6Addr}, sync::Arc, @@ -21,12 +22,12 @@ mod ffi { ) -> Result; #[swift_bridge(swift_name = "bumpSockets")] - fn bump_sockets(&self) -> bool; + fn bump_sockets(&self); #[swift_bridge(swift_name = "disableSomeRoamingForBrokenMobileSemantics")] - fn disable_some_roaming_for_broken_mobile_semantics(&self) -> bool; + fn disable_some_roaming_for_broken_mobile_semantics(&self); - fn disconnect(&mut self) -> bool; + fn disconnect(&mut self); } extern "Swift" { @@ -78,45 +79,57 @@ unsafe impl Sync for ffi::CallbackHandler {} pub struct CallbackHandler(Arc); impl Callbacks for CallbackHandler { + type Error = std::convert::Infallible; + fn on_set_interface_config( &self, tunnel_address_v4: Ipv4Addr, tunnel_address_v6: Ipv6Addr, dns_address: Ipv4Addr, - ) { + ) -> Result<(), Self::Error> { self.0.on_set_interface_config( tunnel_address_v4.to_string(), tunnel_address_v6.to_string(), dns_address.to_string(), - ) + ); + Ok(()) } - fn on_tunnel_ready(&self) { - self.0.on_tunnel_ready() + fn on_tunnel_ready(&self) -> Result<(), Self::Error> { + self.0.on_tunnel_ready(); + Ok(()) } - fn on_add_route(&self, route: String) { - self.0.on_add_route(route) + fn on_add_route(&self, route: IpNetwork) -> Result<(), Self::Error> { + self.0.on_add_route(route.to_string()); + Ok(()) } - fn on_remove_route(&self, route: String) { - self.0.on_remove_route(route) + fn on_remove_route(&self, route: IpNetwork) -> Result<(), Self::Error> { + self.0.on_remove_route(route.to_string()); + Ok(()) } - fn on_update_resources(&self, resource_list: Vec) { + fn on_update_resources( + &self, + resource_list: Vec, + ) -> Result<(), Self::Error> { self.0.on_update_resources( serde_json::to_string(&resource_list) .expect("developer error: failed to serialize resource list"), - ) + ); + Ok(()) } - fn on_disconnect(&self, error: Option<&Error>) { + fn on_disconnect(&self, error: Option<&Error>) -> Result<(), Self::Error> { self.0 - .on_disconnect(error.map(ToString::to_string).unwrap_or_default()) + .on_disconnect(error.map(ToString::to_string).unwrap_or_default()); + Ok(()) } - fn on_error(&self, error: &Error) { - self.0.on_error(error.to_string()) + fn on_error(&self, error: &Error) -> Result<(), Self::Error> { + self.0.on_error(error.to_string()); + Ok(()) } } @@ -148,17 +161,16 @@ impl WrappedSession { .map_err(|err| err.to_string()) } - fn bump_sockets(&self) -> bool { - // TODO: See https://github.com/WireGuard/wireguard-apple/blob/2fec12a6e1f6e3460b6ee483aa00ad29cddadab1/Sources/WireGuardKitGo/api-apple.go#L177 - todo!() + fn bump_sockets(&self) { + self.session.bump_sockets() } - fn disable_some_roaming_for_broken_mobile_semantics(&self) -> bool { - // TODO: See https://github.com/WireGuard/wireguard-apple/blob/2fec12a6e1f6e3460b6ee483aa00ad29cddadab1/Sources/WireGuardKitGo/api-apple.go#LL197C6-L197C50 - todo!() + fn disable_some_roaming_for_broken_mobile_semantics(&self) { + self.session + .disable_some_roaming_for_broken_mobile_semantics() } - fn disconnect(&mut self) -> bool { + fn disconnect(&mut self) { self.session.disconnect(None) } } diff --git a/rust/connlib/clients/headless/Cargo.toml b/rust/connlib/clients/headless/Cargo.toml index c90370320..bfed7f466 100644 --- a/rust/connlib/clients/headless/Cargo.toml +++ b/rust/connlib/clients/headless/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] firezone-client-connlib = { path = "../../libs/client" } +ip_network = "0.4" url = { version = "2.3.1", default-features = false } tracing-subscriber = { version = "0.3" } tracing = { version = "0.1" } diff --git a/rust/connlib/clients/headless/src/main.rs b/rust/connlib/clients/headless/src/main.rs index afc3efe57..4cc1573ce 100644 --- a/rust/connlib/clients/headless/src/main.rs +++ b/rust/connlib/clients/headless/src/main.rs @@ -1,5 +1,6 @@ use anyhow::{Context, Result}; use clap::Parser; +use ip_network::IpNetwork; use std::{ net::{Ipv4Addr, Ipv6Addr}, str::FromStr, @@ -12,32 +13,46 @@ use url::Url; pub struct CallbackHandler; impl Callbacks for CallbackHandler { + type Error = std::convert::Infallible; + fn on_set_interface_config( &self, _tunnel_address_v4: Ipv4Addr, _tunnel_address_v6: Ipv6Addr, _dns_address: Ipv4Addr, - ) { + ) -> Result<(), Self::Error> { + Ok(()) } - fn on_tunnel_ready(&self) { + fn on_tunnel_ready(&self) -> Result<(), Self::Error> { tracing::trace!("Tunnel connected"); + Ok(()) } - fn on_add_route(&self, _route: String) {} + fn on_add_route(&self, _route: IpNetwork) -> Result<(), Self::Error> { + Ok(()) + } - fn on_remove_route(&self, _route: String) {} + fn on_remove_route(&self, _route: IpNetwork) -> Result<(), Self::Error> { + Ok(()) + } - fn on_update_resources(&self, resource_list: Vec) { + fn on_update_resources( + &self, + resource_list: Vec, + ) -> Result<(), Self::Error> { tracing::trace!("Resources updated, current list: {resource_list:?}"); + Ok(()) } - fn on_disconnect(&self, error: Option<&Error>) { + fn on_disconnect(&self, error: Option<&Error>) -> Result<(), Self::Error> { tracing::trace!("Tunnel disconnected: {error:?}"); + Ok(()) } - fn on_error(&self, error: &Error) { + fn on_error(&self, error: &Error) -> Result<(), Self::Error> { tracing::warn!("Encountered recoverable error: {error}"); + Ok(()) } } diff --git a/rust/connlib/gateway/Cargo.toml b/rust/connlib/gateway/Cargo.toml index eb9e07c6c..f610d3d93 100644 --- a/rust/connlib/gateway/Cargo.toml +++ b/rust/connlib/gateway/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] firezone-gateway-connlib = { path = "../libs/gateway" } +ip_network = "0.4" url = { version = "2.3.1", default-features = false } tracing-subscriber = { version = "0.3" } tracing = { version = "0.1" } diff --git a/rust/connlib/gateway/src/main.rs b/rust/connlib/gateway/src/main.rs index fe2a86ac4..1a2fc6568 100644 --- a/rust/connlib/gateway/src/main.rs +++ b/rust/connlib/gateway/src/main.rs @@ -1,4 +1,5 @@ use anyhow::{Context, Result}; +use ip_network::IpNetwork; use std::{ net::{Ipv4Addr, Ipv6Addr}, str::FromStr, @@ -11,32 +12,46 @@ use url::Url; pub struct CallbackHandler; impl Callbacks for CallbackHandler { + type Error = std::convert::Infallible; + fn on_set_interface_config( &self, _tunnel_address_v4: Ipv4Addr, _tunnel_address_v6: Ipv6Addr, _dns_address: Ipv4Addr, - ) { + ) -> Result<(), Self::Error> { + Ok(()) } - fn on_tunnel_ready(&self) { + fn on_tunnel_ready(&self) -> Result<(), Self::Error> { tracing::trace!("Tunnel connected with address"); + Ok(()) } - fn on_add_route(&self, _route: String) {} + fn on_add_route(&self, _route: IpNetwork) -> Result<(), Self::Error> { + Ok(()) + } - fn on_remove_route(&self, _route: String) {} + fn on_remove_route(&self, _route: IpNetwork) -> Result<(), Self::Error> { + Ok(()) + } - fn on_update_resources(&self, resource_list: Vec) { + fn on_update_resources( + &self, + resource_list: Vec, + ) -> Result<(), Self::Error> { tracing::trace!("Resources updated, current list: {resource_list:?}"); + Ok(()) } - fn on_disconnect(&self, error: Option<&Error>) { + fn on_disconnect(&self, error: Option<&Error>) -> Result<(), Self::Error> { tracing::trace!("Tunnel disconnected: {error:?}"); + Ok(()) } - fn on_error(&self, error: &Error) { + fn on_error(&self, error: &Error) -> Result<(), Self::Error> { tracing::warn!("Encountered recoverable error: {error}"); + Ok(()) } } diff --git a/rust/connlib/libs/client/src/control.rs b/rust/connlib/libs/client/src/control.rs index 0a081e11b..85780098f 100644 --- a/rust/connlib/libs/client/src/control.rs +++ b/rust/connlib/libs/client/src/control.rs @@ -100,7 +100,7 @@ impl ControlPlane { ) .await { - self.tunnel.callbacks().on_error(&e); + let _ = self.tunnel.callbacks().on_error(&e); } } @@ -142,12 +142,12 @@ impl ControlPlane { .await { tunnel.cleanup_connection(resource_id); - tunnel.callbacks().on_error(&err); + let _ = tunnel.callbacks().on_error(&err); } } Err(err) => { tunnel.cleanup_connection(resource_id); - tunnel.callbacks().on_error(&err); + let _ = tunnel.callbacks().on_error(&err); } } }); @@ -175,7 +175,7 @@ impl ControlPlane { tracing::error!( "An offline error came back with a reference to a non-valid resource id" ); - self.tunnel.callbacks().on_error(&Error::ControlProtocolError); + let _ = self.tunnel.callbacks().on_error(&Error::ControlProtocolError); return; }; // TODO: Rate limit the number of attempts of getting the relays before just trying a local network connection @@ -183,9 +183,10 @@ impl ControlPlane { } None => { tracing::error!( - "An offline portal error came without a reference that originated the error" - ); - self.tunnel + "An offline portal error came without a reference that originated the error" + ); + let _ = self + .tunnel .callbacks() .on_error(&Error::ControlProtocolError); } diff --git a/rust/connlib/libs/common/src/error.rs b/rust/connlib/libs/common/src/error.rs index 1df212db0..798c92c06 100644 --- a/rust/connlib/libs/common/src/error.rs +++ b/rust/connlib/libs/common/src/error.rs @@ -60,6 +60,16 @@ pub enum ConnlibError { /// Error when reading system's interface #[error("Error while reading system's interface")] IfaceRead(std::io::Error), + #[error("`on_set_interface_config` failed: {0}")] + OnSetInterfaceConfigFailed(String), + #[error("`on_tunnel_ready` failed: {0}")] + OnTunnelReadyFailed(String), + #[error("`on_add_route` failed: {0}")] + OnAddRouteFailed(String), + #[error("`on_remove_route` failed: {0}")] + OnRemoveRouteFailed(String), + #[error("`on_update_resources` failed: {0}")] + OnUpdateResourcesFailed(String), /// Glob for errors without a type. #[error("Other error: {0}")] Other(&'static str), diff --git a/rust/connlib/libs/common/src/lib.rs b/rust/connlib/libs/common/src/lib.rs index ba2bdafd0..6d0b1b7c7 100644 --- a/rust/connlib/libs/common/src/lib.rs +++ b/rust/connlib/libs/common/src/lib.rs @@ -13,7 +13,7 @@ pub mod messages; pub use error::ConnlibError as Error; pub use error::Result; -pub use session::{Callbacks, ControlSession, Session, DNS_SENTINEL}; +pub use session::{CallbackErrorFacade, Callbacks, ControlSession, Session, DNS_SENTINEL}; const VERSION: &str = env!("CARGO_PKG_VERSION"); const LIB_NAME: &str = "connlib"; diff --git a/rust/connlib/libs/common/src/session.rs b/rust/connlib/libs/common/src/session.rs index b8f52e147..cd78411e0 100644 --- a/rust/connlib/libs/common/src/session.rs +++ b/rust/connlib/libs/common/src/session.rs @@ -1,12 +1,16 @@ use async_trait::async_trait; use backoff::{backoff::Backoff, ExponentialBackoffBuilder}; use boringtun::x25519::{PublicKey, StaticSecret}; +use ip_network::IpNetwork; use parking_lot::Mutex; use rand::{distributions::Alphanumeric, thread_rng, Rng}; use rand_core::OsRng; use std::{ + error::Error as StdError, + fmt::{Debug, Display}, marker::PhantomData, net::{Ipv4Addr, Ipv6Addr}, + result::Result as StdResult, sync::Arc, time::Duration, }; @@ -46,34 +50,123 @@ pub trait ControlSession { /// A session is created using [Session::connect], then to stop a session we use [Session::disconnect]. pub struct Session { runtime: Arc>>, - callbacks: CB, + callbacks: CallbackErrorFacade, _phantom: PhantomData<(T, U, V, R, M)>, } /// Traits that will be used by connlib to callback the client upper layers. pub trait Callbacks: Clone + Send + Sync { + /// Error returned when a callback fails. + type Error: Debug + Display + StdError; + /// Called when the tunnel address is set. fn on_set_interface_config( &self, tunnel_address_v4: Ipv4Addr, tunnel_address_v6: Ipv6Addr, dns_address: Ipv4Addr, - ); + ) -> StdResult<(), Self::Error>; /// Called when the tunnel is connected. - fn on_tunnel_ready(&self); + fn on_tunnel_ready(&self) -> StdResult<(), Self::Error>; /// Called when when a route is added. - fn on_add_route(&self, route: String); + fn on_add_route(&self, route: IpNetwork) -> StdResult<(), Self::Error>; /// Called when when a route is removed. - fn on_remove_route(&self, route: String); + fn on_remove_route(&self, route: IpNetwork) -> StdResult<(), Self::Error>; /// Called when the resource list changes. - fn on_update_resources(&self, resource_list: Vec); + fn on_update_resources( + &self, + resource_list: Vec, + ) -> StdResult<(), Self::Error>; /// Called when the tunnel is disconnected. /// /// If the tunnel disconnected due to a fatal error, `error` is the error /// that caused the disconnect. - fn on_disconnect(&self, error: Option<&Error>); + fn on_disconnect(&self, error: Option<&Error>) -> StdResult<(), Self::Error>; /// Called when there's a recoverable error. - fn on_error(&self, error: &Error); + fn on_error(&self, error: &Error) -> StdResult<(), Self::Error>; +} + +#[derive(Clone)] +pub struct CallbackErrorFacade(pub CB); + +impl Callbacks for CallbackErrorFacade { + type Error = Error; + + fn on_set_interface_config( + &self, + tunnel_address_v4: Ipv4Addr, + tunnel_address_v6: Ipv6Addr, + dns_address: Ipv4Addr, + ) -> Result<()> { + let result = self + .0 + .on_set_interface_config(tunnel_address_v4, tunnel_address_v6, dns_address) + .map_err(|err| Error::OnSetInterfaceConfigFailed(err.to_string())); + if let Err(err) = result.as_ref() { + tracing::error!("{err}"); + } + result + } + + fn on_tunnel_ready(&self) -> Result<()> { + let result = self + .0 + .on_tunnel_ready() + .map_err(|err| Error::OnTunnelReadyFailed(err.to_string())); + if let Err(err) = result.as_ref() { + tracing::error!("{err}"); + } + result + } + + fn on_add_route(&self, route: IpNetwork) -> Result<()> { + let result = self + .0 + .on_add_route(route) + .map_err(|err| Error::OnAddRouteFailed(err.to_string())); + if let Err(err) = result.as_ref() { + tracing::error!("{err}"); + } + result + } + + fn on_remove_route(&self, route: IpNetwork) -> Result<()> { + let result = self + .0 + .on_remove_route(route) + .map_err(|err| Error::OnRemoveRouteFailed(err.to_string())); + if let Err(err) = result.as_ref() { + tracing::error!("{err}"); + } + result + } + + fn on_update_resources(&self, resource_list: Vec) -> Result<()> { + let result = self + .0 + .on_update_resources(resource_list) + .map_err(|err| Error::OnUpdateResourcesFailed(err.to_string())); + if let Err(err) = result.as_ref() { + tracing::error!("{err}"); + } + result + } + + fn on_disconnect(&self, error: Option<&Error>) -> Result<()> { + if let Err(err) = self.0.on_disconnect(error) { + tracing::error!("`on_disconnect` failed: {err}"); + } + // There's nothing we can really do if `on_disconnect` fails. + Ok(()) + } + + fn on_error(&self, error: &Error) -> Result<()> { + if let Err(err) = self.0.on_error(error) { + tracing::error!("`on_error` failed: {err}"); + } + // There's nothing we really want to do if `on_error` fails. + Ok(()) + } } macro_rules! fatal_error { @@ -128,6 +221,7 @@ where // Big question here however is how do we get the result? We could block here await the result and spawn a new task. // but then platforms should know that this function is blocking. + let callbacks = CallbackErrorFacade(callbacks); let this = Self { runtime: Mutex::new(Some( tokio::runtime::Builder::new_multi_thread() @@ -155,7 +249,7 @@ where } if cfg!(feature = "mock") { - Self::connect_mock(this.callbacks.clone()); + Self::connect_mock(Arc::clone(&this.runtime), this.callbacks.clone()); } else { Self::connect_inner( Arc::clone(&this.runtime), @@ -172,7 +266,7 @@ where runtime: Arc>>, portal_url: Url, token: String, - callbacks: CB, + callbacks: CallbackErrorFacade, ) { let runtime_disconnector = Arc::clone(&runtime); runtime.lock().as_ref().unwrap().spawn(async move { @@ -206,7 +300,7 @@ where let topic = T::socket_path().to_string(); let internal_sender = connection.sender_with_topic(topic.clone()); fatal_error!( - T::start(private_key, control_plane_receiver, internal_sender, callbacks.clone()).await, + T::start(private_key, control_plane_receiver, internal_sender, callbacks.0.clone()).await, &runtime_disconnector, &callbacks ); @@ -218,7 +312,7 @@ where let result = connection.start(vec![topic.clone()], || exponential_backoff.reset()).await; if let Some(t) = exponential_backoff.next_backoff() { tracing::warn!("Error during connection to the portal, retrying in {} seconds", t.as_secs()); - callbacks.on_error(&result.err().unwrap_or(Error::PortalConnectionError(tokio_tungstenite::tungstenite::Error::ConnectionClosed))); + let _ = callbacks.on_error(&result.err().unwrap_or(Error::PortalConnectionError(tokio_tungstenite::tungstenite::Error::ConnectionClosed))); tokio::time::sleep(t).await; } else { tracing::error!("Connection to the portal error, check your internet or the status of the portal.\nDisconnecting interface."); @@ -235,41 +329,57 @@ where }); } - fn connect_mock(callbacks: CB) { + fn connect_mock(runtime: Arc>>, callbacks: CallbackErrorFacade) { std::thread::sleep(Duration::from_secs(1)); - callbacks.on_set_interface_config( - "100.100.111.2".parse().unwrap(), - "fd00:0222:2021:1111::2".parse().unwrap(), - DNS_SENTINEL, + fatal_error!( + callbacks.on_set_interface_config( + "100.100.111.2".parse().unwrap(), + "fd00:0222:2021:1111::2".parse().unwrap(), + DNS_SENTINEL, + ), + &runtime, + &callbacks + ); + fatal_error!(callbacks.on_tunnel_ready(), &runtime, &callbacks); + let handle = { + let callbacks = callbacks.clone(); + std::thread::spawn(move || -> Result<()> { + std::thread::sleep(Duration::from_secs(3)); + let resources = vec![ + ResourceDescriptionCidr { + id: Uuid::new_v4(), + address: "8.8.4.4".parse::().unwrap().into(), + name: "Google Public DNS IPv4".to_string(), + }, + ResourceDescriptionCidr { + id: Uuid::new_v4(), + address: "2001:4860:4860::8844".parse::().unwrap().into(), + name: "Google Public DNS IPv6".to_string(), + }, + ]; + for resource in &resources { + callbacks.on_add_route(resource.address)?; + } + callbacks.on_update_resources( + resources + .into_iter() + .map(ResourceDescription::Cidr) + .collect(), + ) + }) + }; + fatal_error!( + handle.join().expect("mock thread panicked"), + &runtime, + &callbacks ); - callbacks.on_tunnel_ready(); - std::thread::spawn(move || { - std::thread::sleep(Duration::from_secs(3)); - let resources = vec![ - ResourceDescriptionCidr { - id: Uuid::new_v4(), - address: "8.8.4.4".parse::().unwrap().into(), - name: "Google Public DNS IPv4".to_string(), - }, - ResourceDescriptionCidr { - id: Uuid::new_v4(), - address: "2001:4860:4860::8844".parse::().unwrap().into(), - name: "Google Public DNS IPv6".to_string(), - }, - ]; - for resource in &resources { - callbacks.on_add_route(serde_json::to_string(&resource.address).unwrap()); - } - callbacks.on_update_resources( - resources - .into_iter() - .map(ResourceDescription::Cidr) - .collect(), - ); - }); } - fn disconnect_inner(runtime: &Mutex>, callbacks: &CB, error: Option) { + fn disconnect_inner( + runtime: &Mutex>, + callbacks: &CallbackErrorFacade, + error: Option, + ) { // 1. Close the websocket connection // 2. Free the device handle (UNIX) // 3. Close the file descriptor (UNIX) @@ -283,26 +393,25 @@ where // implement them :) *runtime.lock() = None; - callbacks.on_disconnect(error.as_ref()); + let _ = callbacks.on_disconnect(error.as_ref()); } /// Cleanup a [Session]. /// /// For now this just drops the runtime, which should drop all pending tasks. /// Further cleanup should be done here. (Otherwise we can just drop [Session]). - pub fn disconnect(&mut self, error: Option) -> bool { - Self::disconnect_inner(&self.runtime, &self.callbacks, error); - true + pub fn disconnect(&mut self, error: Option) { + Self::disconnect_inner(&self.runtime, &self.callbacks, error) } - /// TODO - pub fn bump_sockets(&self) -> bool { - true + // TODO: See https://github.com/WireGuard/wireguard-apple/blob/2fec12a6e1f6e3460b6ee483aa00ad29cddadab1/Sources/WireGuardKitGo/api-apple.go#L177 + pub fn bump_sockets(&self) { + tracing::error!("`bump_sockets` is unimplemented"); } - /// TODO - pub fn disable_some_roaming_for_broken_mobile_semantics(&self) -> bool { - true + // TODO: See https://github.com/WireGuard/wireguard-apple/blob/2fec12a6e1f6e3460b6ee483aa00ad29cddadab1/Sources/WireGuardKitGo/api-apple.go#LL197C6-L197C50 + pub fn disable_some_roaming_for_broken_mobile_semantics(&self) { + tracing::error!("`disable_some_roaming_for_broken_mobile_semantics` is unimplemented"); } } diff --git a/rust/connlib/libs/gateway/src/control.rs b/rust/connlib/libs/gateway/src/control.rs index 4d157efee..4ca9cbd95 100644 --- a/rust/connlib/libs/gateway/src/control.rs +++ b/rust/connlib/libs/gateway/src/control.rs @@ -90,12 +90,12 @@ impl ControlPlane { .await { tunnel.cleanup_connection(connection_request.device.id); - tunnel.callbacks().on_error(&err); + let _ = tunnel.callbacks().on_error(&err); } } Err(err) => { tunnel.cleanup_connection(connection_request.device.id); - tunnel.callbacks().on_error(&err); + let _ = tunnel.callbacks().on_error(&err); } } }); diff --git a/rust/connlib/libs/tunnel/Cargo.toml b/rust/connlib/libs/tunnel/Cargo.toml index a4188d9f1..e3aa5edbc 100644 --- a/rust/connlib/libs/tunnel/Cargo.toml +++ b/rust/connlib/libs/tunnel/Cargo.toml @@ -9,7 +9,6 @@ tokio = { version = "1.27", default-features = false, features = ["rt", "rt-mult thiserror = { version = "1.0", default-features = false } rand_core = { version = "0.6", default-features = false, features = ["getrandom"] } serde = { version = "1.0", default-features = false, features = ["derive", "std"] } -serde_json = { version = "1.0", default-features = false, features = ["std"] } futures = { version = "0.3", default-features = false, features = ["std", "async-await", "executor"] } futures-util = { version = "0.3", default-features = false, features = ["std", "async-await", "async-await-macro"] } tracing = { version = "0.1", default-features = false, features = ["std", "attributes"] } diff --git a/rust/connlib/libs/tunnel/src/control_protocol.rs b/rust/connlib/libs/tunnel/src/control_protocol.rs index 186f3dddc..cc4c7dee7 100644 --- a/rust/connlib/libs/tunnel/src/control_protocol.rs +++ b/rust/connlib/libs/tunnel/src/control_protocol.rs @@ -174,7 +174,7 @@ where let Some(gateway_public_key) = tunnel.gateway_public_keys.lock().remove(&resource_id) else { tunnel.cleanup_connection(resource_id); tracing::warn!("Opened ICE channel with gateway without ever receiving public key"); - tunnel.callbacks.on_error(&Error::ControlProtocolError); + let _ = tunnel.callbacks.on_error(&Error::ControlProtocolError); return; }; let peer_config = PeerConfig { @@ -186,7 +186,7 @@ where if let Err(e) = tunnel.handle_channel_open(d, index, peer_config, None, resource_id, None).await { tracing::error!("Couldn't establish wireguard link after channel was opened: {e}"); - tunnel.callbacks.on_error(&e); + let _ = tunnel.callbacks.on_error(&e); tunnel.cleanup_connection(resource_id); } tunnel.awaiting_connection.lock().remove(&resource_id); @@ -302,10 +302,10 @@ where Box::pin(async move { { let mut iface_config = tunnel.iface_config.lock().await; - for ip in &peer.ips { + for &ip in &peer.ips { if let Err(e) = iface_config.add_route(ip, tunnel.callbacks()).await { - tunnel.callbacks.on_error(&e); + let _ = tunnel.callbacks.on_error(&e); } } } @@ -321,7 +321,7 @@ where ) .await { - tunnel.callbacks.on_error(&e); + let _ = tunnel.callbacks.on_error(&e); tracing::error!( "Couldn't establish wireguard link after opening channel: {e}" ); @@ -331,7 +331,7 @@ where if let Some(conn) = conn { if let Err(e) = conn.close().await { tracing::error!("Problem while trying to close channel: {e:?}"); - tunnel.callbacks().on_error(&e.into()); + let _ = tunnel.callbacks().on_error(&e.into()); } } } diff --git a/rust/connlib/libs/tunnel/src/lib.rs b/rust/connlib/libs/tunnel/src/lib.rs index f2cb2925f..55e496a4e 100644 --- a/rust/connlib/libs/tunnel/src/lib.rs +++ b/rust/connlib/libs/tunnel/src/lib.rs @@ -38,7 +38,7 @@ use std::{ use libs_common::{ messages::{Id, Interface as InterfaceConfig, ResourceDescription}, - Result, + CallbackErrorFacade, Result, }; use device_channel::{create_iface, DeviceChannel}; @@ -146,7 +146,7 @@ pub struct Tunnel { resources: RwLock, control_signaler: C, gateway_public_keys: Mutex>, - callbacks: CB, + callbacks: CallbackErrorFacade, } impl Tunnel @@ -208,7 +208,7 @@ where resources, awaiting_connection, control_signaler, - callbacks, + callbacks: CallbackErrorFacade(callbacks), }) } @@ -221,7 +221,7 @@ where { let mut iface_config = self.iface_config.lock().await; for ip in resource_description.ips() { - iface_config.add_route(&ip, self.callbacks()).await?; + iface_config.add_route(ip, self.callbacks()).await?; } } let resource_list = { @@ -229,7 +229,7 @@ where resources.insert(resource_description); resources.resource_list() }; - self.callbacks.on_update_resources(resource_list); + self.callbacks.on_update_resources(resource_list)?; Ok(()) } @@ -247,14 +247,14 @@ where .await .expect("Couldn't initiate interface"); iface_config - .add_route(&DNS_SENTINEL.into(), self.callbacks()) + .add_route(DNS_SENTINEL.into(), self.callbacks()) .await?; } self.start_timers(); self.start_iface_handler(); - self.callbacks.on_tunnel_ready(); + self.callbacks.on_tunnel_ready()?; tracing::trace!("Started background loops"); @@ -389,7 +389,7 @@ where } Err(TunnResult::Err(e)) => { tracing::error!("Wireguard error: {e:?}"); - tunnel.callbacks().on_error(&e.into()); + let _ = tunnel.callbacks().on_error(&e.into()); continue; } Err(_) => { @@ -419,7 +419,7 @@ where } TunnResult::Err(err) => { tracing::error!("Error decapsulating packet: {err:?}"); - tunnel.callbacks().on_error(&err.into()); + let _ = tunnel.callbacks().on_error(&err.into()); continue; } TunnResult::WriteToNetwork(packet) => { @@ -449,13 +449,13 @@ where async fn write4_device_infallible(&self, packet: &[u8]) { if let Err(e) = self.device_channel.write4(packet).await { - self.callbacks.on_error(&e.into()); + let _ = self.callbacks.on_error(&e.into()); } } async fn write6_device_infallible(&self, packet: &[u8]) { if let Err(e) = self.device_channel.write6(packet).await { - self.callbacks.on_error(&e.into()); + let _ = self.callbacks.on_error(&e.into()); } } @@ -485,13 +485,13 @@ where Ok(res) => res, Err(err) => { tracing::error!("Couldn't read packet from interface: {err}"); - dev.callbacks.on_error(&err.into()); + let _ = dev.callbacks.on_error(&err.into()); continue; } }, Err(err) => { tracing::error!("Couldn't obtain iface mtu: {err}"); - dev.callbacks.on_error(&err); + let _ = dev.callbacks.on_error(&err); continue; } } @@ -572,7 +572,7 @@ where // Not a deadlock because this is a different task dev.awaiting_connection.lock().remove(&id); tracing::error!("couldn't start protocol for new connection to resource: {e}"); - dev.callbacks.on_error(&e); + let _ = dev.callbacks.on_error(&e); } }); } @@ -591,7 +591,7 @@ where } TunnResult::Err(e) => { tracing::error!(message = "Encapsulate error for resource corresponding to {dst_addr}", error = ?e); - dev.callbacks.on_error(&e.into()); + let _ = dev.callbacks.on_error(&e.into()); } TunnResult::WriteToNetwork(packet) => { tracing::trace!("writing iface packet to peer: {dst_addr}"); @@ -612,11 +612,11 @@ where tracing::error!( "Problem while trying to close channel: {e:?}" ); - dev.callbacks().on_error(&e.into()); + let _ = dev.callbacks().on_error(&e.into()); } } } - dev.callbacks.on_error(&e.into()); + let _ = dev.callbacks.on_error(&e.into()); } } _ => panic!("Unexpected result from encapsulate"), @@ -629,7 +629,7 @@ where self.next_index.lock().next() } - pub fn callbacks(&self) -> &CB { + pub fn callbacks(&self) -> &CallbackErrorFacade { &self.callbacks } } diff --git a/rust/connlib/libs/tunnel/src/peer.rs b/rust/connlib/libs/tunnel/src/peer.rs index 087d54165..bcec0b868 100644 --- a/rust/connlib/libs/tunnel/src/peer.rs +++ b/rust/connlib/libs/tunnel/src/peer.rs @@ -40,7 +40,7 @@ impl Peer { pub(crate) async fn send_infallible(&self, data: &[u8], callbacks: &CB) { if let Err(e) = self.channel.write(&Bytes::copy_from_slice(data)).await { tracing::error!("Couldn't send packet to connected peer: {e}"); - callbacks.on_error(&e.into()); + let _ = callbacks.on_error(&e.into()); } } diff --git a/rust/connlib/libs/tunnel/src/resource_sender.rs b/rust/connlib/libs/tunnel/src/resource_sender.rs index daaa76650..959887159 100644 --- a/rust/connlib/libs/tunnel/src/resource_sender.rs +++ b/rust/connlib/libs/tunnel/src/resource_sender.rs @@ -60,7 +60,7 @@ where let mut address = r.address.split(':'); let Some(dst_addr) = address.next() else { tracing::error!("invalid DNS name for resource: {}", r.address); - self.callbacks().on_error(&Error::InvalidResource(r.address.clone())); + let _ = self.callbacks().on_error(&Error::InvalidResource(r.address.clone())); return; }; let Ok(mut dst_addr) = format!("{dst_addr}:0").to_socket_addrs() else { diff --git a/rust/connlib/libs/tunnel/src/tun_android.rs b/rust/connlib/libs/tunnel/src/tun_android.rs index c5f6d427a..69cb813a5 100644 --- a/rust/connlib/libs/tunnel/src/tun_android.rs +++ b/rust/connlib/libs/tunnel/src/tun_android.rs @@ -1,7 +1,7 @@ use super::InterfaceConfig; use ip_network::IpNetwork; use libc::{close, open, O_RDWR}; -use libs_common::{Callbacks, Error, Result, DNS_SENTINEL}; +use libs_common::{CallbackErrorFacade, Callbacks, Error, Result, DNS_SENTINEL}; use std::{ os::fd::{AsRawFd, RawFd}, sync::Arc, @@ -73,15 +73,18 @@ impl IfaceConfig { pub async fn set_iface_config( &mut self, config: &InterfaceConfig, - callbacks: &impl Callbacks, + callbacks: &CallbackErrorFacade, ) -> Result<()> { callbacks.on_set_interface_config(config.ipv4, config.ipv6, DNS_SENTINEL); Ok(()) } - pub async fn add_route(&mut self, route: &IpNetwork, callbacks: &impl Callbacks) -> Result<()> { - callbacks.on_add_route(serde_json::to_string(route)?); - Ok(()) + pub async fn add_route( + &mut self, + route: IpNetwork, + callbacks: &CallbackErrorFacade, + ) -> Result<()> { + callbacks.on_add_route(route) } pub async fn up(&mut self) -> Result<()> { diff --git a/rust/connlib/libs/tunnel/src/tun_darwin.rs b/rust/connlib/libs/tunnel/src/tun_darwin.rs index 3500a0654..1733031d0 100644 --- a/rust/connlib/libs/tunnel/src/tun_darwin.rs +++ b/rust/connlib/libs/tunnel/src/tun_darwin.rs @@ -5,7 +5,7 @@ use libc::{ CTLIOCGINFO, F_GETFL, F_SETFL, IF_NAMESIZE, IPPROTO_IP, O_NONBLOCK, PF_SYSTEM, SOCK_DGRAM, SOCK_STREAM, SYSPROTO_CONTROL, UTUN_OPT_IFNAME, }; -use libs_common::{Callbacks, Error, Result, DNS_SENTINEL}; +use libs_common::{CallbackErrorFacade, Callbacks, Error, Result, DNS_SENTINEL}; use std::{ ffi::{c_int, c_short, c_uchar}, io, @@ -266,15 +266,17 @@ impl IfaceConfig { pub async fn set_iface_config( &mut self, config: &InterfaceConfig, - callbacks: &impl Callbacks, + callbacks: &CallbackErrorFacade, ) -> Result<()> { - callbacks.on_set_interface_config(config.ipv4, config.ipv6, DNS_SENTINEL); - Ok(()) + callbacks.on_set_interface_config(config.ipv4, config.ipv6, DNS_SENTINEL) } - pub async fn add_route(&mut self, route: &IpNetwork, callbacks: &impl Callbacks) -> Result<()> { - callbacks.on_add_route(serde_json::to_string(route)?); - Ok(()) + pub async fn add_route( + &mut self, + route: IpNetwork, + callbacks: &CallbackErrorFacade, + ) -> Result<()> { + callbacks.on_add_route(route) } pub async fn up(&mut self) -> Result<()> { diff --git a/rust/connlib/libs/tunnel/src/tun_linux.rs b/rust/connlib/libs/tunnel/src/tun_linux.rs index b63c43377..14da88da3 100644 --- a/rust/connlib/libs/tunnel/src/tun_linux.rs +++ b/rust/connlib/libs/tunnel/src/tun_linux.rs @@ -4,7 +4,7 @@ use libc::{ close, fcntl, ioctl, open, read, sockaddr, sockaddr_in, write, F_GETFL, F_SETFL, IFF_MULTI_QUEUE, IFF_NO_PI, IFF_TUN, IFNAMSIZ, O_NONBLOCK, O_RDWR, }; -use libs_common::{Callbacks, Error, Result}; +use libs_common::{CallbackErrorFacade, Callbacks, Error, Result}; use netlink_packet_route::{rtnl::link::nlas::Nla, RT_SCOPE_UNIVERSE}; use rtnetlink::{new_connection, Handle}; use std::{ @@ -178,8 +178,8 @@ fn get_last_error() -> Error { impl IfaceConfig { pub async fn add_route( &mut self, - route: &IpNetwork, - _callbacks: &impl Callbacks, + route: IpNetwork, + _callbacks: &CallbackErrorFacade, ) -> Result<()> { let req = self .0 @@ -224,7 +224,7 @@ impl IfaceConfig { pub async fn set_iface_config( &mut self, config: &InterfaceConfig, - _callbacks: &impl Callbacks, + _callbacks: &CallbackErrorFacade, ) -> Result<()> { let ips = self .0 diff --git a/rust/connlib/libs/tunnel/src/tun_win.rs b/rust/connlib/libs/tunnel/src/tun_win.rs index 078d52dfa..2e3b47b07 100644 --- a/rust/connlib/libs/tunnel/src/tun_win.rs +++ b/rust/connlib/libs/tunnel/src/tun_win.rs @@ -1,6 +1,6 @@ use super::InterfaceConfig; use ip_network::IpNetwork; -use libs_common::{Callbacks, Result}; +use libs_common::{CallbackErrorFacade, Callbacks, Result}; #[derive(Debug)] pub(crate) struct IfaceConfig; @@ -10,15 +10,15 @@ impl IfaceConfig { pub async fn set_iface_config( &mut self, _config: &InterfaceConfig, - _callbacks: &impl Callbacks, + _callbacks: &CallbackErrorFacade, ) -> Result<()> { todo!() } pub async fn add_route( &mut self, - _route: &IpNetwork, - _callbacks: &impl Callbacks, + _route: IpNetwork, + _callbacks: &CallbackErrorFacade, ) -> Result<()> { todo!() }