From 0db3e36bf4a05db0d0fd72fbbb623e6b74d78e76 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 14 Apr 2022 07:24:03 +0200 Subject: [PATCH] Make network settings consistent - Group DNS "Enabled" and protocol into configuration - Make DNS servers / search domains optional - Make proxy bypass domains optional Also refine a comment about future on-demand. --- Passepartout/App/Shared/L10n/Core+L10n.swift | 18 ++++++++ .../App/Shared/L10n/TunnelKit+L10n.swift | 15 ------- .../App/iOS/Views/NetworkSettingsView.swift | 41 ++++++++++--------- .../Extensions/OnDemand+Rules.swift | 6 ++- .../OpenVPNSettings+VPNConfiguration.swift | 24 ++++++++--- .../WireGuardSettings+VPNConfiguration.swift | 13 ++++-- .../Managers/AppManager+Migrations.swift | 2 +- .../Managers/VPNManager+Configuration.swift | 8 +--- .../DataModels/Network.swift | 34 ++++++++++----- .../Extensions/OpenVPNSettings+Network.swift | 22 +++++----- ...ssepartoutProfiles+StrippableContent.swift | 6 +-- .../WireGuardSettings+Network.swift | 6 +-- .../Pickers/Picker+Network.swift | 6 +-- 13 files changed, 117 insertions(+), 84 deletions(-) diff --git a/Passepartout/App/Shared/L10n/Core+L10n.swift b/Passepartout/App/Shared/L10n/Core+L10n.swift index 557207f4..6dd595d3 100644 --- a/Passepartout/App/Shared/L10n/Core+L10n.swift +++ b/Passepartout/App/Shared/L10n/Core+L10n.swift @@ -105,6 +105,24 @@ extension Network.Choice { } } +extension Network.DNSSettings.ConfigurationType { + var localizedDescription: String { + switch self { + case .plain: + return Unlocalized.DNS.plain + + case .https: + return Unlocalized.Network.https + + case .tls: + return Unlocalized.Network.tls + + case .disabled: + return L10n.Global.Strings.disabled + } + } +} + extension Network.ProxySettings.ConfigurationType { var localizedDescription: String { switch self { diff --git a/Passepartout/App/Shared/L10n/TunnelKit+L10n.swift b/Passepartout/App/Shared/L10n/TunnelKit+L10n.swift index d85fcebd..da4359ef 100644 --- a/Passepartout/App/Shared/L10n/TunnelKit+L10n.swift +++ b/Passepartout/App/Shared/L10n/TunnelKit+L10n.swift @@ -48,21 +48,6 @@ extension VPNStatus { } } -extension DNSProtocol { - var localizedDescription: String { - switch self { - case .plain: - return Unlocalized.DNS.plain - - case .https: - return Unlocalized.Network.https - - case .tls: - return Unlocalized.Network.tls - } - } -} - extension DataCount { var localizedDescription: String { let down = received.descriptionAsDataUnit diff --git a/Passepartout/App/iOS/Views/NetworkSettingsView.swift b/Passepartout/App/iOS/Views/NetworkSettingsView.swift index e0367d43..83373def 100644 --- a/Passepartout/App/iOS/Views/NetworkSettingsView.swift +++ b/Passepartout/App/iOS/Views/NetworkSettingsView.swift @@ -122,29 +122,30 @@ extension NetworkSettingsView { Toggle(L10n.Global.Strings.automatic, isOn: $settings.isAutomaticDNS.animation()) if !settings.isAutomaticDNS { - Toggle(L10n.Global.Strings.enabled, isOn: $settings.dns.isDNSEnabled.animation()) + themeTextPicker( + // FIXME: l10n, refactor string id to "global.strings.configuration" + L10n.Profile.Sections.Configuration.header, + selection: $settings.dns.configurationType, + values: Network.DNSSettings.availableConfigurationTypes(forVPNProtocol: vpnProtocol), + description: \.localizedDescription + ) - if settings.dns.isDNSEnabled { - themeTextPicker( - L10n.Global.Strings.protocol, - selection: $settings.dns.dnsProtocol, - values: Network.DNSSettings.availableProtocols(forVPNProtocol: vpnProtocol), - description: \.localizedDescription - ) - switch settings.dns.dnsProtocol { - case .plain: - EmptyView() + switch settings.dns.configurationType { + case .plain: + EmptyView() - case .https: - dnsManualHTTPSRow + case .https: + dnsManualHTTPSRow - case .tls: - dnsManualTLSRow - } + case .tls: + dnsManualTLSRow + + case .disabled: + EmptyView() } } } - if !settings.isAutomaticDNS && settings.dns.isDNSEnabled { + if !settings.isAutomaticDNS && settings.dns.configurationType != .disabled { dnsManualServers dnsManualDomains } @@ -163,7 +164,7 @@ extension NetworkSettingsView { private var dnsManualServers: some View { Section { EditableTextList( - elements: $settings.dns.dnsServers, + elements: $settings.dns.dnsServers ?? [], allowsDuplicates: false, mapping: mapNotEmpty ) { @@ -184,7 +185,7 @@ extension NetworkSettingsView { private var dnsManualDomains: some View { Section { EditableTextList( - elements: $settings.dns.dnsSearchDomains, + elements: $settings.dns.dnsSearchDomains ?? [], allowsDuplicates: false, mapping: mapNotEmpty ) { @@ -250,7 +251,7 @@ extension NetworkSettingsView { private var proxyManualBypassDomains: some View { Section { EditableTextList( - elements: $settings.proxy.proxyBypassDomains, + elements: $settings.proxy.proxyBypassDomains ?? [], allowsDuplicates: false, mapping: mapNotEmpty ) { diff --git a/PassepartoutCore/Sources/PassepartoutCore/Extensions/OnDemand+Rules.swift b/PassepartoutCore/Sources/PassepartoutCore/Extensions/OnDemand+Rules.swift index 58039db2..2b329fb0 100644 --- a/PassepartoutCore/Sources/PassepartoutCore/Extensions/OnDemand+Rules.swift +++ b/PassepartoutCore/Sources/PassepartoutCore/Extensions/OnDemand+Rules.swift @@ -29,9 +29,11 @@ import NetworkExtension extension Profile.OnDemand { func rules(withCustomRules: Bool) -> [NEOnDemandRule] { - // TODO: on-demand, drop when "trusted networks" -> "on-demand" + // TODO: on-demand, drop hardcoding when "trusted networks" -> "on-demand" +// isEnabled = true +// policy = .excluding assert(policy == .excluding) - + var rules: [NEOnDemandRule] = [] if withCustomRules { #if os(iOS) diff --git a/PassepartoutCore/Sources/PassepartoutCore/Extensions/OpenVPNSettings+VPNConfiguration.swift b/PassepartoutCore/Sources/PassepartoutCore/Extensions/OpenVPNSettings+VPNConfiguration.swift index 4fe77781..8801169b 100644 --- a/PassepartoutCore/Sources/PassepartoutCore/Extensions/OpenVPNSettings+VPNConfiguration.swift +++ b/PassepartoutCore/Sources/PassepartoutCore/Extensions/OpenVPNSettings+VPNConfiguration.swift @@ -97,13 +97,27 @@ extension OpenVPN.ConfigurationBuilder { break case .manual: - isDNSEnabled = settings.isDNSEnabled + let isDNSEnabled = settings.configurationType != .disabled + self.isDNSEnabled = isDNSEnabled - if settings.isDNSEnabled { - dnsProtocol = settings.dnsProtocol - dnsServers = settings.dnsServers.filter { !$0.isEmpty } + switch settings.configurationType { + case .plain: + dnsProtocol = .plain + + case .https: + dnsProtocol = .https dnsHTTPSURL = settings.dnsHTTPSURL + + case .tls: + dnsProtocol = .tls dnsTLSServerName = settings.dnsTLSServerName + + case .disabled: + break + } + + if isDNSEnabled { + dnsServers = settings.dnsServers?.filter { !$0.isEmpty } searchDomains = settings.dnsSearchDomains } } @@ -121,7 +135,7 @@ extension OpenVPN.ConfigurationBuilder { case .manual: httpProxy = settings.proxyServer httpsProxy = settings.proxyServer - proxyBypassDomains = settings.proxyBypassDomains.filter { !$0.isEmpty } + proxyBypassDomains = settings.proxyBypassDomains?.filter { !$0.isEmpty } proxyAutoConfigurationURL = nil case .pac: diff --git a/PassepartoutCore/Sources/PassepartoutCore/Extensions/WireGuardSettings+VPNConfiguration.swift b/PassepartoutCore/Sources/PassepartoutCore/Extensions/WireGuardSettings+VPNConfiguration.swift index e5683c1d..a576de5d 100644 --- a/PassepartoutCore/Sources/PassepartoutCore/Extensions/WireGuardSettings+VPNConfiguration.swift +++ b/PassepartoutCore/Sources/PassepartoutCore/Extensions/WireGuardSettings+VPNConfiguration.swift @@ -89,12 +89,17 @@ extension WireGuard.ConfigurationBuilder { break case .manual: - if settings.isDNSEnabled { - dnsServers = settings.dnsServers - dnsSearchDomains = settings.dnsSearchDomains.filter { !$0.isEmpty } - } else { + switch settings.configurationType { + case .plain: + dnsServers = settings.dnsServers ?? [] + dnsSearchDomains = settings.dnsSearchDomains?.filter { !$0.isEmpty } ?? [] + + case .disabled: dnsServers = [] dnsSearchDomains = [] + + default: + fatalError("Invalid DNS configuration for WireGuard: \(settings.configurationType)") } } } diff --git a/PassepartoutCore/Sources/PassepartoutCore/Managers/AppManager+Migrations.swift b/PassepartoutCore/Sources/PassepartoutCore/Managers/AppManager+Migrations.swift index 59f7ca2c..fd96f99a 100644 --- a/PassepartoutCore/Sources/PassepartoutCore/Managers/AppManager+Migrations.swift +++ b/PassepartoutCore/Sources/PassepartoutCore/Managers/AppManager+Migrations.swift @@ -288,7 +288,7 @@ extension AppManager { // dns (manual["dnsProtocol"] as? String).map { - settings.dns.dnsProtocol = DNSProtocol(rawValue: $0) ?? .plain + settings.dns.configurationType = .init(rawValue: $0) ?? .plain } settings.dns.dnsServers = manual["dnsServers"] as? [String] ?? [] settings.dns.dnsSearchDomains = manual["dnsSearchDomains"] as? [String] ?? [] diff --git a/PassepartoutCore/Sources/PassepartoutCore/Managers/VPNManager+Configuration.swift b/PassepartoutCore/Sources/PassepartoutCore/Managers/VPNManager+Configuration.swift index 5bc5f633..d0c7a309 100644 --- a/PassepartoutCore/Sources/PassepartoutCore/Managers/VPNManager+Configuration.swift +++ b/PassepartoutCore/Sources/PassepartoutCore/Managers/VPNManager+Configuration.swift @@ -58,8 +58,6 @@ extension VPNManager { withCustomRules: isOnDemandRulesSupported() ) - var cfg: VPNConfiguration - switch profile.currentVPNProtocol { case .openVPN: let settings: Profile.OpenVPNSettings @@ -71,7 +69,7 @@ extension VPNManager { } settings = hostSettings } - cfg = try settings.vpnConfiguration(parameters) + return try settings.vpnConfiguration(parameters) case .wireGuard: let settings: Profile.WireGuardSettings @@ -83,10 +81,8 @@ extension VPNManager { } settings = hostSettings } - cfg = try settings.vpnConfiguration(parameters) + return try settings.vpnConfiguration(parameters) } - - return cfg } catch { pp_log.error("Unable to build VPNConfiguration: \(error)") diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/DataModels/Network.swift b/PassepartoutCore/Sources/PassepartoutProfiles/DataModels/Network.swift index aadec6dc..93cfb2de 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/DataModels/Network.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/DataModels/Network.swift @@ -51,21 +51,21 @@ public protocol GatewaySettingsProviding { } public protocol DNSSettingsProviding { - var dnsProtocol: DNSProtocol { get } + var dnsProtocol: DNSProtocol? { get } - var dnsServers: [String] { get } + var dnsServers: [String]? { get } + var dnsSearchDomains: [String]? { get } + var dnsHTTPSURL: URL? { get } var dnsTLSServerName: String? { get } - - var dnsSearchDomains: [String] { get } } public protocol ProxySettingsProviding { var proxyServer: Proxy? { get } - var proxyBypassDomains: [String] { get } + var proxyBypassDomains: [String]? { get } var proxyAutoConfigurationURL: URL? { get } } @@ -88,15 +88,27 @@ extension Network { extension Network { public struct DNSSettings: Codable, Equatable, NetworkChoiceRepresentable, DNSSettingsProviding { + public enum ConfigurationType: String, Codable { + case plain + + case https + + case tls + + case disabled + } + public var choice: Network.Choice - public var isDNSEnabled = true - - public var dnsProtocol: DNSProtocol = .plain + public var configurationType: ConfigurationType = .plain - public var dnsServers: [String] = [] + public var dnsProtocol: DNSProtocol? { + DNSProtocol(rawValue: configurationType.rawValue) + } - public var dnsSearchDomains: [String] = [] + public var dnsServers: [String]? + + public var dnsSearchDomains: [String]? public var dnsHTTPSURL: URL? @@ -122,7 +134,7 @@ extension Network { public var proxyPort: UInt16? - public var proxyBypassDomains: [String] = [] + public var proxyBypassDomains: [String]? public var proxyAutoConfigurationURL: URL? diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/OpenVPNSettings+Network.swift b/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/OpenVPNSettings+Network.swift index 89dad833..9baf8767 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/OpenVPNSettings+Network.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/OpenVPNSettings+Network.swift @@ -43,18 +43,18 @@ extension Profile.OpenVPNSettings: GatewaySettingsProviding { extension Profile.OpenVPNSettings: DNSSettingsProviding { // not a dhcp-option - public var dnsProtocol: DNSProtocol { - return .plain + public var dnsProtocol: DNSProtocol? { + return (configuration.isDNSEnabled ?? true) ? .plain : nil } // dhcp-option DNS - public var dnsServers: [String] { - return configuration.dnsServers ?? [] + public var dnsServers: [String]? { + return configuration.dnsServers } // dhcp-option DOMAIN/DOMAIN-SEARCH - public var dnsSearchDomains: [String] { - return configuration.searchDomains ?? [] + public var dnsSearchDomains: [String]? { + return configuration.searchDomains } // not a dhcp-option @@ -75,15 +75,15 @@ extension Profile.OpenVPNSettings: ProxySettingsProviding { return configuration.httpsProxy ?? configuration.httpProxy } + // dhcp-option PROXY_BYPASS + public var proxyBypassDomains: [String]? { + return configuration.proxyBypassDomains + } + // dhcp-option PROXY_AUTO_CONFIG_URL public var proxyAutoConfigurationURL: URL? { return configuration.proxyAutoConfigurationURL } - - // dhcp-option PROXY_BYPASS - public var proxyBypassDomains: [String] { - return configuration.proxyBypassDomains ?? [] - } } extension Profile.OpenVPNSettings: MTUSettingsProviding { diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/PassepartoutProfiles+StrippableContent.swift b/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/PassepartoutProfiles+StrippableContent.swift index 0546bc41..e416641d 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/PassepartoutProfiles+StrippableContent.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/PassepartoutProfiles+StrippableContent.swift @@ -51,11 +51,11 @@ extension Profile.OnDemand: StrippableContent { extension Profile.NetworkSettings: StrippableContent { public var stripped: Self { var copy = self - copy.dns.dnsServers = copy.dns.dnsServers.compactMap(\.strippedNotEmpty) - copy.dns.dnsSearchDomains = copy.dns.dnsSearchDomains.compactMap(\.strippedNotEmpty) + copy.dns.dnsServers = copy.dns.dnsServers?.compactMap(\.strippedNotEmpty) + copy.dns.dnsSearchDomains = copy.dns.dnsSearchDomains?.compactMap(\.strippedNotEmpty) copy.dns.dnsTLSServerName = copy.dns.dnsTLSServerName?.strippedNotEmpty copy.proxy.proxyAddress = copy.proxy.proxyAddress?.strippedNotEmpty - copy.proxy.proxyBypassDomains = copy.proxy.proxyBypassDomains.compactMap(\.strippedNotEmpty) + copy.proxy.proxyBypassDomains = copy.proxy.proxyBypassDomains?.compactMap(\.strippedNotEmpty) return copy } } diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/WireGuardSettings+Network.swift b/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/WireGuardSettings+Network.swift index 6af3381d..de379824 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/WireGuardSettings+Network.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/Extensions/WireGuardSettings+Network.swift @@ -28,15 +28,15 @@ import TunnelKitCore import TunnelKitWireGuard extension Profile.WireGuardSettings: DNSSettingsProviding { - public var dnsProtocol: DNSProtocol { + public var dnsProtocol: DNSProtocol? { return .plain } - public var dnsServers: [String] { + public var dnsServers: [String]? { return configuration.dnsServers } - public var dnsSearchDomains: [String] { + public var dnsSearchDomains: [String]? { return configuration.dnsSearchDomains } diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/Pickers/Picker+Network.swift b/PassepartoutCore/Sources/PassepartoutProfiles/Pickers/Picker+Network.swift index b6669b9d..2b629f2b 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/Pickers/Picker+Network.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/Pickers/Picker+Network.swift @@ -28,13 +28,13 @@ import TunnelKitCore import PassepartoutProviders extension Network.DNSSettings { - public static func availableProtocols(forVPNProtocol vpnProtocol: VPNProtocolType) -> [DNSProtocol] { + public static func availableConfigurationTypes(forVPNProtocol vpnProtocol: VPNProtocolType) -> [ConfigurationType] { switch vpnProtocol { case .openVPN: - return [.plain, .https, .tls] + return [.plain, .https, .tls, .disabled] case .wireGuard: - return [.plain] + return [.plain, .disabled] } } }