Skip to content

Commit

Permalink
Replace unsafe calls to C-functions with Direction::want_pktap
Browse files Browse the repository at this point in the history
Use the safe `pcap` function `Direction::want_pktap` to initialize the
packet capture in the macOS split tunnel implemenation. This replaces
parts of our auto-generated FFI to interface with `libpcap`, thus
getting rid of some use of `unsafe`.
  • Loading branch information
MarkusPettersson98 committed Aug 28, 2024
1 parent a6496e1 commit 27c4644
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 53 deletions.
20 changes: 1 addition & 19 deletions talpid-core/src/split_tunnel/macos/bindings.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// automatically generated by rust-bindgen 0.69.2

pub const PTH_FLAG_DIR_OUT: u32 = 2;
pub const PCAP_ERRBUF_SIZE: u32 = 256;
pub type __int32_t = ::std::os::raw::c_int;
pub type __darwin_pid_t = __int32_t;
pub type __darwin_uuid_t = [::std::os::raw::c_uchar; 16usize];
Expand Down Expand Up @@ -287,22 +286,5 @@ fn bindgen_test_layout_pktap_header() {
)
);
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct pcap {
_unused: [u8; 0],
}
pub type pcap_t = pcap;
extern "C" {
pub fn pcap_create(
arg1: *const ::std::os::raw::c_char,
arg2: *mut ::std::os::raw::c_char,
) -> *mut pcap_t;
}
extern "C" {
pub fn pcap_set_want_pktap(
arg1: *mut pcap_t,
arg2: ::std::os::raw::c_int,
) -> ::std::os::raw::c_int;
}

pub const BIOCSWANTPKTAP: u64 = 3221504639;
42 changes: 8 additions & 34 deletions talpid-core/src/split_tunnel/macos/tun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
//! either the default interface or a VPN tunnel interface.
use super::{
bindings::{
pcap_create, pcap_set_want_pktap, pktap_header, PCAP_ERRBUF_SIZE, PTH_FLAG_DIR_OUT,
},
bindings::{pktap_header, PTH_FLAG_DIR_OUT},
bpf,
default::DefaultInterface,
};
Expand All @@ -25,7 +23,6 @@ use std::{
ffi::{c_uint, CStr},
io::{self, IoSlice, Write},
net::{Ipv4Addr, Ipv6Addr},
ptr::NonNull,
};
use talpid_routing::RouteManagerHandle;
use tokio::{
Expand Down Expand Up @@ -57,7 +54,7 @@ pub enum Error {
EnableNonblock(#[source] pcap::Error),
/// pcap_create failed
#[error("pcap_create failed: {}", _0)]
CreatePcap(String),
CreatePcap(#[source] pcap::Error),
/// Failed to create packet stream
#[error("Failed to create packet stream")]
CreateStream(#[source] pcap::Error),
Expand Down Expand Up @@ -806,8 +803,13 @@ fn fix_ipv6_checksums(
fn capture_outbound_packets(
utun_iface: &str,
) -> Result<impl Stream<Item = Result<PktapPacket, Error>> + Send, Error> {
let cap = pktap_capture()?
// We want to create a pktap "pseudo-device" and capture data on it using a bpf device.
// This provides packet data plus a pktap header including process information.
// libpcap will do the heavy lifting for us if we simply request a "pktap" device.
let cap = pcap::Capture::from_device("pktap")
.map_err(Error::CreatePcap)?
.immediate_mode(true)
.want_pktap(true)
.open()
.map_err(Error::CaptureSplitTunnelDevice)?;

Expand Down Expand Up @@ -894,31 +896,3 @@ impl PacketCodec for PktapCodec {
})
}
}

/// Create a pktap interface using `libpcap`
fn pktap_capture() -> Result<pcap::Capture<pcap::Inactive>, Error> {
// We want to create a pktap "pseudo-device" and capture data on it using a bpf device.
// This provides packet data plus a pktap header including process information.
// libpcap will do the heavy lifting for us if we simply request a "pktap" device.

let mut errbuf = [0u8; PCAP_ERRBUF_SIZE as usize];

let pcap = unsafe { pcap_create(c"pktap".as_ptr(), errbuf.as_mut_ptr() as _) };
if pcap.is_null() {
let errstr = CStr::from_bytes_until_nul(&errbuf)
.unwrap()
.to_string_lossy()
.into_owned();
return Err(Error::CreatePcap(errstr));
}
unsafe { pcap_set_want_pktap(pcap, 1) };

// TODO: Upstream setting "want pktap" directly on Capture
// If we had that, we could have simply used pcap::Capture::from_device("pktap")
// TODO: Also upstream exposure of a raw handle to pcap_t on Capture<Inactive>

// just casting a pointer to a private type using _. that's fine, apparently
Ok(pcap::Capture::from(unsafe {
NonNull::new_unchecked(pcap as *mut _)
}))
}

0 comments on commit 27c4644

Please sign in to comment.