Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions AUDIT_OPEN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <host>: 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

Expand Down
81 changes: 81 additions & 0 deletions internal/ssh/hostkey_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package ssh

import (
"bytes"
"crypto/ed25519"
"crypto/rand"
"crypto/rsa"
"errors"
"net"
"os"
"os/exec"
Expand All @@ -11,6 +14,7 @@ import (
"testing"

"golang.org/x/crypto/ssh"
"golang.org/x/crypto/ssh/knownhosts"
)

func testPublicKey(t *testing.T) ssh.PublicKey {
Expand Down Expand Up @@ -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
Expand Down
37 changes: 34 additions & 3 deletions internal/ssh/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"golang.org/x/term"
)


// Compile-time check: RemoteExecutor implements Executor.
var _ Executor = (*RemoteExecutor)(nil)

Expand Down Expand Up @@ -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 <one>, 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,
Expand Down Expand Up @@ -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
Expand Down
Loading