feat(ssh): known_hosts verification + TOFU + --insecure opt-out (closes #101)#135
Merged
Conversation
) Every SSH connection the CLI makes used ssh.InsecureIgnoreHostKey(), which was justified in v0.1.x as "personal VPS use" but becomes unsafe in the post-#98/#103 world: the same SSH channel now carries state-mutating Admin API calls (upsert, deploy, delete), compose archive uploads, and a growing number of ops commands. A MITM on the path can silently reroute those to an attacker-controlled responder and misreport phase/active_target back to the operator. New default: - Resolve ~/.ssh/known_hosts (override: SSH_KNOWN_HOSTS env var). - On first connect to an unknown host, prompt operator to accept and pin the key (TOFU). - When CONOHA_NO_INPUT is set or stdin isn't a TTY, refuse the connection with a message pointing at --insecure and ssh-keyscan. - On host-key mismatch (pinned key differs from what the server presented), return HostKeyMismatchError with a recovery hint ('ssh-keygen -R <host>' to remove the stale pin after a legit rebuild). Opt-out: - --insecure global flag (persistent, applies to every subcommand). - CONOHA_SSH_INSECURE env var for CI / wrappers. - Preserves the old behavior bit-for-bit when set; documented as the explicit lab / throwaway-VPS knob. Connect() reads insecure state from either ConnectConfig.Insecure or the env, so no caller changes were needed across the 7 existing Connect sites. Tests: - Insecure callback accepts any key. - Pre-seeded known_hosts + mismatched server key → HostKeyMismatchError with the 'ssh-keygen -R' hint. - Unknown host + noInput → helpful refusal message. - Missing known_hosts is auto-created. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…own_hosts - cmd/gpu/setup.go: explicit `_ =` discard on Close() (errcheck does not exclude *ssh.Client; the io.Closer exclusion only matches the interface type itself). - internal/ssh/knownhosts.go: drop redundant `remote != nil` outer guard; type assertion on a nil interface returns ok=false. (staticcheck S1020.)
…osed on non-TTY TOFU Three follow-ups from PR #135 review: 1. Remove ConnectConfig.Insecure — no caller sets it. The --insecure flag / CONOHA_SSH_INSECURE env var routes through configpkg.IsSSHInsecure inside Connect, which is the single source of truth. The unused field was a misleading second knob. 2. Alias the config import as `configpkg` to remove the package-vs-local shadowing trap in Connect (the previous `config -> clientCfg` rename only fixed the immediate collision; an import alias prevents recurrence). 3. Guard TOFU prompt with term.IsTerminal so a non-TTY stdin (CI, build script piping a heredoc) fails closed instead of letting an attacker- controlled `yes\n` silently trust an unknown host. The function-level doc already promised this behavior. Adds TestHostKeyCallback_UnknownHost_NonTTYFailsClosed; skips when stdin happens to be a real TTY (interactive `go test` runs).
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
2 participants
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.
Summary
Switch from `ssh.InsecureIgnoreHostKey()` to real host-key verification via `~/.ssh/known_hosts`, with TOFU on first connect and a distinct mismatch error path.
Why
Every SSH session this CLI opens now carries state-mutating proxy Admin API calls, compose archive uploads, and accessory secrets. A MITM can reroute `curl --unix-socket` to an attacker socket and silently misreport `active_target` / `phase` back. The `InsecureIgnoreHostKey()` shortcut was tolerable for git-push-only v0.1.x; not for v0.2+.
Behavior
Opt-out
Breaking
Users upgrading with existing servers will see a TOFU prompt on first SSH-touching command. Script users need either `--insecure`, `CONOHA_SSH_INSECURE=1`, or a pre-seeded `known_hosts` (via `ssh-keyscan`).
Test plan
Follow-ups (not in this PR)