From d791e52d82b098a87e2dbb495d34e4779fedaf82 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Wed, 14 Oct 2026 08:12:37 +0800 Subject: [PATCH] fix: stop auto-accepting changed jump host keys (#1620) A jump hop whose key no longer matched the pinned fingerprint was silently accepted and the pin overwritten. Background status probes reconnect to jump hosts every cycle, so a MITM on that hop got the credentials with no user interaction. Changed jump host keys now go through the same path as any other host: prompt when there is a terminal socket, otherwise refuse. --- src/backend/hosts/host-key-verifier.ts | 22 ------- .../tests/hosts/host-key-verifier.test.ts | 57 ++++++++++++++++++- 2 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/backend/hosts/host-key-verifier.ts b/src/backend/hosts/host-key-verifier.ts index 89b54688c..11c14331a 100644 --- a/src/backend/hosts/host-key-verifier.ts +++ b/src/backend/hosts/host-key-verifier.ts @@ -263,28 +263,6 @@ export class SSHHostKeyVerifier { changeCount: host.hostKeyChangedCount || 0, }); - if (isJumpHost) { - await this.updateHostKey( - hostId, - fingerprint, - keyType, - algorithm, - host.hostKeyChangedCount || 0, - ); - sshLogger.warn("Jump host key changed - auto-accepted", { - operation: "host_key_updated", - hostId, - ip, - port, - fingerprint, - keyType, - userId, - isJumpHost: true, - }); - verify(true); - return; - } - if (!ws) { sshLogger.error( "Host key changed - please connect via Terminal to verify the new key", diff --git a/src/backend/tests/hosts/host-key-verifier.test.ts b/src/backend/tests/hosts/host-key-verifier.test.ts index 1e26cfb23..5e42db8f4 100644 --- a/src/backend/tests/hosts/host-key-verifier.test.ts +++ b/src/backend/tests/hosts/host-key-verifier.test.ts @@ -1,10 +1,16 @@ import { EventEmitter } from "node:events"; import type { WebSocket } from "ws"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { SSHHostKeyVerifier } from "../../hosts/host-key-verifier.js"; +const repo = vi.hoisted(() => ({ + updateHostKey: vi.fn(async () => {}), + storeHostKey: vi.fn(async () => {}), + touchHostKeyLastVerified: vi.fn(async () => {}), +})); + vi.mock("../../database/repositories/factory.js", () => ({ - createCurrentHostResolutionRepository: () => ({}), + createCurrentHostResolutionRepository: () => repo, })); vi.mock("../../utils/logger.js", () => ({ sshLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, @@ -57,3 +63,50 @@ describe("quick connect host keys", () => { expect(await verifyUnsaved(null)).toBe(true); }); }); + +const pinned = { + hostKeyFingerprint: Buffer.from("old-key").toString("hex"), + hostKeyType: "ssh-ed25519", + hostKeyAlgorithm: "sha256", + hostKeyChangedCount: 0, + name: "bastion", +}; + +async function verifyJumpHop(socket: WebSocket | null, key: string) { + const verifier = await SSHHostKeyVerifier.createHostVerifier( + 7, + "10.0.0.1", + 22, + socket, + "user", + true, + pinned, + ); + return new Promise((resolve) => verifier(Buffer.from(key), resolve)); +} + +describe("jump host key changes", () => { + beforeEach(() => { + repo.updateHostKey.mockClear(); + }); + + it("refuses a changed key and keeps the pin when nobody can be asked", async () => { + expect(await verifyJumpHop(null, "new-key")).toBe(false); + expect(repo.updateHostKey).not.toHaveBeenCalled(); + }); + + it("asks the user and only replaces the pin when they accept", async () => { + const no = socketAnswering("reject"); + expect(await verifyJumpHop(no.socket, "new-key")).toBe(false); + expect(no.sent[0]).toContain("host_key_changed"); + expect(repo.updateHostKey).not.toHaveBeenCalled(); + + const yes = socketAnswering("accept"); + expect(await verifyJumpHop(yes.socket, "new-key")).toBe(true); + expect(repo.updateHostKey).toHaveBeenCalledTimes(1); + }); + + it("still accepts the pinned key without asking", async () => { + expect(await verifyJumpHop(null, "old-key")).toBe(true); + }); +});