fix(ssh): refuse the known_hosts lines ssh itself refuses to parse

Three ways a line could be trusted by us and invisible to the user's own ssh —
or, worse, raise a CHANGED alarm from an entry ssh drops.

Buffer.from does not fail on bad base64, it SKIPS invalid characters, so
`<key>!!!` and a blob with `@@` spliced into it both decoded to the correct key
and matched. Verified live against OpenSSH 10.2p1 on 127.0.0.1:2224: the
unmodified control reached authentication and all three malformed variants
produced "No ED25519 host key is known". A re-encode-and-compare makes us agree.

`<key>AAAA` is the interesting one, and the reason the first fix was not enough:
68 characters plus 4 is still legal base64, and the algorithm header still reads
ssh-ed25519, so neither the base64 check nor the existing header check sees
anything wrong. ssh parses the whole key structure. We decoded 54 bytes where an
ed25519 key is 51 and reported `mismatch` — a man-in-the-middle warning caused by
a typo in a file ssh silently ignores.

So the blob is now walked as what it is: a run of length-prefixed fields that
must consume it exactly. Algorithm-agnostic on purpose, so a key type we do not
model is checked as well as one we do. It also rejects a length prefix that
overruns the buffer, which readHostKeyType only checked for the first field.

And ssh's extract_salt demands exactly one SHA1 digest — "expected salt len 20,
got 16" — where we accepted any non-empty salt. A short salt is still a usable
HMAC key for us, so a hand-crafted line could match for us and be a parse error
for ssh. ssh-keygen -H always writes 20 bytes, so refusing loses no real entry.

Worth recording that ssh-keygen -F cannot answer any of this: it matches host
names and prints lines without ever decoding the key, so it reports "found" for
all four blobs. The real client was the only instrument that worked.

Found by a parity review; the base64 finding as reported was right about the
behaviour and wrong about the mechanism for the padded case, which is what led to
the structural check.

1,533 SSH tests pass.

src/main/ssh/ssh-known-hosts.test.ts

@@ -253,6 +253,60 @@ describe('matching a presented key', () => {
  * client's verdict recorded from its own output. Everything else in this file states what we believe
  * ssh does; these state what it did.
  */
+describe('lines ssh itself refuses to parse', () => {
+  const ED = 'AAAAC3NzaC1lZDI1NTE5AAAAIKqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq'
+
+  // Buffer.from SKIPS invalid base64 characters instead of failing, so each of these decodes to
+  // something. Verified live against OpenSSH 10.2p1 on 127.0.0.1:2224: the valid control reached
+  // authentication and ALL THREE of these produced "No ED25519 host key is known" — ssh drops the
+  // line. Honouring them means granting trust from a line the user's own ssh ignores, and for the
+  // padded variant it means raising a CHANGED alarm from one.
+  it.each([
+    ['trailing junk', `${ED}!!!`],
+    ['invalid characters spliced into the middle', ED.replace(/^(.{20})/, '$1@@')]
+  ])('drops a key field with %s, as ssh does', (_label, keyField) => {
+    expect(parseKnownHostsLine(`example.com ssh-ed25519 ${keyField}`)).toBeUndefined()
+  })
+
+  // Still VALID base64 — 68 characters plus 4 is a legal length — and the algorithm header still
+  // reads ssh-ed25519, so neither the base64 check nor the header check catches it. ssh parses the
+  // whole key structure and drops the line; we decoded 54 bytes where the key is 51 and reported a
+  // CHANGED alarm from an entry ssh ignores.
+  it('drops a key blob with trailing bytes its own structure does not account for', () => {
+    expect(parseKnownHostsLine(`example.com ssh-ed25519 ${ED}AAAA`)).toBeUndefined()
+  })
+
+  it('drops a key blob whose own length prefix overruns the buffer', () => {
+    const overrun = Buffer.concat([
+      Buffer.from([0, 0, 0, 11]),
+      Buffer.from('ssh-ed25519'),
+      Buffer.from([0, 0, 0, 99])
+    ])
+    expect(
+      parseKnownHostsLine(`example.com ssh-ed25519 ${overrun.toString('base64')}`)
+    ).toBeUndefined()
+  })
+
+  it('still accepts the unmodified key field', () => {
+    expect(parseKnownHostsLine(`example.com ssh-ed25519 ${ED}`)).toBeDefined()
+  })
+
+  // ssh's extract_salt demands exactly one SHA1 digest: "expected salt len 20, got 16". A shorter
+  // salt is still a usable HMAC key for us, so we could match a hand-crafted line that is invisible
+  // to the user's ssh. ssh-keygen -H always writes 20 bytes, so refusing loses no real entry.
+  it('drops a hashed entry whose salt is not a full SHA1 digest', () => {
+    const shortSalt = Buffer.alloc(16, 1).toString('base64')
+    const hash = Buffer.alloc(20, 2).toString('base64')
+    expect(parseKnownHostsLine(`|1|${shortSalt}|${hash} ssh-ed25519 ${ED}`)).toBeUndefined()
+  })
+
+  it('accepts a hashed entry whose salt is a full SHA1 digest', () => {
+    const salt = Buffer.alloc(20, 1).toString('base64')
+    const hash = Buffer.alloc(20, 2).toString('base64')
+    expect(parseKnownHostsLine(`|1|${salt}|${hash} ssh-ed25519 ${ED}`)).toBeDefined()
+  })
+})
+
 describe('agreement with a live OpenSSH client', () => {
   const SERVER_KEY = 'AAAAC3NzaC1lZDI1NTE5AAAAIM2eSSqUqU9LERdg8qNjFiU59unM+JyfwFHLkMxR13oq'
   const OTHER_KEY = 'AAAAC3NzaC1lZDI1NTE5AAAAIKmVZ4Z+MJ3VYnZmZmZmZmZmZmZmZmZmZmZmZmZmZmZm'

src/main/ssh/ssh-known-hosts.ts

@@ -52,15 +52,52 @@ export function readHostKeyType(key: Buffer): string | undefined {
   return key.subarray(4, 4 + length).toString('utf8')
 }
 
+/**
+ * Whether the blob is exactly consumed by its own SSH-wire structure.
+ *
+ * A public key blob is a run of length-prefixed fields — name, then the algorithm's parameters — and
+ * nothing else. Checking only the algorithm header leaves trailing bytes undetected, and ssh parses
+ * the whole structure: verified live against OpenSSH 10.2p1, a valid ed25519 key with four extra
+ * base64 characters appended is still valid base64 and still reports `ssh-ed25519`, but ssh reports
+ * "No ED25519 host key is known" and drops the line, where we decoded 54 bytes instead of 51 and
+ * raised a CHANGED alarm from an entry the user's own ssh ignores.
+ *
+ * Deliberately algorithm-agnostic: the field walk is the same for every key type, so a type we do
+ * not model is checked as well as one we do.
+ */
+function isWellFormedHostKeyBlob(key: Buffer): boolean {
+  let offset = 0
+  while (offset < key.length) {
+    if (offset + 4 > key.length) {
+      return false
+    }
+    const fieldLength = key.readUInt32BE(offset)
+    offset += 4
+    if (fieldLength > key.length - offset) {
+      return false
+    }
+    offset += fieldLength
+  }
+  return offset === key.length
+}
+
 /** `SHA256:...` exactly as `ssh-keygen -lf` prints it, base64 with padding stripped. */
 export function formatHostKeyFingerprint(sha256Base64: string): string {
   return `SHA256:${sha256Base64.replace(/=+$/, '')}`
 }
 
 function decodeKey(raw: string): Buffer | undefined {
-  // Why the round-trip check: Buffer.from never throws on bad base64, it silently truncates.
+  // Buffer.from never throws on bad base64 — it silently SKIPS invalid characters, so `<valid>!!!`
+  // and a blob with `@@` spliced into it both decode to the same correct key. ssh rejects those
+  // lines outright ("parse error in hostkeys file"), so accepting them grants trust from a line the
+  // user's own ssh ignores; `<valid>AAAA` is worse still, decoding to different bytes that still
+  // parse, which reads as a CHANGED key. Re-encoding and comparing is what makes us as strict.
   const decoded = Buffer.from(raw, 'base64')
-  return decoded.length > 0 ? decoded : undefined
+  if (decoded.length === 0) {
+    return undefined
+  }
+  const canonical = decoded.toString('base64').replace(/=+$/, '')
+  return canonical === raw.replace(/=+$/, '') ? decoded : undefined
 }
 
 function parseHashedPatterns(field: string): KnownHostsEntry['hashed'] | undefined {
@@ -71,7 +108,10 @@ function parseHashedPatterns(field: string): KnownHostsEntry['hashed'] | undefin
   }
   const salt = Buffer.from(parts[2] ?? '', 'base64')
   const hash = Buffer.from(parts[3] ?? '', 'base64')
-  if (salt.length === 0 || hash.length !== SHA1_DIGEST_BYTES) {
+  // ssh requires BOTH to be exactly one SHA1 digest — extract_salt rejects anything else with
+  // "expected salt len 20, got N". Accepting a shorter salt would let us match a line ssh treats as
+  // a parse error, so the entry would be invisible to the user's own ssh but trusted by us.
+  if (salt.length !== SHA1_DIGEST_BYTES || hash.length !== SHA1_DIGEST_BYTES) {
     return undefined
   }
   return { salt, hash }
@@ -109,7 +149,7 @@ export function parseKnownHostsLine(line: string): KnownHostsEntry | undefined {
   }
 
   const key = decodeKey(keyBase64)
-  if (!key || readHostKeyType(key) !== keyType) {
+  if (!key || readHostKeyType(key) !== keyType || !isWellFormedHostKeyBlob(key)) {
     return undefined
   }