diff --git a/AUDIT_OPEN.md b/AUDIT_OPEN.md index 689e985..aecfde6 100644 --- a/AUDIT_OPEN.md +++ b/AUDIT_OPEN.md @@ -1191,11 +1191,15 @@ alarm rather than the coverage gap it is — the natural first reaction (re-scan with `-t ed25519`, per most docs) is exactly what produces the state. -Status: OPEN (ergonomics/diagnostics, not correctness — failing closed is -right). Candidate fix: include the presented key type and the on-file types -in the error, or document "scan without -t" in the error string. Found -while provisioning the ship delivery worker; worked around by scanning all -algorithms. +Status: CLOSED 2026-09-22, fixed in `078f610`: both host-key callback +paths (default and accept-new) wrap a knownhosts mismatch with the host, +the presented key type, the on-file key types and the remediation — +`host key mismatch for : server presented ssh-rsa, known_hosts has +no matching entry (has ssh-ed25519) — scan all algorithms (ssh-keyscan +without -t), not just one`. Failing closed is unchanged; the enriched +error still unwraps to `*knownhosts.KeyError` +(TestHostKeyMismatchNamesAlgorithms). Found while provisioning the ship +delivery worker; worked around by scanning all algorithms. ## Programme slice (2026-09-22, later still) — C02: commit-pinned builds diff --git a/internal/ssh/hostkey_test.go b/internal/ssh/hostkey_test.go index c68419e..6c0c673 100644 --- a/internal/ssh/hostkey_test.go +++ b/internal/ssh/hostkey_test.go @@ -1,8 +1,11 @@ package ssh import ( + "bytes" "crypto/ed25519" "crypto/rand" + "crypto/rsa" + "errors" "net" "os" "os/exec" @@ -11,6 +14,7 @@ import ( "testing" "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/knownhosts" ) func testPublicKey(t *testing.T) ssh.PublicKey { @@ -84,6 +88,83 @@ func TestAcceptNewHostKeyCallback_WriteSuccessRecordsKey(t *testing.T) { var _ = net.Addr(fakeAddr{}) // compile-time interface check +// 2026-09-22 live finding (ship S14 trusted-copy provisioning): a +// known_hosts carrying only the host's ed25519 line made every connection +// presenting a different algorithm fail with a bare "knownhosts: key +// mismatch" — the error named neither the algorithm presented nor the ones +// on file, so an algorithm-coverage gap read as a MITM alarm. Both callback +// paths must name the host, the presented algorithm, the on-file algorithms +// and the scan-without--t remediation, while still failing closed (and +// remaining classifiable as *knownhosts.KeyError by callers). +func TestHostKeyMismatchNamesAlgorithms(t *testing.T) { + dir := t.TempDir() + sshDir := filepath.Join(dir, ".ssh") + if err := os.MkdirAll(sshDir, 0700); err != nil { + t.Fatal(err) + } + knownHostsPath := filepath.Join(sshDir, "known_hosts") + + enrolled := testPublicKey(t) + line := knownhosts.Line([]string{knownhosts.Normalize("203.0.113.7:22")}, enrolled) + if err := os.WriteFile(knownHostsPath, []byte(line+"\n"), 0644); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(knownHostsPath) + if err != nil { + t.Fatal(err) + } + + rsaPriv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating rsa test key: %v", err) + } + presented, err := ssh.NewPublicKey(&rsaPriv.PublicKey) + if err != nil { + t.Fatalf("wrapping rsa test key: %v", err) + } + + assertEnriched := func(name string, err error) { + t.Helper() + if err == nil { + t.Fatalf("%s: expected a mismatch error, got nil", name) + } + for _, want := range []string{ + "203.0.113.7:22", + "ssh-rsa", + "ssh-ed25519", + "ssh-keyscan without -t", + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("%s: error does not name %q:\n%s", name, want, err) + } + } + var keyErr *knownhosts.KeyError + if !errors.As(err, &keyErr) { + t.Errorf("%s: wrapped error no longer unwraps to *knownhosts.KeyError: %v", name, err) + } + } + + t.Setenv("HOME", dir) + strict, err := defaultHostKeyCallback() + if err != nil { + t.Fatalf("defaultHostKeyCallback: %v", err) + } + assertEnriched("defaultHostKeyCallback", strict("203.0.113.7:22", fakeAddr{}, presented)) + + acceptNew := acceptNewHostKeyCallback(knownHostsPath) + assertEnriched("acceptNewHostKeyCallback", acceptNew("203.0.113.7:22", fakeAddr{}, presented)) + + // Failing closed is unchanged: the mismatch must not enroll the presented + // key (accept-new) or alter known_hosts in any way. + after, err := os.ReadFile(knownHostsPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(before, after) { + t.Error("known_hosts was modified by a rejected host key — a mismatch must not enroll anything") + } +} + // TestPublicKeyBytes_DerivesFromPrivateKey is the A32 regression: with an // explicit identity and NO .pub file, the public key is DERIVED from the // private key instead of falling through to an unrelated default; a diff --git a/internal/ssh/remote.go b/internal/ssh/remote.go index f68ffe1..bfb6c88 100644 --- a/internal/ssh/remote.go +++ b/internal/ssh/remote.go @@ -19,7 +19,6 @@ import ( "golang.org/x/term" ) - // Compile-time check: RemoteExecutor implements Executor. var _ Executor = (*RemoteExecutor)(nil) @@ -255,7 +254,39 @@ func defaultHostKeyCallback() (gossh.HostKeyCallback, error) { if err != nil { return nil, fmt.Errorf("parsing known_hosts: %w", err) } - return callback, nil + return func(hostname string, remote net.Addr, key gossh.PublicKey) error { + if err := callback(hostname, remote, key); err != nil { + return mismatchHint(hostname, key, err) + } + return nil + }, nil +} + +// mismatchHint enriches a knownhosts mismatch error with what the raw +// "knownhosts: key mismatch" omits: the host, the algorithm the server +// presented, and the algorithms known_hosts holds for that host. A +// known_hosts scanned for a single algorithm (ssh-keyscan -t , the +// shape most guides produce) makes every connection that negotiates a +// different algorithm fail without naming either side — which reads as a +// MITM alarm rather than the algorithm-coverage gap it is (2026-09-22 live +// finding from ship's delivery provisioning). Verification is unchanged — +// every mismatch still fails closed; only the message gains context. +// Non-mismatch failures (revoked keys, database problems) pass through. +func mismatchHint(hostname string, key gossh.PublicKey, err error) error { + var keyErr *knownhosts.KeyError + if !errors.As(err, &keyErr) || len(keyErr.Want) == 0 { + return err + } + onFile := make([]string, 0, len(keyErr.Want)) + seen := make(map[string]bool, len(keyErr.Want)) + for _, known := range keyErr.Want { + if t := known.Key.Type(); !seen[t] { + seen[t] = true + onFile = append(onFile, t) + } + } + return fmt.Errorf("host key mismatch for %s: server presented %s, known_hosts has no matching entry (has %s) — scan all algorithms (ssh-keyscan without -t), not just one: %w", + hostname, key.Type(), strings.Join(onFile, ", "), err) } // resolveSigners finds and loads SSH private keys. For an EXPLICIT key path, @@ -396,7 +427,7 @@ func acceptNewHostKeyCallback(knownHostsPath string) gossh.HostKeyCallback { // is rejected. var keyErr *knownhosts.KeyError if !errors.As(err, &keyErr) || len(keyErr.Want) != 0 { - return err + return mismatchHint(hostname, key, err) } } // Append to known_hosts. Ensure the parent directory exists first (a