From ef08403f809d2da792a9ed86b9977764f0e22eac Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 14 Aug 2026 04:20:45 -0700 Subject: [PATCH] fix(ssh): stop the fallback pass accepting a changed key, and refusing a new one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One wrong loop scope, two opposite errors, both reproduced against a live OpenSSH 10.2p1 client and an ed25519-only sshd on 127.0.0.1:2223. ACCEPTING A CHANGED KEY. ssh runs the bare-host fallback only when the port-qualified lookup matched no plain entry of ANY key type. We ran it unless pass 0 produced a match or a SAME-TYPE mismatch. So with an off-port RSA entry plus a bare, correct ed25519 line — an ordinary shape, an old off-port entry beside one written by a port-22 connect — ssh printed IDENTIFICATION HAS CHANGED and refused, with no "checking without port identifier" in -v because the fallback never ran, while we reached the bare line and returned `match`. REFUSING A NEW ONE. sawKnownHostOtherType and sawCertAuthority were declared outside the pass loop, so an entry found only on the fallback pass could set them. A bare ssh-rsa entry, dialed on a non-default port against an ed25519-only server, made ssh add the host and connect — plain first contact — where we returned unknown-type-known-host and hard-failed. That is Gitea/Forgejo, dev containers, Gerrit, Vagrant: an off-port service on a host already in known_hosts. So the flags are per-pass now, and pass 0 decides as soon as it finds any plain entry for the host. Which of the two rejections it reports only picks the message; ssh calls both HOST_CHANGED. Also drops the type check from the match test: byte equality already implies the types agree, because the blob carries its own algorithm name and parsing rejects any line whose declared type disagrees with it. Reverting either half of the scope fix fails exactly the two new tests. 1,526 SSH tests pass. --- src/main/ssh/ssh-known-hosts.test.ts | 25 +++++++++++++++ src/main/ssh/ssh-known-hosts.ts | 46 ++++++++++++++++------------ 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/main/ssh/ssh-known-hosts.test.ts b/src/main/ssh/ssh-known-hosts.test.ts index 856f7290269..4db946dd486 100644 --- a/src/main/ssh/ssh-known-hosts.test.ts +++ b/src/main/ssh/ssh-known-hosts.test.ts @@ -267,6 +267,31 @@ describe('agreement with a live OpenSSH client', () => { ).toBe('match') }) + // THE accept-a-changed-key case. Live on 127.0.0.1:2223 against an ed25519-only server, with an + // off-port RSA entry AND a bare correct ed25519 line, ssh printed IDENTIFICATION HAS CHANGED and + // refused — and printed NO "checking without port identifier", so the fallback never ran. ssh + // gates that pass on the port-qualified lookup matching no plain entry of ANY type; gating it on + // "no match and no same-type mismatch" instead reaches the bare line and returns `match`. + it('does not fall back past an off-port entry of another type, as ssh did not', () => { + const contents = `${line('[127.0.0.1]:2223', RSA_A, 'ssh-rsa')}\n${line('127.0.0.1', SERVER_KEY)}` + expect(verdict(contents, { host: '127.0.0.1', port: 2223, key: SERVER_KEY })).toBe( + 'unknown-type-known-host' + ) + }) + + // The same flags, the opposite error. Live, a BARE ssh-rsa entry dialed on 2223 against the same + // ed25519-only server made ssh add the host and connect — plain first contact. Letting the + // fallback pass set unknown-type-known-host refuses a host ssh accepts. + it('treats an other-type entry found only on the fallback pass as unknown, as ssh did', () => { + expect( + verdict(line('127.0.0.1', RSA_A, 'ssh-rsa'), { + host: '127.0.0.1', + port: 2223, + key: SERVER_KEY + }) + ).toBe('unknown') + }) + // Live: ssh reached authentication, so it accepted the bare line for a non-default port. it('falls back to the bare host line on a non-default port, as ssh did', () => { expect( diff --git a/src/main/ssh/ssh-known-hosts.ts b/src/main/ssh/ssh-known-hosts.ts index af42e2815b4..c912b179528 100644 --- a/src/main/ssh/ssh-known-hosts.ts +++ b/src/main/ssh/ssh-known-hosts.ts @@ -221,43 +221,51 @@ export function matchKnownHosts( } } - let sawKnownHostOtherType = false + // A CA line is a MARKER, so it never satisfies a plain host key and never stops the fallback pass. + // Tracked across passes because it describes the host either way. let sawCertAuthority = false for (let passIndex = 0; passIndex < passes.length; passIndex += 1) { const candidates = passes[passIndex]! + // Per pass, never carried forward. These describe what THIS candidate form knows, and the + // fallback form's knowledge is not admissible as evidence of a change — see below. let sawSameTypeForHost = false + let sawPlainEntryForHost = false for (const entry of entries) { if (entry.marker === 'revoked' || !matchesHost(entry, candidates)) { continue } if (entry.marker === 'cert-authority') { - // A CA line only validates certificates; it never matches a plain host key. sawCertAuthority = true continue } - if (entry.keyType === query.keyType) { - if (entry.key.equals(query.key)) { - return 'match' - } - sawSameTypeForHost = true - } else { - sawKnownHostOtherType = true + // Byte equality implies the types agree: the blob carries its own algorithm name, and parsing + // already rejected any line whose declared type disagreed with it. + if (entry.key.equals(query.key)) { + return 'match' } + sawPlainEntryForHost = true + sawSameTypeForHost ||= entry.keyType === query.keyType } - // Only the first (most specific) pass may report a change. On the fallback pass OpenSSH - // downgrades a wrong key to "not known", so reporting mismatch there would be a false alarm. - if (sawSameTypeForHost && passIndex === 0) { - return 'mismatch' + // The first pass decides if it knows this host AT ALL. OpenSSH runs the bare-host fallback only + // when the port-qualified lookup matched no plain entry of ANY type; when one was there, its + // verdict is final. Verified live against OpenSSH 10.2p1 on 127.0.0.1:2223 — an off-port RSA + // entry plus a bare, correct ed25519 line makes ssh print IDENTIFICATION HAS CHANGED with no + // "checking without port identifier", where continuing to the fallback returns `match` and + // ACCEPTS A CHANGED KEY. + if (passIndex === 0 && sawPlainEntryForHost) { + // Both refuse. The distinction only picks the message: a changed key of the type we hold, or + // a type we have never seen for a host we do know. ssh calls both HOST_CHANGED. + return sawSameTypeForHost ? 'mismatch' : 'unknown-type-known-host' } } - if (sawCertAuthority) { - return 'ca-only' - } - // We hold a key for this host, just not of the presented type. Not first contact — an attacker - // who cannot forge the known type could otherwise present a different one to get a soft outcome. - return sawKnownHostOtherType ? 'unknown-type-known-host' : 'unknown' + // Nothing after the first pass may report a change. On the fallback pass OpenSSH downgrades any + // non-match to "not known" — including an entry of another type, which it treats as plain first + // contact. Verified live: a bare ssh-rsa entry, dialed on 2223 against an ed25519-only server, + // makes ssh add the host and connect. Reporting unknown-type-known-host there refuses a host ssh + // accepts, which is why these flags are scoped to their pass. + return sawCertAuthority ? 'ca-only' : 'unknown' }