Refactor device creation

Change the way tun devices are created to prevent
two from overlapping. This caused PQ to panic when
taking read-locks.

1 files changed, 90 insertions, 149 deletions

diff --git a/talpid-wireguard/src/boringtun/mod.rs b/talpid-wireguard/src/boringtun/mod.rs
index c8fab5d64a..e790ef70f9 100644
--- a/talpid-wireguard/src/boringtun/mod.rs
+++ b/talpid-wireguard/src/boringtun/mod.rs
@@ -56,6 +56,31 @@ pub struct BoringTun {
     interface_name: String,
 }
 
+impl BoringTun {
+    async fn new(
+        tun: Arc<AsyncDevice>,
+        #[cfg(target_os = "android")] android_tun: Arc<Tun>,
+        config: Config,
+        interface_name: String,
+    ) -> Result<Self, TunnelError> {
+        let devices = create_devices(
+            &config,
+            tun.clone(),
+            #[cfg(target_os = "android")]
+            android_tun.clone(),
+        )
+        .await?;
+        Ok(Self {
+            config,
+            interface_name,
+            tun,
+            #[cfg(target_os = "android")]
+            android_tun,
+            devices: Some(devices),
+        })
+    }
+}
+
 enum Devices {
     Singlehop {
         device: SinglehopDevice,
@@ -153,25 +178,15 @@ pub async fn open_boringtun_tunnel(
     log::info!("passing tunnel dev to boringtun");
     let async_tun = Arc::new(async_tun);
 
-    let mut boringtun = BoringTun {
-        config: config.clone(),
-        interface_name,
-        tun: async_tun.clone(),
+    let boringtun = BoringTun::new(
+        async_tun,
         #[cfg(target_os = "android")]
-        android_tun: tun.clone(),
-        devices: Some(
-            create_devices(
-                config,
-                async_tun,
-                #[cfg(target_os = "android")]
-                tun,
-            )
-            .await,
-        ),
-    };
-
-    // FIXME: double clone
-    boringtun.set_config(config.clone()).await?;
+        tun.clone(),
+        config.clone(),
+        interface_name,
+    )
+    .await
+    .inspect_err(|e| log::error!("Failed to open BoringTun: {e:?}"))?;
 
     log::info!(
         "This tunnel was brought to you by...
@@ -192,13 +207,13 @@ async fn create_devices(
     config: &Config,
     async_tun: Arc<AsyncDevice>,
     #[cfg(target_os = "android")] tun: Arc<Tun>,
-) -> Devices {
+) -> Result<Devices, TunnelError> {
     let (entry_api, entry_api_server) = ApiServer::new();
     let boringtun_entry_config = DeviceConfig {
         api: Some(entry_api_server),
     };
 
-    if config.exit_peer.is_some() {
+    if let Some(exit_peer) = &config.exit_peer {
         // multihop
 
         let source_v4 = config.tunnel.addresses.iter().find_map(|ip| match ip {
@@ -237,12 +252,36 @@ async fn create_devices(
         let entry_device =
             EntryDevice::new(factory, channel.clone(), channel, boringtun_entry_config).await;
 
-        Devices::Multihop {
+        let private_key = &config.tunnel.private_key;
+        let peer = &config.entry_peer;
+        let set_cmd = create_set_command(
+            #[cfg(target_os = "linux")]
+            config.fwmark,
+            private_key,
+            peer,
+        );
+        entry_api.send(set_cmd).await.map_err(|err| {
+            log::error!("Failed to set boringtun config: {err:#}");
+            TunnelError::SetConfigError
+        })?;
+
+        let set_cmd = create_set_command(
+            #[cfg(target_os = "linux")]
+            config.fwmark,
+            private_key,
+            exit_peer,
+        );
+        exit_api.send(set_cmd).await.map_err(|err| {
+            log::error!("Failed to set boringtun config: {err:#}");
+            TunnelError::SetConfigError
+        })?;
+
+        Ok(Devices::Multihop {
             entry_device,
             entry_api,
             exit_device,
             exit_api,
-        }
+        })
     } else {
         #[cfg(target_os = "android")]
         let factory = AndroidUdpSocketFactory { tun };
@@ -258,10 +297,25 @@ async fn create_devices(
         )
         .await;
 
-        Devices::Singlehop {
+        log::info!("configuring boringtun device");
+        let private_key = &config.tunnel.private_key;
+        let peer = &config.entry_peer;
+        let set_cmd = create_set_command(
+            #[cfg(target_os = "linux")]
+            config.fwmark,
+            private_key,
+            peer,
+        );
+
+        entry_api.send(set_cmd).await.map_err(|err| {
+            log::error!("Failed to set boringtun config: {err:#}");
+            TunnelError::SetConfigError
+        })?;
+
+        Ok(Devices::Singlehop {
             device,
             api: entry_api,
-        }
+        })
     }
 }
 
@@ -355,22 +409,9 @@ impl Tunnel for BoringTun {
                         #[cfg(target_os = "android")]
                         self.android_tun.clone(),
                     )
-                    .await,
+                    .await?,
                 );
             }
-            match self.devices.as_mut().unwrap() {
-                Devices::Singlehop { api, .. } => {
-                    set_boringtun_config(api, &self.config).await?;
-                }
-                Devices::Multihop {
-                    entry_api,
-                    exit_api,
-                    ..
-                } => {
-                    set_boringtun_entry_config(entry_api, &self.config).await?;
-                    set_boringtun_exit_config(exit_api, &self.config).await?;
-                }
-            }
             Ok(())
         })
     }
@@ -381,118 +422,22 @@ impl Tunnel for BoringTun {
     }
 }
 
-async fn set_boringtun_config(
-    tx: &mut ApiClient,
-    config: &Config,
-) -> Result<(), crate::TunnelError> {
-    log::info!("configuring boringtun device");
-    let mut set_cmd = Set::builder()
-        .private_key(config.tunnel.private_key.to_bytes())
-        .listen_port(0u16)
-        .replace_peers()
-        .build();
-
-    #[cfg(target_os = "linux")]
-    {
-        set_cmd.fwmark = config.fwmark;
-    }
-
-    for peer in config.peers() {
-        let mut boring_peer = Peer::builder()
-            .public_key(*peer.public_key.as_bytes())
-            .endpoint(peer.endpoint)
-            .allowed_ip(
-                peer.allowed_ips
-                    .iter()
-                    .map(|net| AllowedIP {
-                        addr: net.ip(),
-                        cidr: net.prefix(),
-                    })
-                    .collect(),
-            )
-            .build();
-
-        if let Some(psk) = &peer.psk {
-            boring_peer.preshared_key = Some(SetUnset::Set((*psk.as_bytes()).into()));
-        }
-
-        let boring_peer = SetPeer::builder().peer(boring_peer).build();
-
-        set_cmd.peers.push(boring_peer);
-    }
-
-    tx.send(set_cmd).await.map_err(|err| {
-        log::error!("Failed to set boringtun config: {err:#}");
-        TunnelError::SetConfigError
-    })?;
-    Ok(())
-}
-
-async fn set_boringtun_entry_config(
-    tx: &mut ApiClient,
-    config: &Config,
-) -> Result<(), crate::TunnelError> {
-    log::info!("configuring boringtun device");
-    let mut set_cmd = Set::builder()
-        .private_key(config.tunnel.private_key.to_bytes())
-        .listen_port(0u16)
-        .replace_peers()
-        .build();
-
-    #[cfg(target_os = "linux")]
-    {
-        set_cmd.fwmark = config.fwmark;
-    }
-
-    let peer = &config.entry_peer;
-    let mut boring_peer = Peer::builder()
-        .public_key(*peer.public_key.as_bytes())
-        .endpoint(peer.endpoint)
-        .allowed_ip(
-            peer.allowed_ips
-                .iter()
-                .map(|net| AllowedIP {
-                    addr: net.ip(),
-                    cidr: net.prefix(),
-                })
-                .collect(),
-        )
-        .build();
-
-    if let Some(psk) = &peer.psk {
-        boring_peer.preshared_key = Some(SetUnset::Set((*psk.as_bytes()).into()));
-    }
-
-    let boring_peer = SetPeer::builder().peer(boring_peer).build();
-
-    set_cmd.peers.push(boring_peer);
-
-    tx.send(set_cmd).await.map_err(|err| {
-        log::error!("Failed to set boringtun config: {err:#}");
-        TunnelError::SetConfigError
-    })?;
-    Ok(())
-}
-
-async fn set_boringtun_exit_config(
-    tx: &mut ApiClient,
-    config: &Config,
-) -> Result<(), crate::TunnelError> {
-    log::info!("configuring boringtun device");
+fn create_set_command(
+    #[cfg(target_os = "linux")] fwmark: Option<u32>,
+    private_key: &talpid_types::net::wireguard::PrivateKey,
+    peer: &talpid_types::net::wireguard::PeerConfig,
+) -> Set {
     let mut set_cmd = Set::builder()
-        .private_key(config.tunnel.private_key.to_bytes())
+        .private_key(private_key.to_bytes())
         .listen_port(0u16)
         .replace_peers()
         .build();
 
     #[cfg(target_os = "linux")]
     {
-        set_cmd.fwmark = config.fwmark;
+        set_cmd.fwmark = fwmark;
     }
 
-    // TODO: don't unwrap
-    let peer = config.exit_peer.as_ref().unwrap();
-
     let mut boring_peer = Peer::builder()
         .public_key(*peer.public_key.as_bytes())
         .endpoint(peer.endpoint)
@@ -511,15 +456,11 @@ async fn set_boringtun_exit_config(
         boring_peer.preshared_key = Some(SetUnset::Set((*psk.as_bytes()).into()));
     }
 
-    let boring_peer = SetPeer::builder().peer(boring_peer).build();
-
-    set_cmd.peers.push(boring_peer);
+    set_cmd
+        .peers
+        .push(SetPeer::builder().peer(boring_peer).build());
 
-    tx.send(set_cmd).await.map_err(|err| {
-        log::error!("Failed to set boringtun config: {err:#}");
-        TunnelError::SetConfigError
-    })?;
-    Ok(())
+    set_cmd
 }
 
 #[cfg(target_os = "windows")]