From cae371bb400ad3de9bc59f96c18a091fb577ac44 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 29 Oct 2022 12:24:28 +0200 Subject: [PATCH] Split IPv4/6 settings and routes (#298) * Postpone setting route gateway Resolve in NetworkSettingsBuilder. * Store routes separately from IP*Settings Parse as optionals to avoid empty arrays. * Deprecate routes stored in IP*Settings * Apply routes from new fields * Update CHANGELOG --- CHANGELOG.md | 1 + Sources/TunnelKitCore/IPv4Settings.swift | 19 +++++++--- Sources/TunnelKitCore/IPv6Settings.swift | 19 +++++++--- .../NetworkSettingsBuilder.swift | 20 +++++----- .../TunnelKitOpenVPNCore/Configuration.swift | 22 +++++++++++ .../ConfigurationParser.swift | 38 ++++++++++++------- Tests/TunnelKitOpenVPNTests/PushTests.swift | 2 +- 7 files changed, 86 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8014329..1ffc34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - OpenVPN: Deal with remote options properly. [#297](https://github.com/passepartoutvpn/tunnelkit/pull/297) +- OpenVPN: Routes from configuration file are ignored. [#278](https://github.com/passepartoutvpn/tunnelkit/issues/278) - 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 574b48d..0bd859b 100644 --- a/Sources/TunnelKitCore/IPv4Settings.swift +++ b/Sources/TunnelKitCore/IPv4Settings.swift @@ -37,10 +37,10 @@ public struct IPv4Settings: Codable, Equatable, CustomStringConvertible { /// The address mask. public let mask: String - /// The address of the gateway (uses default gateway if not set). - public let gateway: String + /// The address of the gateway (falls back to global gateway). + public let gateway: String? - public init(_ destination: String, _ mask: String?, _ gateway: String) { + public init(_ destination: String, _ mask: String?, _ gateway: String?) { self.destination = destination self.mask = mask ?? "255.255.255.255" self.gateway = gateway @@ -49,7 +49,7 @@ public struct IPv4Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - "{\(destination.maskedDescription)/\(mask) \(gateway.maskedDescription)}" + "{\(destination.maskedDescription)/\(mask) \(gateway?.maskedDescription ?? "*")}" } } @@ -63,8 +63,17 @@ public struct IPv4Settings: Codable, Equatable, CustomStringConvertible { public let defaultGateway: String /// The additional routes. + @available(*, deprecated, message: "Store routes separately") public let routes: [Route] + public init(address: String, addressMask: String, defaultGateway: String) { + self.address = address + self.addressMask = addressMask + self.defaultGateway = defaultGateway + self.routes = [] + } + + @available(*, deprecated, message: "Store routes separately") public init(address: String, addressMask: String, defaultGateway: String, routes: [Route]) { self.address = address self.addressMask = addressMask @@ -75,6 +84,6 @@ public struct IPv4Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - "addr \(address.maskedDescription) netmask \(addressMask) gw \(defaultGateway.maskedDescription) routes \(routes.map(\.maskedDescription))" + "addr \(address.maskedDescription) netmask \(addressMask) gw \(defaultGateway.maskedDescription)" } } diff --git a/Sources/TunnelKitCore/IPv6Settings.swift b/Sources/TunnelKitCore/IPv6Settings.swift index 9cb073a..d9c6cda 100644 --- a/Sources/TunnelKitCore/IPv6Settings.swift +++ b/Sources/TunnelKitCore/IPv6Settings.swift @@ -37,10 +37,10 @@ public struct IPv6Settings: Codable, Equatable, CustomStringConvertible { /// The address prefix length. public let prefixLength: UInt8 - /// The address of the gateway (uses default gateway if not set). - public let gateway: String + /// The address of the gateway (falls back to global gateway). + public let gateway: String? - public init(_ destination: String, _ prefixLength: UInt8?, _ gateway: String) { + public init(_ destination: String, _ prefixLength: UInt8?, _ gateway: String?) { self.destination = destination self.prefixLength = prefixLength ?? 3 self.gateway = gateway @@ -49,7 +49,7 @@ public struct IPv6Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - "{\(destination.maskedDescription)/\(prefixLength) \(gateway.maskedDescription)}" + "{\(destination.maskedDescription)/\(prefixLength) \(gateway?.maskedDescription ?? "*")}" } } @@ -63,8 +63,17 @@ public struct IPv6Settings: Codable, Equatable, CustomStringConvertible { public let defaultGateway: String /// The additional routes. + @available(*, deprecated, message: "Store routes separately") public let routes: [Route] + public init(address: String, addressPrefixLength: UInt8, defaultGateway: String) { + self.address = address + self.addressPrefixLength = addressPrefixLength + self.defaultGateway = defaultGateway + self.routes = [] + } + + @available(*, deprecated, message: "Store routes separately") public init(address: String, addressPrefixLength: UInt8, defaultGateway: String, routes: [Route]) { self.address = address self.addressPrefixLength = addressPrefixLength @@ -75,6 +84,6 @@ public struct IPv6Settings: Codable, Equatable, CustomStringConvertible { // MARK: CustomStringConvertible public var description: String { - "addr \(address.maskedDescription)/\(addressPrefixLength) gw \(defaultGateway.maskedDescription) routes \(routes.map(\.maskedDescription))" + "addr \(address.maskedDescription)/\(addressPrefixLength) gw \(defaultGateway.maskedDescription)" } } diff --git a/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift b/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift index b5a9e16..6d5d4e5 100644 --- a/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift +++ b/Sources/TunnelKitOpenVPNAppExtension/NetworkSettingsBuilder.swift @@ -104,19 +104,17 @@ extension NetworkSettingsBuilder { 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 { + var routes = localOptions.routes4 ?? [] + if pullRoutes, let remoteRoutes = remoteOptions.routes4 { 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 { + var routes = localOptions.routes6 ?? [] + if pullRoutes, let remoteRoutes = remoteOptions.routes6 { routes.append(contentsOf: remoteRoutes) } return routes @@ -169,9 +167,10 @@ extension NetworkSettingsBuilder { for r in allRoutes4 { let ipv4Route = NEIPv4Route(destinationAddress: r.destination, subnetMask: r.mask) - ipv4Route.gatewayAddress = r.gateway + let gw = r.gateway ?? ipv4.defaultGateway + ipv4Route.gatewayAddress = gw neRoutes.append(ipv4Route) - log.info("Routing.IPv4: Adding route \(r.destination)/\(r.mask) -> \(r.gateway)") + log.info("Routing.IPv4: Adding route \(r.destination)/\(r.mask) -> \(gw)") } ipv4Settings.includedRoutes = neRoutes @@ -196,9 +195,10 @@ extension NetworkSettingsBuilder { for r in allRoutes6 { let ipv6Route = NEIPv6Route(destinationAddress: r.destination, networkPrefixLength: r.prefixLength as NSNumber) - ipv6Route.gatewayAddress = r.gateway + let gw = r.gateway ?? ipv6.defaultGateway + ipv6Route.gatewayAddress = gw neRoutes.append(ipv6Route) - log.info("Routing.IPv6: Adding route \(r.destination)/\(r.prefixLength) -> \(r.gateway)") + log.info("Routing.IPv6: Adding route \(r.destination)/\(r.prefixLength) -> \(gw)") } ipv6Settings.includedRoutes = neRoutes diff --git a/Sources/TunnelKitOpenVPNCore/Configuration.swift b/Sources/TunnelKitOpenVPNCore/Configuration.swift index e7d02f3..5256074 100644 --- a/Sources/TunnelKitOpenVPNCore/Configuration.swift +++ b/Sources/TunnelKitOpenVPNCore/Configuration.swift @@ -257,6 +257,12 @@ extension OpenVPN { /// The settings for IPv6. `OpenVPNSession` only evaluates this server-side. public var ipv6: IPv6Settings? + /// The IPv4 routes if `ipv4` is nil. + public var routes4: [IPv4Settings.Route]? + + /// The IPv6 routes if `ipv6` is nil. + public var routes6: [IPv6Settings.Route]? + /// Set false to ignore DNS settings, even when pushed. public var isDNSEnabled: Bool? @@ -356,6 +362,8 @@ extension OpenVPN { peerId: peerId, ipv4: ipv4, ipv6: ipv6, + routes4: routes4, + routes6: routes6, isDNSEnabled: isDNSEnabled, dnsProtocol: dnsProtocol, dnsServers: dnsServers, @@ -468,6 +476,12 @@ extension OpenVPN { /// - Seealso: `ConfigurationBuilder.ipv6` public let ipv6: IPv6Settings? + /// - Seealso: `ConfigurationBuilder.routes4` + public let routes4: [IPv4Settings.Route]? + + /// - Seealso: `ConfigurationBuilder.routes6` + public let routes6: [IPv6Settings.Route]? + /// - Seealso: `ConfigurationBuilder.isDNSEnabled` public let isDNSEnabled: Bool? @@ -597,6 +611,8 @@ extension OpenVPN.Configuration { builder.peerId = peerId builder.ipv4 = ipv4 builder.ipv6 = ipv6 + builder.routes4 = routes4 + builder.routes6 = routes6 builder.isDNSEnabled = isDNSEnabled builder.dnsProtocol = dnsProtocol builder.dnsServers = dnsServers @@ -631,6 +647,12 @@ extension OpenVPN.Configuration { log.info("\tIPv4: \(ipv4?.description ?? "not configured")") log.info("\tIPv6: \(ipv6?.description ?? "not configured")") } + if let routes = routes4 { + log.info("\tRoutes (IPv4): \(routes)") + } + if let routes = routes6 { + log.info("\tRoutes (IPv6): \(routes)") + } if let cipher = cipher { log.info("\tCipher: \(cipher)") diff --git a/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift b/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift index 812cbb1..c2e7d20 100644 --- a/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift +++ b/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift @@ -284,8 +284,8 @@ extension OpenVPN { var optIfconfig4Arguments: [String]? var optIfconfig6Arguments: [String]? var optGateway4Arguments: [String]? - var optRoutes4: [(String, String, String?)] = [] // address, netmask, gateway - var optRoutes6: [(String, UInt8, String?)] = [] // destination, prefix, gateway + var optRoutes4: [(String, String, String?)]? // address, netmask, gateway + var optRoutes6: [(String, UInt8, String?)]? // destination, prefix, gateway var optDNSServers: [String]? var optSearchDomains: [String]? var optHTTPProxy: Proxy? @@ -619,7 +619,10 @@ extension OpenVPN { if gateway == "vpn_gateway" { gateway = nil } - optRoutes4.append((address, mask, gateway)) + if optRoutes4 == nil { + optRoutes4 = [] + } + optRoutes4?.append((address, mask, gateway)) } Regex.route6.enumerateSpacedArguments(in: line) { let routeEntryArguments = $0 @@ -637,7 +640,10 @@ extension OpenVPN { if gateway == "vpn_gateway" { gateway = nil } - optRoutes6.append((destination, prefix, gateway)) + if optRoutes6 == nil { + optRoutes6 = [] + } + optRoutes6?.append((destination, prefix, gateway)) } Regex.gateway.enumerateSpacedArguments(in: line) { optGateway4Arguments = $0 @@ -726,6 +732,12 @@ extension OpenVPN { // MARK: Post-processing // ensure that non-nil network settings also imply non-empty + if let array = optRoutes4 { + assert(!array.isEmpty) + } + if let array = optRoutes6 { + assert(!array.isEmpty) + } if let array = optDNSServers { assert(!array.isEmpty) } @@ -857,17 +869,16 @@ extension OpenVPN { addressMask4 = "255.255.255.255" defaultGateway4 = ifconfig4Arguments[1] } - let routes4 = optRoutes4.map { - IPv4Settings.Route($0.0, $0.1, $0.2 ?? defaultGateway4) - } sessionBuilder.ipv4 = IPv4Settings( address: address4, addressMask: addressMask4, - defaultGateway: defaultGateway4, - routes: routes4 + defaultGateway: defaultGateway4 ) } + sessionBuilder.routes4 = optRoutes4?.map { + IPv4Settings.Route($0.0, $0.1, $0.2) + } if let ifconfig6Arguments = optIfconfig6Arguments { guard ifconfig6Arguments.count == 2 else { @@ -883,17 +894,16 @@ extension OpenVPN { let address6 = address6Components[0] let defaultGateway6 = ifconfig6Arguments[1] - let routes6 = optRoutes6.map { - IPv6Settings.Route($0.0, $0.1, $0.2 ?? defaultGateway6) - } sessionBuilder.ipv6 = IPv6Settings( address: address6, addressPrefixLength: addressPrefix6, - defaultGateway: defaultGateway6, - routes: routes6 + defaultGateway: defaultGateway6 ) } + sessionBuilder.routes6 = optRoutes6?.map { + IPv6Settings.Route($0.0, $0.1, $0.2) + } sessionBuilder.dnsServers = optDNSServers sessionBuilder.searchDomains = optSearchDomains diff --git a/Tests/TunnelKitOpenVPNTests/PushTests.swift b/Tests/TunnelKitOpenVPNTests/PushTests.swift index f2d28a6..aed35c7 100644 --- a/Tests/TunnelKitOpenVPNTests/PushTests.swift +++ b/Tests/TunnelKitOpenVPNTests/PushTests.swift @@ -76,7 +76,7 @@ class PushTests: XCTestCase { let reply = try! OpenVPN.PushReply(message: msg)! reply.debug() - let route = reply.options.ipv4!.routes.first! + let route = reply.options.routes4!.first! XCTAssertEqual(route.destination, "192.168.0.0") XCTAssertEqual(route.mask, "255.255.255.0")