OpenVPN: Fix potential deadlock on disconnection (#1097)

NE link writes are blocking and don't support timeout. When shutting
down a UDP session, the OCCPacket may fail to send and lock the session
in a stale state ("Active" but dead), with an infinite loop of "Failed
TUN read" messages in the log.

- First, [add cancellation handlers to NE UDP/TCP
sockets](https://github.com/passepartoutvpn/passepartoutkit-source/pull/464)
- Then, rather than writing the exit packet in the foreground and
scheduling cancellation:
  - Write the packet in a background task
  - Wait until timeout in the actor
  - Cancel the pending write and go ahead

This may still leak if NE socket cancellation doesn't work, but will
prevent deadlock.
This commit is contained in:
Davide 2025-01-23 10:22:11 +01:00 committed by GitHub
parent f3d90a9a93
commit 60e3460966
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 39 additions and 25 deletions

@ -1 +1 @@
Subproject commit 3d3977cb6c3ac9806c15fdbae8ef585efc81c042
Subproject commit 32582232866c835c77ad5a52b24f1ceca6042a84

View File

@ -217,32 +217,11 @@ extension OpenVPNSession: OpenVPNSessionProtocol {
sessionState = .stopping
// shut down after sending exit notification if link is unreliable (normally UDP)
if error == nil || (error as? PassepartoutError)?.code == .networkChanged,
let link, !link.isReliable,
let currentDataChannel {
if error == nil || (error as? PassepartoutError)?.code == .networkChanged {
do {
if let packets = try currentDataChannel.encrypt(packets: [OCCPacket.exit.serialized()]) {
pp_log(.openvpn, .info, "Send OCCPacket exit")
let timeoutMillis = Int((timeout ?? options.writeTimeout) * 1000.0)
let writeTask = Task {
try await link.writePackets(packets)
try Task.checkCancellation()
}
let timeoutTask = Task {
try await Task.sleep(milliseconds: timeoutMillis)
try Task.checkCancellation()
pp_log(.openvpn, .info, "Cancelled OCCPacket")
writeTask.cancel()
}
try await writeTask.value
timeoutTask.cancel()
pp_log(.openvpn, .info, "Sent OCCPacket correctly")
}
try await sendExitPacket(timeout: timeout)
} catch {
pp_log(.openvpn, .error, "Unable to send OCCPacket exit: \(error)")
pp_log(.openvpn, .error, "Unable to send exit packet: \(error)")
}
}
@ -259,6 +238,41 @@ extension OpenVPNSession: OpenVPNSessionProtocol {
}
private extension OpenVPNSession {
func sendExitPacket(timeout: TimeInterval?) async throws {
guard let link, !link.isReliable, let currentDataChannel else {
return
}
guard let packets = try currentDataChannel.encrypt(packets: [OCCPacket.exit.serialized()]) else {
pp_log(.openvpn, .error, "Encrypted to empty OCCPacket packets")
assertionFailure("Empty OCCPacket packets?")
return
}
pp_log(.openvpn, .info, "Send OCCPacket exit")
let timeoutMillis = Int((timeout ?? options.writeTimeout) * 1000.0)
let timeoutTask = Task {
try await Task.sleep(milliseconds: timeoutMillis)
}
let writeTask = Task {
try await link.writePackets(packets)
timeoutTask.cancel()
do {
try Task.checkCancellation()
} catch {
pp_log(.openvpn, .error, "Cancelled OCCPacket: \(error)")
}
}
do {
try await timeoutTask.value
} catch {
pp_log(.openvpn, .info, "Cancelled OCCPacket write timeout (completed earlier): \(error)")
}
writeTask.cancel()
pp_log(.openvpn, .info, "Sent OCCPacket correctly")
}
func cleanup() async {
link?.shutdown()
for neg in negotiators.values {