From 40139cbef09896dbb5291696a2e8d492bcbee2c7 Mon Sep 17 00:00:00 2001
From: Davide De Rosa <keeshux@gmail.com>
Date: Tue, 9 Jul 2019 12:14:20 +0200
Subject: [PATCH] Replace key flag with session-wide isRenegotiating

Prevent new if one in progress.

Fixes #105
---
 .../Protocols/OpenVPN/OpenVPNSession.swift    | 21 +++++++++++++------
 .../Protocols/OpenVPN/SessionKey.swift        | 10 ++++-----
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/TunnelKit/Sources/Protocols/OpenVPN/OpenVPNSession.swift b/TunnelKit/Sources/Protocols/OpenVPN/OpenVPNSession.swift
index bbd66e9..24350be 100644
--- a/TunnelKit/Sources/Protocols/OpenVPN/OpenVPNSession.swift
+++ b/TunnelKit/Sources/Protocols/OpenVPN/OpenVPNSession.swift
@@ -116,6 +116,8 @@ public class OpenVPNSession: Session {
     
     private var currentKeyIdx: UInt8?
     
+    private var isRenegotiating: Bool
+    
     private var negotiationKey: OpenVPN.SessionKey {
         guard let key = keys[negotiationKeyIdx] else {
             fatalError("Keys are empty or index \(negotiationKeyIdx) not found in \(keys.keys)")
@@ -196,6 +198,7 @@ public class OpenVPNSession: Session {
         keys = [:]
         oldKeys = []
         negotiationKeyIdx = 0
+        isRenegotiating = false
         lastPing = BidirectionalState(withResetValue: Date.distantPast)
         isStopping = false
         
@@ -324,6 +327,7 @@ public class OpenVPNSession: Session {
         oldKeys.removeAll()
         negotiationKeyIdx = 0
         currentKeyIdx = nil
+        isRenegotiating = false
         
         nextPushRequestDate = nil
         connectedDate = nil
@@ -487,7 +491,7 @@ public class OpenVPNSession: Session {
                 }
                 
             case .softResetV1:
-                if !negotiationKey.softReset {
+                if !isRenegotiating {
                     softReset(isServerInitiated: true)
                 }
 
@@ -580,7 +584,7 @@ public class OpenVPNSession: Session {
         continuatedPushReplyMessage = nil
         pushReply = nil
         negotiationKeyIdx = 0
-        let newKey = OpenVPN.SessionKey(id: UInt8(negotiationKeyIdx))
+        let newKey = OpenVPN.SessionKey(id: UInt8(negotiationKeyIdx), timeout: CoreConfiguration.OpenVPN.negotiationTimeout)
         keys[negotiationKeyIdx] = newKey
         log.debug("Negotiation key index is \(negotiationKeyIdx)")
 
@@ -614,6 +618,10 @@ public class OpenVPNSession: Session {
     
     // Ruby: soft_reset
     private func softReset(isServerInitiated: Bool) {
+        guard !isRenegotiating else {
+            log.warning("Renegotiation already in progress")
+            return
+        }
         if isServerInitiated {
             log.debug("Handle soft reset")
         } else {
@@ -622,12 +630,12 @@ public class OpenVPNSession: Session {
         
         resetControlChannel(forNewSession: false)
         negotiationKeyIdx = max(1, (negotiationKeyIdx + 1) % OpenVPN.ProtocolMacros.numberOfKeys)
-        let newKey = OpenVPN.SessionKey(id: UInt8(negotiationKeyIdx))
+        let newKey = OpenVPN.SessionKey(id: UInt8(negotiationKeyIdx), timeout: CoreConfiguration.OpenVPN.softNegotiationTimeout)
         keys[negotiationKeyIdx] = newKey
         log.debug("Negotiation key index is \(negotiationKeyIdx)")
 
         negotiationKey.state = .softReset
-        negotiationKey.softReset = true
+        isRenegotiating = true
         loopNegotiation()
         if !isServerInitiated {
             enqueueControlPackets(code: .softResetV1, key: UInt8(negotiationKeyIdx), payload: Data())
@@ -694,8 +702,9 @@ public class OpenVPNSession: Session {
         log.debug("TLS.ifconfig: Send pulled ciphertext (\(cipherTextOut.count) bytes)")
         enqueueControlPackets(code: .controlV1, key: negotiationKey.id, payload: cipherTextOut)
         
-        if negotiationKey.softReset {
+        if isRenegotiating {
             completeConnection()
+            isRenegotiating = false
         }
         nextPushRequestDate = Date().addingTimeInterval(CoreConfiguration.OpenVPN.pushRequestInterval)
     }
@@ -862,7 +871,7 @@ public class OpenVPNSession: Session {
             }
             
             negotiationKey.controlState = .preIfConfig
-            nextPushRequestDate = Date().addingTimeInterval(negotiationKey.softReset ? CoreConfiguration.OpenVPN.softResetDelay : CoreConfiguration.OpenVPN.retransmissionLimit)
+            nextPushRequestDate = Date().addingTimeInterval(isRenegotiating ? CoreConfiguration.OpenVPN.softResetDelay : CoreConfiguration.OpenVPN.retransmissionLimit)
             pushRequest()
         }
         
diff --git a/TunnelKit/Sources/Protocols/OpenVPN/SessionKey.swift b/TunnelKit/Sources/Protocols/OpenVPN/SessionKey.swift
index 76e9626..e549acc 100644
--- a/TunnelKit/Sources/Protocols/OpenVPN/SessionKey.swift
+++ b/TunnelKit/Sources/Protocols/OpenVPN/SessionKey.swift
@@ -53,6 +53,8 @@ extension OpenVPN {
 
         let id: UInt8 // 3-bit
         
+        let timeout: TimeInterval
+        
         let startTime: Date
         
         var state = State.invalid
@@ -70,16 +72,14 @@ extension OpenVPN {
         
         var dataPath: DataPath?
         
-        var softReset: Bool
-
         private var isTLSConnected: Bool
         
-        init(id: UInt8) {
+        init(id: UInt8, timeout: TimeInterval) {
             self.id = id
+            self.timeout = timeout
 
             startTime = Date()
             state = .invalid
-            softReset = false
             isTLSConnected = false
         }
 
@@ -90,8 +90,6 @@ extension OpenVPN {
         
         // Ruby: Key.negotiate_timeout
         func didNegotiationTimeOut(link: LinkInterface) -> Bool {
-            let timeout = (softReset ? CoreConfiguration.OpenVPN.softNegotiationTimeout : CoreConfiguration.OpenVPN.negotiationTimeout)
-            
             return ((controlState != .connected) && (-startTime.timeIntervalSinceNow > timeout))
         }