feat: verify SSH host keys against known_hosts - #334
Open
NatLee wants to merge 4 commits into
Open
Conversation
Connections previously accepted whatever key the server presented: no hostVerifier was passed to ssh2, so a man-in-the-middle was undetectable (jeanp413#266). Every connection — the target and each ProxyJump hop — is now verified against known_hosts. A new knownHosts module replaces the never-wired hostfile.ts. It reads UserKnownHostsFile/GlobalKnownHostsFile (plaintext and |1| hashed entries, [host]:port, wildcards, negations, HostKeyAlias), compares keys per type like OpenSSH, lowercases hostnames the way ssh does, and appends accepted keys to the user file. Behavior is governed by two settings, as discussed in jeanp413#266: - remote.SSH.verifyUnknownHosts (accept|ask|reject, default accept): accept mirrors OpenSSH's accept-new, so existing users see no change while gaining protection from the second connection on. - remote.SSH.verifyKnownHosts (accept|ask|reject, default ask): a changed key shows both fingerprints, with cancel as the default. Safety details: a changed key is only rewritten in place when its line covers just that host — rewriting a shared or wildcard line would re-pin every other host it names — otherwise the accepted key is appended to the user file, which also covers read-only global entries; a known_hosts read failure downgrades auto-accept to an explicit prompt instead of failing open; UserKnownHostsFile 'none' connects without persisting.
A key marked @Revoked is refused before any other consideration and no setting overrides it, matching OpenSSH. A revoked entry for a different key doesn't affect the presented one, and a lone revoked entry doesn't count as a recorded key. @cert-authority remains unsupported since ssh2 has no certificate support.
The changed-key e2e assertion assumed an 11-character key type when slicing the blob; compare against the forged base64 itself instead, and also assert the update happened in place. Comments added where the matching semantics aren't obvious: hashed-entry format, OpenSSH negation rules, the stale-line guard doubling as @Revoked protection, and why an approval isn't persisted while the trust store is unreadable.
Approving a changed key only rewrote the offending line when it named a single host in a user-owned file; otherwise the new key was appended and the old entry left in place. Since any matching same-type entry verifies, both keys stayed trusted — an attacker holding the old key could connect silently, which is exactly what the warning is about. Now every conflicting entry is cleared: a single-host line is rewritten, a line naming several hosts loses just this host (what ssh-keygen -R does). If anything survives — a wildcard line, or a file the user doesn't own — the connection is refused and the surviving file:line is reported, following ssh, which refuses on a changed key rather than half-trusting. Also from review: - lowercase HostKeyAlias, so a mixed-case alias matches hashed entries - one port per ProxyJump hop, shared by the tunnel and the host key lookup; a jump host no longer inherits the target's Port - report the 'none' UserKnownHostsFile case instead of logging a write "to undefined", and don't promise to remember a key that can't be Tests: the reject case now asserts no prompt was shown (it passed with the policy removed before), the wildcard case asserts the old key is refused rather than that the new one works, and readFailures, removeHostFromEntry and findConflictingEntries are covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #266. Implements the plan discussed there, with the settings shape proposed in the threads.
What this does
Every connection — the target and each
ProxyJumphop — now verifies the server's host key againstknown_hoststhrough ssh2'shostVerifier(raw key blob, so nohostHash). Previously no verifier was passed at all, so any key was accepted silently.The never-wired
hostfile.tsis replaced by aknownHostsmodule that follows OpenSSH behavior:|1|hashed entries,[host]:port, comma lists,*/?wildcards,!negationsUserKnownHostsFile/GlobalKnownHostsFilerespected (including thenonekeyword → connect without persisting),HostKeyAliasused for lookup and recording when setSettings
remote.SSH.verifyUnknownHostsaccept/ask/rejectacceptremote.SSH.verifyKnownHostsaccept/ask/rejectaskaccepton unknown hosts mirrors OpenSSH'saccept-new: existing users see no change on their first connect, and get mismatch protection from the second on. A changed key shows both fingerprints with cancel as the default action.Safety details worth reviewing
ssh-keygen -Rdoes). Shared lines are never re-keyed wholesale — that would re-pin every other host they name. If anything survives (a wildcard line, or a file the user doesn't own) the connection is refused and the survivingfile:lineis reported, following ssh, which refuses on a changed key rather than half-trusting it. Appending alongside a stale entry would leave both keys valid, which is a fail-open.unknowncan't be trusted, so auto-accept is downgraded to an explicit prompt that names the unreadable file.Tests
[host]:port, CRLF, negations, aliases, quoted paths with spaces,none, unreadable files, shared-line protection, comment preservation,@revokedprecedence, conflict detectionrejectrefuses without prompting → cancelled prompt refuses → accepted prompt updates in place and connects → a wildcard line that can't be cleared refuses and names the linessh-keygen -F: 100 lookups over a real 86-entry known_hosts in both plaintext andssh-keygen -Hhashed form, plus synthetic multi-name/wildcard/negation/port/case/comment lines — no divergence from OpenSSH's own matcher. Also exercised end to end against a real host: silent match, tampered key rejected,@revokedrefused even withaccept, and an auto-recorded entry that is byte-identical to the one ssh writes and thatssh-keygen -Ffinds.Known limitation
The
askmodals run inside ssh2's handshake, which is bounded byreadyTimeout(default 60s). Answering after the timeout still records the decision, and the retry then connects against the recorded key — the approval isn't lost, but that first attempt reports a failed connection. Noted in a comment; solving it properly means pausing/retrying around the handshake and felt like its own PR.@revokedentries are honored: a revoked key is refused unconditionally, no setting overrides it.@cert-authorityis skipped for now — verifying CA-signed host certificates needs certificate support in ssh2, which doesn't exist yet (#261, #95).