diff --git a/src/main/ssh/ssh-agent-sign-observer.ts b/src/main/ssh/ssh-agent-sign-observer.ts new file mode 100644 index 00000000000..5fee2e60828 --- /dev/null +++ b/src/main/ssh/ssh-agent-sign-observer.ts @@ -0,0 +1,54 @@ +import { + BaseAgent, + createAgent, + type IdentityCallback, + type ParsedKey, + type SignCallback, + type SigningRequestOptions +} from 'ssh2' + +type AgentPublicKey = Parameters[0] + +/** + * Delegates to the real agent and records when the host asked it to sign. ssh2 only signs after the + * host accepted the agent key (PK_OK), so a sign is the only evidence that an agent key partially + * succeeded: ssh2 swallows that failure's partialSuccess flag and method list for agent keys. + */ +class SignObservingAgent extends BaseAgent { + private resolved: BaseAgent | null = null + + constructor( + private readonly source: BaseAgent | string, + private readonly onSign: () => void + ) { + super() + } + + // Resolved on first use so building an auth queue never opens an agent the attempt won't reach. + private get agent(): BaseAgent { + this.resolved ??= typeof this.source === 'string' ? createAgent(this.source) : this.source + return this.resolved + } + + getIdentities(callback: IdentityCallback): void { + this.agent.getIdentities(callback) + } + + sign( + pubKey: AgentPublicKey, + data: Buffer, + optionsOrCallback?: SigningRequestOptions | SignCallback, + callback?: SignCallback + ): void { + this.onSign() + if (typeof optionsOrCallback === 'function') { + this.agent.sign(pubKey, data, optionsOrCallback) + return + } + this.agent.sign(pubKey, data, optionsOrCallback ?? {}, callback) + } +} + +export function observeAgentSigning(agent: BaseAgent | string, onSign: () => void): BaseAgent { + return new SignObservingAgent(agent, onSign) +} diff --git a/src/main/ssh/ssh-auth-resolution.ts b/src/main/ssh/ssh-auth-resolution.ts index c7e65e26d92..70fd76eb6fe 100644 --- a/src/main/ssh/ssh-auth-resolution.ts +++ b/src/main/ssh/ssh-auth-resolution.ts @@ -157,7 +157,7 @@ export function resolvePrivateKeys( return defaultKey ? [defaultKey] : [] } -function isUnencryptedPrivateKey(contents: Buffer): boolean { +export function isUnencryptedPrivateKey(contents: Buffer): boolean { const parsed = utils.parseKey(contents) as ParsedKey | ParsedKey[] | Error if (parsed instanceof Error) { return false diff --git a/src/main/ssh/ssh-connection-auth-fallback.test.ts b/src/main/ssh/ssh-connection-auth-fallback.test.ts index 20c973e1b2c..0a49985ae21 100644 --- a/src/main/ssh/ssh-connection-auth-fallback.test.ts +++ b/src/main/ssh/ssh-connection-auth-fallback.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { utils } from 'ssh2' import { clientInstances, emitSshEvent, @@ -9,7 +10,11 @@ import { resetSshConnectionMocks, ssh2Mock } from './ssh-connection-test-harness' -import { createCallbacks, createTarget } from './ssh-connection-test-fixtures' +import { + createCallbacks, + createTarget, + walkInitialAuthLadder +} from './ssh-connection-test-fixtures' import { SshConnection } from './ssh-connection' import { resolveWithSshG } from './ssh-config-parser' @@ -100,6 +105,44 @@ describe('SshConnection', () => { } }) + it('holds the keyboard-interactive challenge until the deferred default key has been tried', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + const 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') + const keyBytes = Buffer.from(realSsh2.utils.generateKeyPairSync('ecdsa', { bits: 256 }).private) + writeFileSync(join(tempDir, '.ssh', 'id_ed25519'), keyBytes) + vi.mocked(utils.parseKey).mockImplementation(realSsh2.utils.parseKey) + vi.stubEnv('HOME', tempDir) + vi.stubEnv('USERPROFILE', tempDir) + ssh2Mock.connectSequence = [new Error('All configured authentication methods failed'), 'ready'] + + try { + const onCredentialRequest = vi.fn() + const conn = new SshConnection(createTarget(), createCallbacks({ onCredentialRequest })) + + await conn.connect() + + expect(clientInstances).toHaveLength(2) + // The agent-first attempt defers ~/.ssh/id_ed25519, so its ladder ends at the agent: + // answering the host's password challenge there would preempt the key that authenticates. + expect( + walkInitialAuthLadder(clientInstances[0].lastConnectConfig, [ + 'publickey', + 'keyboard-interactive' + ]) + ).toEqual(['none', 'agent']) + expect(clientInstances[1].lastConnectConfig).toMatchObject({ + privateKey: keyBytes + }) + expect(onCredentialRequest).not.toHaveBeenCalled() + } finally { + vi.mocked(utils.parseKey).mockReset() + rmSync(tempDir, { recursive: true, force: true }) + } + }) + 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 bed6e633092..f33771e3159 100644 --- a/src/main/ssh/ssh-connection-test-fixtures.ts +++ b/src/main/ssh/ssh-connection-test-fixtures.ts @@ -1,6 +1,7 @@ import { EventEmitter } from 'node:events' import { vi } from 'vitest' import type { Mock } from 'vitest' +import type { AnyAuthMethod, AuthenticationType } from 'ssh2' import type { SshConnectionCallbacks } from './ssh-connection' import type { SshResolvedConfig } from './ssh-config-parser' import type { SshTarget } from '../../shared/ssh-types' @@ -16,6 +17,40 @@ export function createTarget(overrides?: Partial): SshTarget { } } +/** Walks an attempt's initial auth ladder against a host that keeps listing `hostMethods`. */ +export function walkInitialAuthLadder( + config: unknown, + hostMethods: AuthenticationType[] +): string[] { + const authHandler = + typeof config === 'object' && config !== null && 'authHandler' in config + ? config.authHandler + : undefined + if (typeof authHandler !== 'function') { + throw new Error('connect config has no auth handler') + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: ssh2 calls the handler with null authsLeft first and passes false to next() once exhausted; its typings omit both. + const handler = authHandler as ( + authsLeft: AuthenticationType[] | null, + partialSuccess: boolean, + next: (attempt: AuthenticationType | AnyAuthMethod | false) => void + ) => void + const walked: (AuthenticationType | AnyAuthMethod)[] = [] + let exhausted = false + let firstCall = true + while (!exhausted) { + handler(firstCall ? null : hostMethods, false, (attempt) => { + if (attempt === false) { + exhausted = true + } else { + walked.push(attempt) + } + }) + firstCall = false + } + return walked.map((attempt) => (typeof attempt === 'string' ? attempt : attempt.type)) +} + export function createResolvedConfig(overrides?: Partial): SshResolvedConfig { return { hostname: 'example.com', diff --git a/src/main/ssh/ssh-connection-utils.test.ts b/src/main/ssh/ssh-connection-utils.test.ts index 5c1feafe1f4..ac545b3572b 100644 --- a/src/main/ssh/ssh-connection-utils.test.ts +++ b/src/main/ssh/ssh-connection-utils.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest' import { join } from 'node:path' -import { BaseAgent, utils, type ParsedKey } from 'ssh2' +import { BaseAgent, utils, type AuthenticationType, type ParsedKey } from 'ssh2' vi.mock('os', () => ({ homedir: () => '/home/testuser' @@ -31,6 +31,7 @@ import { resolveAgentSocket } from './ssh-connection-utils' import { resolveEffectiveProxy } from './ssh-proxy-command' +import { walkInitialAuthLadder } from './ssh-connection-test-fixtures' import type { SshTarget } from '../../shared/ssh-types' import type { SshResolvedConfig } from './ssh-config-parser' @@ -338,6 +339,10 @@ describe('findDefaultKeyFile', () => { // ── buildConnectConfig ────────────────────────────────────────────── +// What an OpenSSH host that collects its password through keyboard-interactive lists after 'none'. +const PASSWORD_HOST_METHODS: AuthenticationType[] = ['publickey', 'keyboard-interactive'] +const UNENCRYPTED_KEY = Buffer.from(utils.generateKeyPairSync('ecdsa', { bits: 256 }).private) + function makeTarget(overrides?: Partial): SshTarget { return { id: 'test-1', @@ -656,24 +661,106 @@ describe('buildConnectConfig', () => { expect(config.privateKey).toEqual(Buffer.from('custom-key')) }) - it('uses agent auth without probing when resolved identityFile is a default path (expanded)', () => { + it('defers a default-path resolved identityFile to the no-agent retry', () => { + mockReadFileSync.mockReturnValue(UNENCRYPTED_KEY) const config = buildConnectConfig( makeTarget(), makeResolved({ identityFile: [testHomePath('.ssh', 'id_ed25519')] }) ) expect(config.agent).toBe('/tmp/agent.sock') + // No privateKey, so ssh2 cannot parse (and demand a passphrase for) the key before the agent + // has been tried, and no password challenge, so the deferred key is reached before any dialog. expect(config.privateKey).toBeUndefined() - expect(mockReadFileSync).not.toHaveBeenCalled() + expect(walkInitialAuthLadder(config, PASSWORD_HOST_METHODS)).toEqual(['none', 'agent']) }) - it('does not probe default key files before agent auth', () => { + it('defers an existing default key file to the no-agent retry', () => { mockExistsSync.mockImplementation( (p: unknown) => String(p) === testHomePath('.ssh', 'id_ed25519') ) + mockReadFileSync.mockReturnValue(UNENCRYPTED_KEY) const config = buildConnectConfig(makeTarget(), null) expect(config.agent).toBe('/tmp/agent.sock') expect(config.privateKey).toBeUndefined() - expect(mockExistsSync).not.toHaveBeenCalled() + expect(walkInitialAuthLadder(config, PASSWORD_HOST_METHODS)).toEqual(['none', 'agent']) + // A host that does not list it can only demand it as a second factor, after the agent key + // partially succeeds; ssh2 reports that with the stale pre-agent list, so it stays offered. + expect(walkInitialAuthLadder(config, ['publickey'])).toEqual([ + 'none', + 'agent', + 'keyboard-interactive' + ]) + }) + + it('reads a deferred default key only once the agent-first attempt reaches the challenge', () => { + const defaultKeyPath = testHomePath('.ssh', 'id_ed25519') + mockExistsSync.mockImplementation((p: unknown) => String(p) === defaultKeyPath) + mockReadFileSync.mockReturnValue(UNENCRYPTED_KEY) + const config = buildConnectConfig(makeTarget(), null) + // A FIFO or slow mount there must not block an agent login that never needs the file. + expect(mockReadFileSync).not.toHaveBeenCalledWith(defaultKeyPath) + expect(walkInitialAuthLadder(config, PASSWORD_HOST_METHODS)).toEqual(['none', 'agent']) + expect(mockReadFileSync).toHaveBeenCalledWith(defaultKeyPath) + }) + + it('keeps the challenge when several keys wait, which could spend the retry auth budget', () => { + mockReadFileSync.mockReturnValue(UNENCRYPTED_KEY) + const config = buildConnectConfig( + makeTarget(), + makeResolved({ + identityFile: [testHomePath('.ssh', 'id_ed25519'), testHomePath('.ssh', 'id_ecdsa')] + }) + ) + expect(config.privateKey).toBeUndefined() + expect(walkInitialAuthLadder(config, PASSWORD_HOST_METHODS)).toEqual([ + 'none', + 'agent', + 'keyboard-interactive' + ]) + }) + + it('keeps the challenge on a keyboard-interactive-only host, where the deferred key cannot help', () => { + mockExistsSync.mockImplementation( + (p: unknown) => String(p) === testHomePath('.ssh', 'id_ed25519') + ) + mockReadFileSync.mockReturnValue(UNENCRYPTED_KEY) + const config = buildConnectConfig(makeTarget(), null) + expect(walkInitialAuthLadder(config, ['keyboard-interactive'])).toEqual([ + 'none', + 'agent', + 'keyboard-interactive' + ]) + }) + + it('keeps the challenge when a deferred key file does not parse as a private key', () => { + mockExistsSync.mockImplementation( + (p: unknown) => String(p) === testHomePath('.ssh', 'id_ed25519') + ) + // ssh2 connect() throws on either before any auth, so the retry could never log in. + const unusableKeys = [ + Buffer.from('not a private key'), + Buffer.from(utils.generateKeyPairSync('ecdsa', { bits: 256 }).public) + ] + for (const contents of unusableKeys) { + mockReadFileSync.mockReturnValue(contents) + const config = buildConnectConfig(makeTarget(), null) + expect(walkInitialAuthLadder(config, PASSWORD_HOST_METHODS)).toEqual([ + 'none', + 'agent', + 'keyboard-interactive' + ]) + } + }) + + it('offers the keyboard-interactive challenge when the agent defers no key', () => { + const config = buildConnectConfig(makeTarget(), null) + expect(config.agent).toBe('/tmp/agent.sock') + expect(config.privateKey).toBeUndefined() + expect(walkInitialAuthLadder(config, PASSWORD_HOST_METHODS)).toEqual([ + 'none', + 'agent', + 'keyboard-interactive' + ]) }) it('provides fallback key when no agent is available', () => { diff --git a/src/main/ssh/ssh-connection-utils.ts b/src/main/ssh/ssh-connection-utils.ts index 48057b9c088..e2a4fdef433 100644 --- a/src/main/ssh/ssh-connection-utils.ts +++ b/src/main/ssh/ssh-connection-utils.ts @@ -3,6 +3,7 @@ import type { SshTarget, SshConnectionState } from '../../shared/ssh-types' import type { SshResolvedConfig } from './ssh-config-parser' import { findEncryptedPrivateKeyPath, + isUnencryptedPrivateKey, resolveAgentConfigValue, resolveAgentSocket, resolvePrivateKeys, @@ -179,7 +180,7 @@ export function createSshOperationAbortError(): Error & { name: string } { return error } -type BuildConnectConfigOptions = { +export type BuildConnectConfigOptions = { includeAgent?: boolean includePrivateKey?: boolean } @@ -227,10 +228,32 @@ export function buildConnectConfig( (options.includePrivateKey ?? !agent) ? resolvePrivateKeys(target, resolved) : resolveUnencryptedExplicitPrivateKeys(target, resolved) + let deferralDecision: boolean | undefined configurePrivateKeyAuthentication( config as ConnectConfig, keys, - findEncryptedPrivateKeyPath(keys) + findEncryptedPrivateKeyPath(keys), + { + // Why: SshConnection retries with the deferred keys only after an attempt that carried none, + // so only that attempt holds the host's password challenge back for them. An encrypted key + // would swap that dialog for a passphrase one, and a file that is no usable private key + // aborts the retry before any auth, so hold it back only if no deferred key needs input. + // One key only: each rejected key spends the host's MaxAuthTries, so several could exhaust + // the retry before it reaches the challenge this attempt skipped. + // 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 + ? () => { + if (deferralDecision === undefined) { + const deferredKeys = resolvePrivateKeys(target, resolved) + deferralDecision = + deferredKeys.length === 1 && isUnencryptedPrivateKey(deferredKeys[0].contents) + } + return deferralDecision + } + : undefined + } ) return config as ConnectConfig diff --git a/src/main/ssh/ssh-multi-factor-authentication.test.ts b/src/main/ssh/ssh-multi-factor-authentication.test.ts index 4b22c9e6157..c6c53135711 100644 --- a/src/main/ssh/ssh-multi-factor-authentication.test.ts +++ b/src/main/ssh/ssh-multi-factor-authentication.test.ts @@ -1,20 +1,25 @@ -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { homedir, tmpdir } from 'node:os' import type * as Os from 'node:os' import { join } from 'node:path' import { + BaseAgent, Client, Server as Ssh2Server, utils, type AuthContext, type Connection, + type IdentityCallback, type KeyboardAuthContext, - type PasswordAuthContext + type ParsedKey, + type PasswordAuthContext, + type SignCallback, + type SigningRequestOptions } from 'ssh2' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { SshTarget } from '../../shared/ssh-types' import type { SshResolvedConfig } from './ssh-config-parser' -import { buildConnectConfig } from './ssh-connection-utils' +import { buildConnectConfig, type BuildConnectConfigOptions } from './ssh-connection-utils' vi.mock('node:os', async (importOriginal) => { const actual = await importOriginal() @@ -26,7 +31,44 @@ const MAX_AUTH_TRIES = 6 const PASSWORD = 'stage-one-password' const PASSCODE = '123456' -type AuthStage = 'password' | 'keyboard-interactive' +type AuthStage = 'password' | 'keyboard-interactive' | 'publickey' + +function generatePrivateKey(): string { + return utils.generateKeyPairSync('ecdsa', { bits: 256 }).private +} + +function parseFixtureKey(privateKey: string): ParsedKey { + const key = utils.parseKey(privateKey) + if (key instanceof Error || Array.isArray(key)) { + throw new Error('fixture key did not parse to a single key') + } + return key +} + +/** An ssh-agent stand-in holding one key, so the agent rung can satisfy a publickey stage. */ +class SingleKeyAgent extends BaseAgent { + private readonly key = parseFixtureKey(generatePrivateKey()) + + getIdentities(callback: IdentityCallback): void { + callback(undefined, [this.key]) + } + + sign( + _pubKey: ParsedKey, + data: Buffer, + optionsOrCallback?: SigningRequestOptions | SignCallback, + callback?: SignCallback + ): void { + const options = typeof optionsOrCallback === 'function' ? {} : optionsOrCallback + const done = typeof optionsOrCallback === 'function' ? optionsOrCallback : callback + const signature = this.key.sign(data, options?.hash) + if (signature instanceof Error) { + done?.(signature) + return + } + done?.(undefined, signature) + } +} type MfaServer = { port: number @@ -34,8 +76,14 @@ type MfaServer = { close: () => Promise } -/** An OpenSSH-style `AuthenticationMethods a,b` host: each stage partial-succeeds into the next. */ -async function startMultiFactorServer(stages: AuthStage[]): Promise { +/** + * An OpenSSH-style `AuthenticationMethods a,b` host: each stage partial-succeeds into the next. + * A stage given as a list accepts any of its methods; `authorizedKey` limits publickey to one key. + */ +async function startMultiFactorServer( + stages: (AuthStage | AuthStage[])[], + authorizedKey?: ParsedKey +): Promise { const attempts: string[] = [] const connections = new Set() // Ed25519 keygen can produce an invalid 31-byte key; ECDSA points always start with 0x04. @@ -46,7 +94,7 @@ async function startMultiFactorServer(stages: AuthStage[]): Promise { connection.on('close', () => connections.delete(connection)) let stage = 0 let failures = 0 - const remaining = (): AuthStage[] => [stages[stage]!] + const remaining = (): AuthStage[] => [stages[stage]!].flat() const fail = (context: AuthContext): void => { failures += 1 if (failures >= MAX_AUTH_TRIES) { @@ -61,7 +109,7 @@ async function startMultiFactorServer(stages: AuthStage[]): Promise { context.reject(remaining(), false) return } - if (context.method !== stages[stage]) { + if (!remaining().some((method) => method === context.method)) { fail(context) return } @@ -78,6 +126,24 @@ async function startMultiFactorServer(stages: AuthStage[]): Promise { context.reject(remaining(), true) return } + if (context.method === 'publickey') { + if (authorizedKey && !authorizedKey.getPublicSSH().equals(context.key.data)) { + fail(context) + return + } + // Query phase: claim the key is acceptable so the client sends the signature. + if (!context.signature) { + context.accept() + return + } + stage += 1 + if (stage === stages.length) { + context.accept() + return + } + context.reject(remaining(), true) + return + } const keyboard = context as KeyboardAuthContext keyboard.prompt( [{ prompt: 'Duo passcode:', echo: false }], @@ -159,13 +225,15 @@ function connectWithOrcaConfig( target: SshTarget, resolved: SshResolvedConfig | null, password: string | undefined, - answers: string[] + answers: string[], + buildOptions: BuildConnectConfigOptions = { includeAgent: false, includePrivateKey: true }, + agent?: BaseAgent ): { ready: Promise; prompts: string[] } { const prompts: string[] = [] - const config = buildConnectConfig(target, resolved, { - includeAgent: false, - includePrivateKey: true - }) + const config = buildConnectConfig(target, resolved, buildOptions) + if (agent) { + config.agent = agent + } if (password != null) { config.password = password } @@ -257,4 +325,121 @@ describe('multi-stage SSH authentication', () => { await server.close() } }) + + it('keeps the challenge on the agent-first attempt when the waiting ~/.ssh key is encrypted', async () => { + // Deferring here would only trade the password dialog for a passphrase one on the key retry. + mkdirSync(join(tempDir, '.ssh')) + writeFileSync( + join(tempDir, '.ssh', 'id_ed25519'), + utils.generateKeyPairSync('ecdsa', { + bits: 256, + passphrase: 'fixture-passphrase', + cipher: 'aes256-ctr', + rounds: 1 + }).private + ) + const server = await startMultiFactorServer( + [['publickey', 'keyboard-interactive']], + parseFixtureKey(generatePrivateKey()) + ) + try { + const { ready, prompts } = connectWithOrcaConfig( + makeTarget(server.port, { identityAgent: join(tempDir, 'agent.sock') }), + null, + undefined, + [PASSCODE], + { includeAgent: true }, + new SingleKeyAgent() + ) + + await expect(ready).resolves.toBeUndefined() + expect(prompts).toEqual(['Duo passcode:']) + } finally { + await server.close() + } + }) + + describe('on the agent-first attempt, which defers ~/.ssh/id_ed25519 to the key retry', () => { + let deferredKey: ParsedKey + + beforeEach(() => { + const privateKey = generatePrivateKey() + deferredKey = parseFixtureKey(privateKey) + mkdirSync(join(tempDir, '.ssh')) + writeFileSync(join(tempDir, '.ssh', 'id_ed25519'), privateKey) + }) + + it('holds a first-factor challenge back until the attempt that carries the deferred key', async () => { + // A host taking either a key or a keyboard-interactive password, where only the deferred key + // is authorized: a challenge on the agent-first attempt is a dialog the key makes unnecessary. + const server = await startMultiFactorServer( + [['publickey', 'keyboard-interactive']], + deferredKey + ) + try { + const target = makeTarget(server.port, { identityAgent: join(tempDir, 'agent.sock') }) + const agentFirst = connectWithOrcaConfig( + target, + null, + undefined, + [PASSCODE], + { includeAgent: true }, + new SingleKeyAgent() + ) + await expect(agentFirst.ready).rejects.toThrow( + 'All configured authentication methods failed' + ) + expect(agentFirst.prompts).toEqual([]) + + const keyRetry = connectWithOrcaConfig(target, null, undefined, [PASSCODE]) + await expect(keyRetry.ready).resolves.toBeUndefined() + expect(keyRetry.prompts).toEqual([]) + } finally { + await server.close() + } + }) + + it('answers the second factor after the agent key partially succeeds', async () => { + const server = await startMultiFactorServer(['publickey', 'keyboard-interactive']) + try { + const { ready, prompts } = connectWithOrcaConfig( + makeTarget(server.port, { identityAgent: join(tempDir, 'agent.sock') }), + null, + undefined, + [PASSCODE], + { includeAgent: true }, + new SingleKeyAgent() + ) + + await expect(ready).resolves.toBeUndefined() + expect(prompts).toEqual(['Duo passcode:']) + } finally { + await server.close() + } + }) + + it('answers the second factor when the host first offered both a key and the challenge', async () => { + // ssh2 keeps the pre-agent list (publickey and keyboard-interactive) after the agent key + // partially succeeds, so only the agent's signature shows this challenge is a second factor. + const server = await startMultiFactorServer([ + ['publickey', 'keyboard-interactive'], + 'keyboard-interactive' + ]) + try { + const { ready, prompts } = connectWithOrcaConfig( + makeTarget(server.port, { identityAgent: join(tempDir, 'agent.sock') }), + null, + undefined, + [PASSCODE], + { includeAgent: true }, + new SingleKeyAgent() + ) + + await expect(ready).resolves.toBeUndefined() + expect(prompts).toEqual(['Duo passcode:']) + } finally { + await server.close() + } + }) + }) }) diff --git a/src/main/ssh/ssh-multi-key-authentication.test.ts b/src/main/ssh/ssh-multi-key-authentication.test.ts index 7cee28a6fda..ea069965be1 100644 --- a/src/main/ssh/ssh-multi-key-authentication.test.ts +++ b/src/main/ssh/ssh-multi-key-authentication.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' import { utils, type AnyAuthMethod, @@ -24,6 +25,7 @@ vi.mock('fs', () => ({ import { buildConnectConfig } from './ssh-connection-utils' import { getPassphrasePrivateKeyPath } from './ssh-private-key-authentication' import { buildSshArgs } from './system-ssh-args' +import { walkInitialAuthLadder } from './ssh-connection-test-fixtures' import type { SshTarget } from '../../shared/ssh-types' import type { SshResolvedConfig } from './ssh-config-parser' @@ -128,7 +130,9 @@ describe('ordered SSH private-key authentication', () => { const config = buildConnectConfig(makeTarget({ identityFile: undefined }), null) expect(nextAuth(config, true)).toMatchObject({ type: 'none' }) - expect(nextAuth(config, false)).toMatchObject({ type: 'agent', agent: '/tmp/agent.sock' }) + // The queued agent observes signing for the challenge deferral; ssh2 still reads the socket. + expect(config.agent).toBe('/tmp/agent.sock') + expect(nextAuth(config, false)).toMatchObject({ type: 'agent' }) expect(nextAuth(config, false)).toBe('keyboard-interactive') }) @@ -174,6 +178,28 @@ describe('ordered SSH private-key authentication', () => { expect(partialSuccessAuth(config, ['keyboard-interactive'])).toBe('keyboard-interactive') }) + it('keeps the password challenge on an agent attempt that carries a key, because no key retry follows it', () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + // Real key bytes, so '/keys/explicit' counts as unencrypted and IdentitiesOnly builds its agent. + const keyBytes = Buffer.from(utils.generateKeyPairSync('ecdsa', { bits: 256 }).private) + mockReadFileSync.mockReturnValue(keyBytes) + const config = buildConnectConfig( + makeTarget(), + makeResolved({ + identityFile: [join('/home/testuser', '.ssh', 'id_ed25519'), '/keys/explicit'] + }) + ) + + // The password retry reuses this config, so holding the challenge back here would leave a + // keyboard-interactive password host with no attempt that ever offers it. + expect(walkInitialAuthLadder(config, ['publickey', 'keyboard-interactive'])).toEqual([ + 'none', + 'publickey', + 'agent', + 'keyboard-interactive' + ]) + }) + it('stops re-offering methods the host no longer accepts after a partial success', () => { const config = buildConnectConfig(makeTarget(), makeResolved(), { includeAgent: false, diff --git a/src/main/ssh/ssh-private-key-authentication.ts b/src/main/ssh/ssh-private-key-authentication.ts index 3daa7a8c844..1903fdc957a 100644 --- a/src/main/ssh/ssh-private-key-authentication.ts +++ b/src/main/ssh/ssh-private-key-authentication.ts @@ -1,5 +1,6 @@ import type { AnyAuthMethod, AuthenticationType, ConnectConfig, NextAuthHandler } from 'ssh2' import type { PrivateKeyFile } from './ssh-auth-resolution' +import { observeAgentSigning } from './ssh-agent-sign-observer' const passphraseKeyPaths = new WeakMap() @@ -15,7 +16,8 @@ function authMethodName(attempt: AuthenticationType | AnyAuthMethod): Authentica function buildAuthQueue( config: ConnectConfig, - keys: PrivateKeyFile[] + keys: PrivateKeyFile[], + onAgentSign?: () => void ): (AuthenticationType | AnyAuthMethod)[] { const username = config.username ?? '' const queue: (AuthenticationType | AnyAuthMethod)[] = [{ type: 'none', username }] @@ -31,7 +33,8 @@ function buildAuthQueue( }) } if (config.agent) { - queue.push({ type: 'agent', username, agent: config.agent }) + const agent = onAgentSign ? observeAgentSigning(config.agent, onAgentSign) : config.agent + queue.push({ type: 'agent', username, agent }) } if (config.tryKeyboard) { queue.push('keyboard-interactive') @@ -42,7 +45,8 @@ function buildAuthQueue( export function configurePrivateKeyAuthentication( config: ConnectConfig, keys: PrivateKeyFile[], - passphraseKeyPath?: string + passphraseKeyPath?: string, + options: { deferKeyboardInteractive?: () => boolean } = {} ): void { const firstKey = keys[0] if (firstKey) { @@ -59,9 +63,16 @@ export function configurePrivateKeyAuthentication( // the list exhausted — reported to the user as "All configured authentication methods failed". let queue: (AuthenticationType | AnyAuthMethod)[] = [] let partialSuccessStagesLeft = MAX_PARTIAL_SUCCESS_STAGES + let agentSigned = false + const onAgentSign = options.deferKeyboardInteractive + ? () => { + agentSigned = true + } + : undefined config.authHandler = (authsLeft, partialSuccess, next) => { if (authsLeft == null) { - queue = buildAuthQueue(config, keys) + agentSigned = false + queue = buildAuthQueue(config, keys, onAgentSign) partialSuccessStagesLeft = MAX_PARTIAL_SUCCESS_STAGES } else if (partialSuccess && partialSuccessStagesLeft > 0) { // A stage was accepted and the host now demands another method. Restart from a fresh queue @@ -69,12 +80,27 @@ export function configurePrivateKeyAuthentication( // exhausts MaxAuthTries before the challenge is ever shown. partialSuccessStagesLeft -= 1 const offered = Array.isArray(authsLeft) ? authsLeft : [] - queue = buildAuthQueue(config, keys).filter((attempt) => { + queue = buildAuthQueue(config, keys, onAgentSign).filter((attempt) => { const method = authMethodName(attempt) return method !== 'none' && offered.includes(method) }) } - const attempt = queue.shift() + let attempt = queue.shift() + // Why: a deferring attempt must not show the host's password challenge before its deferred keys + // are tried. A host that lists keyboard-interactive offers it as a first factor, so skip it while + // publickey is listed too (otherwise the deferred key cannot help). After an agent sign the host + // accepted an agent key, so the challenge is its second factor; ssh2 hides that partial success. + if ( + attempt === 'keyboard-interactive' && + !agentSigned && + partialSuccessStagesLeft === MAX_PARTIAL_SUCCESS_STAGES && + Array.isArray(authsLeft) && + authsLeft.includes('keyboard-interactive') && + authsLeft.includes('publickey') && + options.deferKeyboardInteractive?.() + ) { + attempt = queue.shift() + } next((attempt ?? false) as Parameters[0]) } }