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

@@ -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)
 

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))"
     }
 }

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))"
     }
 }

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)
+        for r in allRoutes4 {
-        let computedRoutes = (pullRoutes ? (remoteOptions.ipv4?.routes ?? localOptions.ipv4?.routes) : localOptions.ipv4?.routes) ?? []
-        for r in computedRoutes {
             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)
+        for r in allRoutes6 {
-        let computedRoutes = (pullRoutes ? (remoteOptions.ipv6?.routes ?? localOptions.ipv6?.routes) : localOptions.ipv6?.routes) ?? []
-        for r in computedRoutes {
             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)")
+//                log.warning("DNS: No servers provided, using fall-back servers: \(fallbackDNSServers)")
-//                    dnsSettings = NEDNSSettings(servers: fallbackDNSServers)
+//                dnsSettings = NEDNSSettings(servers: fallbackDNSServers)
                 if isGateway {
                     log.warning("DNS: No settings provided")
                 } else {
@@ -240,7 +275,8 @@ extension NetworkSettingsBuilder {
             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 {
+        if proxySettings != nil {
-            proxySettings?.exceptionList = bypass
+            let bypass = allProxyBypassDomains
-            log.info("Routing: Setting proxy by-pass list: \(bypass)")
+            if !bypass.isEmpty {
+                proxySettings?.exceptionList = bypass
+                log.info("Routing: Setting proxy by-pass list: \(bypass)")
+            }
         }
         return proxySettings
     }

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,

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")