xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(P1-C): gate FIDO2 system-SSH transport on an OpenSSH binary (#12029) 

* fix(ssh): gate FIDO2 system-transport on an OpenSSH binary

`ssh -G` echoes OpenSSH's built-in default identity list for every host, so
`usesDefaultPaths` was almost never true and the security-key gate returned
`!usesDefaultPaths || findSystemSsh() !== null` — forcing system transport
without checking that an `ssh` binary exists. `spawnSystemSsh()` then throws
`No system ssh binary found`, hard-failing connections that worked on ssh2.

The same flag also stopped the default scan at the first existing normal
private key, so a host that only accepts a FIDO2 key never reached system
OpenSSH when `~/.ssh/id_rsa` happened to exist.

Both decisions are independent of where an identity path came from: always
require `findSystemSsh() !== null` before forcing system transport, and scan
every candidate identity instead of stopping on the first normal key.
`shouldUseSystemSshTransport()` is untouched, so ProxyCommand / ProxyJump /
ProxyUseFdpass keep their intentional system transport.

* test(ssh): isolate connection tests from the developer's own FIDO2 keys

Transport selection now scans every default identity instead of stopping at
the first normal key, so a `~/.ssh/id_ed25519_sk` on the machine running the
suite would decide which transport the default-target tests take. Mock
`findSystemSsh` to null by default and opt the two security-key tests in.
This commit is contained in:
Jinjing 2026-08-01 18:45:37 -07:00 committed by GitHub
parent dbfffa6530
commit de75003df9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 102 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/main/ssh/ssh-connection.test.ts
View File

@ -148,17 +148,23 @@ vi.mock('ssh2', () => {
})
const {
findSystemSshMock,
getOrcaControlSocketPathMock,
removeControlSocketPathMock,
spawnSystemSshCommandMock,
spawnSystemSshMock
} = vi.hoisted(() => ({
findSystemSshMock: vi.fn<() => string | null>(),
getOrcaControlSocketPathMock: vi.fn(),
removeControlSocketPathMock: vi.fn(),
spawnSystemSshMock: vi.fn(),
spawnSystemSshCommandMock: vi.fn()
}))
// Why: security-key transport selection scans the real ~/.ssh defaults, so a developer's own
// FIDO2 key would otherwise decide which transport these tests take.
vi.mock('./system-ssh-binary', () => ({ findSystemSsh: findSystemSshMock }))
vi.mock('./ssh-system-fallback', () => ({
getOrcaControlSocketPath: getOrcaControlSocketPathMock,
spawnSystemSsh: spawnSystemSshMock,
@ -331,6 +337,8 @@ describe('SshConnection', () => {
vi.mocked(writeFileViaSystemSsh).mockResolvedValue(undefined)
vi.mocked(resolveWithSshG).mockReset()
vi.mocked(resolveWithSshG).mockResolvedValue(null)
findSystemSshMock.mockReset()
findSystemSshMock.mockReturnValue(null)
vi.unstubAllEnvs()
})
@ -1697,6 +1705,7 @@ describe('SshConnection', () => {
})
it('uses system SSH before ssh2 parses a security-key private key', async () => {
findSystemSshMock.mockReturnValue('/usr/bin/ssh')
const directory = mkdtempSync(join(tmpdir(), 'orca-security-key-connect-'))
const keyPath = join(directory, 'id_ed25519_sk')
writeFileSync(
@ -1718,6 +1727,7 @@ describe('SshConnection', () => {
})
it('uses system SSH for an agent-backed security-key public identity', async () => {
findSystemSshMock.mockReturnValue('/usr/bin/ssh')
const directory = mkdtempSync(join(tmpdir(), 'orca-security-key-agent-connect-'))
const identityPath = join(directory, 'id_ed25519_sk')
writeFileSync(

110
src/main/ssh/ssh-security-key-identity.test.ts
View File

@ -53,6 +53,29 @@ async function writeKey(contents: Buffer, filename = 'security key'): Promise<st
return keyPath
}
async function createDefaultKeyHome(files: Record<string, Buffer>): Promise<string> {
const directory = await mkdtemp(join(tmpdir(), 'orca-default-key-home-'))
tempDirs.push(directory)
await mkdir(join(directory, '.ssh'))
for (const [name, contents] of Object.entries(files)) {
await writeFile(join(directory, '.ssh', name), contents)
}
return directory
}
// Why: `ssh -G` echoes this list, already home-expanded, for every host — configured or not.
function listBuiltInDefaultIdentityFiles(home: string): string[] {
return [
'id_rsa',
'id_ecdsa',
'id_ecdsa_sk',
'id_ed25519',
'id_ed25519_sk',
'id_xmss',
'id_dsa'
].map((name) => join(home, '.ssh', name))
}
afterEach(async () => {
vi.unstubAllEnvs()
await Promise.all(tempDirs.splice(0).map((directory) => rm(directory, { recursive: true })))
@ -155,13 +178,9 @@ describe('isOpenSshSecurityKeyPrivateKey', () => {
describe('requiresSystemSshForSecurityKey', () => {
it('uses default FIDO2 identities only when config resolution is unavailable', async () => {
const directory = await mkdtemp(join(tmpdir(), 'orca-security-key-home-'))
tempDirs.push(directory)
await mkdir(join(directory, '.ssh'))
await writeFile(
join(directory, '.ssh', 'id_ed25519_sk'),
createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
)
const directory = await createDefaultKeyHome({
id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
})
vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(true)
@ -170,28 +189,58 @@ describe('requiresSystemSshForSecurityKey', () => {
).resolves.toBe(false)
})
it('keeps a regular default ahead of a dormant FIDO2 identity', async () => {
const directory = await mkdtemp(join(tmpdir(), 'orca-regular-key-home-'))
tempDirs.push(directory)
await mkdir(join(directory, '.ssh'))
await writeFile(join(directory, '.ssh', 'id_rsa'), createOpenSshPrivateKeyFixture(['ssh-rsa']))
await writeFile(
join(directory, '.ssh', 'id_ed25519_sk'),
createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
)
it('reaches a default FIDO2 identity that a regular default key precedes', async () => {
const directory = await createDefaultKeyHome({
id_rsa: createOpenSshPrivateKeyFixture(['ssh-rsa']),
id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
})
vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(true)
findSystemSshMock.mockReturnValue(null)
await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(false)
})
it('leaves regular-only defaults on ssh2', async () => {
const directory = await createDefaultKeyHome({
id_rsa: createOpenSshPrivateKeyFixture(['ssh-rsa'])
})
vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(false)
})
it('keeps password and agent fallback when default FIDO2 needs unavailable OpenSSH', async () => {
const directory = await mkdtemp(join(tmpdir(), 'orca-no-system-ssh-home-'))
tempDirs.push(directory)
await mkdir(join(directory, '.ssh'))
await writeFile(
join(directory, '.ssh', 'id_ed25519_sk'),
createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
it('treats resolved built-in default identities as unconfigured, not as forced transport', async () => {
const directory = await createDefaultKeyHome({
id_rsa: createOpenSshPrivateKeyFixture(['ssh-rsa']),
id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
})
const identityFile = listBuiltInDefaultIdentityFiles(directory)
await expect(requiresSystemSshForSecurityKey(createTarget(), { identityFile })).resolves.toBe(
true
)
findSystemSshMock.mockReturnValue(null)
await expect(requiresSystemSshForSecurityKey(createTarget(), { identityFile })).resolves.toBe(
false
)
})
it('keeps password and agent fallback when a configured FIDO2 identity has no OpenSSH', async () => {
const keyPath = await writeKey(createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY]))
findSystemSshMock.mockReturnValue(null)
await expect(
requiresSystemSshForSecurityKey(createTarget({ identityFile: keyPath }), null)
).resolves.toBe(false)
})
it('keeps password and agent fallback when default FIDO2 needs unavailable OpenSSH', async () => {
const directory = await createDefaultKeyHome({
id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
})
vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
findSystemSshMock.mockReturnValue(null)
@ -199,17 +248,10 @@ describe('requiresSystemSshForSecurityKey', () => {
})
it('ignores an orphan regular sidecar before a valid default FIDO2 identity', async () => {
const directory = await mkdtemp(join(tmpdir(), 'orca-orphan-sidecar-home-'))
tempDirs.push(directory)
await mkdir(join(directory, '.ssh'))
await writeFile(
join(directory, '.ssh', 'id_ed25519.pub'),
createOpenSshPublicKeyFixture('ssh-ed25519')
)
await writeFile(
join(directory, '.ssh', 'id_ed25519_sk'),
createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
)
const directory = await createDefaultKeyHome({
'id_ed25519.pub': createOpenSshPublicKeyFixture('ssh-ed25519'),
id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
})
vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(true)

37
src/main/ssh/ssh-transport-selection.ts
View File

@ -21,8 +21,6 @@ const READ_CHUNK_BYTES = 64 * 1024
const READ_OPEN_FLAGS =
constants.O_RDONLY | (process.platform === 'win32' ? 0 : constants.O_NONBLOCK)
type IdentityInspection = { privateIdentityExists: boolean; requiresSystemSsh: boolean }
async function readBoundedKeyFile(path: string): Promise<Buffer | null> {
let handle: Awaited<ReturnType<typeof open>> | undefined
try {
@ -59,21 +57,15 @@ async function readBoundedKeyFile(path: string): Promise<Buffer | null> {
}
}
async function inspectIdentityPath(keyPath: string): Promise<IdentityInspection> {
async function identityRequiresSystemSsh(keyPath: string): Promise<boolean> {
const resolvedPath = resolveSshConfigHomePath(keyPath)
const identity = await readBoundedKeyFile(resolvedPath)
// Why: a present private key wins; a `.pub` beside it may describe a key already replaced.
if (identity !== null) {
return {
privateIdentityExists: true,
requiresSystemSsh:
isOpenSshSecurityKeyPublicKey(identity) || isOpenSshSecurityKeyPrivateKey(identity)
}
return isOpenSshSecurityKeyPublicKey(identity) || isOpenSshSecurityKeyPrivateKey(identity)
}
const publicIdentity = await readBoundedKeyFile(`${resolvedPath}.pub`)
return {
privateIdentityExists: false,
requiresSystemSsh: publicIdentity !== null && isOpenSshSecurityKeyPublicKey(publicIdentity)
}
return publicIdentity !== null && isOpenSshSecurityKeyPublicKey(publicIdentity)
}
export function shouldUseSystemSshTransport(
@ -102,16 +94,19 @@ export async function requiresSystemSshForSecurityKey(
target: SshTarget,
resolved: Pick<SshResolvedConfig, 'identityFile'> | null
): Promise<boolean> {
const configuredPaths = resolveIdentityFilePaths(target, resolved)
const usesDefaultPaths = configuredPaths.length === 0 && !resolved && !target.identityFile
const identityPaths = usesDefaultPaths ? listDefaultIdentityFilePaths() : configuredPaths
const resolvedPaths = resolveIdentityFilePaths(target, resolved)
// Why: `ssh -G` already echoes OpenSSH's built-in defaults, so its list is the real candidate
// set; guess at the defaults only when config resolution failed outright.
const identityPaths =
resolvedPaths.length === 0 && !resolved && !target.identityFile
? listDefaultIdentityFilePaths()
: resolvedPaths
for (const keyPath of identityPaths) {
const inspection = await inspectIdentityPath(keyPath)
if (inspection.requiresSystemSsh) {
return !usesDefaultPaths || findSystemSsh() !== null
}
if (usesDefaultPaths && inspection.privateIdentityExists) {
return false
if (await identityRequiresSystemSsh(keyPath)) {
// Why: scan every candidate — an earlier normal key never rules out a security key the host
// requires — but forcing system transport with no binary hard-fails a connection ssh2 could
// still have served over agent or password auth.
return findSystemSsh() !== null
}
}
return false