Replace key flag with session-wide isRenegotiating

Prevent new if one in progress.

Fixes #105
This commit is contained in:
Davide De Rosa 2019-07-09 12:14:20 +02:00
parent 0f2234f1d1
commit 40139cbef0
2 changed files with 19 additions and 12 deletions

View File

@ -116,6 +116,8 @@ public class OpenVPNSession: Session {
private var currentKeyIdx: UInt8? private var currentKeyIdx: UInt8?
private var isRenegotiating: Bool
private var negotiationKey: OpenVPN.SessionKey { private var negotiationKey: OpenVPN.SessionKey {
guard let key = keys[negotiationKeyIdx] else { guard let key = keys[negotiationKeyIdx] else {
fatalError("Keys are empty or index \(negotiationKeyIdx) not found in \(keys.keys)") fatalError("Keys are empty or index \(negotiationKeyIdx) not found in \(keys.keys)")
@ -196,6 +198,7 @@ public class OpenVPNSession: Session {
keys = [:] keys = [:]
oldKeys = [] oldKeys = []
negotiationKeyIdx = 0 negotiationKeyIdx = 0
isRenegotiating = false
lastPing = BidirectionalState(withResetValue: Date.distantPast) lastPing = BidirectionalState(withResetValue: Date.distantPast)
isStopping = false isStopping = false
@ -324,6 +327,7 @@ public class OpenVPNSession: Session {
oldKeys.removeAll() oldKeys.removeAll()
negotiationKeyIdx = 0 negotiationKeyIdx = 0
currentKeyIdx = nil currentKeyIdx = nil
isRenegotiating = false
nextPushRequestDate = nil nextPushRequestDate = nil
connectedDate = nil connectedDate = nil
@ -487,7 +491,7 @@ public class OpenVPNSession: Session {
} }
case .softResetV1: case .softResetV1:
if !negotiationKey.softReset { if !isRenegotiating {
softReset(isServerInitiated: true) softReset(isServerInitiated: true)
} }
@ -580,7 +584,7 @@ public class OpenVPNSession: Session {
continuatedPushReplyMessage = nil continuatedPushReplyMessage = nil
pushReply = nil pushReply = nil
negotiationKeyIdx = 0 negotiationKeyIdx = 0
let newKey = OpenVPN.SessionKey(id: UInt8(negotiationKeyIdx)) let newKey = OpenVPN.SessionKey(id: UInt8(negotiationKeyIdx), timeout: CoreConfiguration.OpenVPN.negotiationTimeout)
keys[negotiationKeyIdx] = newKey keys[negotiationKeyIdx] = newKey
log.debug("Negotiation key index is \(negotiationKeyIdx)") log.debug("Negotiation key index is \(negotiationKeyIdx)")
@ -614,6 +618,10 @@ public class OpenVPNSession: Session {
// Ruby: soft_reset // Ruby: soft_reset
private func softReset(isServerInitiated: Bool) { private func softReset(isServerInitiated: Bool) {
guard !isRenegotiating else {
log.warning("Renegotiation already in progress")
return
}
if isServerInitiated { if isServerInitiated {
log.debug("Handle soft reset") log.debug("Handle soft reset")
} else { } else {
@ -622,12 +630,12 @@ public class OpenVPNSession: Session {
resetControlChannel(forNewSession: false) resetControlChannel(forNewSession: false)
negotiationKeyIdx = max(1, (negotiationKeyIdx + 1) % OpenVPN.ProtocolMacros.numberOfKeys) 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 keys[negotiationKeyIdx] = newKey
log.debug("Negotiation key index is \(negotiationKeyIdx)") log.debug("Negotiation key index is \(negotiationKeyIdx)")
negotiationKey.state = .softReset negotiationKey.state = .softReset
negotiationKey.softReset = true isRenegotiating = true
loopNegotiation() loopNegotiation()
if !isServerInitiated { if !isServerInitiated {
enqueueControlPackets(code: .softResetV1, key: UInt8(negotiationKeyIdx), payload: Data()) 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)") log.debug("TLS.ifconfig: Send pulled ciphertext (\(cipherTextOut.count) bytes)")
enqueueControlPackets(code: .controlV1, key: negotiationKey.id, payload: cipherTextOut) enqueueControlPackets(code: .controlV1, key: negotiationKey.id, payload: cipherTextOut)
if negotiationKey.softReset { if isRenegotiating {
completeConnection() completeConnection()
isRenegotiating = false
} }
nextPushRequestDate = Date().addingTimeInterval(CoreConfiguration.OpenVPN.pushRequestInterval) nextPushRequestDate = Date().addingTimeInterval(CoreConfiguration.OpenVPN.pushRequestInterval)
} }
@ -862,7 +871,7 @@ public class OpenVPNSession: Session {
} }
negotiationKey.controlState = .preIfConfig 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() pushRequest()
} }

View File

@ -53,6 +53,8 @@ extension OpenVPN {
let id: UInt8 // 3-bit let id: UInt8 // 3-bit
let timeout: TimeInterval
let startTime: Date let startTime: Date
var state = State.invalid var state = State.invalid
@ -70,16 +72,14 @@ extension OpenVPN {
var dataPath: DataPath? var dataPath: DataPath?
var softReset: Bool
private var isTLSConnected: Bool private var isTLSConnected: Bool
init(id: UInt8) { init(id: UInt8, timeout: TimeInterval) {
self.id = id self.id = id
self.timeout = timeout
startTime = Date() startTime = Date()
state = .invalid state = .invalid
softReset = false
isTLSConnected = false isTLSConnected = false
} }
@ -90,8 +90,6 @@ extension OpenVPN {
// Ruby: Key.negotiate_timeout // Ruby: Key.negotiate_timeout
func didNegotiationTimeOut(link: LinkInterface) -> Bool { func didNegotiationTimeOut(link: LinkInterface) -> Bool {
let timeout = (softReset ? CoreConfiguration.OpenVPN.softNegotiationTimeout : CoreConfiguration.OpenVPN.negotiationTimeout)
return ((controlState != .connected) && (-startTime.timeIntervalSinceNow > timeout)) return ((controlState != .connected) && (-startTime.timeIntervalSinceNow > timeout))
} }