Adjust Network Extension implementation

Author: keeshux

Sources/PartoutOS/AppleNE/Modules/DNSModule+NE.swift

@@ -6,52 +6,82 @@ import NetworkExtension
 
 extension DNSModule: NESettingsApplying {
     public func apply(_ ctx: PartoutLoggerContext, to settings: inout NEPacketTunnelNetworkSettings) {
-        var dnsSettings: NEDNSSettings?
+        let dnsSettings: NEDNSSettings
         let rawServers = servers.map(\.rawValue)
 
+        // Former DNS settings are always overridden, even with empty servers
         switch protocolType {
         case .cleartext:
-            if !rawServers.isEmpty {
-                dnsSettings = NEDNSSettings(servers: rawServers)
-                pp_log(ctx, .os, .info, "\t\tServers: \(servers.map { $0.asSensitiveAddress(ctx) })")
-            } else {
-                pp_log(ctx, .os, .info, "\t\tServers: empty")
+            guard !rawServers.isEmpty else {
+                pp_log(ctx, .os, .info, "\t\tSkip DNS settings, cleartext requires non-empty servers")
+                return
             }
-
+            dnsSettings = NEDNSSettings(servers: rawServers)
+            pp_log(ctx, .os, .info, "\t\tServers: \(servers.map { $0.asSensitiveAddress(ctx) })")
         case .https(let url):
             let specificSettings = NEDNSOverHTTPSSettings(servers: rawServers)
             specificSettings.serverURL = url
             dnsSettings = specificSettings
             pp_log(ctx, .os, .info, "\t\tServers: \(servers.map { $0.asSensitiveAddress(ctx) })")
             pp_log(ctx, .os, .info, "\t\tDoH URL: \(url.absoluteString.asSensitiveAddress(ctx))")
-
         case .tls(let hostname):
             let specificSettings = NEDNSOverTLSSettings(servers: rawServers)
             specificSettings.serverName = hostname
             dnsSettings = specificSettings
             pp_log(ctx, .os, .info, "\t\tServers: \(servers.map { $0.asSensitiveAddress(ctx) })")
             pp_log(ctx, .os, .info, "\t\tDoT hostname: \(hostname.asSensitiveAddress(ctx))")
-
         @unknown default:
             break
         }
 
-        if dnsSettings != nil {
-            domainName.map {
-                dnsSettings?.domainName = $0.rawValue
-                pp_log(ctx, .os, .info, "\t\tDomain: \($0.asSensitiveAddress(ctx))")
-            }
-            searchDomains.map {
-                guard !$0.isEmpty else {
-                    return
-                }
-                dnsSettings?.searchDomains = $0.map(\.rawValue)
-                pp_log(ctx, .os, .info, "\t\tSearch domains: \($0.map { $0.asSensitiveAddress(ctx) })")
-            }
-        } else {
-            pp_log(ctx, .os, .info, "\t\tSkip DNS settings")
+        // Main domain (if set)
+        domainName.map {
+            dnsSettings.domainName = $0.rawValue
+            pp_log(ctx, .os, .info, "\t\tDomain: \($0.asSensitiveAddress(ctx))")
+        }
+
+        // Apply domains with the given policy
+        let domains = searchDomains ?? []
+        let domainsDescription = domains.map { $0.asSensitiveAddress(ctx) }
+        let searchDomains = domains.map(\.rawValue)
+        //
+        // Credit for .matchDomains:
+        // https://github.com/WireGuard/wireguard-apple/pull/11
+        //
+        switch domainPolicy {
+        case .search:
+            dnsSettings.searchDomains = searchDomains
+            // XXX: This works around a Network Extension bug. We add the
+            // search domains here because .searchDomains is ineffective when
+            // the VPN is not the default gateway
+            dnsSettings.matchDomains = [""] + searchDomains
+            dnsSettings.matchDomainsNoSearch = false
+            pp_log(ctx, .os, .info, "\t\tSearch-only domains: \(domainsDescription)")
+        case .match:
+            let matchDomains = !searchDomains.isEmpty ? searchDomains : [""]
+            dnsSettings.searchDomains = nil
+            dnsSettings.matchDomains = matchDomains
+            dnsSettings.matchDomainsNoSearch = true
+            pp_log(ctx, .os, .info, "\t\tMatch-only domains: \(domainsDescription)")
+        default:
+            let matchDomains = !searchDomains.isEmpty ? searchDomains : [""]
+            dnsSettings.searchDomains = searchDomains
+            dnsSettings.matchDomains = matchDomains
+            dnsSettings.matchDomainsNoSearch = false
+            pp_log(ctx, .os, .info, "\t\tMatch/Search domains: \(domainsDescription)")
+        }
+
+        //
+        // This is why we guard before committing .matchDomains:
+        // https://git.zx2c4.com/wireguard-apple/commit/?id=20bdf46792905de8862ae7641e50e0f9f99ec946
+        //
+        assert(dnsSettings.matchDomains != nil)
+        if dnsSettings.servers.isEmpty {
+            pp_log(ctx, .os, .error, "\t\tIgnoring match domains without bootstrap DNS servers")
+            dnsSettings.matchDomains = nil
         }
 
+        // Commit to tunnel settings
         settings.dnsSettings = dnsSettings
     }
 }

Sources/PartoutOS/AppleNE/Modules/Profile+NE.swift

@@ -55,9 +55,11 @@ extension Profile {
         // 4. configure DNS for domain-based routing
 
         if let dnsSettings = neSettings.dnsSettings {
-
-            // route DNS through VPN first unless no servers provided
-            if !dnsSettings.servers.isEmpty {
+            // Route DNS through VPN first unless:
+            // - No servers provided
+            // - .matchDomains is not configured
+            // This is a fallback as it *SHOULD* be accomplished by DNSModule+NE
+            if !dnsSettings.servers.isEmpty, dnsSettings.matchDomains == nil {
                 neSettings.dnsSettings?.matchDomains = [""]
             }
         }

Tests/PartoutOSTests/AppleNE/NESettingsApplyingTests.swift

@@ -105,18 +105,18 @@ struct NESettingsApplyingTests {
         let module = try DNSModule.Builder(
             protocolType: .cleartext,
             servers: ["1.1.1.1", "2.2.2.2"],
-            domainName: "domain.com",
-            searchDomains: ["one.com", "two.com"]
+            domains: ["domain.com", "one.com", "two.com"]
         ).build()
 
         var sut = NEPacketTunnelNetworkSettings(tunnelRemoteAddress: "")
         module.apply(.global, to: &sut)
 
         let dnsSettings = try #require(sut.dnsSettings)
+        let expSearchDomains = module.searchDomains?.map(\.rawValue)
         #expect(dnsSettings.dnsProtocol == .cleartext)
         #expect(dnsSettings.servers == module.servers.map(\.rawValue))
         #expect(dnsSettings.domainName == module.domainName?.rawValue)
-        #expect(dnsSettings.searchDomains == module.searchDomains?.map(\.rawValue))
+        #expect(dnsSettings.searchDomains == expSearchDomains)
     }
 
     @Test

Tests/PartoutOSTests/AppleNE/ProfileNetworkSettingsTests.swift

@@ -95,13 +95,14 @@ struct ProfileNetworkSettingsTests {
 
         //
 
-        dnsModuleBuilder.searchDomains = ["domain.com"]
+        dnsModuleBuilder.domains = ["domain.com"]
+        dnsModuleBuilder.domainPolicy = .search
         sut = try Profile.Builder(
             modules: [connectionModule, ipModule, try dnsModuleBuilder.build()],
             activatingModules: true
         ).build().networkSettings(with: nil)
 
-        #expect(sut.dnsSettings?.matchDomains == [""])
+        #expect(sut.dnsSettings?.matchDomains == ["", "domain.com"])
     }
 
     // MARK: With remote info