From ef5625a3bc5d75a1b6d579d6c695da8c08adf9d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Mon, 6 Apr 2026 12:42:22 +0700 Subject: [PATCH 1/2] fix: replace semaphore deadlock pattern with async/await in SSH host key verification --- CHANGELOG.md | 1 + TablePro/Core/SSH/HostKeyVerifier.swift | 134 +++++++++---------- TablePro/Core/SSH/LibSSH2TunnelFactory.swift | 18 +-- TablePro/Core/SSH/SSHTunnelManager.swift | 28 ++-- 4 files changed, 89 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a728c248..81226dd59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Use sheet presentation for password and TOTP prompts instead of blocking modal dialogs - Fix localized strings with interpolation creating untranslatable dynamic keys - Fix crash when closing window during SSH tunnel connection (use-after-free in libssh2) +- Fix potential deadlock in SSH host key verification prompts (semaphore → async/await) - Fix data race in ConnectionStorage, GroupStorage, and TagStorage (added @MainActor isolation) ### Added diff --git a/TablePro/Core/SSH/HostKeyVerifier.swift b/TablePro/Core/SSH/HostKeyVerifier.swift index c6608b9e3..c03339a40 100644 --- a/TablePro/Core/SSH/HostKeyVerifier.swift +++ b/TablePro/Core/SSH/HostKeyVerifier.swift @@ -15,8 +15,6 @@ internal enum HostKeyVerifier { private static let logger = Logger(subsystem: "com.TablePro", category: "HostKeyVerifier") /// Verify the host key, prompting the user if needed. - /// This method blocks the calling thread while showing UI prompts. - /// Must be called from a background thread. /// - Parameters: /// - keyData: The raw host key bytes from the SSH session /// - keyType: The key type string (e.g. "ssh-rsa", "ssh-ed25519") @@ -28,7 +26,7 @@ internal enum HostKeyVerifier { keyType: String, hostname: String, port: Int - ) throws { + ) async throws { let result = HostKeyStore.shared.verify( keyData: keyData, keyType: keyType, @@ -43,7 +41,7 @@ internal enum HostKeyVerifier { case .unknown(let fingerprint, let keyType): logger.info("Unknown host key for [\(hostname)]:\(port), prompting user") - let accepted = promptUnknownHost( + let accepted = await promptUnknownHost( hostname: hostname, port: port, fingerprint: fingerprint, @@ -62,7 +60,7 @@ internal enum HostKeyVerifier { case .mismatch(let expected, let actual): logger.warning("Host key mismatch for [\(hostname)]:\(port)") - let accepted = promptHostKeyMismatch( + let accepted = await promptHostKeyMismatch( hostname: hostname, port: port, expected: expected, @@ -83,85 +81,87 @@ internal enum HostKeyVerifier { // MARK: - UI Prompts - /// Show a dialog asking the user whether to trust an unknown host - /// Blocks the calling thread until the user responds. + @MainActor private static func promptUnknownHost( hostname: String, port: Int, fingerprint: String, keyType: String - ) -> Bool { - let semaphore = DispatchSemaphore(value: 0) - var accepted = false - + ) async -> Bool { let hostDisplay = "[\(hostname)]:\(port)" let title = String(localized: "Unknown SSH Host") - let message = String(localized: """ - The authenticity of host '\(hostDisplay)' can't be established. - - \(keyType) key fingerprint is: - \(fingerprint) - - Are you sure you want to continue connecting? - """) - - DispatchQueue.main.async { - let alert = NSAlert() - alert.messageText = title - alert.informativeText = message - alert.alertStyle = .informational - alert.addButton(withTitle: String(localized: "Trust")) - alert.addButton(withTitle: String(localized: "Cancel")) - - let response = alert.runModal() - accepted = (response == .alertFirstButtonReturn) - semaphore.signal() - } + let message = String( + format: String(localized: """ + The authenticity of host '%@' can't be established. + + %@ key fingerprint is: + %@ + + Are you sure you want to continue connecting? + """), + hostDisplay, + keyType, + fingerprint + ) - semaphore.wait() - return accepted + let alert = NSAlert() + alert.messageText = title + alert.informativeText = message + alert.alertStyle = .informational + alert.addButton(withTitle: String(localized: "Trust")) + alert.addButton(withTitle: String(localized: "Cancel")) + + if let window = NSApp.keyWindow { + return await withCheckedContinuation { continuation in + alert.beginSheetModal(for: window) { response in + continuation.resume(returning: response == .alertFirstButtonReturn) + } + } + } + return alert.runModal() == .alertFirstButtonReturn } - /// Show a warning dialog about a changed host key (potential MITM attack) - /// Blocks the calling thread until the user responds. + @MainActor private static func promptHostKeyMismatch( hostname: String, port: Int, expected: String, actual: String - ) -> Bool { - let semaphore = DispatchSemaphore(value: 0) - var accepted = false - + ) async -> Bool { let hostDisplay = "[\(hostname)]:\(port)" let title = String(localized: "SSH Host Key Changed") - let message = String(localized: """ - WARNING: The host key for '\(hostDisplay)' has changed! - - This could mean someone is doing something malicious, or the server was reinstalled. - - Previous fingerprint: \(expected) - Current fingerprint: \(actual) - """) - - DispatchQueue.main.async { - let alert = NSAlert() - alert.messageText = title - alert.informativeText = message - alert.alertStyle = .critical - alert.addButton(withTitle: String(localized: "Connect Anyway")) - alert.addButton(withTitle: String(localized: "Disconnect")) - - // Make "Disconnect" the default button (Return key) instead of "Connect Anyway" - alert.buttons[1].keyEquivalent = "\r" - alert.buttons[0].keyEquivalent = "" - - let response = alert.runModal() - accepted = (response == .alertFirstButtonReturn) - semaphore.signal() - } + let message = String( + format: String(localized: """ + WARNING: The host key for '%@' has changed! + + This could mean someone is doing something malicious, or the server was reinstalled. + + Previous fingerprint: %@ + Current fingerprint: %@ + """), + hostDisplay, + expected, + actual + ) - semaphore.wait() - return accepted + let alert = NSAlert() + alert.messageText = title + alert.informativeText = message + alert.alertStyle = .critical + alert.addButton(withTitle: String(localized: "Connect Anyway")) + alert.addButton(withTitle: String(localized: "Disconnect")) + + // Make "Disconnect" the default button (Return key) instead of "Connect Anyway" + alert.buttons[1].keyEquivalent = "\r" + alert.buttons[0].keyEquivalent = "" + + if let window = NSApp.keyWindow { + return await withCheckedContinuation { continuation in + alert.beginSheetModal(for: window) { response in + continuation.resume(returning: response == .alertFirstButtonReturn) + } + } + } + return alert.runModal() == .alertFirstButtonReturn } } diff --git a/TablePro/Core/SSH/LibSSH2TunnelFactory.swift b/TablePro/Core/SSH/LibSSH2TunnelFactory.swift index 12b7210a0..98923aea5 100644 --- a/TablePro/Core/SSH/LibSSH2TunnelFactory.swift +++ b/TablePro/Core/SSH/LibSSH2TunnelFactory.swift @@ -38,10 +38,10 @@ internal enum LibSSH2TunnelFactory { remoteHost: String, remotePort: Int, localPort: Int - ) throws -> LibSSH2Tunnel { + ) async throws -> LibSSH2Tunnel { _ = initialized - let chain = try buildAuthenticatedChain( + let chain = try await buildAuthenticatedChain( config: config, credentials: credentials, queueLabel: "com.TablePro.ssh.hop.\(connectionId.uuidString)" @@ -83,10 +83,10 @@ internal enum LibSSH2TunnelFactory { static func testConnection( config: SSHConfiguration, credentials: SSHTunnelCredentials - ) throws { + ) async throws { _ = initialized - let chain = try buildAuthenticatedChain( + let chain = try await buildAuthenticatedChain( config: config, credentials: credentials, queueLabel: "com.TablePro.ssh.test-hop" @@ -119,7 +119,7 @@ internal enum LibSSH2TunnelFactory { config: SSHConfiguration, credentials: SSHTunnelCredentials, queueLabel: String - ) throws -> AuthenticatedChain { + ) async throws -> AuthenticatedChain { let targetHost: String let targetPort: Int @@ -141,7 +141,7 @@ internal enum LibSSH2TunnelFactory { do { // Verify host key - try verifyHostKey(session: session, hostname: targetHost, port: targetPort) + try await verifyHostKey(session: session, hostname: targetHost, port: targetPort) // Authenticate first hop if let firstJump = config.jumpHosts.first { @@ -218,7 +218,7 @@ internal enum LibSSH2TunnelFactory { do { // Verify host key for next hop - try verifyHostKey(session: nextSession, hostname: nextHost, port: nextPort) + try await verifyHostKey(session: nextSession, hostname: nextHost, port: nextPort) // Authenticate next hop if jumpIndex + 1 < jumps.count { @@ -416,7 +416,7 @@ internal enum LibSSH2TunnelFactory { session: OpaquePointer, hostname: String, port: Int - ) throws { + ) async throws { var keyLength = 0 var keyType: Int32 = 0 guard let keyPtr = libssh2_session_hostkey(session, &keyLength, &keyType) else { @@ -426,7 +426,7 @@ internal enum LibSSH2TunnelFactory { let keyData = Data(bytes: keyPtr, count: keyLength) let keyTypeName = HostKeyStore.keyTypeName(keyType) - try HostKeyVerifier.verify( + try await HostKeyVerifier.verify( keyData: keyData, keyType: keyTypeName, hostname: hostname, diff --git a/TablePro/Core/SSH/SSHTunnelManager.swift b/TablePro/Core/SSH/SSHTunnelManager.swift index 847b2672c..6a941be88 100644 --- a/TablePro/Core/SSH/SSHTunnelManager.swift +++ b/TablePro/Core/SSH/SSHTunnelManager.swift @@ -102,16 +102,14 @@ actor SSHTunnelManager { // Try ports until one works for localPort in localPortCandidates() { do { - let tunnel = try await Task.detached { - try LibSSH2TunnelFactory.createTunnel( - connectionId: connectionId, - config: config, - credentials: credentials, - remoteHost: remoteHost, - remotePort: remotePort, - localPort: localPort - ) - }.value + let tunnel = try await LibSSH2TunnelFactory.createTunnel( + connectionId: connectionId, + config: config, + credentials: credentials, + remoteHost: remoteHost, + remotePort: remotePort, + localPort: localPort + ) tunnel.onDeath = { [weak self] id in Task { [weak self] in @@ -176,12 +174,10 @@ actor SSHTunnelManager { config: SSHConfiguration, credentials: SSHTunnelCredentials ) async throws { - try await Task.detached { - try LibSSH2TunnelFactory.testConnection( - config: config, - credentials: credentials - ) - }.value + try await LibSSH2TunnelFactory.testConnection( + config: config, + credentials: credentials + ) } /// Check if a tunnel exists for a connection From 1d2a201fd60daeb9579a0502054233bf1c34776b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Mon, 6 Apr 2026 12:51:09 +0700 Subject: [PATCH 2/2] fix: restore Task.detached for blocking libssh2 I/O to avoid holding cooperative threads --- TablePro/Core/SSH/SSHTunnelManager.swift | 28 ++++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/TablePro/Core/SSH/SSHTunnelManager.swift b/TablePro/Core/SSH/SSHTunnelManager.swift index 6a941be88..c7cf9b702 100644 --- a/TablePro/Core/SSH/SSHTunnelManager.swift +++ b/TablePro/Core/SSH/SSHTunnelManager.swift @@ -102,14 +102,16 @@ actor SSHTunnelManager { // Try ports until one works for localPort in localPortCandidates() { do { - let tunnel = try await LibSSH2TunnelFactory.createTunnel( - connectionId: connectionId, - config: config, - credentials: credentials, - remoteHost: remoteHost, - remotePort: remotePort, - localPort: localPort - ) + let tunnel = try await Task.detached { + try await LibSSH2TunnelFactory.createTunnel( + connectionId: connectionId, + config: config, + credentials: credentials, + remoteHost: remoteHost, + remotePort: remotePort, + localPort: localPort + ) + }.value tunnel.onDeath = { [weak self] id in Task { [weak self] in @@ -174,10 +176,12 @@ actor SSHTunnelManager { config: SSHConfiguration, credentials: SSHTunnelCredentials ) async throws { - try await LibSSH2TunnelFactory.testConnection( - config: config, - credentials: credentials - ) + try await Task.detached { + try await LibSSH2TunnelFactory.testConnection( + config: config, + credentials: credentials + ) + }.value } /// Check if a tunnel exists for a connection