From 575f1564c8ea1d1f7222ca60ab74dde872d0f874 Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Fri, 1 Nov 2024 10:54:42 +0100 Subject: [PATCH 1/8] Skip testAPIConnectionViaBridges because staging bridges are down --- ios/MullvadVPNUITests/ConnectivityTests.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ios/MullvadVPNUITests/ConnectivityTests.swift b/ios/MullvadVPNUITests/ConnectivityTests.swift index 32a219d05e83..a1a49ffd8e23 100644 --- a/ios/MullvadVPNUITests/ConnectivityTests.swift +++ b/ios/MullvadVPNUITests/ConnectivityTests.swift @@ -15,6 +15,11 @@ class ConnectivityTests: LoggedOutUITestCase { /// Verifies that the app still functions when API has been blocked func testAPIConnectionViaBridges() throws { + let skipReason = """ + This test is currently skipped because shadowsocks bridges cannot be reached + from the staging environment + """ + try XCTSkipIf(true, skipReason) firewallAPIClient.removeRules() let hasTimeAccountNumber = getAccountWithTime() From 5d97dac295689e57c782b34e360c68a9ffa266ff Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Fri, 1 Nov 2024 14:02:42 +0100 Subject: [PATCH 2/8] Allow API connections in blocked state --- ios/MullvadVPN/TransportMonitor/TransportMonitor.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift b/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift index c2bbcfb73526..e4aee9cbe007 100644 --- a/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift +++ b/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift @@ -47,6 +47,9 @@ final class TransportMonitor: RESTTransportProvider { private func shouldByPassVPN(tunnel: any TunnelProtocol) -> Bool { switch tunnel.status { case .connected: + if case .error = tunnelManager.tunnelStatus.state { + return true + } return tunnelManager.isConfigurationLoaded && tunnelManager.deviceState == .revoked case .connecting, .reasserting: From 038be31659eba06001e15185256bd64ee4e105bc Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Fri, 1 Nov 2024 15:32:47 +0100 Subject: [PATCH 3/8] Set the accessibilityIdentifier on the correct element --- .../APIAccess/Edit/EditAccessMethodViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ios/MullvadVPN/Coordinators/Settings/APIAccess/Edit/EditAccessMethodViewController.swift b/ios/MullvadVPN/Coordinators/Settings/APIAccess/Edit/EditAccessMethodViewController.swift index c3f73d7de992..676ef4b4940b 100644 --- a/ios/MullvadVPN/Coordinators/Settings/APIAccess/Edit/EditAccessMethodViewController.swift +++ b/ios/MullvadVPN/Coordinators/Settings/APIAccess/Edit/EditAccessMethodViewController.swift @@ -45,9 +45,9 @@ class EditAccessMethodViewController: UIViewController { override func viewDidLoad() { super.viewDidLoad() - view.accessibilityIdentifier = .editAccessMethodView view.backgroundColor = .secondaryColor + tableView.accessibilityIdentifier = .editAccessMethodView tableView.backgroundColor = .secondaryColor tableView.delegate = self From dd66d1246b005594a16841ec945bf98914e80899 Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Tue, 5 Nov 2024 10:13:35 +0100 Subject: [PATCH 4/8] Allow override of encrypted DNS domain name --- ios/MullvadRustRuntime/EncryptedDNSProxy.swift | 2 +- .../include/mullvad_rust_runtime.h | 2 +- mullvad-daemon/src/api.rs | 2 +- .../src/config_resolver.rs | 5 +++-- mullvad-encrypted-dns-proxy/src/state.rs | 4 ++-- mullvad-ios/src/encrypted_dns_proxy.rs | 16 ++++++++++++++-- 6 files changed, 22 insertions(+), 9 deletions(-) diff --git a/ios/MullvadRustRuntime/EncryptedDNSProxy.swift b/ios/MullvadRustRuntime/EncryptedDNSProxy.swift index 690e2459d099..5179cbc51b97 100644 --- a/ios/MullvadRustRuntime/EncryptedDNSProxy.swift +++ b/ios/MullvadRustRuntime/EncryptedDNSProxy.swift @@ -20,7 +20,7 @@ public class EncryptedDNSProxy { private let state: OpaquePointer public init() { - state = encrypted_dns_proxy_init() + state = encrypted_dns_proxy_init("frakta.eu") proxyConfig = ProxyHandle(context: nil, port: 0) } diff --git a/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h b/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h index 26904b89dfe8..ad13d1f6e086 100644 --- a/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h +++ b/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h @@ -34,7 +34,7 @@ extern const uint16_t CONFIG_SERVICE_PORT; /** * Initializes a valid pointer to an instance of `EncryptedDnsProxyState`. */ -struct EncryptedDnsProxyState *encrypted_dns_proxy_init(void); +struct EncryptedDnsProxyState *encrypted_dns_proxy_init(const char *domain_name); /** * This must be called only once to deallocate `EncryptedDnsProxyState`. diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs index 2558dbfee86a..a0fef2c98428 100644 --- a/mullvad-daemon/src/api.rs +++ b/mullvad-daemon/src/api.rs @@ -609,7 +609,7 @@ impl AccessModeSelector { ApiConnectionMode::Proxied(ProxyConfig::from(proxy)) } AccessMethod::BuiltIn(BuiltInAccessMethod::EncryptedDnsProxy) => { - if let Err(error) = encrypted_dns_proxy_cache.fetch_configs().await { + if let Err(error) = encrypted_dns_proxy_cache.fetch_configs("frakta.eu").await { log::warn!("Failed to fetch new Encrypted DNS Proxy configurations"); log::debug!("{error:#?}"); } diff --git a/mullvad-encrypted-dns-proxy/src/config_resolver.rs b/mullvad-encrypted-dns-proxy/src/config_resolver.rs index b763183a1b1e..96aa64e938a2 100644 --- a/mullvad-encrypted-dns-proxy/src/config_resolver.rs +++ b/mullvad-encrypted-dns-proxy/src/config_resolver.rs @@ -61,8 +61,9 @@ pub fn default_resolvers() -> Vec { ] } -pub async fn resolve_default_config() -> Result, Error> { - resolve_configs(&default_resolvers(), "frakta.eu").await +pub async fn resolve_default_config(domain: &str) -> Result, Error> { + // TODO: We should remove the default value here and just force the callers to provide a domain instead + resolve_configs(&default_resolvers(), domain).await } /// Look up the `domain` towards the given `resolvers`, and try to deserialize all the returned diff --git a/mullvad-encrypted-dns-proxy/src/state.rs b/mullvad-encrypted-dns-proxy/src/state.rs index daad7123beea..3d6a26a0ce09 100644 --- a/mullvad-encrypted-dns-proxy/src/state.rs +++ b/mullvad-encrypted-dns-proxy/src/state.rs @@ -59,8 +59,8 @@ impl EncryptedDnsProxyState { } /// Fetch a config, but error out only when no existing configuration was there. - pub async fn fetch_configs(&mut self) -> Result<(), FetchConfigError> { - match resolve_default_config().await { + pub async fn fetch_configs(&mut self, domain: &str) -> Result<(), FetchConfigError> { + match resolve_default_config(domain).await { Ok(new_configs) => { self.configurations = HashSet::from_iter(new_configs.into_iter()); } diff --git a/mullvad-ios/src/encrypted_dns_proxy.rs b/mullvad-ios/src/encrypted_dns_proxy.rs index cf4219897e18..d371044d48d0 100644 --- a/mullvad-ios/src/encrypted_dns_proxy.rs +++ b/mullvad-ios/src/encrypted_dns_proxy.rs @@ -1,5 +1,6 @@ use crate::ProxyHandle; +use libc::c_char; use mullvad_encrypted_dns_proxy::state::{EncryptedDnsProxyState as State, FetchConfigError}; use mullvad_encrypted_dns_proxy::Forwarder; use std::{ @@ -9,10 +10,13 @@ use std::{ }; use tokio::{net::TcpListener, task::JoinHandle}; +use std::ffi::CStr; + /// A thin wrapper around [`mullvad_encrypted_dns_proxy::state::EncryptedDnsProxyState`] that /// can start a local forwarder (see [`Self::start`]). pub struct EncryptedDnsProxyState { state: State, + domain: String, } #[derive(Debug)] @@ -47,7 +51,7 @@ impl From for i32 { impl EncryptedDnsProxyState { async fn start(&mut self) -> Result { self.state - .fetch_configs() + .fetch_configs(&self.domain) .await .map_err(Error::FetchConfig)?; let proxy_configuration = self.state.next_configuration().ok_or(Error::NoConfigs)?; @@ -79,9 +83,17 @@ impl EncryptedDnsProxyState { /// Initializes a valid pointer to an instance of `EncryptedDnsProxyState`. #[no_mangle] -pub unsafe extern "C" fn encrypted_dns_proxy_init() -> *mut EncryptedDnsProxyState { +pub unsafe extern "C" fn encrypted_dns_proxy_init( + domain_name: *const c_char, +) -> *mut EncryptedDnsProxyState { + let domain = unsafe { + let c_str = CStr::from_ptr(domain_name); + String::from_utf8_lossy(c_str.to_bytes()) + }; + let state = Box::new(EncryptedDnsProxyState { state: State::default(), + domain: domain.into_owned(), }); Box::into_raw(state) } From 954d6f0abb42762991b20890ca9b572b9117f2fa Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Thu, 7 Nov 2024 09:36:42 +0100 Subject: [PATCH 5/8] Add encrypted DNS proxy domain as part of config --- ios/Configurations/Api.xcconfig.template | 5 +++++ ios/MullvadREST/ApiHandlers/RESTDefaults.swift | 2 ++ ios/MullvadREST/Info.plist | 2 ++ .../Transport/EncryptedDNS/EncryptedDNSTransport.swift | 2 +- ios/MullvadRustRuntime/EncryptedDNSProxy.swift | 6 ++++-- ios/MullvadVPNTests/Info.plist | 2 ++ ios/MullvadVPNUITests/Info.plist | 2 ++ 7 files changed, 18 insertions(+), 3 deletions(-) diff --git a/ios/Configurations/Api.xcconfig.template b/ios/Configurations/Api.xcconfig.template index 8d3b16010917..0fb8b09e9007 100644 --- a/ios/Configurations/Api.xcconfig.template +++ b/ios/Configurations/Api.xcconfig.template @@ -8,6 +8,11 @@ API_HOST_NAME[config=Release] = api.$(HOST_NAME) API_HOST_NAME[config=MockRelease] = api.$(HOST_NAME) API_HOST_NAME[config=Staging] = api.$(HOST_NAME) +ENCRYPTED_DNS_HOST_NAME[config=Debug] = frakta.eu +ENCRYPTED_DNS_HOST_NAME[config=Release] = frakta.eu +ENCRYPTED_DNS_HOST_NAME[config=MockRelease] = frakta.eu +ENCRYPTED_DNS_HOST_NAME[config=Staging] = stagemole.frakta.eu + API_ENDPOINT[config=Debug] = 45.83.223.196:443 API_ENDPOINT[config=Release] = 45.83.223.196:443 API_ENDPOINT[config=MockRelease] = 45.83.223.196:443 diff --git a/ios/MullvadREST/ApiHandlers/RESTDefaults.swift b/ios/MullvadREST/ApiHandlers/RESTDefaults.swift index 1f59b10f9233..8875096931e3 100644 --- a/ios/MullvadREST/ApiHandlers/RESTDefaults.swift +++ b/ios/MullvadREST/ApiHandlers/RESTDefaults.swift @@ -21,6 +21,8 @@ extension REST { /// Default API endpoint. public static let defaultAPIEndpoint = AnyIPEndpoint(string: infoDictionary["ApiEndpoint"] as! String)! + public static let encryptedDNSHostname = infoDictionary["EncryptedDnsHostName"] as! String + /// Disables API IP address cache when in staging environment and sticks to using default API endpoint instead. public static let isStagingEnvironment = false diff --git a/ios/MullvadREST/Info.plist b/ios/MullvadREST/Info.plist index b66a823a4074..644beb120a8f 100644 --- a/ios/MullvadREST/Info.plist +++ b/ios/MullvadREST/Info.plist @@ -6,5 +6,7 @@ $(API_HOST_NAME) ApiEndpoint $(API_ENDPOINT) + EncryptedDnsHostName + $(ENCRYPTED_DNS_HOST_NAME) diff --git a/ios/MullvadREST/Transport/EncryptedDNS/EncryptedDNSTransport.swift b/ios/MullvadREST/Transport/EncryptedDNS/EncryptedDNSTransport.swift index 230743bdfa8b..2910a14eed96 100644 --- a/ios/MullvadREST/Transport/EncryptedDNS/EncryptedDNSTransport.swift +++ b/ios/MullvadREST/Transport/EncryptedDNS/EncryptedDNSTransport.swift @@ -22,7 +22,7 @@ public final class EncryptedDNSTransport: RESTTransport { public init(urlSession: URLSession) { self.urlSession = urlSession - self.encryptedDnsProxy = EncryptedDNSProxy() + self.encryptedDnsProxy = EncryptedDNSProxy(domain: REST.encryptedDNSHostname) } public func stop() { diff --git a/ios/MullvadRustRuntime/EncryptedDNSProxy.swift b/ios/MullvadRustRuntime/EncryptedDNSProxy.swift index 5179cbc51b97..85c605fac5db 100644 --- a/ios/MullvadRustRuntime/EncryptedDNSProxy.swift +++ b/ios/MullvadRustRuntime/EncryptedDNSProxy.swift @@ -18,9 +18,11 @@ public class EncryptedDNSProxy { private var stateLock = NSLock() private var didStart = false private let state: OpaquePointer + private let domain: String - public init() { - state = encrypted_dns_proxy_init("frakta.eu") + public init(domain: String) { + self.domain = domain + state = encrypted_dns_proxy_init(domain) proxyConfig = ProxyHandle(context: nil, port: 0) } diff --git a/ios/MullvadVPNTests/Info.plist b/ios/MullvadVPNTests/Info.plist index 8eb7b6a836da..bd4375ba323a 100644 --- a/ios/MullvadVPNTests/Info.plist +++ b/ios/MullvadVPNTests/Info.plist @@ -2,6 +2,8 @@ + EncryptedDnsHostName + $(ENCRYPTED_DNS_HOST_NAME) ApiHostName $(API_HOST_NAME) ApiEndpoint diff --git a/ios/MullvadVPNUITests/Info.plist b/ios/MullvadVPNUITests/Info.plist index e01e57495e19..229e9483278b 100644 --- a/ios/MullvadVPNUITests/Info.plist +++ b/ios/MullvadVPNUITests/Info.plist @@ -10,6 +10,8 @@ $(API_ENDPOINT) ApiHostName $(API_HOST_NAME) + EncryptedDnsHostName + $(ENCRYPTED_DNS_HOST_NAME) AttachAppLogsOnFailure $(ATTACH_APP_LOGS_ON_FAILURE) DisplayName From e6fe960ac7e7e9087efc0e82a61c16c2cea107fe Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Mon, 11 Nov 2024 10:06:14 +0100 Subject: [PATCH 6/8] Improve rust documentation --- .../include/mullvad_rust_runtime.h | 9 +++++++++ mullvad-encrypted-dns-proxy/src/config_resolver.rs | 4 ++-- mullvad-encrypted-dns-proxy/src/state.rs | 2 +- mullvad-ios/src/encrypted_dns_proxy.rs | 14 ++++++++++++-- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h b/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h index ad13d1f6e086..a45c6ed6c328 100644 --- a/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h +++ b/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h @@ -33,6 +33,15 @@ extern const uint16_t CONFIG_SERVICE_PORT; /** * Initializes a valid pointer to an instance of `EncryptedDnsProxyState`. + * + * # Safety + * + * * [domain_name] must not be non-null. + * + * * [domain_name] pointer must be [valid](core::ptr#safety) + * + * * The caller must ensure that the pointer to the [domain_name] string contains a nul terminator + * at the end of the string. */ struct EncryptedDnsProxyState *encrypted_dns_proxy_init(const char *domain_name); diff --git a/mullvad-encrypted-dns-proxy/src/config_resolver.rs b/mullvad-encrypted-dns-proxy/src/config_resolver.rs index 96aa64e938a2..82edd886f529 100644 --- a/mullvad-encrypted-dns-proxy/src/config_resolver.rs +++ b/mullvad-encrypted-dns-proxy/src/config_resolver.rs @@ -61,12 +61,12 @@ pub fn default_resolvers() -> Vec { ] } +/// Calls [resolve_configs] with a given `domain` using known DoH resolvers provided by [default_resolvers] pub async fn resolve_default_config(domain: &str) -> Result, Error> { - // TODO: We should remove the default value here and just force the callers to provide a domain instead resolve_configs(&default_resolvers(), domain).await } -/// Look up the `domain` towards the given `resolvers`, and try to deserialize all the returned +/// Looks up the `domain` towards the given `resolvers`, and try to deserialize all the returned /// AAAA records into [`ProxyConfig`](config::ProxyConfig)s. pub async fn resolve_configs( resolvers: &[Nameserver], diff --git a/mullvad-encrypted-dns-proxy/src/state.rs b/mullvad-encrypted-dns-proxy/src/state.rs index 3d6a26a0ce09..8b6c3c988627 100644 --- a/mullvad-encrypted-dns-proxy/src/state.rs +++ b/mullvad-encrypted-dns-proxy/src/state.rs @@ -58,7 +58,7 @@ impl EncryptedDnsProxyState { Some(selected_config) } - /// Fetch a config, but error out only when no existing configuration was there. + /// Fetch a config from `domain`, but error out only when no existing configuration was there. pub async fn fetch_configs(&mut self, domain: &str) -> Result<(), FetchConfigError> { match resolve_default_config(domain).await { Ok(new_configs) => { diff --git a/mullvad-ios/src/encrypted_dns_proxy.rs b/mullvad-ios/src/encrypted_dns_proxy.rs index d371044d48d0..3c89faf89502 100644 --- a/mullvad-ios/src/encrypted_dns_proxy.rs +++ b/mullvad-ios/src/encrypted_dns_proxy.rs @@ -82,12 +82,22 @@ impl EncryptedDnsProxyState { } /// Initializes a valid pointer to an instance of `EncryptedDnsProxyState`. +/// +/// # Safety +/// +/// * [domain_name] must not be non-null. +/// +/// * [domain_name] pointer must be [valid](core::ptr#safety) +/// +/// * The caller must ensure that the pointer to the [domain_name] string contains a nul terminator +/// at the end of the string. #[no_mangle] pub unsafe extern "C" fn encrypted_dns_proxy_init( domain_name: *const c_char, ) -> *mut EncryptedDnsProxyState { - let domain = unsafe { - let c_str = CStr::from_ptr(domain_name); + // SAFETY: domain_name points to a valid region of memory and contains a nul terminator. + let domain = { + let c_str = unsafe { CStr::from_ptr(domain_name) }; String::from_utf8_lossy(c_str.to_bytes()) }; From 0dfccf7cae9fd6107633b82bf74efffb04d5189e Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Mon, 11 Nov 2024 15:23:09 +0100 Subject: [PATCH 7/8] Disable testAppStillFunctioningWhenAPIDown because of ATS in iOS 18 --- ios/MullvadVPNUITests/ConnectivityTests.swift | 6 ++++++ mullvad-ios/src/encrypted_dns_proxy.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ios/MullvadVPNUITests/ConnectivityTests.swift b/ios/MullvadVPNUITests/ConnectivityTests.swift index a1a49ffd8e23..6b66a2d5142c 100644 --- a/ios/MullvadVPNUITests/ConnectivityTests.swift +++ b/ios/MullvadVPNUITests/ConnectivityTests.swift @@ -143,6 +143,12 @@ class ConnectivityTests: LoggedOutUITestCase { // swiftlint:disable function_body_length /// Test that the app is functioning when API is down. To simulate API being down we create a dummy access method func testAppStillFunctioningWhenAPIDown() throws { + let skipReason = """ + This test is currently skipped due to a bug in iOS 18 where ATS shuts down the + connection to the API in the blocked state, despite being explicitly disabled, + and after the checks in SSLPinningURLSessionDelegate return no error. + """ + try XCTSkipIf(true, skipReason) let hasTimeAccountNumber = getAccountWithTime() addTeardownBlock { diff --git a/mullvad-ios/src/encrypted_dns_proxy.rs b/mullvad-ios/src/encrypted_dns_proxy.rs index 3c89faf89502..2aa83d833dc0 100644 --- a/mullvad-ios/src/encrypted_dns_proxy.rs +++ b/mullvad-ios/src/encrypted_dns_proxy.rs @@ -95,8 +95,8 @@ impl EncryptedDnsProxyState { pub unsafe extern "C" fn encrypted_dns_proxy_init( domain_name: *const c_char, ) -> *mut EncryptedDnsProxyState { - // SAFETY: domain_name points to a valid region of memory and contains a nul terminator. let domain = { + // SAFETY: domain_name points to a valid region of memory and contains a nul terminator. let c_str = unsafe { CStr::from_ptr(domain_name) }; String::from_utf8_lossy(c_str.to_bytes()) }; From 93fcbf1168e78343e5fbca6b57b52c6838060992 Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Tue, 19 Nov 2024 10:30:51 +0100 Subject: [PATCH 8/8] Fix typo --- ios/MullvadVPN/TransportMonitor/TransportMonitor.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift b/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift index e4aee9cbe007..57d1340fa486 100644 --- a/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift +++ b/ios/MullvadVPN/TransportMonitor/TransportMonitor.swift @@ -37,14 +37,14 @@ final class TransportMonitor: RESTTransportProvider { tunnel.status == .connecting || tunnel.status == .reasserting || tunnel.status == .connected } - if let tunnel, shouldByPassVPN(tunnel: tunnel) { + if let tunnel, shouldBypassVPN(tunnel: tunnel) { return PacketTunnelTransport(tunnel: tunnel) } else { return transportProvider.makeTransport() } } - private func shouldByPassVPN(tunnel: any TunnelProtocol) -> Bool { + private func shouldBypassVPN(tunnel: any TunnelProtocol) -> Bool { switch tunnel.status { case .connected: if case .error = tunnelManager.tunnelStatus.state {