connlib: Refine callbacks (#1776)

This follows-up on the discussion in #1744 and brings connlib in line
with the callback revisions outlined in firezone/product#586

(It also adds some logging to the Apple bridge that was helpful when
testing this)

---------

Co-authored-by: Roopesh Chander <roop@roopc.net>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
This commit is contained in:
Francesca Lovebloom
2023-07-17 11:26:46 -07:00
committed by GitHub
parent ed8a74e642
commit e413e96ccb
13 changed files with 216 additions and 101 deletions

View File

@@ -10,6 +10,7 @@ use jni::{
objects::{JClass, JObject, JString, JValue},
JNIEnv,
};
use std::net::Ipv4Addr;
/// This should be called once after the library is loaded by the system.
#[allow(non_snake_case)]
@@ -30,11 +31,23 @@ pub extern "system" fn Java_dev_firezone_connlib_Logger_init(_: JNIEnv, _: JClas
pub struct CallbackHandler;
impl Callbacks for CallbackHandler {
fn on_update_resources(&self, _resource_list: ResourceList) {
fn on_set_interface_config(&self, _tunnel_addresses: TunnelAddresses, _dns_address: Ipv4Addr) {
todo!()
}
fn on_connect(&self, _tunnel_addresses: TunnelAddresses) {
fn on_tunnel_ready(&self) {
todo!()
}
fn on_add_route(&self, _route: String) {
todo!()
}
fn on_remove_route(&self, _route: String) {
todo!()
}
fn on_update_resources(&self, _resource_list: ResourceList) {
todo!()
}
@@ -70,12 +83,12 @@ pub unsafe extern "system" fn Java_dev_firezone_connlib_Session_connect(
let tunnel_addresses = env.new_string(tunnelAddressesJSON).unwrap();
match env.call_method(
callback,
"onConnect",
"onTunnelReady",
"(Ljava/lang/String;)Z",
&[JValue::from(&tunnel_addresses)],
) {
Ok(res) => log::trace!("`onConnect` returned `{res:?}`"),
Err(err) => log::error!("Failed to call `onConnect`: {err}"),
Ok(res) => log::trace!("`onTunnelReady` returned `{res:?}`"),
Err(err) => log::error!("Failed to call `onTunnelReady`: {err}"),
}
Box::into_raw(session)

View File

@@ -1,8 +1,5 @@
//
// Callbacks.swift
// connlib
//
// Created by Jamil Bou Kheir on 4/3/23.
// CallbackHandler.swift
//
import NetworkExtension
@@ -19,31 +16,48 @@ extension SwiftConnlibError: @unchecked Sendable {}
extension SwiftConnlibError: Error {}
public protocol CallbackHandlerDelegate: AnyObject {
func onConnect(tunnelAddressIPv4: String, tunnelAddressIPv6: String)
func onUpdateResources(resourceList: String)
func onDisconnect()
func onError(error: Error, isRecoverable: Bool)
func onConnect(tunnelAddressIPv4: String, tunnelAddressIPv6: String)
func onUpdateResources(resourceList: String)
func onDisconnect()
func onError(error: Error, isRecoverable: Bool)
}
public class CallbackHandler {
public weak var delegate: CallbackHandlerDelegate?
public weak var delegate: CallbackHandlerDelegate?
private let logger = Logger(subsystem: "dev.firezone.firezone", category: "callbackhandler")
func onUpdateResources(resourceList: ResourceList) {
delegate?.onUpdateResources(resourceList: resourceList.resources.toString())
}
func onSetInterfaceConfig(tunnelAddresses: TunnelAddresses, dnsAddress: RustString) {
logger.debug("CallbackHandler.onSetInterfaceConfig: IPv4: \(tunnelAddresses.address4.toString(), privacy: .public), IPv6: \(tunnelAddresses.address6.toString(), privacy: .public), DNS: \(dnsAddress.toString(), privacy: .public)")
// Unimplemented
}
func onConnect(tunnelAddresses: TunnelAddresses) {
delegate?.onConnect(
tunnelAddressIPv4: tunnelAddresses.address4.toString(),
tunnelAddressIPv6: tunnelAddresses.address6.toString()
)
}
func onTunnelReady() {
logger.debug("CallbackHandler.onTunnelReady")
// Unimplemented
}
func onDisconnect() {
delegate?.onDisconnect()
}
func onAddRoute(route: RustString) {
logger.debug("CallbackHandler.onAddRoute: \(route.toString(), privacy: .public)")
// Unimplemented
}
func onError(error: SwiftConnlibError, error_type: SwiftErrorType) {
delegate?.onError(error: error, isRecoverable: error_type == .Recoverable)
}
func onRemoveRoute(route: RustString) {
logger.debug("CallbackHandler.onRemoveRoute: \(route.toString(), privacy: .public)")
// Unimplemented
}
func onUpdateResources(resourceList: ResourceList) {
logger.debug("CallbackHandler.onUpdateResources: \(resourceList.resources.toString(), privacy: .public)")
delegate?.onUpdateResources(resourceList: resourceList.resources.toString())
}
func onDisconnect() {
logger.debug("CallbackHandler.onDisconnect")
delegate?.onDisconnect()
}
func onError(error: SwiftConnlibError, error_type: SwiftErrorType) {
logger.debug("CallbackHandler.onError: \(error, privacy: .public) (\(error_type == .Recoverable ? "Recoverable" : "Fatal", privacy: .public)")
delegate?.onError(error: error, isRecoverable: error_type == .Recoverable)
}
}

View File

@@ -5,7 +5,7 @@
use firezone_client_connlib::{
Callbacks, Error, ErrorType, ResourceList, Session, TunnelAddresses,
};
use std::sync::Arc;
use std::{net::Ipv4Addr, sync::Arc};
#[swift_bridge::bridge]
mod ffi {
@@ -73,12 +73,21 @@ mod ffi {
extern "Swift" {
type CallbackHandler;
#[swift_bridge(swift_name = "onSetInterfaceConfig")]
fn on_set_interface_config(&self, tunnelAddresses: TunnelAddresses, dnsAddress: String);
#[swift_bridge(swift_name = "onTunnelReady")]
fn on_tunnel_ready(&self);
#[swift_bridge(swift_name = "onAddRoute")]
fn on_add_route(&self, route: String);
#[swift_bridge(swift_name = "onRemoveRoute")]
fn on_remove_route(&self, route: String);
#[swift_bridge(swift_name = "onUpdateResources")]
fn on_update_resources(&self, resourceList: ResourceList);
#[swift_bridge(swift_name = "onConnect")]
fn on_connect(&self, tunnelAddresses: TunnelAddresses);
#[swift_bridge(swift_name = "onDisconnect")]
fn on_disconnect(&self);
@@ -165,12 +174,25 @@ unsafe impl Sync for ffi::CallbackHandler {}
pub struct CallbackHandler(Arc<ffi::CallbackHandler>);
impl Callbacks for CallbackHandler {
fn on_update_resources(&self, resource_list: ResourceList) {
self.0.on_update_resources(resource_list.into())
fn on_set_interface_config(&self, tunnel_addresses: TunnelAddresses, dns_address: Ipv4Addr) {
self.0
.on_set_interface_config(tunnel_addresses.into(), dns_address.to_string())
}
fn on_connect(&self, tunnel_addresses: TunnelAddresses) {
self.0.on_connect(tunnel_addresses.into())
fn on_tunnel_ready(&self) {
self.0.on_tunnel_ready()
}
fn on_add_route(&self, route: String) {
self.0.on_add_route(route)
}
fn on_remove_route(&self, route: String) {
self.0.on_remove_route(route)
}
fn on_update_resources(&self, resource_list: ResourceList) {
self.0.on_update_resources(resource_list.into())
}
fn on_disconnect(&self) {

View File

@@ -1,6 +1,6 @@
use anyhow::{Context, Result};
use clap::Parser;
use std::str::FromStr;
use std::{net::Ipv4Addr, str::FromStr};
use firezone_client_connlib::{
get_user_agent, Callbacks, Error, ErrorType, ResourceList, Session, TunnelAddresses,
@@ -11,12 +11,18 @@ use url::Url;
pub struct CallbackHandler;
impl Callbacks for CallbackHandler {
fn on_update_resources(&self, resource_list: ResourceList) {
tracing::trace!("Resources updated, current list: {resource_list:?}");
fn on_set_interface_config(&self, _tunnel_addresses: TunnelAddresses, _dns_address: Ipv4Addr) {}
fn on_tunnel_ready(&self) {
tracing::trace!("Tunnel connected");
}
fn on_connect(&self, tunnel_addresses: TunnelAddresses) {
tracing::trace!("Tunnel connected with address: {tunnel_addresses:?}");
fn on_add_route(&self, _route: String) {}
fn on_remove_route(&self, _route: String) {}
fn on_update_resources(&self, resource_list: ResourceList) {
tracing::trace!("Resources updated, current list: {resource_list:?}");
}
fn on_disconnect(&self) {

View File

@@ -1,5 +1,5 @@
use anyhow::{Context, Result};
use std::str::FromStr;
use std::{net::Ipv4Addr, str::FromStr};
use firezone_gateway_connlib::{
Callbacks, Error, ErrorType, ResourceList, Session, TunnelAddresses,
@@ -10,12 +10,18 @@ use url::Url;
pub struct CallbackHandler;
impl Callbacks for CallbackHandler {
fn on_update_resources(&self, resource_list: ResourceList) {
tracing::trace!("Resources updated, current list: {resource_list:?}");
fn on_set_interface_config(&self, _tunnel_addresses: TunnelAddresses, _dns_address: Ipv4Addr) {}
fn on_tunnel_ready(&self) {
tracing::trace!("Tunnel connected with address");
}
fn on_connect(&self, tunnel_addresses: TunnelAddresses) {
tracing::trace!("Tunnel connected with address: {tunnel_addresses:?}");
fn on_add_route(&self, _route: String) {}
fn on_remove_route(&self, _route: String) {}
fn on_update_resources(&self, resource_list: ResourceList) {
tracing::trace!("Resources updated, current list: {resource_list:?}");
}
fn on_disconnect(&self) {

View File

@@ -14,7 +14,9 @@ pub mod messages;
pub use error::ConnlibError as Error;
pub use error::Result;
pub use session::{Callbacks, ControlSession, ResourceList, Session, TunnelAddresses};
pub use session::{
Callbacks, ControlSession, ResourceList, Session, TunnelAddresses, DNS_SENTINEL,
};
const VERSION: &str = env!("CARGO_PKG_VERSION");
const LIB_NAME: &str = "connlib";

View File

@@ -19,6 +19,8 @@ use crate::{
Error, Result,
};
pub const DNS_SENTINEL: Ipv4Addr = Ipv4Addr::new(100, 100, 111, 1);
// TODO: Not the most tidy trait for a control-plane.
/// Trait that represents a control-plane.
#[async_trait]
@@ -65,10 +67,16 @@ pub struct TunnelAddresses {
// Evaluate doing this not static
/// Traits that will be used by connlib to callback the client upper layers.
pub trait Callbacks: Clone + Send + Sync {
/// Called when there's a change in the resource list.
fn on_update_resources(&self, resource_list: ResourceList);
/// Called when the tunnel address is set.
fn on_connect(&self, tunnel_addresses: TunnelAddresses);
fn on_set_interface_config(&self, tunnel_addresses: TunnelAddresses, dns_address: Ipv4Addr);
/// Called when the tunnel is connected.
fn on_tunnel_ready(&self);
/// Called when when a route is added.
fn on_add_route(&self, route: String);
/// Called when when a route is removed.
fn on_remove_route(&self, route: String);
/// Called when the resource list changes.
fn on_update_resources(&self, resource_list: ResourceList);
/// Called when the tunnel is disconnected.
fn on_disconnect(&self);
/// Called when there's an error.
@@ -207,27 +215,38 @@ where
fn connect_mock(callbacks: CB) {
std::thread::sleep(Duration::from_secs(1));
callbacks.on_connect(TunnelAddresses {
address4: "100.100.111.2".parse().unwrap(),
address6: "fd00:0222:2021:1111::2".parse().unwrap(),
});
callbacks.on_set_interface_config(
TunnelAddresses {
address4: "100.100.111.2".parse().unwrap(),
address6: "fd00:0222:2021:1111::2".parse().unwrap(),
},
DNS_SENTINEL,
);
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::<Ipv4Addr>().unwrap().into(),
name: "Google Public DNS IPv4".to_string(),
},
ResourceDescriptionCidr {
id: Uuid::new_v4(),
address: "2001:4860:4860::8844".parse::<Ipv6Addr>().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(ResourceList {
resources: vec![
serde_json::to_string(&ResourceDescription::Cidr(ResourceDescriptionCidr {
id: Uuid::new_v4(),
address: "8.8.4.4".parse::<Ipv4Addr>().unwrap().into(),
name: "Google Public DNS IPv4".to_string(),
}))
.unwrap(),
serde_json::to_string(&ResourceDescription::Cidr(ResourceDescriptionCidr {
id: Uuid::new_v4(),
address: "2001:4860:4860::8844".parse::<Ipv6Addr>().unwrap().into(),
name: "Google Public DNS IPv6".to_string(),
}))
.unwrap(),
],
resources: resources
.into_iter()
.map(|resource| {
serde_json::to_string(&ResourceDescription::Cidr(resource)).unwrap()
})
.collect(),
});
});
}

View File

@@ -281,7 +281,8 @@ where
{
let mut iface_config = tunnel.iface_config.lock().await;
for ip in &peer.ips {
if let Err(e) = iface_config.add_route(ip).await {
if let Err(e) = iface_config.add_route(ip, tunnel.callbacks()).await
{
tunnel.callbacks.on_error(&e, Recoverable);
}
}

View File

@@ -13,7 +13,7 @@ use ip_network::IpNetwork;
use ip_network_table::IpNetworkTable;
use libs_common::{
error_type::ErrorType::{Fatal, Recoverable},
Callbacks, TunnelAddresses,
Callbacks,
};
use async_trait::async_trait;
@@ -219,7 +219,7 @@ where
{
let mut iface_config = self.iface_config.lock().await;
for ip in resource_description.ips() {
if let Err(err) = iface_config.add_route(&ip).await {
if let Err(err) = iface_config.add_route(&ip, self.callbacks()).await {
self.callbacks.on_error(&err, Fatal);
}
}
@@ -243,7 +243,7 @@ where
{
let mut iface_config = self.iface_config.lock().await;
iface_config
.set_iface_config(config)
.set_iface_config(config, self.callbacks())
.await
.expect("Couldn't initiate interface");
iface_config
@@ -255,10 +255,7 @@ where
self.start_timers();
self.start_iface_handler();
self.callbacks.on_connect(TunnelAddresses {
address4: config.ipv4,
address6: config.ipv6,
});
self.callbacks.on_tunnel_ready();
tracing::trace!("Started background loops");

View File

@@ -1,7 +1,7 @@
use super::InterfaceConfig;
use ip_network::IpNetwork;
use libc::{close, open, O_RDWR};
use libs_common::{Error, Result};
use libs_common::{Callbacks, Error, Result, TunnelAddresses, DNS_SENTINEL};
use std::{
os::fd::{AsRawFd, RawFd},
sync::Arc,
@@ -69,14 +69,24 @@ impl IfaceDevice {
}
impl IfaceConfig {
pub async fn add_route(&mut self, route: &IpNetwork) -> Result<()> {
tracing::error!("`add_route` unimplemented on Android: `{route:#?}`");
#[tracing::instrument(level = "trace", skip(self, callbacks))]
pub async fn set_iface_config(
&mut self,
config: &InterfaceConfig,
callbacks: &impl Callbacks,
) -> Result<()> {
callbacks.on_set_interface_config(
TunnelAddresses {
address4: config.ipv4,
address6: config.ipv6,
},
DNS_SENTINEL,
);
Ok(())
}
#[tracing::instrument(level = "trace", skip(self))]
pub async fn set_iface_config(&mut self, _config: &InterfaceConfig) -> Result<()> {
tracing::error!("`set_iface_config` unimplemented on Android: `{_config:#?}`");
pub async fn add_route(&mut self, route: &IpNetwork, callbacks: &impl Callbacks) -> Result<()> {
callbacks.on_add_route(serde_json::to_string(route)?);
Ok(())
}

View File

@@ -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::{Error, Result};
use libs_common::{Callbacks, Error, Result, TunnelAddresses, DNS_SENTINEL};
use std::{
ffi::{c_int, c_short, c_uchar},
io,
@@ -262,14 +262,24 @@ impl IfaceDevice {
// So, these functions take a mutable &self, this is not necessary in theory but it's correct!
impl IfaceConfig {
pub async fn add_route(&mut self, route: &IpNetwork) -> Result<()> {
tracing::error!("`add_route` unimplemented on macOS: `{route:#?}`");
#[tracing::instrument(level = "trace", skip(self, callbacks))]
pub async fn set_iface_config(
&mut self,
config: &InterfaceConfig,
callbacks: &impl Callbacks,
) -> Result<()> {
callbacks.on_set_interface_config(
TunnelAddresses {
address4: config.ipv4,
address6: config.ipv6,
},
DNS_SENTINEL,
);
Ok(())
}
#[tracing::instrument(level = "trace", skip(self))]
pub async fn set_iface_config(&mut self, config: &InterfaceConfig) -> Result<()> {
tracing::error!("`set_iface_config` unimplemented on macOS: `{config:#?}`");
pub async fn add_route(&mut self, route: &IpNetwork, callbacks: &impl Callbacks) -> Result<()> {
callbacks.on_add_route(serde_json::to_string(route)?);
Ok(())
}

View File

@@ -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::{Error, Result};
use libs_common::{Callbacks, Error, Result};
use netlink_packet_route::rtnl::link::nlas::Nla;
use rtnetlink::{new_connection, Handle};
use std::{
@@ -177,7 +177,11 @@ fn get_last_error() -> Error {
}
impl IfaceConfig {
pub async fn add_route(&mut self, route: &IpNetwork) -> Result<()> {
pub async fn add_route(
&mut self,
route: &IpNetwork,
_callbacks: &impl Callbacks,
) -> Result<()> {
let req = self
.0
.handle
@@ -218,8 +222,12 @@ impl IfaceConfig {
Ok(())
}
#[tracing::instrument(level = "trace", skip(self))]
pub async fn set_iface_config(&mut self, config: &InterfaceConfig) -> Result<()> {
#[tracing::instrument(level = "trace", skip(self, _callbacks))]
pub async fn set_iface_config(
&mut self,
config: &InterfaceConfig,
_callbacks: &impl Callbacks,
) -> Result<()> {
let ips = self
.0
.handle

View File

@@ -1,22 +1,29 @@
use super::InterfaceConfig;
use ip_network::IpNetwork;
use libs_common::Result;
use libs_common::{Callbacks, Result};
#[derive(Debug)]
pub(crate) struct IfaceConfig;
impl IfaceConfig {
// It's easier to not make these functions async, setting these should not block the thread for too long
#[tracing::instrument(level = "trace", skip(self))]
pub async fn set_iface_config(&mut self, _config: &InterfaceConfig) -> Result<()> {
#[tracing::instrument(level = "trace", skip(self, _callbacks))]
pub async fn set_iface_config(
&mut self,
_config: &InterfaceConfig,
_callbacks: &impl Callbacks,
) -> Result<()> {
todo!()
}
pub async fn add_route(
&mut self,
_route: &IpNetwork,
_callbacks: &impl Callbacks,
) -> Result<()> {
todo!()
}
pub async fn up(&mut self) -> Result<()> {
todo!()
}
pub async fn add_route(&mut self, _route: &IpNetwork) -> Result<()> {
todo!()
}
}