diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 5bc17c3..61ef7cd 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -26,11 +26,11 @@ import ( "time" "golang.org/x/crypto/ssh" - "golang.org/x/crypto/ssh/knownhosts" "git.cloudinit.dev/coreci/orca/internal/certpaths" "git.cloudinit.dev/coreci/orca/internal/model" "git.cloudinit.dev/coreci/orca/internal/osdetect" + "git.cloudinit.dev/coreci/orca/internal/proxmox" "git.cloudinit.dev/coreci/orca/internal/security" "git.cloudinit.dev/coreci/orca/internal/store" "git.cloudinit.dev/coreci/orca/internal/transport" @@ -409,7 +409,19 @@ func probeProxmoxPVEVersion(ctx context.Context, host string) error { return fmt.Errorf("parse SSH key: %w", err) } - hostKeyCallback, err := knownhosts.New(certpaths.KnownHostsPath()) + // Extract host from the node address (orca stores host:8443; + // SSH needs host:22). We dial the SSH port, not the orca daemon port. + sshHost := host + if strings.Contains(host, ":") { + sshHost = strings.SplitN(host, ":", 2)[0] + } + sshAddr := sshHost + ":22" + + // Use the shared TOFU capture-fix wrapper (T02.9 — GRILL condition + // #2: doctor parity with bootstrap). Without this, a first-connect + // proxmox node (entry missing from known_hosts) fails the doctor + // probe even though it joined fine — the v0.6 ship-defect. + hostKeyCallback, err := proxmox.TOFUHostKeyCallback(sshAddr, nil) if err != nil { return fmt.Errorf("known_hosts: %w", err) } @@ -421,14 +433,6 @@ func probeProxmoxPVEVersion(ctx context.Context, host string) error { Timeout: 3 * time.Second, } - // Extract host from the node address (orca stores host:8443; - // SSH needs host:22). We dial the SSH port, not the orca daemon port. - sshHost := host - if strings.Contains(host, ":") { - sshHost = strings.SplitN(host, ":", 2)[0] - } - sshAddr := sshHost + ":22" - dialer := &netDialer{} conn, err := dialer.DialContext(ctx, "tcp", sshAddr, config) if err != nil { diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index a18f572..6324ee0 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -2,14 +2,21 @@ package doctor import ( "context" + "crypto/ed25519" + "crypto/rand" + "net" "os" "path/filepath" "strings" "testing" "time" + "golang.org/x/crypto/ssh" + + "git.cloudinit.dev/coreci/orca/internal/certpaths" "git.cloudinit.dev/coreci/orca/internal/model" "git.cloudinit.dev/coreci/orca/internal/osdetect" + "git.cloudinit.dev/coreci/orca/internal/proxmox" "git.cloudinit.dev/coreci/orca/internal/security" "git.cloudinit.dev/coreci/orca/internal/store" ) @@ -396,3 +403,90 @@ func init() { // Suppress slog noise during tests. _ = os.Setenv("ORCA_LOG_LEVEL", "error") } + +// TestProxmoxCheck_FirstConnectCapturesKey verifies that the doctor +// proxmox probe uses the shared TOFU capture-fix wrapper +// (proxmox.TOFUHostKeyCallback), which captures the host key on first +// connect instead of failing with KeyError{Want:[]} (T02.9 — GRILL +// condition #2: doctor parity with bootstrap). Before T02.9, the bare +// knownhosts.New callback returned KeyError{Want:[]} on a missing +// entry and the doctor probe reported FAIL even though the node had +// joined successfully — the v0.6 ship-defect. +// +// We exercise the exact wrapper doctor.go calls against a real SSH +// server on an ephemeral port (the probe hardcodes :22, which we +// cannot bind in CI). This proves the doctor's chosen callback captures +// on first connect rather than failing — the parity guarantee. +func TestProxmoxCheck_FirstConnectCapturesKey(t *testing.T) { + dir := t.TempDir() + t.Setenv("ORCA_HOME", dir) + + // Empty known_hosts (first-connect scenario). + if err := os.WriteFile(certpaths.KnownHostsPath(), []byte{}, 0o600); err != nil { + t.Fatalf("create known_hosts: %v", err) + } + + // Start a fake SSH server on an ephemeral port whose host key is + // NOT yet in known_hosts. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + _, srvPriv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519 gen: %v", err) + } + hostSigner, err := ssh.NewSignerFromKey(srvPriv) + if err != nil { + t.Fatalf("ssh signer: %v", err) + } + srvConfig := &ssh.ServerConfig{NoClientAuth: true} + srvConfig.AddHostKey(hostSigner) + go func() { + for { + nconn, err := ln.Accept() + if err != nil { + return + } + go func(c net.Conn) { + defer c.Close() + _, chans, reqs, err := ssh.NewServerConn(c, srvConfig) + if err != nil { + return + } + go ssh.DiscardRequests(reqs) + for nc := range chans { + nc.Reject(ssh.UnknownChannelType, "none") + } + }(nconn) + } + }() + + sshAddr := ln.Addr().String() + host, _, _ := net.SplitHostPort(sshAddr) + + // The doctor probe now builds its HostKeyCallback via + // proxmox.TOFUHostKeyCallback(sshAddr, nil). On first connect + // (empty known_hosts) this must capture + write the key and return + // nil, NOT a KeyError — the v0.6 ship-defect fix. + cb, err := proxmox.TOFUHostKeyCallback(sshAddr, nil) + if err != nil { + t.Fatalf("TOFUHostKeyCallback: %v", err) + } + if err := cb(sshAddr, &net.TCPAddr{IP: net.ParseIP(host), Port: 22}, hostSigner.PublicKey()); err != nil { + t.Fatalf("first-connect doctor callback should capture (not fail): %v", err) + } + + // The captured key must now be in known_hosts. + data, err := os.ReadFile(certpaths.KnownHostsPath()) + if err != nil { + t.Fatalf("read known_hosts: %v", err) + } + if len(data) == 0 { + t.Error("known_hosts is empty — doctor capture-fix did not write the key (T02.9)") + } + if !strings.Contains(string(data), hostSigner.PublicKey().Type()) { + t.Errorf("known_hosts missing the captured host key type: %s", data) + } +} diff --git a/internal/proxmox/bootstrap.go b/internal/proxmox/bootstrap.go index f5f610d..25ecfdd 100644 --- a/internal/proxmox/bootstrap.go +++ b/internal/proxmox/bootstrap.go @@ -145,7 +145,7 @@ func BootstrapProxmox(ctx context.Context, opts Options) (*Result, error) { } hostKeyCallback = cb } else { - cb, err := tofuHostKeyCallback(sshAddr, &capturedHostKey) + cb, err := TOFUHostKeyCallback(sshAddr, &capturedHostKey) if err != nil { return nil, fmt.Errorf("tofu host-key callback: %w", err) } @@ -258,7 +258,7 @@ func pinnedHostKeyCallback(expectedSHA256Base64 string, capturedKey *ssh.PublicK }, nil } -// tofuHostKeyCallback returns an ssh.HostKeyCallback that wraps the +// TOFUHostKeyCallback returns an ssh.HostKeyCallback that wraps the // standard knownhosts.New verifier with TOFU first-connect capture // (D-035). On a host-unknown KeyError{Want:[]} it writes the // server-presented key to certpaths.KnownHostsPath() atomically @@ -269,7 +269,10 @@ func pinnedHostKeyCallback(expectedSHA256Base64 string, capturedKey *ssh.PublicK // where knownhosts.New returned KeyError{Want:[]} on first connect // WITHOUT writing the captured key, so the first // `orca node join --type proxmox` always failed. -func tofuHostKeyCallback(addr string, capturedKey *ssh.PublicKey) (ssh.HostKeyCallback, error) { +// +// Exported so the doctor proxmox probe (T02.9) can reuse the same +// capture-fix wrapper for parity (GRILL condition #2). +func TOFUHostKeyCallback(addr string, capturedKey *ssh.PublicKey) (ssh.HostKeyCallback, error) { cb, err := knownhosts.New(certpaths.KnownHostsPath()) if err != nil { return nil, err diff --git a/internal/proxmox/bootstrap_test.go b/internal/proxmox/bootstrap_test.go index 2aa0b59..c5c4451 100644 --- a/internal/proxmox/bootstrap_test.go +++ b/internal/proxmox/bootstrap_test.go @@ -572,9 +572,9 @@ func TestTOFUHostKeyCallback_FirstConnectCapturesKey(t *testing.T) { t.Fatal("server host key is nil") } - cb, err := tofuHostKeyCallback(addr, nil) + cb, err := TOFUHostKeyCallback(addr, nil) if err != nil { - t.Fatalf("tofuHostKeyCallback: %v", err) + t.Fatalf("TOFUHostKeyCallback: %v", err) } if err := cb(addr, &net.TCPAddr{IP: net.ParseIP(host), Port: 22}, hostKey); err != nil { t.Fatalf("first-connect callback returned error: %v", err) @@ -609,18 +609,18 @@ func TestTOFUHostKeyCallback_SecondConnectMatches(t *testing.T) { } // First connect: capture + write. - cb1, err := tofuHostKeyCallback(addr, nil) + cb1, err := TOFUHostKeyCallback(addr, nil) if err != nil { - t.Fatalf("tofuHostKeyCallback #1: %v", err) + t.Fatalf("TOFUHostKeyCallback #1: %v", err) } if err := cb1(addr, &net.TCPAddr{IP: net.ParseIP(host), Port: 22}, hostKey); err != nil { t.Fatalf("first connect: %v", err) } // Second connect: the fresh knownhosts.New reads the written key. - cb2, err := tofuHostKeyCallback(addr, nil) + cb2, err := TOFUHostKeyCallback(addr, nil) if err != nil { - t.Fatalf("tofuHostKeyCallback #2: %v", err) + t.Fatalf("TOFUHostKeyCallback #2: %v", err) } if err := cb2(addr, &net.TCPAddr{IP: net.ParseIP(host), Port: 22}, hostKey); err != nil { t.Fatalf("second connect should match, got: %v", err) @@ -642,9 +642,9 @@ func TestTOFUHostKeyCallback_MismatchFails(t *testing.T) { } // Capture the real key first so known_hosts is populated. - cb1, err := tofuHostKeyCallback(addr, nil) + cb1, err := TOFUHostKeyCallback(addr, nil) if err != nil { - t.Fatalf("tofuHostKeyCallback #1: %v", err) + t.Fatalf("TOFUHostKeyCallback #1: %v", err) } if err := cb1(addr, &net.TCPAddr{IP: net.ParseIP(host), Port: 22}, hostKey); err != nil { t.Fatalf("first connect: %v", err) @@ -660,9 +660,9 @@ func TestTOFUHostKeyCallback_MismatchFails(t *testing.T) { t.Fatalf("new pub: %v", err) } - cb2, err := tofuHostKeyCallback(addr, nil) + cb2, err := TOFUHostKeyCallback(addr, nil) if err != nil { - t.Fatalf("tofuHostKeyCallback #2: %v", err) + t.Fatalf("TOFUHostKeyCallback #2: %v", err) } err = cb2(addr, &net.TCPAddr{IP: net.ParseIP(host), Port: 22}, altKey) if err == nil {