Repository navigation
Conversation
The browser client accepted whatever host key was presented:
// Handle server authentication (accept any server for now)
// For server authentication, just accept
// In production, you'd verify the host key
e.authenticationPromise = Promise.resolve({});
So anything in the relay path — a compromised gateway or signal server, or
anyone able to influence which endpoint the gateway dials — could impersonate
a target host. Publickey auth signs over the session id, so a man-in-the-middle
cannot replay the user's credentials to the real host, but it can present a
convincing shell and read everything typed into it.
Trust on first use, as OpenSSH does it. The first connection shows the
fingerprint in OpenSSH's own format so it can be checked against ssh-keygen on
the host, and pins it. Later connections must present the same key; a changed
one aborts rather than prompting, since re-approving is how people are trained
to click through the prompt that matters.
Pins are deployment-wide, not per-user: one operator accepting a key protects
everyone, and a man-in-the-middle cannot work through users one at a time. A
pin is never silently replaced — clearing one is a deliberate admin act.
Verification failures fail closed. A missing key, an unreadable key, an
unreachable pin store or an unexpected error all refuse the connection, since
falling through to "accept" is how verification quietly stops happening.
This is the stopgap ahead of VVK-signed host certificates, which remove the
prompt entirely because the client already trusts the quorum.
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.
UV is right: we never validated the SSH host key. This closes it, as the stopgap ahead of VVK-signed host certificates.
The gap
sshClient.ts accepted every server, and said so:
nullmeans reject in this library;{}accepts. Every SSH path went through it — SFTP and SCP take an existing session rather than opening their own, so this was the single choke point.What an attacker gets. Publickey auth signs over the session identifier, so a man-in-the-middle cannot replay the user's credentials to the real host — no credential theft, no lateral access as that user. What it can do is impersonate the host completely: the operator gets a shell that looks right, types real commands into it, pastes secrets into it, and reads fabricated output. The relevant adversary is anything in the relay path — a compromised gateway or signal server, or anyone able to influence which endpoint the gateway dials.
What this does
Trust on first use, matching OpenSSH's model:
SHA256:…, formatted exactly asssh-keygen -lfprints it so the two can be compared by eye, and pins it on confirmation.Three decisions worth calling out:
Pins are deployment-wide, not per-user. One operator accepting a key protects everyone, and a man-in-the-middle can't work through users one at a time until someone clicks accept.
Verification fails closed. A missing key, an unreadable key, an unreachable pin store, or any unexpected error refuses the connection. Falling through to "accept" on error is how verification quietly stops happening.
Pins are never silently replaced. The pin endpoint returns 409 on a different key rather than overwriting, and after a first-use pin the client re-checks what actually got stored — so two simultaneous first connections can't let the second one's key win.
Targets prefer the server record (
server:<id>) so a pin survives the host changing address, falling back toaddr:host:portfor ad-hoc connections and gateway backends with no server record.Testing
344 tests pass, 14 new on the verification logic — including the fingerprint format checked against a known SHA-256 value, mismatch on a changed algorithm with identical bytes, rejection of a truncated key (guarding against a prefix comparison creeping in), and that a pin survives an address change.
Not covered by tests: the
onAuthenticatingwiring itself and the dialog. Those need a live SSH session, so the end-to-end path is unverified — worth connecting to a real host, confirming the prompt appears with a fingerprint matchingssh-keygen -lfon that host, then changing the host key and confirming the connection is refused.Note on scope
Gateway-mode connections pass
serverIdas the host, so their target key isserver:<backend>rather than an address. That is stable and pins correctly, but the fingerprint dialog shows the backend name rather than an IP. Fine for now; worth revisiting when certificates land.