Skip to content

Commit de3c283

Browse files
committed
attempt to fix socks5 udp handling for MacOS
Right now if UDP receives empty message, it goes into hot loop (consuming entire core). This is probably not something that was intented. Fix is two part: * Close the udp session on error. * After the empty message wait at least 50ms before attempting again.
1 parent d3deb8a commit de3c283

1 file changed

Lines changed: 54 additions & 14 deletions

File tree

MacOS/ProxyBridge/extension/AppProxyProvider.swift

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,37 @@ class AppProxyProvider: NETransparentProxyProvider {
339339
}
340340

341341
override func stopProxy(with reason: NEProviderStopReason, completionHandler: @escaping () -> Void) {
342+
tcpConnectionsLock.lock()
343+
let drainedUDPResources = udpResources
344+
udpResources.removeAll()
345+
tcpConnectionsLock.unlock()
346+
347+
for (flow, resources) in drainedUDPResources {
348+
resources.udpSession?.cancel()
349+
resources.tcpConnection.cancel()
350+
flow.closeReadWithError(nil)
351+
flow.closeWriteWithError(nil)
352+
}
353+
342354
completionHandler()
343355
}
344356

345-
private var udpTCPConnections: [NEAppProxyUDPFlow: NWTCPConnection] = [:]
357+
private var udpResources: [NEAppProxyUDPFlow: (tcpConnection: NWTCPConnection, udpSession: NWUDPSession)] = [:]
346358
private let tcpConnectionsLock = NSLock()
359+
private let udpRetryQueue = DispatchQueue(label: "com.interceptsuite.ProxyBridge.udp-retry", qos: .utility)
360+
361+
private func cleanupUDPResources(for clientFlow: NEAppProxyUDPFlow, error: Error? = nil) {
362+
tcpConnectionsLock.lock()
363+
let removedResources = udpResources.removeValue(forKey: clientFlow)
364+
tcpConnectionsLock.unlock()
365+
366+
if let (tcpConnection, udpSession) = removedResources {
367+
udpSession.cancel()
368+
tcpConnection.cancel()
369+
}
370+
clientFlow.closeReadWithError(error)
371+
clientFlow.closeWriteWithError(error)
372+
}
347373

348374
override func handleAppMessage(_ messageData: Data, completionHandler: ((Data?) -> Void)?) {
349375
guard let message = try? JSONSerialization.jsonObject(with: messageData) as? [String: Any],
@@ -638,6 +664,7 @@ class AppProxyProvider: NETransparentProxyProvider {
638664

639665
if let error = error {
640666
self.log("Failed to open UDP flow: \(error.localizedDescription)", level: "ERROR")
667+
self.cleanupUDPResources(for: flow, error: error)
641668
return
642669
}
643670

@@ -664,20 +691,20 @@ class AppProxyProvider: NETransparentProxyProvider {
664691
guard let self = self else { return }
665692

666693
if let error = error {
667-
self.log("SOCKS5 UDP greeting failed: \(error.localizedDescription)", level: "ERROR")
694+
self.failUDPSetup(for: clientFlow, tcpConnection: tcpConnection, message: "SOCKS5 UDP greeting failed: \(error.localizedDescription)", error: error)
668695
return
669696
}
670697

671698
tcpConnection.readMinimumLength(2, maximumLength: 2) { [weak self] data, error in
672699
guard let self = self else { return }
673700

674701
if let error = error {
675-
self.log("SOCKS5 UDP greeting response failed: \(error.localizedDescription)", level: "ERROR")
702+
self.failUDPSetup(for: clientFlow, tcpConnection: tcpConnection, message: "SOCKS5 UDP greeting response failed: \(error.localizedDescription)", error: error)
676703
return
677704
}
678705

679706
guard let data = data, data.count == 2, data[1] == 0x00 else {
680-
self.log("SOCKS5 UDP greeting response failed", level: "ERROR")
707+
self.failUDPSetup(for: clientFlow, tcpConnection: tcpConnection, message: "SOCKS5 UDP greeting response failed")
681708
return
682709
}
683710

@@ -699,20 +726,20 @@ class AppProxyProvider: NETransparentProxyProvider {
699726
guard let self = self else { return }
700727

701728
if let error = error {
702-
self.log("SOCKS5 UDP ASSOCIATE failed: \(error.localizedDescription)", level: "ERROR")
729+
self.failUDPSetup(for: clientFlow, tcpConnection: tcpConnection, message: "SOCKS5 UDP ASSOCIATE failed: \(error.localizedDescription)", error: error)
703730
return
704731
}
705732

706733
tcpConnection.readMinimumLength(10, maximumLength: 512) { [weak self] data, error in
707734
guard let self = self else { return }
708735

709736
if let error = error {
710-
self.log("SOCKS5 UDP ASSOCIATE response error: \(error.localizedDescription)", level: "ERROR")
737+
self.failUDPSetup(for: clientFlow, tcpConnection: tcpConnection, message: "SOCKS5 UDP ASSOCIATE response error: \(error.localizedDescription)", error: error)
711738
return
712739
}
713740

714741
guard let data = data, data.count >= 10, data[0] == 0x05, data[1] == 0x00 else {
715-
self.log("SOCKS5 UDP ASSOCIATE rejected", level: "ERROR")
742+
self.failUDPSetup(for: clientFlow, tcpConnection: tcpConnection, message: "SOCKS5 UDP ASSOCIATE rejected")
716743
return
717744
}
718745

@@ -722,6 +749,12 @@ class AppProxyProvider: NETransparentProxyProvider {
722749
}
723750
}
724751

752+
private func failUDPSetup(for clientFlow: NEAppProxyUDPFlow, tcpConnection: NWTCPConnection, message: String, error: Error? = nil) {
753+
log(message, level: "ERROR")
754+
cleanupUDPResources(for: clientFlow, error: error)
755+
tcpConnection.cancel()
756+
}
757+
725758
private func parseSOCKS5Address(from data: Data, offset: Int) -> (String, UInt16) {
726759
let atyp = data[offset]
727760

@@ -754,26 +787,32 @@ class AppProxyProvider: NETransparentProxyProvider {
754787
let udpSession = self.createUDPSession(to: relayEndpoint, from: nil)
755788

756789
tcpConnectionsLock.lock()
757-
udpTCPConnections[clientFlow] = tcpConnection
790+
udpResources[clientFlow] = (tcpConnection: tcpConnection, udpSession: udpSession)
758791
tcpConnectionsLock.unlock()
759792

760793
readAndForwardClientUDP(clientFlow: clientFlow, udpSession: udpSession, processPath: processPath)
761794
readAndForwardRelayUDP(clientFlow: clientFlow, udpSession: udpSession)
762795
}
763796

764-
private func readAndForwardClientUDP(clientFlow: NEAppProxyUDPFlow, udpSession: NWUDPSession, processPath: String) {
765-
var isFirstPacket = true
797+
private func readAndForwardClientUDP(clientFlow: NEAppProxyUDPFlow, udpSession: NWUDPSession, processPath: String, emptyReadCount: Int = 0) {
798+
var isFirstPacket = isFirstPacket
766799

767800
clientFlow.readDatagrams { [weak self] datagrams, endpoints, error in
768801
guard let self = self else { return }
769802

770803
if let error = error {
771804
self.log("UDP read error: \(error.localizedDescription)", level: "ERROR")
805+
self.cleanupUDPResources(for: clientFlow, error: error)
772806
return
773807
}
774808

775809
guard let datagrams = datagrams, let endpoints = endpoints, !datagrams.isEmpty else {
776-
self.readAndForwardClientUDP(clientFlow: clientFlow, udpSession: udpSession, processPath: processPath)
810+
let cappedEmptyReadCount = min(emptyReadCount + 1, 4)
811+
let delayMs = 50 * (1 << min(emptyReadCount, 4))
812+
813+
self.udpRetryQueue.asyncAfter(deadline: .now() + .milliseconds(delayMs)) {
814+
self.readAndForwardClientUDP(clientFlow: clientFlow, udpSession: udpSession, processPath: processPath, emptyReadCount: cappedEmptyReadCount)
815+
}
777816
return
778817
}
779818

@@ -800,12 +839,13 @@ class AppProxyProvider: NETransparentProxyProvider {
800839
udpSession.writeDatagram(encapsulated, completionHandler: { error in
801840
if let error = error {
802841
self.log("UDP write error: \(error)", level: "ERROR")
842+
self.cleanupUDPResources(for: clientFlow, error: error)
803843
}
804844
})
805845
}
806846
}
807847

808-
self.readAndForwardClientUDP(clientFlow: clientFlow, udpSession: udpSession, processPath: processPath)
848+
self.readAndForwardClientUDP(clientFlow: clientFlow, udpSession: udpSession, processPath: processPath, emptyReadCount: 0)
809849
}
810850
}
811851

@@ -815,6 +855,7 @@ class AppProxyProvider: NETransparentProxyProvider {
815855

816856
if let error = error {
817857
self.log("UDP relay error: \(error.localizedDescription)", level: "ERROR")
858+
self.cleanupUDPResources(for: clientFlow, error: error)
818859
return
819860
}
820861

@@ -833,6 +874,7 @@ class AppProxyProvider: NETransparentProxyProvider {
833874
clientFlow.writeDatagrams(unwrappedDatagrams, sentBy: unwrappedEndpoints) { error in
834875
if let error = error {
835876
self.log("UDP response write error: \(error.localizedDescription)", level: "ERROR")
877+
self.cleanupUDPResources(for: clientFlow, error: error)
836878
}
837879
}
838880
}
@@ -1355,5 +1397,3 @@ class AppProxyProvider: NETransparentProxyProvider {
13551397
logQueueLock.unlock()
13561398
}
13571399
}
1358-
1359-

0 commit comments

Comments
 (0)