From d008173e551fcb8e977606337ad5e5dc0a2fd675 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 09:22:02 +0000 Subject: [PATCH] ssh: offer every default key, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seed1 rejected DiskPush while `ssh seed1` from a terminal connected as ubuntu. The host was not refusing our keys, as this was first reported to the user: it was never offered the one it accepts. ssh -i ~/.ssh/id_ed25519 seed1 -> Permission denied (publickey,password) ssh -i ~/.ssh/id_rsa seed1 -> RSA OK The default-key fallback added in 0.2.5 took the first identity that exists, and ssh2's `privateKey` holds exactly one. On a machine with both keys that is always id_ed25519, so a host accepting only id_rsa saw one key, refused it, and the connection ended — while ssh(1), which offers each identity in turn, walked straight in. Agent authentication now hands ssh2 an ordered authHandler: the agent first when one is found, then every default identity that exists, in ssh's order. That is what "the way ssh does it" was supposed to mean. Verified against the host that reported it. Before: `[auth] SSH authentication was rejected by seed1.h4kr.com`. After: seed1 lists its home directory, 23 entries. seed2 still lists 51 and still resolves `data -> /mnt/vdb` as a directory. 4 new tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd --- packages/ssh-core/src/identity.test.ts | 32 ++++++++++++++++++++++ packages/ssh-core/src/identity.ts | 14 +++++++++- packages/ssh-core/src/session.ts | 38 ++++++++++++++++---------- 3 files changed, 69 insertions(+), 15 deletions(-) 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