summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md1
-rw-r--r--talpid-core/src/dns/macos.rs113
-rw-r--r--talpid-core/src/dns/mod.rs20
-rw-r--r--talpid-core/src/tunnel_state_machine/connected_state.rs22
-rw-r--r--talpid-core/src/tunnel_state_machine/connecting_state.rs5
-rw-r--r--talpid-core/src/tunnel_state_machine/disconnected_state.rs5
-rw-r--r--talpid-core/src/tunnel_state_machine/error_state.rs5
7 files changed, 136 insertions, 35 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index aa618b61b1..17ea48f7ef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -41,6 +41,7 @@ Line wrap the file at 100 chars. Th
### Fixed
#### macOS
- Fix packets being duplicated on LAN when split tunneling is enabled.
+- Fix DNS not working due to broken PF redirect.
## [2024.6] - 2024-10-23
diff --git a/talpid-core/src/dns/macos.rs b/talpid-core/src/dns/macos.rs
index dbfd056d01..21c2434512 100644
--- a/talpid-core/src/dns/macos.rs
+++ b/talpid-core/src/dns/macos.rs
@@ -2,7 +2,7 @@ use parking_lot::Mutex;
use std::{
collections::{BTreeSet, HashMap},
fmt, mem,
- net::IpAddr,
+ net::{IpAddr, SocketAddr},
sync::{mpsc as sync_mpsc, Arc, RwLock},
thread,
time::Duration,
@@ -12,12 +12,15 @@ use system_configuration::{
array::CFArray,
base::{CFType, TCFType, ToVoid},
dictionary::{CFDictionary, CFMutableDictionary},
+ number::CFNumber,
propertylist::CFPropertyList,
runloop::{kCFRunLoopCommonModes, CFRunLoop},
string::CFString,
},
dynamic_store::{SCDynamicStore, SCDynamicStoreBuilder, SCDynamicStoreCallBackContext},
- sys::schema_definitions::{kSCPropNetDNSServerAddresses, kSCPropNetInterfaceDeviceName},
+ sys::schema_definitions::{
+ kSCPropNetDNSServerAddresses, kSCPropNetDNSServerPort, kSCPropNetInterfaceDeviceName,
+ },
};
use talpid_routing::debounce::BurstGuard;
@@ -25,6 +28,8 @@ use super::ResolvedDnsConfig;
pub type Result<T> = std::result::Result<T, Error>;
+const DNS_PORT: u16 = 53;
+
/// Errors that can happen when setting/monitoring DNS on macOS.
#[derive(thiserror::Error, Debug)]
pub enum Error {
@@ -78,18 +83,20 @@ impl State {
store: &SCDynamicStore,
interface: &str,
servers: &[IpAddr],
+ port: u16,
) -> Result<()> {
talpid_types::detect_flood!();
let servers: Vec<DnsServer> = servers.iter().map(|ip| ip.to_string()).collect();
- let new_settings = DnsSettings::from_server_addresses(&servers, interface.to_string());
+ let new_settings =
+ DnsSettings::from_server_addresses(&servers, interface.to_string(), port);
match &self.dns_settings {
None => {
self.dns_settings = Some(new_settings);
self.update_and_apply_state(store);
}
Some(old_settings) => {
- if new_settings.address_set() != old_settings.address_set() {
+ if new_settings.server_addresses() != old_settings.server_addresses() {
for service_path in self.backup.keys() {
new_settings.save(store, service_path.as_str())?;
}
@@ -117,7 +124,7 @@ impl State {
};
let prev_state = mem::take(&mut self.backup);
- let desired_set = desired_settings.address_set();
+ let desired_set = desired_settings.server_addresses();
self.backup = Self::merge_states(actual_state, prev_state, desired_set);
}
@@ -127,7 +134,7 @@ impl State {
fn merge_states(
new_state: &HashMap<ServicePath, Option<DnsSettings>>,
mut prev_state: HashMap<ServicePath, Option<DnsSettings>>,
- ignore_addresses: BTreeSet<String>,
+ ignore_addresses: BTreeSet<SocketAddr>,
) -> HashMap<ServicePath, Option<DnsSettings>> {
let mut modified_state = HashMap::new();
@@ -135,7 +142,7 @@ impl State {
let old_entry = prev_state.remove(path);
match settings {
// If the service is using the desired addresses, don't save changes
- Some(settings) if settings.address_set() == ignore_addresses => {
+ Some(settings) if settings.server_addresses() == ignore_addresses => {
let settings = old_entry.unwrap_or_else(|| Some(settings.to_owned()));
modified_state.insert(path.to_owned(), settings);
}
@@ -143,7 +150,7 @@ impl State {
settings => {
let servers = settings
.as_ref()
- .map(|settings| settings.server_addresses().join(","))
+ .map(|settings| settings.format_addresses())
.unwrap_or_default();
log::debug!("Saving DNS settings [{}] for {}", servers, path);
modified_state.insert(path.to_owned(), settings.to_owned());
@@ -167,14 +174,19 @@ impl State {
let Some(ref desired_settings) = self.dns_settings else {
return;
};
- let desired_set = desired_settings.address_set();
+ let desired_set = desired_settings.server_addresses();
for (path, settings) in actual_state {
match settings {
// Do nothing if the state is already what we want
- Some(settings) if settings.address_set() == desired_set => (),
+ Some(settings) if settings.server_addresses() == desired_set => (),
// Ignore loopback addresses
- Some(settings) if settings.ips().any(|ip| ip.is_loopback()) => {
+ Some(settings)
+ if settings
+ .server_addresses()
+ .iter()
+ .any(|addr| addr.ip().is_loopback()) =>
+ {
log::trace!("Not updating DNS config: localhost is used");
}
// Apply desired state to service
@@ -221,7 +233,7 @@ struct DnsSettings {
unsafe impl Send for DnsSettings {}
impl DnsSettings {
- pub fn from_server_addresses(server_addresses: &[DnsServer], name: String) -> Self {
+ pub fn from_server_addresses(server_addresses: &[DnsServer], name: String, port: u16) -> Self {
let mut mut_dict = CFMutableDictionary::new();
if !server_addresses.is_empty() {
let cf_string_servers: Vec<CFString> =
@@ -233,6 +245,14 @@ impl DnsSettings {
&server_addresses_key.to_void(),
&server_addresses_value.to_void(),
);
+
+ // Set port if non-standard
+ if port != DNS_PORT {
+ let server_port_key =
+ unsafe { CFString::wrap_under_get_rule(kSCPropNetDNSServerPort) };
+ let server_port_value = CFNumber::from(i32::from(port));
+ mut_dict.add(&server_port_key.to_void(), &server_port_value.to_void());
+ }
}
let dict = mut_dict.to_immutable();
DnsSettings { dict, name }
@@ -261,7 +281,7 @@ impl DnsSettings {
) -> Result<()> {
log::trace!(
"Setting DNS to [{}] for {}",
- self.server_addresses().join(", "),
+ self.format_addresses(),
path.to_string()
);
if store.set(path, self.dict.clone()) {
@@ -271,23 +291,37 @@ impl DnsSettings {
}
}
- pub fn server_addresses(&self) -> Vec<String> {
+ pub fn server_addresses(&self) -> BTreeSet<SocketAddr> {
+ let port = self
+ .dict
+ .find(unsafe { kSCPropNetDNSServerPort }.to_void())
+ .map(|ptr| unsafe { CFType::wrap_under_get_rule(*ptr) })
+ .and_then(|port| port.downcast::<CFNumber>())
+ .and_then(|port| port.to_i32())
+ .and_then(|port| u16::try_from(port).ok())
+ .unwrap_or(DNS_PORT);
+
self.dict
.find(unsafe { kSCPropNetDNSServerAddresses }.to_void())
.map(|array_ptr| unsafe { CFType::wrap_under_get_rule(*array_ptr) })
.and_then(|array| array.downcast::<CFArray>())
.and_then(Self::parse_cf_array_to_strings)
.unwrap_or_default()
+ .into_iter()
+ .flat_map(|addr| addr.parse::<IpAddr>())
+ .map(|ip| SocketAddr::new(ip, port))
+ .collect()
}
- pub fn address_set(&self) -> BTreeSet<String> {
- BTreeSet::from_iter(self.server_addresses())
- }
-
- pub fn ips(&self) -> impl Iterator<Item = IpAddr> {
- self.server_addresses()
- .into_iter()
- .filter_map(|addr| addr.parse::<IpAddr>().ok())
+ fn format_addresses(&self) -> String {
+ let mut s = String::new();
+ for addr in self.server_addresses() {
+ if !s.is_empty() {
+ s.push_str(", ");
+ }
+ s.push_str(&addr.to_string());
+ }
+ s
}
/// Parses a CFArray into a Rust vector of Rust strings, if the array contains CFString
@@ -370,10 +404,11 @@ impl super::DnsMonitorT for DnsMonitor {
}
fn set(&mut self, interface: &str, config: ResolvedDnsConfig) -> Result<()> {
+ let port = config.port;
let servers: Vec<_> = config.addresses().collect();
let mut state = self.state.lock();
- state.apply_new_config(&self.store, interface, &servers)
+ state.apply_new_config(&self.store, interface, &servers, port)
}
fn reset(&mut self) -> Result<()> {
@@ -508,8 +543,13 @@ fn state_to_setup_path(state_path: &str) -> Option<String> {
#[cfg(test)]
mod test {
+ use crate::dns::imp::DNS_PORT;
+
use super::{DnsSettings, State};
- use std::collections::{BTreeSet, HashMap};
+ use std::{
+ collections::{BTreeSet, HashMap},
+ net::SocketAddr,
+ };
/// The initial backup should equal whatever the first provided state is.
#[test]
@@ -523,6 +563,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["1.2.3.4".to_owned()],
"iface_b".to_owned(),
+ DNS_PORT,
)),
),
// One of our states already equals the desired state. It should be stored regardless.
@@ -531,11 +572,12 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["10.64.0.1".to_owned()],
"iface_c".to_owned(),
+ DNS_PORT,
)),
),
]);
- let desired_addresses: BTreeSet<String> = ["10.64.0.1".to_owned()].into();
+ let desired_addresses: BTreeSet<SocketAddr> = ["10.64.0.1:53".parse().unwrap()].into();
let merged_state = State::merge_states(&new_state, prev_state, desired_addresses);
@@ -552,6 +594,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["1.2.3.4".to_owned()],
"iface_b".to_owned(),
+ DNS_PORT,
)),
),
(
@@ -559,6 +602,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["10.64.0.1".to_owned()],
"iface_c".to_owned(),
+ DNS_PORT,
)),
),
(
@@ -566,6 +610,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["1.3.3.7".to_owned()],
"iface_d".to_owned(),
+ DNS_PORT,
)),
),
]);
@@ -576,6 +621,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["10.64.0.1".to_owned()],
"iface_a".to_owned(),
+ DNS_PORT,
)),
),
// This change should be ignored
@@ -584,6 +630,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["10.64.0.1".to_owned()],
"iface_b".to_owned(),
+ DNS_PORT,
)),
),
// This change should be ignored
@@ -592,6 +639,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["4.3.2.1".to_owned()],
"iface_c".to_owned(),
+ DNS_PORT,
)),
),
// This change should NOT be ignored
@@ -600,6 +648,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["4.3.2.1".to_owned()],
"iface_d".to_owned(),
+ DNS_PORT,
)),
),
]);
@@ -610,6 +659,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["1.2.3.4".to_owned()],
"iface_b".to_owned(),
+ DNS_PORT,
)),
),
(
@@ -617,6 +667,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["4.3.2.1".to_owned()],
"iface_c".to_owned(),
+ DNS_PORT,
)),
),
(
@@ -624,11 +675,12 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["4.3.2.1".to_owned()],
"iface_d".to_owned(),
+ DNS_PORT,
)),
),
]);
- let desired_addresses: BTreeSet<String> = ["10.64.0.1".to_owned()].into();
+ let desired_addresses: BTreeSet<SocketAddr> = ["10.64.0.1:53".parse().unwrap()].into();
let merged_state = State::merge_states(&new_state, prev_state, desired_addresses);
@@ -644,6 +696,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["10.64.0.1".to_owned()],
"iface_a".to_owned(),
+ DNS_PORT,
)),
),
(
@@ -651,6 +704,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["1.2.3.4".to_owned()],
"iface_b".to_owned(),
+ DNS_PORT,
)),
),
("c".to_owned(), None),
@@ -658,7 +712,7 @@ mod test {
let new_state = HashMap::from([("c".to_owned(), None)]);
let expected_state = new_state.clone();
- let desired_addresses: BTreeSet<String> = ["10.64.0.1".to_owned()].into();
+ let desired_addresses: BTreeSet<SocketAddr> = ["10.64.0.1:53".parse().unwrap()].into();
let merged_state = State::merge_states(&new_state, prev_state, desired_addresses);
@@ -677,6 +731,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["192.168.100.1".to_owned()],
"iface_a".to_owned(),
+ DNS_PORT,
)),
)]);
let new_state = HashMap::from([(
@@ -684,6 +739,7 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["192.168.1.1".to_owned()],
"iface_a".to_owned(),
+ DNS_PORT,
)),
)]);
let expect_state = HashMap::from([(
@@ -691,10 +747,11 @@ mod test {
Some(DnsSettings::from_server_addresses(
&["192.168.1.1".to_owned()],
"iface_a".to_owned(),
+ DNS_PORT,
)),
)]);
- let desired_addresses: BTreeSet<String> = ["192.168.1.1".to_owned()].into();
+ let desired_addresses: BTreeSet<SocketAddr> = ["192.168.1.1:53".parse().unwrap()].into();
let merged_state = State::merge_states(&new_state, prev_state, desired_addresses);
diff --git a/talpid-core/src/dns/mod.rs b/talpid-core/src/dns/mod.rs
index f803842ef9..b3b3da1310 100644
--- a/talpid-core/src/dns/mod.rs
+++ b/talpid-core/src/dns/mod.rs
@@ -67,11 +67,17 @@ enum InnerDnsConfig {
}
impl DnsConfig {
- pub(crate) fn resolve(&self, default_tun_config: &[IpAddr]) -> ResolvedDnsConfig {
+ pub(crate) fn resolve(
+ &self,
+ default_tun_config: &[IpAddr],
+ #[cfg(target_os = "macos")] port: u16,
+ ) -> ResolvedDnsConfig {
match &self.config {
InnerDnsConfig::Default => ResolvedDnsConfig {
tunnel_config: default_tun_config.to_owned(),
non_tunnel_config: vec![],
+ #[cfg(target_os = "macos")]
+ port,
},
InnerDnsConfig::Override {
tunnel_config,
@@ -79,6 +85,8 @@ impl DnsConfig {
} => ResolvedDnsConfig {
tunnel_config: tunnel_config.to_owned(),
non_tunnel_config: non_tunnel_config.to_owned(),
+ #[cfg(target_os = "macos")]
+ port,
},
}
}
@@ -93,6 +101,9 @@ pub struct ResolvedDnsConfig {
/// For the most part, the tunnel state machine will not handle any of this configuration
/// on non-tunnel interface, only allow them in the firewall.
non_tunnel_config: Vec<IpAddr>,
+ /// Port to use
+ #[cfg(target_os = "macos")]
+ port: u16,
}
impl fmt::Display for ResolvedDnsConfig {
@@ -101,7 +112,12 @@ impl fmt::Display for ResolvedDnsConfig {
Self::fmt_addr_set(f, &self.tunnel_config)?;
f.write_str(" Non-tunnel DNS: ")?;
- Self::fmt_addr_set(f, &self.non_tunnel_config)
+ Self::fmt_addr_set(f, &self.non_tunnel_config)?;
+
+ #[cfg(target_os = "macos")]
+ write!(f, " Port: {}", self.port)?;
+
+ Ok(())
}
}
diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs
index 10d9ac9b72..25c13a0643 100644
--- a/talpid-core/src/tunnel_state_machine/connected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connected_state.rs
@@ -151,11 +151,15 @@ impl ConnectedState {
metadata: &TunnelMetadata,
shared_values: &SharedTunnelStateValues,
) -> ResolvedDnsConfig {
- shared_values.dns_config.resolve(&metadata.gateways())
+ shared_values.dns_config.resolve(
+ &metadata.gateways(),
+ #[cfg(target_os = "macos")]
+ 53,
+ )
}
fn set_dns(&self, shared_values: &mut SharedTunnelStateValues) -> Result<(), BoxedError> {
- let dns_config = Self::resolve_dns(&self.metadata, shared_values);
+ let dns_config: ResolvedDnsConfig = Self::resolve_dns(&self.metadata, shared_values);
#[cfg(not(target_os = "macos"))]
shared_values
@@ -171,8 +175,22 @@ impl ConnectedState {
.filtering_resolver
.enable_forward(dns_config.addresses().collect()),
);
+ shared_values
+ .dns_monitor
+ .set(
+ "lo",
+ crate::dns::DnsConfig::default().resolve(
+ &[std::net::Ipv4Addr::LOCALHOST.into()],
+ shared_values.filtering_resolver.listening_port(),
+ ),
+ )
+ .map_err(BoxedError::new)?;
} else {
log::debug!("Not enabling DNS forwarding since loopback is used");
+ shared_values
+ .dns_monitor
+ .set(&self.metadata.interface, dns_config)
+ .map_err(BoxedError::new)?;
}
Ok(())
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 199aa7eaa7..7a790a11b6 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -60,7 +60,10 @@ impl ConnectingState {
#[cfg(target_os = "macos")]
if let Err(err) = shared_values.dns_monitor.set(
"lo",
- crate::dns::DnsConfig::default().resolve(&[std::net::Ipv4Addr::LOCALHOST.into()]),
+ crate::dns::DnsConfig::default().resolve(
+ &[std::net::Ipv4Addr::LOCALHOST.into()],
+ shared_values.filtering_resolver.listening_port(),
+ ),
) {
log::error!(
"{}",
diff --git a/talpid-core/src/tunnel_state_machine/disconnected_state.rs b/talpid-core/src/tunnel_state_machine/disconnected_state.rs
index baf6012103..c65a04c3f3 100644
--- a/talpid-core/src/tunnel_state_machine/disconnected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/disconnected_state.rs
@@ -137,7 +137,10 @@ impl DisconnectedState {
shared_values.dns_monitor.set(
"lo",
- dns::DnsConfig::default().resolve(&[Ipv4Addr::LOCALHOST.into()]),
+ dns::DnsConfig::default().resolve(
+ &[Ipv4Addr::LOCALHOST.into()],
+ shared_values.filtering_resolver.listening_port(),
+ ),
)
}
}
diff --git a/talpid-core/src/tunnel_state_machine/error_state.rs b/talpid-core/src/tunnel_state_machine/error_state.rs
index eeaf48956b..1b154840e2 100644
--- a/talpid-core/src/tunnel_state_machine/error_state.rs
+++ b/talpid-core/src/tunnel_state_machine/error_state.rs
@@ -37,7 +37,10 @@ impl ErrorState {
if !block_reason.prevents_filtering_resolver() {
if let Err(err) = shared_values.dns_monitor.set(
"lo",
- DnsConfig::default().resolve(&[Ipv4Addr::LOCALHOST.into()]),
+ DnsConfig::default().resolve(
+ &[Ipv4Addr::LOCALHOST.into()],
+ shared_values.filtering_resolver.listening_port(),
+ ),
) {
log::error!(
"{}",
Copyright © 2025 - 2026 Wayne Cole. WTFPL. Attribution appreciated. No warranty. Not liable for bodily damages.