diff --git a/CHANGELOG.md b/CHANGELOG.md index fac3bf2fc7..4d65412976 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Cell editor opening off screen when `Tab` wrapped onto a row below the visible ones. - Cell cursor left on the old column after `Tab` carried the editor to the next one. - Every data grid switching to its accessibility layout after one `Tab` press, with no assistive app attached. +- SSH Agent auth prompting for a private key passphrase instead of reporting that the agent was never reached. (#2583) +- "SSH password rejected" on an SSH connection that has no password, when the server offers no keyboard-interactive. ## [0.69.0] - 2026-08-27 diff --git a/TablePro/Core/SSH/Auth/AgentAuthenticator.swift b/TablePro/Core/SSH/Auth/AgentAuthenticator.swift index df12cc4d81..4c4ad65bd2 100644 --- a/TablePro/Core/SSH/Auth/AgentAuthenticator.swift +++ b/TablePro/Core/SSH/Auth/AgentAuthenticator.swift @@ -12,6 +12,7 @@ internal struct AgentAuthenticator: SSHAuthenticator { private static let logger = Logger(subsystem: "com.TablePro", category: "AgentAuthenticator") let socketPath: String? + let socketOrigin: AgentSocketOrigin /// Resolve SSH_AUTH_SOCK via launchctl for GUI apps that don't inherit shell env. private static func resolveSocketViaLaunchctl() -> String? { @@ -51,7 +52,7 @@ internal struct AgentAuthenticator: SSHAuthenticator { } guard let agent = libssh2_agent_init(session) else { - throw SSHTunnelError.tunnelCreationFailed("Failed to initialize SSH agent") + throw SSHTunnelError.authenticationFailed(reason: .agentUnavailable(socketOrigin)) } defer { @@ -71,17 +72,18 @@ internal struct AgentAuthenticator: SSHAuthenticator { var rc = libssh2_agent_connect(agent) guard rc == 0 else { Self.logger.error("Failed to connect to SSH agent (rc=\(rc))") - throw SSHTunnelError.tunnelCreationFailed("Failed to connect to SSH agent") + throw SSHTunnelError.authenticationFailed(reason: .agentUnavailable(socketOrigin)) } rc = libssh2_agent_list_identities(agent) guard rc == 0 else { Self.logger.error("Failed to list SSH agent identities (rc=\(rc))") - throw SSHTunnelError.tunnelCreationFailed("Failed to list SSH agent identities") + throw SSHTunnelError.authenticationFailed(reason: .agentUnavailable(socketOrigin)) } var previousIdentity: UnsafeMutablePointer? var currentIdentity: UnsafeMutablePointer? + var offeredCount = 0 while true { rc = libssh2_agent_get_identity(agent, ¤tIdentity, previousIdentity) @@ -92,13 +94,14 @@ internal struct AgentAuthenticator: SSHAuthenticator { } if rc < 0 { Self.logger.error("Failed to get SSH agent identity (rc=\(rc))") - throw SSHTunnelError.tunnelCreationFailed("Failed to get SSH agent identity") + throw SSHTunnelError.authenticationFailed(reason: .agentUnavailable(socketOrigin)) } guard let identity = currentIdentity else { break } + offeredCount += 1 let authRc = libssh2_agent_userauth(agent, username, identity) if authRc == 0 { Self.logger.info("SSH agent authentication succeeded") @@ -108,7 +111,14 @@ internal struct AgentAuthenticator: SSHAuthenticator { previousIdentity = identity } - Self.logger.error("SSH agent authentication failed: no identity accepted") + // An agent that answered but offered nothing is a locked or empty agent, which the user + // fixes somewhere entirely different from an agent whose keys the server refused. + guard offeredCount > 0 else { + Self.logger.error("SSH agent offered no identities") + throw SSHTunnelError.authenticationFailed(reason: .agentNoIdentities(socketOrigin)) + } + + Self.logger.error("SSH agent authentication failed: none of \(offeredCount) identities accepted") throw SSHTunnelError.authenticationFailed(reason: .agentRejected) } } diff --git a/TablePro/Core/SSH/Auth/CompositeAuthenticator.swift b/TablePro/Core/SSH/Auth/CompositeAuthenticator.swift index 4c8e1bfd3c..97f3290ca3 100644 --- a/TablePro/Core/SSH/Auth/CompositeAuthenticator.swift +++ b/TablePro/Core/SSH/Auth/CompositeAuthenticator.swift @@ -10,11 +10,22 @@ import CLibSSH2 /// Authenticator that tries multiple auth methods in sequence. /// Used for servers requiring e.g. password + keyboard-interactive (TOTP). +/// +/// The reported failure is the last step that actually offered the server a credential. A later +/// step the server never engaged (keyboard-interactive on a server that issues no prompt) would +/// otherwise bury the real reason: an SSH agent that never answered used to surface as +/// "SSH password rejected" on a connection that has no password. internal struct CompositeAuthenticator: SSHAuthenticator { private static let logger = Logger(subsystem: "com.TablePro", category: "CompositeAuthenticator") let authenticators: [any SSHAuthenticator] + /// Failures after which the remaining steps are not worth running. The SSH Agent chain names + /// the two agent failures that mean no first factor was ever supplied, because the + /// keyboard-interactive step behind them is a second factor and would otherwise ask for a + /// credential of its own instead of reporting the agent. + var endsChainOn: Set = [] + func authenticate(session: OpaquePointer, username: String) throws { var lastError: Error? for (index, authenticator) in authenticators.enumerated() { @@ -25,7 +36,13 @@ internal struct CompositeAuthenticator: SSHAuthenticator { throw error } catch { Self.logger.debug("Authenticator \(index + 1) failed: \(error)") - lastError = error + if lastError == nil || Self.describesAnAttempt(error) { + lastError = error + } + if Self.reason(of: error).map(endsChainOn.contains) == true { + Self.logger.debug("Authenticator \(index + 1) ended the chain") + throw error + } } if libssh2_userauth_authenticated(session) != 0 { @@ -38,4 +55,14 @@ internal struct CompositeAuthenticator: SSHAuthenticator { throw lastError ?? SSHTunnelError.authenticationFailed(reason: .generic) } } + + private static func describesAnAttempt(_ error: any Error) -> Bool { + reason(of: error)?.describesAnAttempt ?? true + } + + private static func reason(of error: any Error) -> AuthFailureReason? { + guard let tunnelError = error as? SSHTunnelError, + case .authenticationFailed(let reason) = tunnelError else { return nil } + return reason + } } diff --git a/TablePro/Core/SSH/Auth/KeyboardInteractiveAuthenticator.swift b/TablePro/Core/SSH/Auth/KeyboardInteractiveAuthenticator.swift index 8462f87b5f..e9f55ec6aa 100644 --- a/TablePro/Core/SSH/Auth/KeyboardInteractiveAuthenticator.swift +++ b/TablePro/Core/SSH/Auth/KeyboardInteractiveAuthenticator.swift @@ -32,6 +32,7 @@ internal final class KeyboardInteractiveContext { let promptProvider: any KeyboardInteractivePromptProvider private(set) var totpAttemptCount = 0 private(set) var interactiveAttemptCount = 0 + private(set) var passwordAnswerCount = 0 private(set) var userCancelled = false var lastError: Error? @@ -45,6 +46,16 @@ internal final class KeyboardInteractiveContext { self.promptProvider = promptProvider } + /// What the rejection was about, named by whichever answer actually went to the server. A + /// server that issued no prompt at all never took a credential from this method, so the + /// failure says nothing about the user's own: it says keyboard-interactive was not on offer. + var failureReason: AuthFailureReason { + if interactiveAttemptCount > 0 { return .keyboardInteractive } + if totpAttemptCount > 0 { return .verificationCode } + if passwordAnswerCount > 0 { return .password } + return .methodUnavailable + } + func nextTotpCode() -> String { guard let totpProvider else { return "" } defer { totpAttemptCount += 1 } @@ -67,6 +78,7 @@ internal final class KeyboardInteractiveContext { switch KeyboardInteractiveAuthenticator.classify(prompt.text) { case .password where password != nil: results[index] = password + passwordAnswerCount += 1 case .totp where totpProvider != nil: results[index] = nextTotpCode() default: @@ -206,10 +218,7 @@ internal struct KeyboardInteractiveAuthenticator: SSHAuthenticator { libssh2_session_last_error(session, &msgPtr, &msgLen, 0) let detail = msgPtr.map { String(cString: $0) } ?? "Unknown error" Self.logger.error("Keyboard-interactive authentication failed: \(detail)") - let reason: AuthFailureReason = context.interactiveAttemptCount > 0 - ? .keyboardInteractive - : (context.totpAttemptCount > 0 ? .verificationCode : .password) - throw SSHTunnelError.authenticationFailed(reason: reason) + throw SSHTunnelError.authenticationFailed(reason: context.failureReason) } Self.logger.info("Keyboard-interactive authentication succeeded") diff --git a/TablePro/Core/SSH/LibSSH2TunnelFactory.swift b/TablePro/Core/SSH/LibSSH2TunnelFactory.swift index 016e03a222..e779314c03 100644 --- a/TablePro/Core/SSH/LibSSH2TunnelFactory.swift +++ b/TablePro/Core/SSH/LibSSH2TunnelFactory.swift @@ -35,7 +35,9 @@ internal enum LibSSH2TunnelFactory { // MARK: - Global Init - private static let initialized: Bool = { + /// libssh2's own header says `libssh2_init` uses global state and must not be called + /// concurrently, so every entry point in the process goes through this one lazy static. + internal static let initialized: Bool = { libssh2_init(0) return true }() @@ -501,8 +503,7 @@ internal enum LibSSH2TunnelFactory { buildKeyFileAuthenticator( keyPath: keyPath, providedPassphrase: credentials.keyPassphrase, - resolved: resolved, - canPrompt: true + resolved: resolved ) } authenticators.append(KeyboardInteractiveAuthenticator( @@ -513,28 +514,28 @@ internal enum LibSSH2TunnelFactory { return CompositeAuthenticator(authenticators: authenticators) case .sshAgent: + // The agent is the credential, so there is no key-file fallback: authenticating with a + // key the user never chose put TablePro's own passphrase prompt over an agent that had + // simply not been reached (#2583). Keyboard-interactive stays, being a second factor the + // same server asked for rather than another credential. let socketPath: String? = resolved.agentSocketPath.isEmpty ? nil : SSHPathUtilities.expandTilde(resolved.agentSocketPath) - var authenticators: [any SSHAuthenticator] = [AgentAuthenticator(socketPath: socketPath)] - - for keyPath in effectiveKeyPaths(for: resolved) { - authenticators.append(buildKeyFileAuthenticator( - keyPath: keyPath, - providedPassphrase: credentials.keyPassphrase, - resolved: resolved, - canPrompt: true - )) - } - - authenticators.append(KeyboardInteractiveAuthenticator( - password: nil, - totpProvider: buildTOTPProvider(config: config, credentials: credentials), - promptProvider: promptProvider - )) - - return CompositeAuthenticator(authenticators: authenticators) + return CompositeAuthenticator( + authenticators: [ + AgentAuthenticator(socketPath: socketPath, socketOrigin: resolved.agentSocketOrigin), + KeyboardInteractiveAuthenticator( + password: nil, + totpProvider: buildTOTPProvider(config: config, credentials: credentials), + promptProvider: promptProvider + ), + ], + endsChainOn: Set( + AgentSocketOrigin.allCases.map(AuthFailureReason.agentUnavailable) + + AgentSocketOrigin.allCases.map(AuthFailureReason.agentNoIdentities) + ) + ) case .keyboardInteractive: return KeyboardInteractiveAuthenticator( @@ -562,19 +563,16 @@ internal enum LibSSH2TunnelFactory { .filter { FileManager.default.isReadableFile(atPath: $0) } } - /// Passphrase resolution is deferred to auth time (not build time) so - /// that, when this authenticator is used as an agent fallback, the user - /// is only prompted if the agent actually fails. + /// Passphrase resolution is deferred to auth time (not build time) so that a key later in + /// the chain only prompts once the ones before it have actually been refused. private static func buildKeyFileAuthenticator( keyPath: String, providedPassphrase: String?, - resolved: ResolvedSSHTarget, - canPrompt: Bool + resolved: ResolvedSSHTarget ) -> any SSHAuthenticator { KeyFileAuthenticator( keyPath: keyPath, providedPassphrase: providedPassphrase, - canPrompt: canPrompt, useKeychain: resolved.useKeychain, addKeysToAgent: resolved.addKeysToAgent ) @@ -586,7 +584,6 @@ internal enum LibSSH2TunnelFactory { private struct KeyFileAuthenticator: SSHAuthenticator { let keyPath: String let providedPassphrase: String? - let canPrompt: Bool let useKeychain: Bool let addKeysToAgent: Bool @@ -617,9 +614,7 @@ internal enum LibSSH2TunnelFactory { } } - // 2. Prompt the user if allowed (key is encrypted, no stored passphrase) - guard canPrompt else { throw SSHTunnelError.authenticationFailed(reason: .privateKey) } - + // 2. Prompt the user (key is encrypted, no stored passphrase) let provider = PromptPassphraseProvider(keyPath: expandedPath) guard let promptResult = provider.providePassphrase() else { throw SSHTunnelError.authenticationFailed(reason: .privateKey) @@ -668,7 +663,6 @@ internal enum LibSSH2TunnelFactory { KeyFileAuthenticator( keyPath: path, providedPassphrase: nil, - canPrompt: true, useKeychain: resolved.useKeychain, addKeysToAgent: resolved.addKeysToAgent ) @@ -678,12 +672,11 @@ internal enum LibSSH2TunnelFactory { : CompositeAuthenticator(authenticators: authenticators) case .sshAgent: let socketPath: String? = resolved.agentSocketPath.isEmpty ? nil : resolved.agentSocketPath - let agent = AgentAuthenticator(socketPath: socketPath) + let agent = AgentAuthenticator(socketPath: socketPath, socketOrigin: resolved.agentSocketOrigin) if !jumpHost.privateKeyPath.isEmpty { let keyAuth = KeyFileAuthenticator( keyPath: jumpHost.privateKeyPath, providedPassphrase: nil, - canPrompt: true, useKeychain: resolved.useKeychain, addKeysToAgent: resolved.addKeysToAgent ) diff --git a/TablePro/Core/SSH/ResolvedSSHTarget.swift b/TablePro/Core/SSH/ResolvedSSHTarget.swift index 7fbc67c797..de59901c78 100644 --- a/TablePro/Core/SSH/ResolvedSSHTarget.swift +++ b/TablePro/Core/SSH/ResolvedSSHTarget.swift @@ -5,6 +5,18 @@ import Foundation +/// Where the agent socket a connection will use came from. `agentSocketPath` collapses three +/// sources into one string, and each is changed somewhere different, so an agent that does not +/// answer can only be reported usefully alongside the source that named it. +enum AgentSocketOrigin: Sendable, Hashable, CaseIterable { + /// The Agent Socket control on the SSH Tunnel pane. + case agentSocketSetting + /// An `IdentityAgent` directive matching this host in `~/.ssh/config`. + case identityAgentDirective + /// `SSH_AUTH_SOCK`, from the process environment or launchd. + case environment +} + struct ResolvedSSHTarget: Sendable, Hashable { let originalHost: String let host: String @@ -12,6 +24,7 @@ struct ResolvedSSHTarget: Sendable, Hashable { let username: String let identityFiles: [String] let agentSocketPath: String + let agentSocketOrigin: AgentSocketOrigin let identitiesOnly: Bool let useKeychain: Bool let addKeysToAgent: Bool diff --git a/TablePro/Core/SSH/SSHConfigResolver.swift b/TablePro/Core/SSH/SSHConfigResolver.swift index dbf933649b..6ee6863ebf 100644 --- a/TablePro/Core/SSH/SSHConfigResolver.swift +++ b/TablePro/Core/SSH/SSHConfigResolver.swift @@ -117,9 +117,18 @@ enum SSHConfigResolver { let effectivePort = formPort ?? merged.port ?? 22 let effectiveUser = !formUser.isEmpty ? formUser : (merged.user ?? "") - let effectiveAgentSocket = !formAgentSocket.isEmpty - ? formAgentSocket - : (merged.identityAgent ?? "") + let effectiveAgentSocket: String + let agentSocketOrigin: AgentSocketOrigin + if !formAgentSocket.isEmpty { + effectiveAgentSocket = formAgentSocket + agentSocketOrigin = .agentSocketSetting + } else if let identityAgent = merged.identityAgent, !identityAgent.isEmpty { + effectiveAgentSocket = identityAgent + agentSocketOrigin = .identityAgentDirective + } else { + effectiveAgentSocket = "" + agentSocketOrigin = .environment + } let effectiveIdentityFiles: [String] if !formIdentityFile.isEmpty { @@ -150,6 +159,7 @@ enum SSHConfigResolver { username: effectiveUser, identityFiles: effectiveIdentityFiles, agentSocketPath: effectiveAgentSocket, + agentSocketOrigin: agentSocketOrigin, identitiesOnly: merged.identitiesOnly ?? false, useKeychain: merged.useKeychain ?? true, addKeysToAgent: merged.addKeysToAgent ?? false, diff --git a/TablePro/Core/SSH/SSHTunnelManager.swift b/TablePro/Core/SSH/SSHTunnelManager.swift index b3de4e9906..783627a7da 100644 --- a/TablePro/Core/SSH/SSHTunnelManager.swift +++ b/TablePro/Core/SSH/SSHTunnelManager.swift @@ -11,15 +11,63 @@ import os /// Why an SSH authentication attempt failed. Drives the user-facing error string so the /// alert points at the actual cause (wrong OTP, missing key, agent rejection) instead of /// the catch-all "credentials or private key" message. -enum AuthFailureReason: Sendable, Equatable, CaseIterable { +enum AuthFailureReason: Sendable, Hashable, CaseIterable { case password case verificationCode case privateKey + case agentUnavailable(AgentSocketOrigin) + case agentNoIdentities(AgentSocketOrigin) case agentRejected case passwordlessRejected case keyboardInteractive + case methodUnavailable case cancelled case generic + + /// Hand-written because the two agent cases carry the socket source they are about, which + /// stops `CaseIterable` synthesising this. + static var allCases: [AuthFailureReason] { + [.password, .verificationCode, .privateKey] + + AgentSocketOrigin.allCases.map(AuthFailureReason.agentUnavailable) + + AgentSocketOrigin.allCases.map(AuthFailureReason.agentNoIdentities) + + [.agentRejected, .passwordlessRejected, .keyboardInteractive, .methodUnavailable, + .cancelled, .generic] + } +} + +extension AuthFailureReason { + /// True when the step never exchanged a credential with the server, so its failure says + /// nothing about why authentication was refused. `CompositeAuthenticator` keeps an earlier, + /// real failure rather than letting one of these displace it. + var describesAnAttempt: Bool { + self != .methodUnavailable + } +} + +private extension AgentSocketOrigin { + /// Names the socket the connect actually used, and the one place that names it. A jump hop + /// has no Agent Socket control of its own, and a shell-launched app can inherit an + /// `SSH_AUTH_SOCK` that is not the agent macOS runs, so neither can be assumed. + var whereTheSocketCameFrom: String { + switch self { + case .agentSocketSetting: + return String(localized: "the Agent Socket setting") + case .identityAgentDirective: + return String(localized: "the IdentityAgent line for this host in ~/.ssh/config") + case .environment: + return String(localized: "SSH_AUTH_SOCK") + } + } + + /// How to put a key in front of that particular agent. + var howToLoadAKey: String { + switch self { + case .agentSocketSetting, .identityAgentDirective: + return String(localized: "Unlock the app that runs it and add the key there.") + case .environment: + return String(localized: "Run ssh-add to load one, or point Agent Socket at another agent.") + } + } } /// Error types for SSH tunnel operations @@ -51,12 +99,25 @@ enum SSHTunnelError: Error, LocalizedError, Equatable, Sendable { return String(localized: "Verification code rejected. Get a new code from your authenticator app and try again.") case .privateKey: return String(localized: "SSH private key rejected. Check the key file or passphrase.") + case .agentUnavailable(let origin): + return String( + format: String(localized: "No SSH agent answered on the socket from %@. Check that agent is running."), + origin.whereTheSocketCameFrom + ) + case .agentNoIdentities(let origin): + return String( + format: String(localized: "The SSH agent from %@ holds no keys. %@"), + origin.whereTheSocketCameFrom, + origin.howToLoadAKey + ) case .agentRejected: return String(localized: "SSH agent did not authenticate. Run ssh-add -l to check loaded keys.") case .passwordlessRejected: return String(localized: "The SSH server did not accept passwordless authentication. Choose Password, Private Key, or SSH Agent.") case .keyboardInteractive: return String(localized: "SSH verification rejected. Check your response and try again.") + case .methodUnavailable: + return String(localized: "The SSH server does not offer that authentication method.") case .cancelled: return String(localized: "SSH authentication cancelled.") case .generic: diff --git a/TablePro/Models/Connection/SSHTypes.swift b/TablePro/Models/Connection/SSHTypes.swift index 652d7399f4..4a33f239de 100644 --- a/TablePro/Models/Connection/SSHTypes.swift +++ b/TablePro/Models/Connection/SSHTypes.swift @@ -60,6 +60,22 @@ enum SSHAgentSocketOption: String, CaseIterable, Identifiable { } } + /// Says which agent the choice actually reaches. `SSH_AUTH_SOCK` is the ssh-agent macOS + /// starts for the login session, so it never finds 1Password however the shell is set up, + /// and the connect used to fail with a passphrase prompt for an unrelated key (#2583). + var explanation: String { + switch self { + case .systemDefault: + return String( + localized: "The ssh-agent macOS runs, from SSH_AUTH_SOCK. 1Password and Secretive listen elsewhere." + ) + case .onePassword: + return String(localized: "1Password's own socket. 1Password has to be running and unlocked.") + case .custom: + return String(localized: "The socket of another agent, such as Secretive or an ssh-agent you started.") + } + } + init(socketPath: String) { let trimmedPath = socketPath.trimmingCharacters(in: .whitespacesAndNewlines) diff --git a/TablePro/Views/Connection/ConnectionSSHTunnelView.swift b/TablePro/Views/Connection/ConnectionSSHTunnelView.swift index e524253f37..7ef40127b7 100644 --- a/TablePro/Views/Connection/ConnectionSSHTunnelView.swift +++ b/TablePro/Views/Connection/ConnectionSSHTunnelView.swift @@ -194,7 +194,7 @@ struct ConnectionSSHTunnelView: View { prompt: Text("/path/to/agent.sock") ) } - Text("Keys are provided by the SSH agent (e.g. 1Password, ssh-agent).") + Text(sshState.agentSocketOption.explanation) .font(.caption) .foregroundStyle(.secondary) } else if sshState.authMethod == .keyboardInteractive { diff --git a/TablePro/Views/Connection/SSHProfileEditorView.swift b/TablePro/Views/Connection/SSHProfileEditorView.swift index 992af355fd..8598177d62 100644 --- a/TablePro/Views/Connection/SSHProfileEditorView.swift +++ b/TablePro/Views/Connection/SSHProfileEditorView.swift @@ -159,7 +159,7 @@ struct SSHProfileEditorView: View { prompt: Text("/path/to/agent.sock") ) } - Text("Keys are provided by the SSH agent (e.g. 1Password, ssh-agent).") + Text(agentSocketOption.explanation) .font(.caption) .foregroundStyle(.secondary) } else if authMethod == .keyboardInteractive { diff --git a/TableProTests/Core/SSH/Auth/AgentAuthenticationReportingTests.swift b/TableProTests/Core/SSH/Auth/AgentAuthenticationReportingTests.swift new file mode 100644 index 0000000000..dd84923b93 --- /dev/null +++ b/TableProTests/Core/SSH/Auth/AgentAuthenticationReportingTests.swift @@ -0,0 +1,314 @@ +// +// AgentAuthenticationReportingTests.swift +// TableProTests +// +// SSH Agent auth used to end in TablePro's own passphrase prompt for a key the user never +// chose: the chain fell through to `~/.ssh/id_*` whenever the agent produced nothing, and the +// agent's own failure was then buried by the keyboard-interactive step that followed, which +// reported "SSH password rejected" on a connection with no password (#2583). +// +// The agent cases run against a real unix socket speaking the agent protocol, because the +// distinction under test is exactly what libssh2 does with the bytes on that socket. +// + +import Foundation +import Testing + +import CLibSSH2 + +@testable import TablePro + +/// Minimal ssh-agent that answers `SSH_AGENTC_REQUEST_IDENTITIES` with a fixed identity list. +private final class FakeSSHAgent: @unchecked Sendable { + let path: String + private let listenFD: Int32 + private let identityBlobs: [(blob: [UInt8], comment: String)] + private var thread: Thread? + + init?(identities: [(blob: [UInt8], comment: String)]) { + identityBlobs = identities + + // sun_path is 104 bytes, and the socket has to sit somewhere every test run can write. + let directory = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("tp-agent-\(UInt32.random(in: 0..= 0 else { return nil } + + var address = sockaddr_un() + address.sun_family = sa_family_t(AF_UNIX) + let pathBytes = Array(path.utf8) + guard pathBytes.count < MemoryLayout.size(ofValue: address.sun_path) else { + Darwin.close(listenFD) + return nil + } + withUnsafeMutableBytes(of: &address.sun_path) { destination in + destination.copyBytes(from: pathBytes) + } + address.sun_len = UInt8(MemoryLayout.size) + + let bound = withUnsafePointer(to: &address) { pointer in + pointer.withMemoryRebound(to: sockaddr.self, capacity: 1) { generic in + Darwin.bind(listenFD, generic, socklen_t(MemoryLayout.size)) + } + } + guard bound == 0, Darwin.listen(listenFD, 4) == 0 else { + Darwin.close(listenFD) + return nil + } + + let thread = Thread { [weak self] in self?.serve() } + thread.start() + self.thread = thread + } + + func stop() { + Darwin.shutdown(listenFD, SHUT_RDWR) + Darwin.close(listenFD) + try? FileManager.default.removeItem( + at: URL(fileURLWithPath: path).deletingLastPathComponent() + ) + } + + private func serve() { + while true { + let clientFD = Darwin.accept(listenFD, nil, nil) + guard clientFD >= 0 else { return } + handle(clientFD) + Darwin.close(clientFD) + } + } + + private func handle(_ clientFD: Int32) { + while let request = readFrame(clientFD), let type = request.first { + let sshAgentcRequestIdentities: UInt8 = 11 + guard type == sshAgentcRequestIdentities else { + let sshAgentFailure: UInt8 = 5 + writeFrame(clientFD, [sshAgentFailure]) + continue + } + writeFrame(clientFD, identitiesAnswer()) + } + } + + private func identitiesAnswer() -> [UInt8] { + let sshAgentIdentitiesAnswer: UInt8 = 12 + var payload: [UInt8] = [sshAgentIdentitiesAnswer] + payload += Self.uint32(UInt32(identityBlobs.count)) + for identity in identityBlobs { + payload += Self.string(identity.blob) + payload += Self.string(Array(identity.comment.utf8)) + } + return payload + } + + private static func uint32(_ value: UInt32) -> [UInt8] { + [UInt8(value >> 24 & 0xFF), UInt8(value >> 16 & 0xFF), UInt8(value >> 8 & 0xFF), UInt8(value & 0xFF)] + } + + private static func string(_ bytes: [UInt8]) -> [UInt8] { + uint32(UInt32(bytes.count)) + bytes + } + + private func readFrame(_ fd: Int32) -> [UInt8]? { + guard let header = readExactly(fd, 4) else { return nil } + let length = (UInt32(header[0]) << 24) | (UInt32(header[1]) << 16) + | (UInt32(header[2]) << 8) | UInt32(header[3]) + guard length > 0, length < 64 * 1024 else { return nil } + return readExactly(fd, Int(length)) + } + + private func readExactly(_ fd: Int32, _ count: Int) -> [UInt8]? { + var buffer = [UInt8](repeating: 0, count: count) + var filled = 0 + while filled < count { + let read: Int = buffer.withUnsafeMutableBytes { destination in + guard let base = destination.baseAddress else { return -1 } + return Darwin.read(fd, base.advanced(by: filled), count - filled) + } + guard read > 0 else { return nil } + filled += read + } + return buffer + } + + private func writeFrame(_ fd: Int32, _ payload: [UInt8]) { + let frame = Self.uint32(UInt32(payload.count)) + payload + var written = 0 + while written < frame.count { + let sent: Int = frame.withUnsafeBytes { source in + guard let base = source.baseAddress else { return -1 } + return Darwin.write(fd, base.advanced(by: written), frame.count - written) + } + guard sent > 0 else { return } + written += sent + } + } +} + +/// A session with no transport. `libssh2_agent_*` never touches one, and +/// `libssh2_userauth_authenticated` only reads a state flag, so the agent and composite paths +/// under test run without an SSH server. +private func withTransportlessSession(_ body: (OpaquePointer) throws -> T) throws -> T { + // libssh2_init uses global state and must not be called concurrently, and Swift Testing runs + // sibling cases in parallel, so this goes through the app's one lazy initializer. + _ = LibSSH2TunnelFactory.initialized + let session = try #require(tablepro_libssh2_session_init()) + defer { libssh2_session_free(session) } + return try body(session) +} + +@Suite("AgentAuthenticator failure reasons", .serialized) +struct AgentAuthenticatorFailureTests { + @Test("A socket path nothing is listening on reports the agent as unreachable (#2583)") + func missingSocketReportsUnavailable() throws { + let missingPath = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("tp-agent-absent.sock").path + + try withTransportlessSession { session in + #expect(throws: SSHTunnelError.authenticationFailed(reason: .agentUnavailable(.agentSocketSetting))) { + try AgentAuthenticator(socketPath: missingPath, socketOrigin: .agentSocketSetting) + .authenticate(session: session, username: "alice") + } + } + } + + @Test("An agent that answers with an empty key list reports no identities (#2583)") + func emptyAgentReportsNoIdentities() throws { + let agent = try #require(FakeSSHAgent(identities: [])) + defer { agent.stop() } + + try withTransportlessSession { session in + #expect(throws: SSHTunnelError.authenticationFailed(reason: .agentNoIdentities(.identityAgentDirective))) { + try AgentAuthenticator(socketPath: agent.path, socketOrigin: .identityAgentDirective) + .authenticate(session: session, username: "alice") + } + } + } +} + +private struct ThrowingAuthenticator: SSHAuthenticator { + let reason: AuthFailureReason + + func authenticate(session: OpaquePointer, username: String) throws { + throw SSHTunnelError.authenticationFailed(reason: reason) + } +} + +@Suite("CompositeAuthenticator failure reporting") +struct CompositeAuthenticatorFailureReportingTests { + private func failureReason( + of authenticators: [any SSHAuthenticator], + endsChainOn: Set = [] + ) throws -> AuthFailureReason? { + try withTransportlessSession { session in + do { + try CompositeAuthenticator(authenticators: authenticators, endsChainOn: endsChainOn) + .authenticate(session: session, username: "alice") + return nil + } catch let error as SSHTunnelError { + guard case .authenticationFailed(let reason) = error else { return nil } + return reason + } + } + } + + @Test("A method the server never engaged does not bury the agent's failure (#2583)") + func unavailableMethodDoesNotBuryAgentFailure() throws { + let reason = try failureReason(of: [ + ThrowingAuthenticator(reason: .agentNoIdentities(.environment)), + ThrowingAuthenticator(reason: .methodUnavailable), + ]) + + #expect(reason == .agentNoIdentities(.environment)) + } + + @Test("A second factor the server did challenge still wins (#1018)") + func engagedSecondFactorStillWins() throws { + let reason = try failureReason(of: [ + ThrowingAuthenticator(reason: .privateKey), + ThrowingAuthenticator(reason: .verificationCode), + ]) + + #expect(reason == .verificationCode) + } + + @Test("An unreachable agent ends the chain instead of letting a second factor prompt (#2583)") + func unreachableAgentEndsTheChain() throws { + final class Spy: SSHAuthenticator, @unchecked Sendable { + var ran = false + func authenticate(session: OpaquePointer, username: String) throws { + ran = true + throw SSHTunnelError.authenticationFailed(reason: .keyboardInteractive) + } + } + let secondFactor = Spy() + + let reason = try failureReason( + of: [ThrowingAuthenticator(reason: .agentUnavailable(.agentSocketSetting)), secondFactor], + endsChainOn: [.agentUnavailable(.agentSocketSetting)] + ) + + #expect(reason == .agentUnavailable(.agentSocketSetting)) + #expect(!secondFactor.ran) + } + + @Test("An agent the server refused still lets the second factor run (#1920)") + func rejectedAgentKeepsTheSecondFactor() throws { + let reason = try failureReason( + of: [ThrowingAuthenticator(reason: .agentRejected), ThrowingAuthenticator(reason: .verificationCode)], + endsChainOn: [.agentUnavailable(.agentSocketSetting), .agentNoIdentities(.agentSocketSetting)] + ) + + #expect(reason == .verificationCode) + } + + @Test("An unavailable method is still reported when it is the only failure") + func unavailableMethodSurvivesAlone() throws { + let reason = try failureReason(of: [ThrowingAuthenticator(reason: .methodUnavailable)]) + + #expect(reason == .methodUnavailable) + } + + @Test("A user cancellation aborts the chain before any later step runs") + func cancellationAbortsTheChain() throws { + let reason = try failureReason(of: [ + ThrowingAuthenticator(reason: .cancelled), + ThrowingAuthenticator(reason: .password), + ]) + + #expect(reason == .cancelled) + } +} + +@Suite("KeyboardInteractiveContext failure reason") +struct KeyboardInteractiveFailureReasonTests { + private final class SilentPromptProvider: KeyboardInteractivePromptProvider, @unchecked Sendable { + func provideResponses(for challenge: KeyboardInteractiveChallenge, attempt: Int) throws -> [String] { + [] + } + } + + private func context(password: String? = nil) -> KeyboardInteractiveContext { + KeyboardInteractiveContext( + password: password, + totpProvider: nil, + promptProvider: SilentPromptProvider() + ) + } + + @Test("A server that issued no prompt reports the method as unavailable, not a bad password (#2583)") + func noPromptIsNotAPasswordRejection() { + #expect(context(password: "hunter2").failureReason == .methodUnavailable) + } + + @Test("A password answered from the fast path reports a password rejection (#1005)") + func answeredPasswordReportsPassword() { + let ctx = context(password: "hunter2") + _ = ctx.responses(name: "", instruction: "", prompts: [KeyboardInteractivePrompt(text: "Password:", echo: false)]) + + #expect(ctx.failureReason == .password) + } +} diff --git a/TableProTests/Core/SSH/Auth/AuthFailureReasonTests.swift b/TableProTests/Core/SSH/Auth/AuthFailureReasonTests.swift index c4d44b0323..c4a398d1d3 100644 --- a/TableProTests/Core/SSH/Auth/AuthFailureReasonTests.swift +++ b/TableProTests/Core/SSH/Auth/AuthFailureReasonTests.swift @@ -52,6 +52,66 @@ struct AuthFailureReasonTests { #expect(description.localizedCaseInsensitiveContains("agent")) } + @Test("An unreachable agent names the socket source, not a key (#2583)") + func agentUnavailableMessage() { + for origin in AgentSocketOrigin.allCases { + let description = SSHTunnelError.authenticationFailed( + reason: .agentUnavailable(origin) + ).errorDescription ?? "" + + #expect(description.localizedCaseInsensitiveContains("agent")) + #expect(!description.localizedCaseInsensitiveContains("private key")) + #expect(!description.localizedCaseInsensitiveContains("passphrase")) + } + } + + @Test("Each socket source sends the user somewhere it can actually be changed (#2583)") + func agentUnavailableNamesItsOwnSource() { + func message(_ origin: AgentSocketOrigin) -> String { + SSHTunnelError.authenticationFailed(reason: .agentUnavailable(origin)).errorDescription ?? "" + } + + #expect(message(.agentSocketSetting).localizedCaseInsensitiveContains("Agent Socket")) + #expect(message(.identityAgentDirective).localizedCaseInsensitiveContains("IdentityAgent")) + #expect(message(.environment).localizedCaseInsensitiveContains("SSH_AUTH_SOCK")) + + #expect(!message(.identityAgentDirective).localizedCaseInsensitiveContains("SSH_AUTH_SOCK")) + #expect(!message(.environment).localizedCaseInsensitiveContains("IdentityAgent")) + } + + @Test("ssh-add is only offered for the agent ssh-add can reach (#2583)") + func agentNoIdentitiesMessage() { + func message(_ origin: AgentSocketOrigin) -> String { + SSHTunnelError.authenticationFailed(reason: .agentNoIdentities(origin)).errorDescription ?? "" + } + + for origin in AgentSocketOrigin.allCases { + #expect(message(origin).localizedCaseInsensitiveContains("agent")) + #expect(!message(origin).localizedCaseInsensitiveContains("password")) + } + + #expect(message(.environment).localizedCaseInsensitiveContains("ssh-add")) + #expect(!message(.agentSocketSetting).localizedCaseInsensitiveContains("ssh-add")) + #expect(!message(.identityAgentDirective).localizedCaseInsensitiveContains("ssh-add")) + } + + @Test("An unavailable method names the server, not a credential (#2583)") + func methodUnavailableMessage() { + let error = SSHTunnelError.authenticationFailed(reason: .methodUnavailable) + let description = error.errorDescription ?? "" + + #expect(description.localizedCaseInsensitiveContains("server")) + #expect(!description.localizedCaseInsensitiveContains("password")) + #expect(!description.localizedCaseInsensitiveContains("private key")) + } + + @Test("Only a method nothing was offered through is exempt from defining a chain's failure") + func onlyMethodUnavailableSkipsAttemptReporting() { + for reason in AuthFailureReason.allCases { + #expect(reason.describesAnAttempt == (reason != .methodUnavailable)) + } + } + @Test("Passwordless reason points at the server, not the user's credentials") func passwordlessRejectedMessage() { let error = SSHTunnelError.authenticationFailed(reason: .passwordlessRejected) diff --git a/TableProTests/Core/SSH/Auth/BuildAuthenticatorTests.swift b/TableProTests/Core/SSH/Auth/BuildAuthenticatorTests.swift index d40c58bcf9..8bba7d36c1 100644 --- a/TableProTests/Core/SSH/Auth/BuildAuthenticatorTests.swift +++ b/TableProTests/Core/SSH/Auth/BuildAuthenticatorTests.swift @@ -26,7 +26,8 @@ struct BuildAuthenticatorTests { host: String = "ssh.example.com", username: String = "alice", port: Int = 22, - identityFiles: [String] = [] + identityFiles: [String] = [], + agentSocketOrigin: AgentSocketOrigin = .environment ) -> ResolvedSSHTarget { ResolvedSSHTarget( originalHost: host, @@ -35,6 +36,7 @@ struct BuildAuthenticatorTests { username: username, identityFiles: identityFiles, agentSocketPath: "", + agentSocketOrigin: agentSocketOrigin, identitiesOnly: false, useKeychain: false, addKeysToAgent: false, @@ -151,6 +153,47 @@ struct BuildAuthenticatorTests { #expect(kbdint.password == nil) } + @Test("SSH agent auth is the agent and a second factor, nothing else (#2583)") + func sshAgentChainHoldsNoKeyFile() throws { + let authenticator = try LibSSH2TunnelFactory.buildAuthenticator( + config: config(authMethod: .sshAgent, totpMode: .none), + resolved: resolved(), + credentials: credentials() + ) + let composite = try #require(authenticator as? CompositeAuthenticator) + + #expect(composite.authenticators.count == 2) + #expect(composite.authenticators.first is AgentAuthenticator) + #expect(composite.authenticators.last is KeyboardInteractiveAuthenticator) + } + + @Test("SSH agent auth never falls back to an identity file from ~/.ssh/config (#2583)") + func sshAgentIgnoresResolvedIdentityFiles() throws { + let authenticator = try LibSSH2TunnelFactory.buildAuthenticator( + config: config(authMethod: .sshAgent, totpMode: .none), + resolved: resolved(identityFiles: ["/home/alice/.ssh/id_ed25519", "/home/alice/.ssh/id_rsa"]), + credentials: credentials() + ) + let composite = try #require(authenticator as? CompositeAuthenticator) + + #expect(composite.authenticators.count == 2) + #expect(composite.authenticators.first is AgentAuthenticator) + #expect(composite.authenticators.last is KeyboardInteractiveAuthenticator) + } + + @Test("Private key auth still tries every resolved identity file") + func privateKeyKeepsEveryIdentityFile() throws { + let authenticator = try LibSSH2TunnelFactory.buildAuthenticator( + config: config(authMethod: .privateKey, totpMode: .none), + resolved: resolved(identityFiles: ["/home/alice/.ssh/id_ed25519", "/home/alice/.ssh/id_rsa"]), + credentials: credentials() + ) + let composite = try #require(authenticator as? CompositeAuthenticator) + + #expect(composite.authenticators.count == 3) + #expect(composite.authenticators.last is KeyboardInteractiveAuthenticator) + } + @Test("None auth method returns a NoneAuthenticator") func noneReturnsNoneAuthenticator() throws { let authenticator = try LibSSH2TunnelFactory.buildAuthenticator( diff --git a/TableProTests/Core/SSH/SSHConfigurationTests.swift b/TableProTests/Core/SSH/SSHConfigurationTests.swift index c46186b802..fbd3cc927f 100644 --- a/TableProTests/Core/SSH/SSHConfigurationTests.swift +++ b/TableProTests/Core/SSH/SSHConfigurationTests.swift @@ -125,6 +125,22 @@ struct SSHConfigurationTests { ) } + @Test("The system default's help text says it is the macOS agent, not 1Password's (#2583)") + func testSystemDefaultExplanationNamesTheMacOSAgent() { + let explanation = SSHAgentSocketOption.systemDefault.explanation + + #expect(explanation.localizedCaseInsensitiveContains("SSH_AUTH_SOCK")) + #expect(explanation.localizedCaseInsensitiveContains("1Password")) + } + + @Test("Every agent socket option carries its own help text (#2583)") + func testEveryAgentSocketOptionExplains() { + let explanations = SSHAgentSocketOption.allCases.map(\.explanation) + + #expect(!explanations.contains("")) + #expect(Set(explanations).count == explanations.count) + } + @Test("Jump hosts validation passes when all valid") func testJumpHostsValidationPasses() { let config = SSHConfiguration( diff --git a/docs/connections/ssh-tunneling.mdx b/docs/connections/ssh-tunneling.mdx index 36bb29e9e9..1763f171e4 100644 --- a/docs/connections/ssh-tunneling.mdx +++ b/docs/connections/ssh-tunneling.mdx @@ -26,7 +26,7 @@ flowchart LR Back on **General**, `localhost` reaches a database on the SSH server itself. One on a unix socket needs [Socket Path](#forwarding-to-a-unix-socket) instead. - A wrong host or a blocked forward names the real reason instead of timing out. [Troubleshooting](#troubleshooting) has the three the SSH side reports. + A wrong host, a blocked forward, or an agent nothing answered on names the real reason instead of timing out. [Troubleshooting](#troubleshooting) has the messages the SSH side reports. @@ -45,7 +45,7 @@ There is no **SSH Tunnel** pane on SQLite, PGlite, libSQL, Beancount, BigQuery, |---|---| | **Password** | The SSH password. Prefer a key on anything production | | **Private Key** | **Key File**, with **Browse** to pick one, and **Passphrase** if the key is encrypted. Leaving **Key File** empty auto-detects from `~/.ssh/config` and the default key locations | -| **SSH Agent** | **Agent Socket**: **SSH_AUTH_SOCK**, **1Password** (its socket at `~/Library/Group Containers/2BUA8C4S2C.com.1password/t/agent.sock`), or **Custom Path** for Secretive or your own `ssh-agent`. Signing stays in the agent; the key is never read | +| **SSH Agent** | **Agent Socket**: **SSH_AUTH_SOCK** for the `ssh-agent` macOS runs, **1Password** for its socket at `~/Library/Group Containers/2BUA8C4S2C.com.1password/t/agent.sock`, or **Custom Path** for Secretive or your own. Signing stays in the agent; the key is never read, and no key file is tried if the agent refuses | | **Keyboard Interactive** | The SSH password, sent through SSH's challenge-response. Use it when the server rejects plain password auth, common with PAM | | **None** | Nothing, for a server that authenticates the connection itself such as a [Tailscale SSH](https://tailscale.com/kb/1193/tailscale-ssh) host. A server that does want credentials fails the connect with a message naming the other methods | @@ -122,6 +122,22 @@ The destination took the connection and never answered, which usually means a fi The socket path does not exist on the server, or `sshd_config` sets `AllowStreamLocalForwarding no`. That is a separate setting from `AllowTcpForwarding`. +### "No SSH agent answered on the socket from …" + +Nothing is listening where that socket points, and the message names which of the three set it: **Agent Socket** on the SSH Tunnel pane, an `IdentityAgent` line for the host in `~/.ssh/config`, or `SSH_AUTH_SOCK`. Change it in the place the message names. + +`SSH_AUTH_SOCK` is the one that catches people out. An app launched from Finder gets it from launchd, which means the `ssh-agent` macOS runs, whatever a shell profile exports. 1Password and Secretive are reached by naming their own socket: switch **Agent Socket** to **1Password**, or to **Custom Path**. + +A jump host has no **Agent Socket** field of its own, so its agent comes from `IdentityAgent` or `SSH_AUTH_SOCK`. + +### "The SSH agent from … holds no keys." + +The agent answered and offered nothing. 1Password serves keys only while it is running and unlocked, and Secretive only while its agent is loaded; unlock it and add the key there. `ssh-add` loads keys into the `SSH_AUTH_SOCK` agent alone, so it is the fix only when that is the socket in the message. + +### "SSH agent did not authenticate. …" + +The agent offered keys and the server accepted none of them. Check the public key is in `~/.ssh/authorized_keys` on the server for the **SSH User** you filled in, and that the key you expect is in the agent rather than only on disk. + ### The tunnel connects and the database refuses the login The database credentials are separate from the SSH ones. Check you did not carry one set into the other.