Skip to content

Commit

Permalink
Merge branch 'prevent-dns-leaks-in-blocking-states-droid-950'
Browse files Browse the repository at this point in the history
  • Loading branch information
albin-mullvad committed May 8, 2024
2 parents 44dcda5 + 25bc197 commit 9869614
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
3 changes: 3 additions & 0 deletions android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ Line wrap the file at 100 chars. Th
* **Security**: in case of vulnerabilities.

## [Unreleased]
### Security
- Fix DNS leaks in blocking states or when no valid DNS has been configured due to an underlying OS
issue. In these cases a dummy DNS will be set to prevent leaks.


## [android/2024.2-beta1] - 2024-04-17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package net.mullvad.talpid

import android.net.VpnService
import android.os.ParcelFileDescriptor
import android.util.Log
import java.net.Inet4Address
import java.net.Inet6Address
import java.net.InetAddress
Expand Down Expand Up @@ -103,6 +104,18 @@ open class TalpidVpnService : VpnService() {
}
}

// Avoids creating a tunnel with no DNS servers or if all DNS servers was invalid,
// since apps then may leak DNS requests.
// https://issuetracker.google.com/issues/337961996
if (invalidDnsServerAddresses.size == config.dnsServers.size) {
Log.w(
"mullvad",
"All DNS servers invalid or non set, using fallback DNS server to " +
"minimize leaks, dnsServers.isEmpty(): ${config.dnsServers.isEmpty()}"
)
addDnsServer(FALLBACK_DUMMY_DNS_SERVER)
}

for (route in config.routes) {
addRoute(route.address, route.prefixLength.toInt())
}
Expand Down Expand Up @@ -148,4 +161,8 @@ open class TalpidVpnService : VpnService() {
private external fun defaultTunConfig(): TunConfig

private external fun waitForTunnelUp(tunFd: Int, isIpv6Enabled: Boolean)

companion object {
private const val FALLBACK_DUMMY_DNS_SERVER = "192.0.2.1"
}
}
41 changes: 30 additions & 11 deletions talpid-tunnel/src/tun_provider/android/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub struct AndroidTunProvider {
object: GlobalRef,
last_tun_config: TunConfig,
allow_lan: bool,
blocking: bool,
custom_dns_servers: Option<Vec<IpAddr>>,
allowed_lan_networks: Vec<IpNetwork>,
}
Expand All @@ -82,6 +83,7 @@ impl AndroidTunProvider {
object: context.vpn_service,
last_tun_config: TunConfig::default(),
allow_lan,
blocking: false,
custom_dns_servers,
allowed_lan_networks,
}
Expand All @@ -105,8 +107,15 @@ impl AndroidTunProvider {
Ok(())
}

/// Retrieve a tunnel device with the provided configuration. Custom DNS and LAN routes are
/// appended to the provided config.
pub fn get_tun(&mut self, mut config: TunConfig) -> Result<VpnServiceTun, Error> {
self.prepare_tun_config(&mut config, false);
self.get_tun_inner(config)
}

/// Retrieve a tunnel device with the provided configuration.
pub fn get_tun(&mut self, config: TunConfig) -> Result<VpnServiceTun, Error> {
fn get_tun_inner(&mut self, config: TunConfig) -> Result<VpnServiceTun, Error> {
let tun_fd = self.get_tun_fd(config.clone())?;

self.last_tun_config = config;
Expand All @@ -122,15 +131,15 @@ impl AndroidTunProvider {
})
}

/// Open a tunnel device that routes everything but custom DNS, and
/// (potentially) LAN routes via the tunnel device.
/// Open a tunnel device that routes everything but (potentially) LAN routes via the tunnel
/// device.
///
/// Will open a new tunnel if there is already an active tunnel. The previous tunnel will be
/// closed.
pub fn create_blocking_tun(&mut self) -> Result<(), Error> {
let mut config = TunConfig::default();
self.prepare_tun_config(&mut config);
let _ = self.get_tun(config)?;
self.prepare_tun_config(&mut config, true);
let _ = self.get_tun_inner(config)?;
Ok(())
}

Expand Down Expand Up @@ -176,9 +185,7 @@ impl AndroidTunProvider {
}
}

fn get_tun_fd(&mut self, mut config: TunConfig) -> Result<RawFd, Error> {
self.prepare_tun_config(&mut config);

fn get_tun_fd(&mut self, config: TunConfig) -> Result<RawFd, Error> {
let env = self.env()?;
let java_config = config.into_java(&env);

Expand All @@ -198,7 +205,7 @@ impl AndroidTunProvider {
fn recreate_tun_if_open(&mut self) -> Result<(), Error> {
let mut actual_config = self.last_tun_config.clone();

self.prepare_tun_config(&mut actual_config);
self.prepare_tun_config(&mut actual_config, self.blocking);

let env = self.env()?;
let java_config = actual_config.into_java(&env);
Expand All @@ -216,9 +223,13 @@ impl AndroidTunProvider {
}
}

fn prepare_tun_config(&self, config: &mut TunConfig) {
fn prepare_tun_config(&mut self, config: &mut TunConfig, blocking: bool) {
self.blocking = blocking;
self.prepare_tun_config_for_allow_lan(config);
self.prepare_tun_config_for_custom_dns(config);
if !blocking {
self.prepare_tun_config_for_custom_dns(config);
}
maybe_set_dummy_dns_servers(config);
}

fn prepare_tun_config_for_allow_lan(&self, config: &mut TunConfig) {
Expand Down Expand Up @@ -324,6 +335,14 @@ impl AndroidTunProvider {
}
}

/// Add dummy servers if no DNS servers are set. Android may sometimes leak DNS otherwise.
fn maybe_set_dummy_dns_servers(config: &mut TunConfig) {
if !config.dns_servers.is_empty() {
return;
}
config.dns_servers = vec!["192.0.2.1".parse().unwrap()];
}

/// Handle to a tunnel device on Android.
pub struct VpnServiceTun {
tunnel: RawFd,
Expand Down

0 comments on commit 9869614

Please sign in to comment.