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.

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

fix(android): send Cidr format instead of IpNetwork format #4134

Merged
merged 6 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions rust/connlib/clients/android/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
// ecosystem, so it's used here for consistency.

use connlib_client_shared::{
file_logger, keypair, Callbacks, Error, LoginUrl, LoginUrlError, ResourceDescription, Session,
file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, LoginUrlError,
ResourceDescription, Session,
};
use ip_network::{Ipv4Network, Ipv6Network};
use jni::{
objects::{GlobalRef, JByteArray, JClass, JObject, JObjectArray, JString, JValue, JValueGen},
strings::JNIString,
Expand Down Expand Up @@ -196,8 +196,8 @@ impl Callbacks for CallbackHandler {

fn on_update_routes(
&self,
route_list_4: Vec<Ipv4Network>,
route_list_6: Vec<Ipv6Network>,
route_list_4: Vec<Cidrv4>,
route_list_6: Vec<Cidrv6>,
) -> Result<Option<RawFd>, Self::Error> {
self.env(|mut env| {
let route_list_4 = env
Expand Down
7 changes: 3 additions & 4 deletions rust/connlib/clients/apple/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
#![allow(clippy::unnecessary_cast, improper_ctypes, non_camel_case_types)]

use connlib_client_shared::{
file_logger, keypair, Callbacks, Error, LoginUrl, ResourceDescription, Session,
file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, ResourceDescription, Session,
};
use ip_network::{Ipv4Network, Ipv6Network};
use secrecy::SecretString;
use std::{
net::{IpAddr, Ipv4Addr, Ipv6Addr},
Expand Down Expand Up @@ -122,8 +121,8 @@ impl Callbacks for CallbackHandler {

fn on_update_routes(
&self,
route_list_4: Vec<Ipv4Network>,
route_list_6: Vec<Ipv6Network>,
route_list_4: Vec<Cidrv4>,
route_list_6: Vec<Cidrv6>,
) -> Result<Option<RawFd>, Self::Error> {
self.inner.on_update_routes(
serde_json::to_string(&route_list_4).unwrap(),
Expand Down
4 changes: 3 additions & 1 deletion rust/connlib/clients/shared/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Main connlib library for clients.
pub use connlib_shared::messages::ResourceDescription;
pub use connlib_shared::{keypair, Callbacks, Error, LoginUrl, LoginUrlError, StaticSecret};
pub use connlib_shared::{
keypair, Callbacks, Cidrv4, Cidrv6, Error, LoginUrl, LoginUrlError, StaticSecret,
};
pub use tracing_appender::non_blocking::WorkerGuard;

use backoff::ExponentialBackoffBuilder;
Expand Down
35 changes: 33 additions & 2 deletions rust/connlib/shared/src/callbacks.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::messages::ResourceDescription;
use ip_network::{Ipv4Network, Ipv6Network};
use serde::Serialize;
use std::error::Error;
use std::fmt::{Debug, Display};
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
Expand All @@ -8,6 +9,36 @@ use std::path::PathBuf;
// Avoids having to map types for Windows
type RawFd = i32;

#[derive(Serialize, Clone, Copy, Debug)]
pub struct Cidrv4 {
conectado marked this conversation as resolved.
Show resolved Hide resolved
address: Ipv4Addr,
prefix: u8,
}

#[derive(Serialize, Clone, Copy, Debug)]
conectado marked this conversation as resolved.
Show resolved Hide resolved
pub struct Cidrv6 {
address: Ipv6Addr,
prefix: u8,
}

impl From<Ipv4Network> for Cidrv4 {
fn from(value: Ipv4Network) -> Self {
Self {
address: value.network_address(),
prefix: value.netmask(),
}
}
}

impl From<Ipv6Network> for Cidrv6 {
fn from(value: Ipv6Network) -> Self {
Self {
address: value.network_address(),
prefix: value.netmask(),
}
}
}

/// 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.
Expand Down Expand Up @@ -35,8 +66,8 @@ pub trait Callbacks: Clone + Send + Sync {
/// Called when the route list changes.
fn on_update_routes(
&self,
_: Vec<Ipv4Network>,
_: Vec<Ipv6Network>,
_: Vec<Cidrv4>,
_: Vec<Cidrv6>,
) -> Result<Option<RawFd>, Self::Error> {
Ok(None)
}
Expand Down
7 changes: 3 additions & 4 deletions rust/connlib/shared/src/callbacks_error_facade.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use ip_network::{Ipv4Network, Ipv6Network};

use crate::callbacks::{Cidrv4, Cidrv6};
use crate::messages::ResourceDescription;
use crate::{Callbacks, Error, Result};
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
Expand Down Expand Up @@ -43,8 +42,8 @@ impl<CB: Callbacks> Callbacks for CallbackErrorFacade<CB> {

fn on_update_routes(
&self,
routes4: Vec<Ipv4Network>,
routes6: Vec<Ipv6Network>,
routes4: Vec<Cidrv4>,
routes6: Vec<Cidrv6>,
) -> Result<Option<RawFd>> {
let result = self
.0
Expand Down
2 changes: 1 addition & 1 deletion rust/connlib/shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub mod windows;

pub use boringtun::x25519::PublicKey;
pub use boringtun::x25519::StaticSecret;
pub use callbacks::Callbacks;
pub use callbacks::{Callbacks, Cidrv4, Cidrv6};
pub use callbacks_error_facade::CallbackErrorFacade;
pub use error::ConnlibError as Error;
pub use error::Result;
Expand Down
15 changes: 7 additions & 8 deletions rust/connlib/tunnel/src/device_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ mod tun;
mod tun;

use crate::ip_packet::{IpPacket, MutableIpPacket};
use connlib_shared::error::ConnlibError;
use connlib_shared::messages::Interface;
use connlib_shared::{Callbacks, Error};
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
use connlib_shared::{error::ConnlibError, messages::Interface, Callbacks, Error};
use connlib_shared::{Cidrv4, Cidrv6};
use ip_network::IpNetwork;
use pnet_packet::Packet;
use std::collections::HashSet;
use std::io;
Expand All @@ -39,18 +38,18 @@ pub struct Device {
}

#[allow(dead_code)]
fn ipv4(ip: &IpNetwork) -> Option<&Ipv4Network> {
fn ipv4(ip: IpNetwork) -> Option<Cidrv4> {
match ip {
IpNetwork::V4(v4) => Some(v4),
IpNetwork::V4(v4) => Some(v4.into()),
IpNetwork::V6(_) => None,
}
}

#[allow(dead_code)]
fn ipv6(ip: &IpNetwork) -> Option<&Ipv6Network> {
fn ipv6(ip: IpNetwork) -> Option<Cidrv6> {
match ip {
IpNetwork::V4(_) => None,
IpNetwork::V6(v6) => Some(v6),
IpNetwork::V6(v6) => Some(v6.into()),
}
}

Expand Down
4 changes: 2 additions & 2 deletions rust/connlib/tunnel/src/device_channel/tun_android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ impl Tun {
) -> Result<()> {
let fd = callbacks
.on_update_routes(
routes.iter().filter_map(ipv4).copied().collect(),
routes.iter().filter_map(ipv6).copied().collect(),
routes.iter().copied().filter_map(ipv4).collect(),
routes.iter().copied().filter_map(ipv6).collect(),
)?
.ok_or(Error::NoFd)?;

Expand Down
4 changes: 2 additions & 2 deletions rust/connlib/tunnel/src/device_channel/tun_darwin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ impl Tun {
) -> Result<()> {
// This will always be None in macos
callbacks.on_update_routes(
routes.iter().filter_map(ipv4).copied().collect(),
routes.iter().filter_map(ipv6).copied().collect(),
routes.iter().copied().filter_map(ipv4).collect(),
routes.iter().copied().filter_map(ipv6).collect(),
ReactorScram marked this conversation as resolved.
Show resolved Hide resolved
)?;

Ok(())
Expand Down
9 changes: 3 additions & 6 deletions swift/apple/FirezoneNetworkExtension/Adapter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -451,18 +451,15 @@ extension Adapter: CallbackHandlerDelegate {
workQueue.async { [weak self] in
guard let self = self else { return }

self.logger.log("Adapter.onUpdateRoutes")

let routes4 = try! JSONDecoder().decode([String].self, from: routeList4.data(using: .utf8)!)
let routes6 = try! JSONDecoder().decode([String].self, from: routeList6.data(using: .utf8)!)
self.logger.log("Adapter.onUpdateRoutes \(routeList4) \(routeList6)")

guard let networkSettings = self.networkSettings else {
self.logger.error("Adapter.onUpdateRoutes: No network settings")
return
}

networkSettings.routes4 = NetworkSettings.parseRoutes4(routes4: routes4)
networkSettings.routes6 = NetworkSettings.parseRoutes6(routes6: routes6)
networkSettings.routes4 = try! JSONDecoder().decode([NetworkSettings.Cidr].self, from: routeList4.data(using: .utf8)!).compactMap { $0.asNEIPv4Route }
networkSettings.routes6 = try! JSONDecoder().decode([NetworkSettings.Cidr].self, from: routeList6.data(using: .utf8)!).compactMap { $0.asNEIPv6Route }

networkSettings.apply(on: packetTunnelProvider, logger: self.logger, completionHandler: nil)
}
Expand Down
49 changes: 10 additions & 39 deletions swift/apple/FirezoneNetworkExtension/NetworkSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -154,47 +154,18 @@ enum IPv4SubnetMaskLookup {
]
}

extension NetworkSettings {
static func parseRoutes4(routes4: [String]) -> [NEIPv4Route] {
return routes4.map { route4 in
let components = route4.split(separator: "/")
let address = String(components[0])
let prefix: Int = Int(components[1])!
let mask = IPv4SubnetMaskLookup.table[prefix]!
return NEIPv4Route(destinationAddress: address, subnetMask: mask)
}
}

static func parseRoutes6(routes6: [String]) -> [NEIPv6Route] {
routes6.map { route6 in
let components = route6.split(separator: "/")
let address = String(components[0])
let prefix: NSNumber = NSNumber(value: Int(components.last!)!)
return NEIPv6Route(destinationAddress: address, networkPrefixLength: prefix)
}
}

private static func validNetworkPrefixLength(fromString string: String, maximumValue: Int) -> Int
{
guard let networkPrefixLength = Int(string) else { return 0 }
if networkPrefixLength < 0 { return 0 }
if networkPrefixLength > maximumValue { return maximumValue }
return networkPrefixLength
}
extension NetworkSettings {
struct Cidr: Codable {
let address: String
let prefix: Int

private static func ipv4SubnetMask(networkPrefixLength: Int) -> String {
precondition(networkPrefixLength >= 0 && networkPrefixLength <= 32)
let mask: UInt32 = 0xFFFF_FFFF
let maxPrefixLength = 32
let octets = 4

let subnetMask = mask & (mask << (maxPrefixLength - networkPrefixLength))
var parts: [String] = []
for idx in 0...(octets - 1) {
let part = String(UInt32(0x0000_00FF) & (subnetMask >> ((octets - 1 - idx) * 8)), radix: 10)
parts.append(part)
}
var asNEIPv4Route: NEIPv4Route {
return NEIPv4Route(destinationAddress: address, subnetMask: String(prefix))
}

return parts.joined(separator: ".")
var asNEIPv6Route: NEIPv6Route {
return NEIPv6Route(destinationAddress: address, networkPrefixLength: NSNumber(value: prefix))
}
}
}
Loading