OpenVPN: Restore and improve negotiation speed (#1095)

The new OpenVPN parser was painfully slow due to allocating NSRegularExpression zillion times, which resulted in poor performance (4x time!) when processing long PUSH_REPLY messages. This is a hard regression from v2 because [TunnelKit created the regexes statically](https://github.com/passepartoutvpn/tunnelkit/blob/339b509ddfd2838ee348d186e9a3016b0f8f447d/Sources/TunnelKitOpenVPNCore/ConfigurationParser.swift#L42). Solution: pre-allocate the regular expressions at parser creation time. Optimize long fragmented replies further by catching PUSH_REPLY continuations early, rather than parsing line by line.
passepartoutvpn · Jan 22, 2025 · a13cc34 · a13cc34
1 parent 8ab7b0d
commit a13cc34
Show file tree

Hide file tree

Showing 6 changed files with 113 additions and 51 deletions.
diff --git a/...es/PassepartoutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/Internal/PushReply.swift b/...es/PassepartoutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/Internal/PushReply.swift
@@ -72,6 +72,9 @@ extension StandardOpenVPNParser {
         guard let prefixIndex = message.range(of: Self.prefix)?.lowerBound else {
             return nil
         }
+        guard !message.contains("push-continuation 2") else {
+            throw StandardOpenVPNParserError.continuationPushReply
+        }
         let original = String(message[prefixIndex...])
         let lines = original.components(separatedBy: ",")
         let options = try parsed(fromLines: lines).configuration

diff --git a/...toutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/StandardOpenVPNParser+Builder.swift b/...toutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/StandardOpenVPNParser+Builder.swift
@@ -90,6 +90,8 @@ extension StandardOpenVPNParser {
 // MARK: - Parsing
 
 extension StandardOpenVPNParser.Builder {
+
+    @inlinable
     mutating func putOption(_ option: StandardOpenVPNParser.Option, line: String, components: [String]) throws {
         switch option {
 

diff --git a/...rtoutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/StandardOpenVPNParser+Option.swift b/...rtoutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/StandardOpenVPNParser+Option.swift
@@ -28,6 +28,10 @@ import Foundation
 extension StandardOpenVPNParser {
     enum Option: String, CaseIterable {
 
+        // MARK: Continuation
+
+        case continuation = "^push-continuation [12]"
+
         // MARK: Unsupported
 
         // check blocks first
@@ -125,9 +129,9 @@ extension StandardOpenVPNParser {
 
         case xorInfo = "^scramble +(xormask|xorptrpos|reverse|obfuscate)[\\s]?([^\\s]+)?"
 
-        // MARK: Continuation
-
-        case continuation = "^push-continuation [12]"
+        func regularExpression() throws -> NSRegularExpression {
+            try NSRegularExpression(pattern: rawValue)
+        }
     }
 }
 
@@ -140,34 +144,4 @@ extension StandardOpenVPNParser.Option {
             return false
         }
     }
-
-    static func parsed(in line: String) -> (option: Self, components: [String])? {
-        assert(allCases.first == .connectionBlock)
-        for option in allCases {
-            guard let components = option.spacedComponents(in: line) else {
-                continue
-            }
-            return (option, components)
-        }
-        return nil
-    }
-}
-
-extension StandardOpenVPNParser.Option {
-    func spacedComponents(in string: String) -> [String]? {
-        let results = NSRegularExpression(rawValue)
-            .matches(in: string, options: [], range: NSRange(location: 0, length: string.count))
-        guard !results.isEmpty else {
-            return nil
-        }
-        assert(results.count == 1)
-        return results.first.map { result in
-            let match = (string as NSString).substring(with: result.range)
-            return match
-                .components(separatedBy: " ")
-                .filter {
-                    !$0.isEmpty
-                }
-        }
-    }
 }
diff --git a/...PassepartoutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/StandardOpenVPNParser.swift b/...PassepartoutOpenVPNOpenSSL/Sources/PassepartoutOpenVPNOpenSSL/StandardOpenVPNParser.swift
@@ -70,6 +70,16 @@ public final class StandardOpenVPNParser {
     /// The decrypter for private keys.
     private let decrypter: PrivateKeyDecrypter?
 
+    private let rxOptions: [(option: Option, rx: NSRegularExpression)] = Option.allCases.compactMap {
+        do {
+            let rx = try $0.regularExpression()
+            return ($0, rx)
+        } catch {
+            assertionFailure("Unable to build regex for '\($0.rawValue)': \(error)")
+            return nil
+        }
+    }
+
     public init(decrypter: PrivateKeyDecrypter? = nil) {
         self.decrypter = decrypter ?? OSSLTLSBox()
     }
@@ -149,11 +159,13 @@ private extension StandardOpenVPNParser {
         var builder = Builder(decrypter: decrypter)
         var isUnknown = true
         for line in lines {
-            guard let result = Option.parsed(in: line) else {
+            let found = try enumerateOptions(in: line) {
+                try builder.putOption($0, line: line, components: $1)
+            }
+            guard found else {
                 builder.putLine(line)
                 continue
             }
-            try builder.putOption(result.option, line: line, components: result.components)
             isUnknown = false
         }
         guard !isUnknown else {
@@ -168,19 +180,6 @@ private extension StandardOpenVPNParser {
     }
 }
 
-private extension String {
-    func trimmedLines() -> [String] {
-        components(separatedBy: .newlines)
-            .map {
-                $0.trimmingCharacters(in: .whitespacesAndNewlines)
-                    .replacingOccurrences(of: "\\s", with: " ", options: .regularExpression)
-            }
-            .filter {
-                !$0.isEmpty
-            }
-    }
-}
-
 // MARK: - ConfigurationDecoder
 
 extension StandardOpenVPNParser: ConfigurationDecoder {
@@ -212,3 +211,68 @@ extension StandardOpenVPNParser: ModuleImporter {
         }
     }
 }
+
+// MARK: - Helpers
+
+private extension StandardOpenVPNParser {
+    func enumerateOptions(
+        in line: String,
+        completion: (_ option: Option, _ components: [String]) throws -> Void
+    ) throws -> Bool {
+        assert(rxOptions.first?.option == .continuation)
+        for pair in rxOptions {
+            var lastError: Error?
+            if pair.rx.enumerateSpacedComponents(in: line, using: { components in
+                do {
+                    try completion(pair.option, components)
+                } catch {
+                    lastError = error
+                }
+            }) {
+                if let lastError {
+                    throw lastError
+                }
+                return true
+            }
+        }
+        return false
+    }
+}
+
+extension NSRegularExpression {
+    func enumerateSpacedComponents(in string: String, using block: ([String]) -> Void) -> Bool {
+        var found = false
+        enumerateMatches(
+            in: string,
+            options: [],
+            range: NSRange(location: 0, length: string.count)
+        ) { result, _, _ in
+            guard let result else {
+                return
+            }
+            let match = (string as NSString)
+                .substring(with: result.range)
+            let components = match
+                .components(separatedBy: " ")
+                .filter {
+                    !$0.isEmpty
+                }
+            found = true
+            block(components)
+        }
+        return found
+    }
+}
+
+private extension String {
+    func trimmedLines() -> [String] {
+        components(separatedBy: .newlines)
+            .map {
+                $0.trimmingCharacters(in: .whitespacesAndNewlines)
+                    .replacingOccurrences(of: "\\s", with: " ", options: .regularExpression)
+            }
+            .filter {
+                !$0.isEmpty
+            }
+    }
+}
diff --git a/...ges/PassepartoutOpenVPNOpenSSL/Tests/PassepartoutOpenVPNOpenSSLTests/PushReplyTests.swift b/...ges/PassepartoutOpenVPNOpenSSL/Tests/PassepartoutOpenVPNOpenSSLTests/PushReplyTests.swift
@@ -189,6 +189,14 @@ final class PushReplyTests: XCTestCase {
         }
     }
 
+    func test_givenMessage_whenParse_thenIsFastEnough() throws {
+        let msg = "PUSH_REPLY,route 87.233.192.218,route 87.233.192.219,route 87.233.192.220,route 87.248.186.252,route 92.241.171.245,route 103.246.200.0 255.255.252.0,route 109.239.140.0 255.255.255.0,route 128.199.0.0 255.255.0.0,route 13.125.0.0 255.255.0.0,route 13.230.0.0 255.254.0.0,route 13.56.0.0 255.252.0.0,route 149.154.160.0 255.255.252.0,route 149.154.164.0 255.255.252.0,route 149.154.168.0 255.255.252.0,route 149.154.172.0 255.255.252.0,route 159.122.128.0 255.255.192.0,route 159.203.0.0 255.255.0.0,route 159.65.0.0 255.255.0.0,route 159.89.0.0 255.255.0.0,route 165.227.0.0 255.255.0.0,route 167.99.0.0 255.255.0.0,route 174.138.0.0 255.255.128.0,route 176.67.169.0 255.255.255.0,route 178.239.88.0 255.255.248.0,route 178.63.0.0 255.255.0.0,route 18.130.0.0 255.255.0.0,route 18.144.0.0 255.255.0.0,route 18.184.0.0 255.254.0.0,route 18.194.0.0 255.254.0.0,route 18.196.0.0 255.254.0.0,route 18.204.0.0 255.252.0.0,push-continuation 2"
+
+        measure {
+            _ = try? parser.pushReply(with: msg)
+        }
+    }
+
     func test_givenMultipleRedirectGateway_whenParse_thenIncludesRoutingPolicies() throws {
         let msg = "PUSH_REPLY,redirect-gateway def1,redirect-gateway bypass-dhcp,redirect-gateway autolocal,dhcp-option DNS 8.8.8.8,route-gateway 10.8.0.1,topology subnet,ping 10,ping-restart 20,ifconfig 10.8.0.2 255.255.255.0,peer-id 0,cipher AES-256-GCM"
 

diff --git a/...toutOpenVPNOpenSSL/Tests/PassepartoutOpenVPNOpenSSLTests/StandardOpenVPNParserTests.swift b/...toutOpenVPNOpenSSL/Tests/PassepartoutOpenVPNOpenSSLTests/StandardOpenVPNParserTests.swift
@@ -31,9 +31,10 @@ final class StandardOpenVPNParserTests: XCTestCase {
     private let parser = StandardOpenVPNParser()
 
     func test_givenOption_whenEnumerateComponents_thenAreParsedCorrectly() throws {
-        let sut = StandardOpenVPNParser.Option.remote
-        let components = try XCTUnwrap(sut.spacedComponents(in: "remote    one.two.com   12345   tcp"))
-        XCTAssertEqual(components, ["remote", "one.two.com", "12345", "tcp"])
+        let sut = try StandardOpenVPNParser.Option.remote.regularExpression()
+        _ = sut.enumerateSpacedComponents(in: "remote    one.two.com   12345   tcp") {
+            XCTAssertEqual($0, ["remote", "one.two.com", "12345", "tcp"])
+        }
     }
 
     // MARK: Lines
@@ -163,6 +164,16 @@ final class StandardOpenVPNParserTests: XCTestCase {
         XCTAssertNil(cfg.warning)
         XCTAssertEqual(cfg4.configuration.xorMethod, OpenVPN.XORMethod.obfuscate(mask: multiMask))
     }
+
+    func test_givenMessage_whenParse_thenIsFastEnough() throws {
+        let msg = "PUSH_REPLY,route 87.233.192.218,route 87.233.192.219,route 87.233.192.220,route 87.248.186.252,route 92.241.171.245,route 103.246.200.0 255.255.252.0,route 109.239.140.0 255.255.255.0,route 128.199.0.0 255.255.0.0,route 13.125.0.0 255.255.0.0,route 13.230.0.0 255.254.0.0,route 13.56.0.0 255.252.0.0,route 149.154.160.0 255.255.252.0,route 149.154.164.0 255.255.252.0,route 149.154.168.0 255.255.252.0,route 149.154.172.0 255.255.252.0,route 159.122.128.0 255.255.192.0,route 159.203.0.0 255.255.0.0,route 159.65.0.0 255.255.0.0,route 159.89.0.0 255.255.0.0,route 165.227.0.0 255.255.0.0,route 167.99.0.0 255.255.0.0,route 174.138.0.0 255.255.128.0,route 176.67.169.0 255.255.255.0,route 178.239.88.0 255.255.248.0,route 178.63.0.0 255.255.0.0,route 18.130.0.0 255.255.0.0,route 18.144.0.0 255.255.0.0,route 18.184.0.0 255.254.0.0,route 18.194.0.0 255.254.0.0,route 18.196.0.0 255.254.0.0,route 18.204.0.0 255.252.0.0,push-continuation 2"
+
+        let parser = StandardOpenVPNParser()
+        let lines = msg.components(separatedBy: ",")
+        measure {
+            _ = try? parser.parsed(fromLines: lines)
+        }
+    }
 }
 
 // MARK: - Helpers