From d097afccdc987b9344aa2620c6045cba532ba3ed Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 16 Apr 2019 12:42:38 +0200 Subject: [PATCH 1/6] Resend PUSH_REQUEST every 2 seconds Regardless of link reliability. --- .../Sources/Core/CoreConfiguration.swift | 2 ++ TunnelKit/Sources/Core/SessionProxy.swift | 23 ++++++------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/TunnelKit/Sources/Core/CoreConfiguration.swift b/TunnelKit/Sources/Core/CoreConfiguration.swift index fc2d894..f5e7f11 100644 --- a/TunnelKit/Sources/Core/CoreConfiguration.swift +++ b/TunnelKit/Sources/Core/CoreConfiguration.swift @@ -67,6 +67,8 @@ struct CoreConfiguration { static let tickInterval = 0.2 + static let pushRequestInterval = 2.0 + static let pingTimeout = 120.0 static let retransmissionLimit = 0.1 diff --git a/TunnelKit/Sources/Core/SessionProxy.swift b/TunnelKit/Sources/Core/SessionProxy.swift index b09b812..7b286da 100644 --- a/TunnelKit/Sources/Core/SessionProxy.swift +++ b/TunnelKit/Sources/Core/SessionProxy.swift @@ -403,12 +403,10 @@ public class SessionProxy { return } - if !isReliableLink { - pushRequest() - flushControlQueue() - } + pushRequest() + flushControlQueue() - guard (negotiationKey.controlState == .connected) else { + guard negotiationKey.controlState == .connected else { queue.asyncAfter(deadline: .now() + CoreConfiguration.tickInterval) { [weak self] in self?.loopNegotiation() } @@ -696,13 +694,11 @@ public class SessionProxy { // Ruby: push_request private func pushRequest() { - guard (negotiationKey.controlState == .preIfConfig) else { + guard negotiationKey.controlState == .preIfConfig else { return } - if !isReliableLink { - guard let targetDate = nextPushRequestDate, (Date() > targetDate) else { - return - } + guard let targetDate = nextPushRequestDate, Date() > targetDate else { + return } log.debug("TLS.ifconfig: Put plaintext (PUSH_REQUEST)") @@ -727,7 +723,7 @@ public class SessionProxy { if negotiationKey.softReset { completeConnection() } - nextPushRequestDate = Date().addingTimeInterval(CoreConfiguration.retransmissionLimit) + nextPushRequestDate = Date().addingTimeInterval(CoreConfiguration.pushRequestInterval) } private func maybeRenegotiate() { @@ -1143,11 +1139,6 @@ public class SessionProxy { // MARK: Acks private func handleAcks() { - - // retry PUSH_REQUEST if ack queue is empty (all sent packets were ack'ed) - if isReliableLink && !controlChannel.hasPendingAcks() { - pushRequest() - } } // Ruby: send_ack From 23b6e3b98efbb1de59b7173c6961434e4cce7f1a Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 16 Apr 2019 23:59:41 +0200 Subject: [PATCH 2/6] Relax negotiation timeouts --- TunnelKit/Sources/AppExtension/Transport/NETCPInterface.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TunnelKit/Sources/AppExtension/Transport/NETCPInterface.swift b/TunnelKit/Sources/AppExtension/Transport/NETCPInterface.swift index e14ce07..1543793 100644 --- a/TunnelKit/Sources/AppExtension/Transport/NETCPInterface.swift +++ b/TunnelKit/Sources/AppExtension/Transport/NETCPInterface.swift @@ -200,9 +200,9 @@ class NETCPLink: LinkInterface { return maxPacketSize } - let negotiationTimeout: TimeInterval = 10.0 + let negotiationTimeout: TimeInterval = 60.0 - let hardResetTimeout: TimeInterval = 5.0 + let hardResetTimeout: TimeInterval = 20.0 func setReadHandler(queue: DispatchQueue, _ handler: @escaping ([Data]?, Error?) -> Void) { loopReadPackets(queue, Data(), handler) From 380ac2beaca7edf7b9d943f4a2d6fa1954f8a33b Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Fri, 22 Mar 2019 11:58:30 +0100 Subject: [PATCH 3/6] Throw to exit PUSH_REPLY parsing on continuation --- TunnelKit/Sources/Core/ConfigurationParser.swift | 14 ++++++++++++++ TunnelKit/Sources/Core/SessionError.swift | 6 ++++++ 2 files changed, 20 insertions(+) diff --git a/TunnelKit/Sources/Core/ConfigurationParser.swift b/TunnelKit/Sources/Core/ConfigurationParser.swift index afc0f28..390ac20 100644 --- a/TunnelKit/Sources/Core/ConfigurationParser.swift +++ b/TunnelKit/Sources/Core/ConfigurationParser.swift @@ -106,6 +106,10 @@ public class ConfigurationParser { static let externalFiles = NSRegularExpression("^(ca|cert|key|tls-auth|tls-crypt) ") static let connection = NSRegularExpression("^") + + // MARK: Continuation + + static let continuation = NSRegularExpression("^push-continuation [12]") } private enum Topology: String { @@ -232,6 +236,16 @@ public class ConfigurationParser { isHandled = true } + // MARK: Continuation + + var isContinuation = false + Regex.continuation.enumerateArguments(in: line) { + isContinuation = ($0.first == "2") + } + guard !isContinuation else { + throw SessionError.continuationPushReply + } + // MARK: Inline content if unsupportedError == nil { diff --git a/TunnelKit/Sources/Core/SessionError.swift b/TunnelKit/Sources/Core/SessionError.swift index b76b715..9d7ae8f 100644 --- a/TunnelKit/Sources/Core/SessionError.swift +++ b/TunnelKit/Sources/Core/SessionError.swift @@ -59,6 +59,12 @@ public enum SessionError: String, Error { /// The provided credentials failed authentication. case badCredentials + /// The PUSH_REPLY is multipart. + case continuationPushReply + + /// The reply to PUSH_REQUEST is malformed. + case malformedPushReply + /// A write operation failed at the link layer (e.g. network unreachable). case failedLinkWrite From 88cd62064aa140d2751129db05ff77eb6429d60e Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 16 Apr 2019 23:55:50 +0200 Subject: [PATCH 4/6] Handle continuation in PUSH_REPLY --- TunnelKit/Sources/Core/SessionProxy.swift | 16 +++++++++++++++- TunnelKitTests/PushTests.swift | 6 ++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/TunnelKit/Sources/Core/SessionProxy.swift b/TunnelKit/Sources/Core/SessionProxy.swift index 7b286da..6c80ba0 100644 --- a/TunnelKit/Sources/Core/SessionProxy.swift +++ b/TunnelKit/Sources/Core/SessionProxy.swift @@ -135,6 +135,8 @@ public class SessionProxy { return link?.isReliable ?? false } + private var continuatedPushReplyMessage: String? + private var pushReply: SessionReply? private var nextPushRequestDate: Date? @@ -368,6 +370,7 @@ public class SessionProxy { nextPushRequestDate = nil connectedDate = nil authenticator = nil + continuatedPushReplyMessage = nil pushReply = nil link = nil if !(tunnel?.isPersistent ?? false) { @@ -605,6 +608,7 @@ public class SessionProxy { log.debug("Send hard reset") resetControlChannel(forNewSession: true) + continuatedPushReplyMessage = nil pushReply = nil negotiationKeyIdx = 0 let newKey = SessionKey(id: UInt8(negotiationKeyIdx)) @@ -911,9 +915,15 @@ public class SessionProxy { log.debug("Received control message: \"\(message)\"") } + let completeMessage: String + if let continuated = continuatedPushReplyMessage { + completeMessage = "\(continuated),\(message)" + } else { + completeMessage = message + } let reply: PushReply do { - guard let optionalReply = try PushReply(message: message) else { + guard let optionalReply = try PushReply(message: completeMessage) else { return } reply = optionalReply @@ -935,6 +945,10 @@ public class SessionProxy { throw SessionError.serverCompression } } + } catch SessionError.continuationPushReply { + continuatedPushReplyMessage = completeMessage.replacingOccurrences(of: "push-continuation", with: "") + // FIXME: strip "PUSH_REPLY" and "push-continuation 2" + return } catch let e { deferStop(.shutdown, e) return diff --git a/TunnelKitTests/PushTests.swift b/TunnelKitTests/PushTests.swift index 9c4495b..907427d 100644 --- a/TunnelKitTests/PushTests.swift +++ b/TunnelKitTests/PushTests.swift @@ -155,4 +155,10 @@ class PushTests: XCTestCase { XCTAssertEqual(reply.options.keepAliveInterval, 10) } + + func testProvost() { + 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 reply = try! SessionProxy.PushReply(message: msg)! + reply.debug() + } } From 6fd6d228bf0579ccfa1cc18cd569c7548c658c92 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 16 Apr 2019 23:53:51 +0200 Subject: [PATCH 5/6] Loop pulling plain text from TLS There might be more data to read. Fixes #71, #73 --- TunnelKit/Sources/Core/SessionProxy.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/TunnelKit/Sources/Core/SessionProxy.swift b/TunnelKit/Sources/Core/SessionProxy.swift index 6c80ba0..c93d200 100644 --- a/TunnelKit/Sources/Core/SessionProxy.swift +++ b/TunnelKit/Sources/Core/SessionProxy.swift @@ -854,8 +854,10 @@ public class SessionProxy { } do { - let controlData = try controlChannel.currentControlData(withTLS: negotiationKey.tls) - handleControlData(controlData) + while true { + let controlData = try controlChannel.currentControlData(withTLS: negotiationKey.tls) + handleControlData(controlData) + } } catch _ { } } From 80f5a3250d84d155d8be98e0146d11b00650b2f0 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Wed, 17 Apr 2019 00:26:56 +0200 Subject: [PATCH 6/6] Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39b4852..bcf4a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Negotiation times out with SoftEther. [#67](https://github.com/keeshux/tunnelkit/issues/67) +- Unable to handle continuated PUSH_REPLY. [#71](https://github.com/keeshux/tunnelkit/issues/71) +- TCP requiring multiple PUSH_REQUEST. [#73](https://github.com/keeshux/tunnelkit/issues/73) ## 1.6.1 (2019-04-07)