diff --git a/packages/ssh-core/src/identity.test.ts b/packages/ssh-core/src/identity.test.ts index 88dd2af..caf8c5b 100644 --- a/packages/ssh-core/src/identity.test.ts +++ b/packages/ssh-core/src/identity.test.ts @@ -4,6 +4,7 @@ import { defaultIdentityPaths, expandTilde, findAgentSocket, + findDefaultIdentities, findDefaultIdentity, } from './identity.js' @@ -91,3 +92,34 @@ describe('findAgentSocket', () => { expect(agentSocketCandidates({}, null)).toEqual([]) }) }) + +describe('findDefaultIdentities', () => { + /** + * The bug this exists to prevent: only the first existing key was offered. + * A host that accepts id_rsa but not id_ed25519 — seed1, in the report that + * prompted this — rejected the connection outright, while `ssh` to the same + * host from a terminal succeeded, because ssh offers each identity in turn. + */ + it('returns every key that exists, in ssh order', () => { + expect(findDefaultIdentities(() => true, HOME)).toEqual([ + '/home/you/.ssh/id_ed25519', + '/home/you/.ssh/id_ecdsa', + '/home/you/.ssh/id_rsa', + '/home/you/.ssh/id_dsa', + ]) + }) + + it('keeps id_rsa when ed25519 also exists, because the server chooses', () => { + const present = ['/home/you/.ssh/id_ed25519', '/home/you/.ssh/id_rsa'] + expect(findDefaultIdentities((path) => present.includes(path), HOME)).toEqual(present) + }) + + it('is empty when the user has no keys', () => { + expect(findDefaultIdentities(() => false, HOME)).toEqual([]) + }) + + it('still reports the first one for callers that want just one', () => { + const only = '/home/you/.ssh/id_rsa' + expect(findDefaultIdentity((path) => path === only, HOME)).toBe(only) + }) +}) diff --git a/packages/ssh-core/src/identity.ts b/packages/ssh-core/src/identity.ts index e82ea97..c49a6ad 100644 --- a/packages/ssh-core/src/identity.ts +++ b/packages/ssh-core/src/identity.ts @@ -34,9 +34,21 @@ export function defaultIdentityPaths(home: string = homedir()): string[] { return DEFAULT_IDENTITY_FILES.map((name) => join(home, '.ssh', name)) } +/** + * Every default identity that exists, in ssh's order. + * + * All of them, not the first: ssh(1) offers each identity in turn until the + * server accepts one, and a host that takes id_rsa but not id_ed25519 is + * ordinary. Offering only the first key made such a host reject us outright + * while `ssh` to the same host from a terminal succeeded. + */ +export function findDefaultIdentities(exists: (path: string) => boolean, home: string = homedir()): string[] { + return defaultIdentityPaths(home).filter((path) => exists(path)) +} + /** The first default identity that exists, or null when the user has no keys. */ export function findDefaultIdentity(exists: (path: string) => boolean, home: string = homedir()): string | null { - return defaultIdentityPaths(home).find((path) => exists(path)) ?? null + return findDefaultIdentities(exists, home)[0] ?? null } /** diff --git a/packages/ssh-core/src/session.ts b/packages/ssh-core/src/session.ts index 4dea102..8f25714 100644 --- a/packages/ssh-core/src/session.ts +++ b/packages/ssh-core/src/session.ts @@ -1,8 +1,8 @@ import { existsSync, readFileSync } from 'node:fs' -import { Client, type ConnectConfig, type SFTPWrapper } from 'ssh2' +import { Client, type AnyAuthMethod, type ConnectConfig, type SFTPWrapper } from 'ssh2' import type { Connection } from '@diskpush/schemas' import { keyTypeOf, sha256Fingerprint } from './fingerprint.js' -import { expandTilde, findAgentSocket, findDefaultIdentity } from './identity.js' +import { expandTilde, findAgentSocket, findDefaultIdentities } from './identity.js' import { appendKnownHost, readKnownHosts, verifyHostKey, type HostKeyVerdict } from './known-hosts.js' export class SshError extends Error { @@ -60,20 +60,18 @@ export class SshSession { } if (connection.authType === 'agent') { - // Both halves, the way ssh(1) does it: an agent if one can be found, and - // the default identity files regardless. Requiring SSH_AUTH_SOCK to be - // exported meant every agent host failed in the desktop app, which is - // launched from a session that exports far less than a login shell. + // Every credential, offered in turn, the way ssh(1) does it: the agent + // first if one can be found, then each default identity that exists. + // + // ssh2's `privateKey` holds exactly one key, so offering only the first + // one meant a host that accepts id_rsa but not id_ed25519 rejected us + // outright — while `ssh` to that same host from a terminal succeeded, + // because it tries them all. An authHandler array is how ssh2 expresses + // "try these, in this order". const agent = options.agentSocket ?? findAgentSocket(existsSync) - if (agent) config.agent = agent + const identities = findDefaultIdentities(existsSync) - const identity = findDefaultIdentity(existsSync) - if (identity) { - config.privateKey = readFileSync(identity) - if (options.passphrase) config.passphrase = options.passphrase - } - - if (!agent && !identity) { + if (!agent && identities.length === 0) { throw new SshError( 'No SSH agent and no default key. Looked for an agent socket, then for ' + '~/.ssh/id_ed25519, id_ecdsa, id_rsa and id_dsa. Set a key file on this connection, ' + @@ -81,6 +79,18 @@ export class SshSession { 'auth', ) } + + const methods: AnyAuthMethod[] = [] + if (agent) methods.push({ type: 'agent', username: connection.username, agent }) + for (const identity of identities) { + methods.push({ + type: 'publickey', + username: connection.username, + key: readFileSync(identity), + ...(options.passphrase ? { passphrase: options.passphrase } : {}), + }) + } + config.authHandler = methods } else if (connection.authType === 'key' || connection.authType === 'key-passphrase') { if (!connection.keyPath) throw new SshError('This connection is set to key authentication but has no key path.', 'auth') // `~` is expanded here rather than trusted to have been expanded by