Skip to content

Commit d1a94ba

Browse files
authored
fix(ssh): reason-aware auth-failure messages and lazy TOTP retry (#1018)
* fix(ssh): reason-aware auth-failure messages and lazy TOTP retry * fix(ssh): drop em-dash and AI-vibe wording from auth-failure strings
1 parent bc18374 commit d1a94ba

13 files changed

Lines changed: 286 additions & 44 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313

1414
### Fixed
1515

16+
- SSH auth-failure alerts now point at the actual cause. The catch-all "Check your credentials or private key." string was wrong for the most common 2FA case (typing a wrong TOTP code), because the credentials were fine; only the verification code was wrong. `SSHTunnelError.authenticationFailed` now carries an `AuthFailureReason` (`password`, `verificationCode`, `privateKey`, `agentRejected`, `generic`), every throw site picks the right one, and the alert text matches: "Verification code rejected. Get a new code from your authenticator app and try again." for kbd-int + TOTP rejections, "SSH password rejected. Check the password and try again." for plain password rejections, and so on. Follow-up to #1005.
17+
- TOTP codes are now fetched lazily from the `TOTPProvider` inside the kbd-int callback rather than once upfront before authentication starts. Fixes two related issues: (1) when the kbd-int handshake straddled a 30-second TOTP rotation boundary, the `AutoTOTPProvider` code that was valid at fetch time had expired by the time PAM validated it; (2) when the server retried after a wrong code (sshd defaults to 3 prompts per session), TablePro replayed the same wrong code instead of asking the user for a new one. `PromptTOTPProvider` now switches its NSAlert wording to "Verification Code Rejected. The previous code wasn't accepted. Wait for your authenticator to refresh, then enter the new code." on retries, matching how OpenSSH re-prompts.
1618
- SSH Auth Method = Password failed against servers that only advertise `keyboard-interactive` (the typical `pam_google_authenticator` setup, where `sshd_config` has `AuthenticationMethods keyboard-interactive`). The bare `userauth_password` request the server doesn't accept was the only attempt, so connection failed even when the user typed the right password. `buildAuthenticator` now always pairs `PasswordAuthenticator` with a `KeyboardInteractiveAuthenticator` that reuses the same password, matching OpenSSH and Sequel Ace's "save the password, the server may also prompt for a code" flow. Follow-up to #1005.
1719
- SSH connections to servers requiring Google Authenticator (or any PAM stack that pairs `Password:` with `Verification code:` over keyboard-interactive) failed when the user picked Auth Method = Password with TOTP enabled. `LibSSH2TunnelFactory.buildAuthenticator` built a `CompositeAuthenticator(PasswordAuthenticator → KeyboardInteractiveAuthenticator)` but passed `password: nil` to the kbd-interactive fallback, so when the server replayed the password challenge the callback answered with an empty string. The fallback now reuses the same SSH password, mirroring the `.keyboardInteractive` path. Fixes #1005.
1820
- Up arrow on the first line of the SQL editor failed to jump to the start of the document when the caret was at the very end of a single-line query (or anywhere outside the half-open `lineStorage.first.range`). Same shape on the symmetric Down-on-last-line path. The first/last-line detection used `NSRange.contains`, which is half-open and excludes the trailing offset, so the caret-at-end-of-document fell through to a geometry probe that found no line above and bailed. Now the check uses `textLineForOffset`, which already handles `offset == length` by returning the last line, and compares line IDs.

TablePro/Core/SSH/Auth/AgentAuthenticator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,6 @@ internal struct AgentAuthenticator: SSHAuthenticator {
110110
}
111111

112112
Self.logger.error("SSH agent authentication failed: no identity accepted")
113-
throw SSHTunnelError.authenticationFailed
113+
throw SSHTunnelError.authenticationFailed(reason: .agentRejected)
114114
}
115115
}

TablePro/Core/SSH/Auth/CompositeAuthenticator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ internal struct CompositeAuthenticator: SSHAuthenticator {
3333
}
3434

3535
if libssh2_userauth_authenticated(session) == 0 {
36-
throw lastError ?? SSHTunnelError.authenticationFailed
36+
throw lastError ?? SSHTunnelError.authenticationFailed(reason: .generic)
3737
}
3838
}
3939
}

TablePro/Core/SSH/Auth/KeyboardInteractiveAuthenticator.swift

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,38 @@ internal enum KBDINTPromptType {
1515
case unknown
1616
}
1717

18-
/// Context passed through the libssh2 session abstract pointer to the C callback
18+
/// Context passed through the libssh2 session abstract pointer to the C callback.
19+
///
20+
/// TOTP codes are fetched lazily inside the callback (not upfront) so that:
21+
/// - `AutoTOTPProvider` generates a code that's still valid when PAM validates it. The
22+
/// upfront approach raced the 30-second window during the SSH handshake.
23+
/// - When the server retries the kbd-int session after a wrong code (PAM defaults to
24+
/// 3 prompts), each retry calls `provideCode(attempt:)` again, matching how OpenSSH
25+
/// re-prompts the user.
1926
internal final class KeyboardInteractiveContext {
20-
var password: String?
21-
var totpCode: String?
27+
let password: String?
28+
let totpProvider: (any TOTPProvider)?
29+
var totpAttemptCount: Int = 0
30+
var lastTotpError: Error?
2231

23-
init(password: String?, totpCode: String?) {
32+
init(password: String?, totpProvider: (any TOTPProvider)?) {
2433
self.password = password
25-
self.totpCode = totpCode
34+
self.totpProvider = totpProvider
35+
}
36+
37+
/// Fetches the next TOTP code. Errors from the provider (user cancelled, missing
38+
/// secret) are stored in `lastTotpError` and surface at the end of the kbd-int session.
39+
/// The C callback can't throw across the libssh2 boundary, so we record the failure
40+
/// and report it after `libssh2_userauth_keyboard_interactive_ex` returns.
41+
func nextTotpCode() -> String {
42+
guard let totpProvider else { return "" }
43+
defer { totpAttemptCount += 1 }
44+
do {
45+
return try totpProvider.provideCode(attempt: totpAttemptCount)
46+
} catch {
47+
lastTotpError = error
48+
return ""
49+
}
2650
}
2751
}
2852

@@ -67,7 +91,7 @@ private let kbdintCallback: @convention(c) (
6791
case .password:
6892
responseText = context.password ?? ""
6993
case .totp:
70-
responseText = context.totpCode ?? ""
94+
responseText = context.nextTotpCode()
7195
case .unknown:
7296
// Fall back to password for unrecognized prompts
7397
responseText = context.password ?? ""
@@ -89,16 +113,9 @@ internal struct KeyboardInteractiveAuthenticator: SSHAuthenticator {
89113
let totpProvider: (any TOTPProvider)?
90114

91115
func authenticate(session: OpaquePointer, username: String) throws {
92-
// Generate TOTP code if a provider is available
93-
let totpCode: String?
94-
if let totpProvider {
95-
totpCode = try totpProvider.provideCode()
96-
} else {
97-
totpCode = nil
98-
}
99-
100-
// Create context and store in session abstract pointer
101-
let context = KeyboardInteractiveContext(password: password, totpCode: totpCode)
116+
// Hand the provider to the callback so it can fetch a fresh code on every challenge
117+
// (see KeyboardInteractiveContext doc comment for why this isn't done upfront).
118+
let context = KeyboardInteractiveContext(password: password, totpProvider: totpProvider)
102119
let contextPtr = Unmanaged.passRetained(context).toOpaque()
103120

104121
defer {
@@ -124,13 +141,22 @@ internal struct KeyboardInteractiveAuthenticator: SSHAuthenticator {
124141
kbdintCallback
125142
)
126143

144+
// Surface a totpProvider error (e.g. user cancelled the NSAlert) verbatim. It's
145+
// already an SSHTunnelError with the right reason.
146+
if let providerError = context.lastTotpError {
147+
throw providerError
148+
}
149+
127150
guard rc == 0 else {
128151
var msgPtr: UnsafeMutablePointer<CChar>?
129152
var msgLen: Int32 = 0
130153
libssh2_session_last_error(session, &msgPtr, &msgLen, 0)
131154
let detail = msgPtr.map { String(cString: $0) } ?? "Unknown error"
132155
Self.logger.error("Keyboard-interactive authentication failed: \(detail)")
133-
throw SSHTunnelError.authenticationFailed
156+
// If a TOTP code was actually delivered to the server, the rejection is most
157+
// likely about that code. Point the user at the authenticator, not the password.
158+
let reason: AuthFailureReason = context.totpAttemptCount > 0 ? .verificationCode : .password
159+
throw SSHTunnelError.authenticationFailed(reason: reason)
134160
}
135161

136162
Self.logger.info("Keyboard-interactive authentication succeeded")

TablePro/Core/SSH/Auth/PasswordAuthenticator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal struct PasswordAuthenticator: SSHAuthenticator {
2626
libssh2_session_last_error(session, &msgPtr, &msgLen, 0)
2727
let detail = msgPtr.map { String(cString: $0) } ?? "Unknown error"
2828
Self.logger.error("Password authentication failed: \(detail)")
29-
throw SSHTunnelError.authenticationFailed
29+
throw SSHTunnelError.authenticationFailed(reason: .password)
3030
}
3131
}
3232
}

TablePro/Core/SSH/Auth/PromptTOTPProvider.swift

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,23 @@ import Foundation
1111
/// This provider blocks the calling thread while the alert is displayed on the main thread.
1212
/// It is intended for interactive SSH sessions where no TOTP secret is configured.
1313
internal final class PromptTOTPProvider: TOTPProvider, @unchecked Sendable {
14-
func provideCode() throws -> String {
14+
func provideCode(attempt: Int) throws -> String {
1515
if Thread.isMainThread {
16-
return try handleResult(showAlert())
16+
return try handleResult(showAlert(attempt: attempt))
1717
}
18-
return try handleResult(DispatchQueue.main.sync { showAlert() })
18+
return try handleResult(DispatchQueue.main.sync { showAlert(attempt: attempt) })
1919
}
2020

2121
// Note: runModal() is intentional here. This method runs on the main thread
2222
// (via DispatchQueue.main.sync from provideCode), so beginSheetModal + semaphore would deadlock.
23-
private func showAlert() -> String? {
23+
private func showAlert(attempt: Int) -> String? {
2424
let alert = NSAlert()
25-
alert.messageText = String(localized: "Verification Code Required")
26-
alert.informativeText = String(
27-
localized: "Enter the TOTP verification code for SSH authentication."
28-
)
25+
alert.messageText = attempt == 0
26+
? String(localized: "Verification Code Required")
27+
: String(localized: "Verification Code Rejected")
28+
alert.informativeText = attempt == 0
29+
? String(localized: "Enter the TOTP verification code for SSH authentication.")
30+
: String(localized: "The previous code wasn't accepted. Wait for your authenticator to refresh, then enter the new code.")
2931
alert.alertStyle = .informational
3032
alert.addButton(withTitle: String(localized: "Connect"))
3133
alert.addButton(withTitle: String(localized: "Cancel"))
@@ -41,7 +43,7 @@ internal final class PromptTOTPProvider: TOTPProvider, @unchecked Sendable {
4143

4244
private func handleResult(_ code: String?) throws -> String {
4345
guard let totpCode = code, !totpCode.isEmpty else {
44-
throw SSHTunnelError.authenticationFailed
46+
throw SSHTunnelError.authenticationFailed(reason: .verificationCode)
4547
}
4648
return totpCode
4749
}

TablePro/Core/SSH/Auth/TOTPProvider.swift

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,19 @@ import Foundation
77

88
/// Protocol for providing TOTP verification codes
99
internal protocol TOTPProvider: Sendable {
10-
/// Generate or obtain a TOTP code
11-
/// - Returns: The TOTP code string
12-
/// - Throws: SSHTunnelError if the code cannot be obtained
13-
func provideCode() throws -> String
10+
/// Generate or obtain a TOTP code.
11+
/// - Parameter attempt: 0 for the first prompt in a session, 1+ for retries when the
12+
/// server rejected an earlier code (wrong digits, expired window). Implementations
13+
/// may use this to vary UI affordances. `PromptTOTPProvider` shows a "previous code
14+
/// was rejected" hint when `attempt > 0`.
15+
/// - Returns: The TOTP code string.
16+
/// - Throws: `SSHTunnelError` if the code cannot be obtained (user cancelled, no secret).
17+
func provideCode(attempt: Int) throws -> String
18+
}
19+
20+
extension TOTPProvider {
21+
/// Convenience for callers that only ever need a single code (test connections, sync probes).
22+
func provideCode() throws -> String { try provideCode(attempt: 0) }
1423
}
1524

1625
/// Automatically generates TOTP codes from a stored secret.
@@ -21,7 +30,7 @@ internal protocol TOTPProvider: Sendable {
2130
internal struct AutoTOTPProvider: TOTPProvider {
2231
let generator: TOTPGenerator
2332

24-
func provideCode() throws -> String {
33+
func provideCode(attempt: Int) throws -> String {
2534
let remaining = generator.secondsRemaining()
2635
if remaining < 5 {
2736
// Brief bounded sleep (max ~6s) to wait for next TOTP period.

TablePro/Core/SSH/LibSSH2TunnelFactory.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ internal enum LibSSH2TunnelFactory {
468468
// Sequel Ace's behavior.
469469
guard let sshPassword = credentials.sshPassword else {
470470
logger.error("SSH password is nil (Keychain lookup may have failed) for \(resolved.host)")
471-
throw SSHTunnelError.authenticationFailed
471+
throw SSHTunnelError.authenticationFailed(reason: .password)
472472
}
473473
let totpProvider = buildTOTPProvider(config: config, credentials: credentials)
474474
return CompositeAuthenticator(authenticators: [
@@ -479,7 +479,7 @@ internal enum LibSSH2TunnelFactory {
479479
case .privateKey:
480480
let keyPaths = effectiveKeyPaths(for: resolved)
481481
guard !keyPaths.isEmpty else {
482-
throw SSHTunnelError.authenticationFailed
482+
throw SSHTunnelError.authenticationFailed(reason: .privateKey)
483483
}
484484
var authenticators: [any SSHAuthenticator] = keyPaths.map { keyPath in
485485
buildKeyFileAuthenticator(
@@ -599,11 +599,11 @@ internal enum LibSSH2TunnelFactory {
599599
}
600600

601601
// 2. Prompt the user if allowed (key is encrypted, no stored passphrase)
602-
guard canPrompt else { throw SSHTunnelError.authenticationFailed }
602+
guard canPrompt else { throw SSHTunnelError.authenticationFailed(reason: .privateKey) }
603603

604604
let provider = PromptPassphraseProvider(keyPath: expandedPath)
605605
guard let promptResult = provider.providePassphrase() else {
606-
throw SSHTunnelError.authenticationFailed
606+
throw SSHTunnelError.authenticationFailed(reason: .privateKey)
607607
}
608608

609609
let retryAuth = PublicKeyAuthenticator(
@@ -643,7 +643,7 @@ internal enum LibSSH2TunnelFactory {
643643
case .privateKey:
644644
let keyPaths = effectiveKeyPaths(for: resolved)
645645
guard !keyPaths.isEmpty else {
646-
throw SSHTunnelError.authenticationFailed
646+
throw SSHTunnelError.authenticationFailed(reason: .privateKey)
647647
}
648648
let authenticators = keyPaths.map { path in
649649
KeyFileAuthenticator(

TablePro/Core/SSH/SSHTunnelManager.swift

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,23 @@
88
import Foundation
99
import os
1010

11+
/// Why an SSH authentication attempt failed. Drives the user-facing error string so the
12+
/// alert points at the actual cause (wrong OTP, missing key, agent rejection) instead of
13+
/// the catch-all "credentials or private key" message.
14+
enum AuthFailureReason: Sendable, Equatable {
15+
case password
16+
case verificationCode
17+
case privateKey
18+
case agentRejected
19+
case generic
20+
}
21+
1122
/// Error types for SSH tunnel operations
12-
enum SSHTunnelError: Error, LocalizedError {
23+
enum SSHTunnelError: Error, LocalizedError, Equatable {
1324
case tunnelCreationFailed(String)
1425
case tunnelAlreadyExists(UUID)
1526
case noAvailablePort
16-
case authenticationFailed
27+
case authenticationFailed(reason: AuthFailureReason)
1728
case connectionTimeout
1829
case hostKeyVerificationFailed
1930
case channelOpenFailed
@@ -26,8 +37,19 @@ enum SSHTunnelError: Error, LocalizedError {
2637
return String(format: String(localized: "SSH tunnel already exists for connection: %@"), id.uuidString)
2738
case .noAvailablePort:
2839
return String(localized: "No available local port for SSH tunnel")
29-
case .authenticationFailed:
30-
return String(localized: "SSH authentication failed. Check your credentials or private key.")
40+
case .authenticationFailed(let reason):
41+
switch reason {
42+
case .password:
43+
return String(localized: "SSH password rejected. Check the password and try again.")
44+
case .verificationCode:
45+
return String(localized: "Verification code rejected. Get a new code from your authenticator app and try again.")
46+
case .privateKey:
47+
return String(localized: "SSH private key rejected. Check the key file or passphrase.")
48+
case .agentRejected:
49+
return String(localized: "SSH agent did not authenticate. Run ssh-add -l to check loaded keys.")
50+
case .generic:
51+
return String(localized: "SSH authentication failed. Check your credentials or private key.")
52+
}
3153
case .connectionTimeout:
3254
return String(localized: "SSH connection timed out")
3355
case .hostKeyVerificationFailed:

TablePro/Resources/Localizable.xcstrings

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43304,6 +43304,9 @@
4330443304
}
4330543305
}
4330643306
}
43307+
},
43308+
"SSH agent did not authenticate. Make sure the right key is loaded (try ssh-add -l)." : {
43309+
4330743310
},
4330843311
"SSH authentication failed. Check your credentials or private key." : {
4330943312
"localizations" : {
@@ -43485,6 +43488,9 @@
4348543488
}
4348643489
}
4348743490
}
43491+
},
43492+
"SSH password rejected. Check the password for this connection." : {
43493+
4348843494
},
4348943495
"SSH Port" : {
4349043496
"localizations" : {
@@ -43510,6 +43516,9 @@
4351043516
},
4351143517
"SSH port must be between 1 and 65535" : {
4351243518

43519+
},
43520+
"SSH private key rejected. Check the key file, passphrase, or pick a different identity." : {
43521+
4351343522
},
4351443523
"SSH Profile" : {
4351543524
"extractionState" : "stale",
@@ -46507,6 +46516,9 @@
4650746516
}
4650846517
}
4650946518
}
46519+
},
46520+
"The previous code wasn't accepted. Wait for your authenticator to roll over and enter the new code." : {
46521+
4651046522
},
4651146523
"The server will be accessible from other devices on your network. Authentication and TLS are enabled automatically." : {
4651246524
"localizations" : {
@@ -50495,6 +50507,12 @@
5049550507
}
5049650508
}
5049750509
}
50510+
},
50511+
"Verification Code Rejected" : {
50512+
50513+
},
50514+
"Verification code rejected. The code may be wrong or expired — try a fresh code from your authenticator." : {
50515+
5049850516
},
5049950517
"Verification Code Required" : {
5050050518
"localizations" : {

0 commit comments

Comments
 (0)