From cad48fcb9d1d1d184c1d817644b2fb131676a73f Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Tue, 27 Aug 2024 14:51:20 +0200
Subject: [PATCH] Replace `unsafe` calls to C-functions with
 `Direction::want_pktap`

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`.
---
 .../src/split_tunnel/macos/bindings.rs        | 20 +--------
 talpid-core/src/split_tunnel/macos/tun.rs     | 44 +++++--------------
 2 files changed, 11 insertions(+), 53 deletions(-)

diff --git a/talpid-core/src/split_tunnel/macos/bindings.rs b/talpid-core/src/split_tunnel/macos/bindings.rs
index 931ac84d2e97..7430029c1470 100644
--- a/talpid-core/src/split_tunnel/macos/bindings.rs
+++ b/talpid-core/src/split_tunnel/macos/bindings.rs
@@ -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];
@@ -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;
diff --git a/talpid-core/src/split_tunnel/macos/tun.rs b/talpid-core/src/split_tunnel/macos/tun.rs
index f5cc58878630..5876881fa8bb 100644
--- a/talpid-core/src/split_tunnel/macos/tun.rs
+++ b/talpid-core/src/split_tunnel/macos/tun.rs
@@ -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,
 };
@@ -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::{
@@ -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),
@@ -806,8 +803,15 @@ 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.
+    //
+    // TODO: upstream exposure of a raw handle to pcap_t on Capture<Inactive>
+    let cap = pcap::Capture::from_device("pktap")
+        .map_err(Error::CreatePcap)?
         .immediate_mode(true)
+        .want_pktap(true)
         .open()
         .map_err(Error::CaptureSplitTunnelDevice)?;
 
@@ -894,31 +898,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 _)
-    }))
-}