From 703d1416ad0c7152e52703bbacc99ee30b9052b5 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 29 Oct 2022 11:00:39 +0200 Subject: [PATCH] Deal with remote options properly (#297) Some take over, some are merged. Also: - Drop non-existing DOMAIN-SEARCH dhcp-option - Only first DNS domain was parsed --- CHANGELOG.md | 2 +- Sources/TunnelKitCore/IPv4Settings.swift | 4 +- Sources/TunnelKitCore/IPv6Settings.swift | 4 +- .../NetworkSettingsBuilder.swift | 93 +++++++++++++------ .../ConfigurationParser.swift | 26 ++---- .../ConfigurationParserTests.swift | 6 +- 6 files changed, 80 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45b9f52..8014329 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- OpenVPN: Prioritize server configuration over client (standard behavior). +- OpenVPN: Deal with remote options properly. [#297](https://github.com/passepartoutvpn/tunnelkit/pull/297) - IPv6 endpoints are parsed improperly. [#293](https://github.com/passepartoutvpn/tunnelkit/issues/293) - Fix abandoned MockVPN. [#285](https://github.com/passepartoutvpn/tunnelkit/pull/285) diff --git a/Sources/TunnelKitCore/IPv4Settings.swift b/Sources/TunnelKitCore/IPv4Settings.swift index 759915a..574b48d 100644 --- a/Sources/TunnelKitCore/IPv4Settings.swift +++ b/Sources/TunnelKitCore/IPv4Settings.swift @@ -49,7 +49,7 @@ public struct IPv4Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - return "{\(destination.maskedDescription)/\(mask) \(gateway.maskedDescription)}" + "{\(destination.maskedDescription)/\(mask) \(gateway.maskedDescription)}" } } @@ -75,6 +75,6 @@ public struct IPv4Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - return "addr \(address.maskedDescription) netmask \(addressMask) gw \(defaultGateway.maskedDescription) routes \(routes.map(\.maskedDescription))" + "addr \(address.maskedDescription) netmask \(addressMask) gw \(defaultGateway.maskedDescription) routes \(routes.map(\.maskedDescription))" } } diff --git a/Sources/TunnelKitCore/IPv6Settings.swift b/Sources/TunnelKitCore/IPv6Settings.swift index 77dfda6..9cb073a 100644 --- a/Sources/TunnelKitCore/IPv6Settings.swift +++ b/Sources/TunnelKitCore/IPv6Settings.swift @@ -49,7 +49,7 @@ public struct IPv6Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - return "{\(destination.maskedDescription)/\(prefixLength) \(gateway.maskedDescription)}" + "{\(destination.maskedDescription)/\(prefixLength) \(gateway.maskedDescription)}" } } @@ -75,6 +75,6 @@ public struct IPv6Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - return "addr \(address.maskedDescription)/\(addressPrefixLength) gw \(defaultGateway.maskedDescription) routes \(routes.map(\.maskedDescription))" + "addr \(address.maskedDescription)/\(addressPrefixLength) gw \(defaultGateway.maskedDescription) routes \(routes.map(\.maskedDescription))" } } diff --git a/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift b/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift index 3cf3021..b5a9e16 100644 --- a/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift +++ b/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift @@ -103,40 +103,78 @@ extension NetworkSettingsBuilder { private var isIPv6Gateway: Bool { routingPolicies?.contains(.IPv6) ?? false } + + // FIXME: local routes are empty, localOptions.ipv4 is always nil (#278) + private var allRoutes4: [IPv4Settings.Route] { + var routes = localOptions.ipv4?.routes ?? [] + if pullRoutes, let remoteRoutes = remoteOptions.ipv4?.routes { + routes.append(contentsOf: remoteRoutes) + } + return routes + } + + // FIXME: local routes are empty, localOptions.ipv6 is always nil (#278) + private var allRoutes6: [IPv6Settings.Route] { + var routes = localOptions.ipv6?.routes ?? [] + if pullRoutes, let remoteRoutes = remoteOptions.ipv6?.routes { + routes.append(contentsOf: remoteRoutes) + } + return routes + } + + private var allDNSServers: [String] { + var servers = localOptions.dnsServers ?? [] + if pullDNS, let remoteServers = remoteOptions.dnsServers { + servers.append(contentsOf: remoteServers) + } + return servers + } + + private var allDNSSearchDomains: [String] { + var searchDomains = localOptions.searchDomains ?? [] + if pullDNS, let remoteSearchDomains = remoteOptions.searchDomains { + searchDomains.append(contentsOf: remoteSearchDomains) + } + return searchDomains + } + + private var allProxyBypassDomains: [String] { + var bypass = localOptions.proxyBypassDomains ?? [] + if pullProxy, let remoteBypass = remoteOptions.proxyBypassDomains { + bypass.append(contentsOf: remoteBypass) + } + return bypass + } } extension NetworkSettingsBuilder { // IPv4/6 address/mask MUST come from server options // routes, instead, can both come from server and local options - // - // FIXME: routes from local options are ignored (#278) private var computedIPv4Settings: NEIPv4Settings? { guard let ipv4 = remoteOptions.ipv4 else { return nil } let ipv4Settings = NEIPv4Settings(addresses: [ipv4.address], subnetMasks: [ipv4.addressMask]) - var routes: [NEIPv4Route] = [] + var neRoutes: [NEIPv4Route] = [] // route all traffic to VPN? if isIPv4Gateway { let defaultRoute = NEIPv4Route.default() defaultRoute.gatewayAddress = ipv4.defaultGateway - routes.append(defaultRoute) + neRoutes.append(defaultRoute) log.info("Routing.IPv4: Setting default gateway to \(ipv4.defaultGateway)") } - // FIXME: this is ineffective until #278 is fixed (localOptions.ipv4 is always nil) - let computedRoutes = (pullRoutes ? (remoteOptions.ipv4?.routes ?? localOptions.ipv4?.routes) : localOptions.ipv4?.routes) ?? [] - for r in computedRoutes { + for r in allRoutes4 { let ipv4Route = NEIPv4Route(destinationAddress: r.destination, subnetMask: r.mask) ipv4Route.gatewayAddress = r.gateway - routes.append(ipv4Route) + neRoutes.append(ipv4Route) log.info("Routing.IPv4: Adding route \(r.destination)/\(r.mask) -> \(r.gateway)") } - ipv4Settings.includedRoutes = routes + ipv4Settings.includedRoutes = neRoutes ipv4Settings.excludedRoutes = [] return ipv4Settings } @@ -146,26 +184,24 @@ extension NetworkSettingsBuilder { return nil } let ipv6Settings = NEIPv6Settings(addresses: [ipv6.address], networkPrefixLengths: [ipv6.addressPrefixLength as NSNumber]) - var routes: [NEIPv6Route] = [] + var neRoutes: [NEIPv6Route] = [] // route all traffic to VPN? if isIPv6Gateway { let defaultRoute = NEIPv6Route.default() defaultRoute.gatewayAddress = ipv6.defaultGateway - routes.append(defaultRoute) + neRoutes.append(defaultRoute) log.info("Routing.IPv6: Setting default gateway to \(ipv6.defaultGateway)") } - // FIXME: this is ineffective until #278 is fixed (localOptions.ipv6 is always nil) - let computedRoutes = (pullRoutes ? (remoteOptions.ipv6?.routes ?? localOptions.ipv6?.routes) : localOptions.ipv6?.routes) ?? [] - for r in computedRoutes { + for r in allRoutes6 { let ipv6Route = NEIPv6Route(destinationAddress: r.destination, networkPrefixLength: r.prefixLength as NSNumber) ipv6Route.gatewayAddress = r.gateway - routes.append(ipv6Route) + neRoutes.append(ipv6Route) log.info("Routing.IPv6: Adding route \(r.destination)/\(r.prefixLength) -> \(r.gateway)") } - ipv6Settings.includedRoutes = routes + ipv6Settings.includedRoutes = neRoutes ipv6Settings.excludedRoutes = [] return ipv6Settings } @@ -188,11 +224,10 @@ extension NetworkSettingsBuilder { return nil } var dnsSettings: NEDNSSettings? - var dnsServers: [String] = [] if #available(iOS 14, macOS 11, *) { switch localOptions.dnsProtocol { case .https: - dnsServers = localOptions.dnsServers ?? [] + let dnsServers = localOptions.dnsServers ?? [] guard let serverURL = localOptions.dnsHTTPSURL else { break } @@ -203,7 +238,7 @@ extension NetworkSettingsBuilder { log.info("\tHTTPS URL: \(serverURL)") case .tls: - dnsServers = localOptions.dnsServers ?? [] + let dnsServers = localOptions.dnsServers ?? [] guard let serverName = localOptions.dnsTLSServerName else { break } @@ -220,13 +255,13 @@ extension NetworkSettingsBuilder { // fall back if dnsSettings == nil { - dnsServers = (pullDNS ? (remoteOptions.dnsServers ?? localOptions.dnsServers) : localOptions.dnsServers) ?? [] + let dnsServers = allDNSServers if !dnsServers.isEmpty { log.info("DNS: Using servers \(dnsServers)") dnsSettings = NEDNSSettings(servers: dnsServers) } else { -// log.warning("DNS: No servers provided, using fall-back servers: \(fallbackDNSServers)") -// dnsSettings = NEDNSSettings(servers: fallbackDNSServers) +// log.warning("DNS: No servers provided, using fall-back servers: \(fallbackDNSServers)") +// dnsSettings = NEDNSSettings(servers: fallbackDNSServers) if isGateway { log.warning("DNS: No settings provided") } else { @@ -239,8 +274,9 @@ extension NetworkSettingsBuilder { if !isGateway { dnsSettings?.matchDomains = [""] } - - if let searchDomains = pullDNS ? (remoteOptions.searchDomains ?? localOptions.searchDomains) : localOptions.searchDomains { + + let searchDomains = allDNSSearchDomains + if !searchDomains.isEmpty { log.info("DNS: Using search domains \(searchDomains)") dnsSettings?.domainName = searchDomains.first dnsSettings?.searchDomains = searchDomains @@ -283,9 +319,12 @@ extension NetworkSettingsBuilder { } // only set if there is a proxy (proxySettings set to non-nil above) - if let bypass = pullProxy ? (remoteOptions.proxyBypassDomains ?? localOptions.proxyBypassDomains) : localOptions.proxyBypassDomains { - proxySettings?.exceptionList = bypass - log.info("Routing: Setting proxy by-pass list: \(bypass)") + if proxySettings != nil { + let bypass = allProxyBypassDomains + if !bypass.isEmpty { + proxySettings?.exceptionList = bypass + log.info("Routing: Setting proxy by-pass list: \(bypass)") + } } return proxySettings } diff --git a/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift b/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift index 0bb59b5..812cbb1 100644 --- a/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift +++ b/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift @@ -113,8 +113,6 @@ extension OpenVPN { static let domain = NSRegularExpression("^dhcp-option +DOMAIN +[^ ]+") - static let domainSearch = NSRegularExpression("^dhcp-option +DOMAIN-SEARCH +[^ ]+") - static let proxy = NSRegularExpression("^dhcp-option +PROXY_(HTTPS? +[^ ]+ +\\d+|AUTO_CONFIG_URL +[^ ]+)") static let proxyBypass = NSRegularExpression("^dhcp-option +PROXY_BYPASS +.+") @@ -289,7 +287,6 @@ extension OpenVPN { var optRoutes4: [(String, String, String?)] = [] // address, netmask, gateway var optRoutes6: [(String, UInt8, String?)] = [] // destination, prefix, gateway var optDNSServers: [String]? - var optDomain: String? var optSearchDomains: [String]? var optHTTPProxy: Proxy? var optHTTPSProxy: Proxy? @@ -655,12 +652,6 @@ extension OpenVPN { optDNSServers?.append($0[1]) } Regex.domain.enumerateSpacedArguments(in: line) { - guard $0.count == 2 else { - return - } - optDomain = $0[1] - } - Regex.domainSearch.enumerateSpacedArguments(in: line) { guard $0.count == 2 else { return } @@ -734,15 +725,6 @@ extension OpenVPN { // MARK: Post-processing - // prepend search domains with main domain (if set) - if let domain = optDomain { - if optSearchDomains == nil { - optSearchDomains = [domain] - } else { - optSearchDomains?.insert(domain, at: 0) - } - } - // ensure that non-nil network settings also imply non-empty if let array = optDNSServers { assert(!array.isEmpty) @@ -875,7 +857,9 @@ extension OpenVPN { addressMask4 = "255.255.255.255" defaultGateway4 = ifconfig4Arguments[1] } - let routes4 = optRoutes4.map { IPv4Settings.Route($0.0, $0.1, $0.2 ?? defaultGateway4) } + let routes4 = optRoutes4.map { + IPv4Settings.Route($0.0, $0.1, $0.2 ?? defaultGateway4) + } sessionBuilder.ipv4 = IPv4Settings( address: address4, @@ -899,7 +883,9 @@ extension OpenVPN { let address6 = address6Components[0] let defaultGateway6 = ifconfig6Arguments[1] - let routes6 = optRoutes6.map { IPv6Settings.Route($0.0, $0.1, $0.2 ?? defaultGateway6) } + let routes6 = optRoutes6.map { + IPv6Settings.Route($0.0, $0.1, $0.2 ?? defaultGateway6) + } sessionBuilder.ipv6 = IPv6Settings( address: address6, diff --git a/Tests/TunnelKitOpenVPNTests/ConfigurationParserTests.swift b/Tests/TunnelKitOpenVPNTests/ConfigurationParserTests.swift index df4bfcd..6c82f79 100644 --- a/Tests/TunnelKitOpenVPNTests/ConfigurationParserTests.swift +++ b/Tests/TunnelKitOpenVPNTests/ConfigurationParserTests.swift @@ -65,9 +65,9 @@ class ConfigurationParserTests: XCTestCase { "dhcp-option DNS 8.8.8.8", "dhcp-option DNS6 ffff::1", "dhcp-option DOMAIN fake-main.net", - "dhcp-option DOMAIN-SEARCH one.com", - "dhcp-option DOMAIN-SEARCH two.com", "dhcp-option DOMAIN main.net", + "dhcp-option DOMAIN one.com", + "dhcp-option DOMAIN two.com", "dhcp-option PROXY_HTTP 1.2.3.4 8081", "dhcp-option PROXY_HTTPS 7.8.9.10 8082", "dhcp-option PROXY_AUTO_CONFIG_URL https://pac/", @@ -77,7 +77,7 @@ class ConfigurationParserTests: XCTestCase { let parsed = try! OpenVPN.ConfigurationParser.parsed(fromLines: lines).configuration XCTAssertEqual(parsed.dnsServers, ["8.8.8.8", "ffff::1"]) - XCTAssertEqual(parsed.searchDomains, ["main.net", "one.com", "two.com"]) + XCTAssertEqual(parsed.searchDomains, ["fake-main.net", "main.net", "one.com", "two.com"]) XCTAssertEqual(parsed.httpProxy?.address, "1.2.3.4") XCTAssertEqual(parsed.httpProxy?.port, 8081) XCTAssertEqual(parsed.httpsProxy?.address, "7.8.9.10")