diff --git a/src/main/ssh/ssh-connection-auth-fallback.test.ts b/src/main/ssh/ssh-connection-auth-fallback.test.ts index 0a49985ae21..09b8af9e8a1 100644 --- a/src/main/ssh/ssh-connection-auth-fallback.test.ts +++ b/src/main/ssh/ssh-connection-auth-fallback.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest' +import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest' import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -143,6 +143,96 @@ describe('SshConnection', () => { } }) + describe('when the agent-first attempt held the challenge back for ~/.ssh/id_ed25519', () => { + let tempDir: string + const hostMethods = ['publickey' as const, 'keyboard-interactive' as const] + const settle = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)) + + beforeEach(async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-home-')) + mkdirSync(join(tempDir, '.ssh')) + // Real key bytes and parser: deferral requires the waiting key to parse as unencrypted. + const realSsh2 = await vi.importActual<{ utils: typeof utils }>('ssh2') + writeFileSync( + join(tempDir, '.ssh', 'id_ed25519'), + realSsh2.utils.generateKeyPairSync('ecdsa', { bits: 256 }).private + ) + vi.mocked(utils.parseKey).mockImplementation(realSsh2.utils.parseKey) + vi.stubEnv('HOME', tempDir) + vi.stubEnv('USERPROFILE', tempDir) + }) + + afterEach(() => { + vi.mocked(utils.parseKey).mockReset() + rmSync(tempDir, { recursive: true, force: true }) + }) + + /** Runs the agent-first attempt to its skipped challenge and fails it, leaving the key retry. */ + async function failAgentFirstAttempt( + conn: SshConnection + ): Promise<{ connected: Promise }> { + const created = nextSshClientCreation() + const connected = conn.connect() + await created + await settle() + expect(walkInitialAuthLadder(clientInstances[0].lastConnectConfig, hostMethods)).toEqual([ + 'none', + 'agent' + ]) + const keyRetryCreated = nextSshClientCreation() + emitSshEvent('error', new Error('All configured authentication methods failed')) + await keyRetryCreated + await settle() + // Wrapped: an async function returning the promise itself would wait for the whole connect. + return { connected } + } + + it('offers the challenge alone when a MaxAuthTries=1 host drops the key retry first', async () => { + ssh2Mock.connectSequence = ['silent', 'silent', 'ready'] + const onCredentialRequest = vi.fn() + const conn = new SshConnection(createTarget(), createCallbacks({ onCredentialRequest })) + + const { connected } = await failAgentFirstAttempt(conn) + // The host disconnects after the rejected key, before the retry reaches keyboard-interactive. + expect(walkInitialAuthLadder(clientInstances[1].lastConnectConfig, hostMethods, 2)).toEqual([ + 'none', + 'publickey' + ]) + const challengeCreated = nextSshClientCreation() + emitSshEvent('error', new Error('Received disconnect: Too many authentication failures')) + await challengeCreated + await connected + + expect(clientInstances).toHaveLength(3) + const challengeConfig = clientInstances[2].lastConnectConfig + expect(challengeConfig).not.toHaveProperty('agent') + expect(challengeConfig).not.toHaveProperty('privateKey') + expect(walkInitialAuthLadder(challengeConfig, hostMethods)).toEqual([ + 'none', + 'keyboard-interactive' + ]) + expect(onCredentialRequest).not.toHaveBeenCalled() + }) + + it('does not repeat the challenge when the key retry already offered it', async () => { + ssh2Mock.connectSequence = ['silent', 'silent'] + const onCredentialRequest = vi.fn(async () => null) + const conn = new SshConnection(createTarget(), createCallbacks({ onCredentialRequest })) + + const { connected } = await failAgentFirstAttempt(conn) + expect(walkInitialAuthLadder(clientInstances[1].lastConnectConfig, hostMethods)).toEqual([ + 'none', + 'publickey', + 'keyboard-interactive' + ]) + emitSshEvent('error', new Error('All configured authentication methods failed')) + + await expect(connected).rejects.toThrow('All configured authentication methods failed') + expect(clientInstances).toHaveLength(2) + }) + }) + it('falls back to direct private key auth when the agent socket is unavailable', async () => { vi.stubEnv('SSH_AUTH_SOCK', '/tmp/stale-agent.sock') const tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-key-')) diff --git a/src/main/ssh/ssh-connection-test-fixtures.ts b/src/main/ssh/ssh-connection-test-fixtures.ts index f33771e3159..c15a787b85d 100644 --- a/src/main/ssh/ssh-connection-test-fixtures.ts +++ b/src/main/ssh/ssh-connection-test-fixtures.ts @@ -17,10 +17,14 @@ export function createTarget(overrides?: Partial): SshTarget { } } -/** Walks an attempt's initial auth ladder against a host that keeps listing `hostMethods`. */ +/** + * Walks an attempt's initial auth ladder against a host that keeps listing `hostMethods`, stopping + * after `maxAttempts` methods as a host that disconnects at its MaxAuthTries would. + */ export function walkInitialAuthLadder( config: unknown, - hostMethods: AuthenticationType[] + hostMethods: AuthenticationType[], + maxAttempts = Number.POSITIVE_INFINITY ): string[] { const authHandler = typeof config === 'object' && config !== null && 'authHandler' in config @@ -38,7 +42,7 @@ export function walkInitialAuthLadder( const walked: (AuthenticationType | AnyAuthMethod)[] = [] let exhausted = false let firstCall = true - while (!exhausted) { + while (!exhausted && walked.length < maxAttempts) { handler(firstCall ? null : hostMethods, false, (attempt) => { if (attempt === false) { exhausted = true diff --git a/src/main/ssh/ssh-connection-utils.ts b/src/main/ssh/ssh-connection-utils.ts index e2a4fdef433..1a42ea7c316 100644 --- a/src/main/ssh/ssh-connection-utils.ts +++ b/src/main/ssh/ssh-connection-utils.ts @@ -183,6 +183,8 @@ export function createSshOperationAbortError(): Error & { name: string } { export type BuildConnectConfigOptions = { includeAgent?: boolean includePrivateKey?: boolean + /** No agent and no key: only the host's keyboard-interactive challenge (and a cached password). */ + keyboardInteractiveOnly?: boolean } // Why: ssh2 tries privateKey before agent, but parses encrypted privateKey @@ -212,7 +214,7 @@ export function buildConnectConfig( tryKeyboard: true } - const shouldIncludeAgent = options.includeAgent ?? true + const shouldIncludeAgent = !options.keyboardInteractiveOnly && (options.includeAgent ?? true) const agentSocket = shouldIncludeAgent ? resolveAgentSocket(target, resolved) : undefined const agent = agentSocket ? resolveAgentConfigValue(agentSocket, target, resolved) : undefined @@ -224,8 +226,9 @@ export function buildConnectConfig( config.agentForward = true } - const keys = - (options.includePrivateKey ?? !agent) + const keys = options.keyboardInteractiveOnly + ? [] + : (options.includePrivateKey ?? !agent) ? resolvePrivateKeys(target, resolved) : resolveUnencryptedExplicitPrivateKeys(target, resolved) let deferralDecision: boolean | undefined @@ -243,7 +246,7 @@ export function buildConnectConfig( // Decided lazily so the deferred files are read only once the agent has failed, as the retry // would read them anyway, never on an agent login that needs no file. deferKeyboardInteractive: - keys.length === 0 + keys.length === 0 && !options.keyboardInteractiveOnly ? () => { if (deferralDecision === undefined) { const deferredKeys = resolvePrivateKeys(target, resolved) diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index c670958c72b..271a78c3411 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -80,7 +80,11 @@ import { } from './ssh-reconnect-error-classification' import { SshReconnectLadder } from './ssh-reconnect-ladder' import { mayUserSshConfigClaimAlias } from './ssh-config-alias-claim' -import { getPassphrasePrivateKeyPath } from './ssh-private-key-authentication' +import { + didDeferKeyboardInteractive, + didOfferKeyboardInteractive, + getPassphrasePrivateKeyPath +} from './ssh-private-key-authentication' import { requiresSystemSshForSecurityKey, shouldUseSystemSshTransport @@ -735,6 +739,32 @@ export class SshConnection { return } } + // Why: a strict MaxAuthTries host can drop the key retry before it reaches the challenge + // the agent-first attempt held back for that key, so offer the challenge on its own. + if ( + isAgentFallbackError(authError) && + didDeferKeyboardInteractive(config) && + !didOfferKeyboardInteractive(keyConfig) + ) { + const challengeConfig = buildConnectConfig(this.target, resolved, { + keyboardInteractiveOnly: true + }) + this.respawnProxy(challengeConfig, effectiveProxy, connectGeneration) + try { + await this.doSsh2Connect(challengeConfig, connectGeneration) + return + } catch (challengeErr) { + if ( + !(challengeErr instanceof Error) || + isHostKeyVerificationError(challengeErr) || + this.keyboardInteractiveCancelled + ) { + this.killProxyIfCurrent(connectGeneration) + throw challengeErr + } + authError = challengeErr + } + } } } } diff --git a/src/main/ssh/ssh-private-key-authentication.ts b/src/main/ssh/ssh-private-key-authentication.ts index 1903fdc957a..03fbba07db4 100644 --- a/src/main/ssh/ssh-private-key-authentication.ts +++ b/src/main/ssh/ssh-private-key-authentication.ts @@ -3,6 +3,8 @@ import type { PrivateKeyFile } from './ssh-auth-resolution' import { observeAgentSigning } from './ssh-agent-sign-observer' const passphraseKeyPaths = new WeakMap() +const deferredKeyboardInteractive = new WeakSet() +const offeredKeyboardInteractive = new WeakSet() // Bounds an `AuthenticationMethods a,b,c` ladder so a host that keeps replying // "partial success" cannot keep the client prompting forever. @@ -99,12 +101,26 @@ export function configurePrivateKeyAuthentication( authsLeft.includes('publickey') && options.deferKeyboardInteractive?.() ) { + deferredKeyboardInteractive.add(config) attempt = queue.shift() } + if (attempt === 'keyboard-interactive') { + offeredKeyboardInteractive.add(config) + } next((attempt ?? false) as Parameters[0]) } } +/** True once this config's handler held the host's first-factor challenge back for a deferred key. */ +export function didDeferKeyboardInteractive(config: ConnectConfig): boolean { + return deferredKeyboardInteractive.has(config) +} + +/** True once this config's handler sent keyboard-interactive to the host. */ +export function didOfferKeyboardInteractive(config: ConnectConfig): boolean { + return offeredKeyboardInteractive.has(config) +} + export function getPassphrasePrivateKeyPath(config: ConnectConfig): string | undefined { return passphraseKeyPaths.get(config) }