9a34887acc
The SSHCopyIDInstaller used StrictHostKeyChecking=accept-new on the ssh-copy-id and sftp-verify connections, so even though the bridge verifies the box host-key fingerprint against the hub descriptor BEFORE installing, the actual install connection was not pinned to that verified key — a MITM could substitute a different key in the gap between the scan and the install (TOCTOU). Now the bridge threads the scanner-verified known_hosts line into KeyInstaller, which writes it to a temp known_hosts and connects with StrictHostKeyChecking=yes + UserKnownHostsFile — the install/verify sessions refuse any key but the one the bridge already matched. Empty known_hosts now refuses to install. Test asserts the installer receives the pinned known_hosts; red-proofed by passing an empty line (the pre-fix TOFU shape) → test fails. Addresses the security-review "host-key TOFU after verify" finding on internal/offsiteapply/seams.go. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
193 lines
6.0 KiB
Go
193 lines
6.0 KiB
Go
package offsiteapply
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"errors"
|
|
"log"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-controller/internal/config"
|
|
)
|
|
|
|
// --- fakes ---
|
|
|
|
type fakeConsumer struct {
|
|
pw string
|
|
err error
|
|
calls int
|
|
panics bool
|
|
}
|
|
|
|
func (f *fakeConsumer) Consume(_ context.Context) (string, error) {
|
|
if f.panics {
|
|
panic("consume must NOT be called (idempotent no-op)")
|
|
}
|
|
f.calls++
|
|
return f.pw, f.err
|
|
}
|
|
|
|
type fakeScanner struct {
|
|
fp, line string
|
|
err error
|
|
}
|
|
|
|
func (f *fakeScanner) Scan(_ context.Context, _ string, _ int) (string, string, error) {
|
|
return f.fp, f.line, f.err
|
|
}
|
|
|
|
type fakeKeyGen struct{ priv, pub string }
|
|
|
|
func (f *fakeKeyGen) Generate() (string, string, error) { return f.priv, f.pub, nil }
|
|
|
|
type fakeInstaller struct {
|
|
err error
|
|
calls int
|
|
gotPub string
|
|
gotPriv string
|
|
gotPw string
|
|
gotKnownHost string
|
|
}
|
|
|
|
func (f *fakeInstaller) Install(_ context.Context, _, _ string, _ int, password, privPEM, pub, knownHosts string) error {
|
|
f.calls++
|
|
f.gotPub, f.gotPriv, f.gotPw, f.gotKnownHost = pub, privPEM, password, knownHosts
|
|
return f.err
|
|
}
|
|
|
|
type fakeEnabler struct {
|
|
err error
|
|
calls int
|
|
gotHost string
|
|
gotKnownHost string
|
|
gotPriv string
|
|
}
|
|
|
|
func (f *fakeEnabler) ConfigureOffbox(_ context.Context, host, _ string, _ int, _, privPEM, knownHosts string) error {
|
|
f.calls++
|
|
f.gotHost, f.gotKnownHost, f.gotPriv = host, knownHosts, privPEM
|
|
return f.err
|
|
}
|
|
|
|
func newBridge(t *testing.T, o config.OffsiteConfig) (*Bridge, *fakeConsumer, *fakeInstaller, *fakeEnabler, *bytes.Buffer) {
|
|
t.Helper()
|
|
cfg := &config.Config{}
|
|
cfg.Offsite = o
|
|
cons := &fakeConsumer{pw: "the-transient-pw"}
|
|
inst := &fakeInstaller{}
|
|
en := &fakeEnabler{}
|
|
var logbuf bytes.Buffer
|
|
b := &Bridge{
|
|
Cfg: cfg,
|
|
Consumer: cons,
|
|
Scanner: &fakeScanner{fp: "SHA256:goodfp", line: "[h]:23 ssh-ed25519 AAAAKEY"},
|
|
KeyGen: &fakeKeyGen{priv: "PRIVPEM", pub: "ssh-ed25519 AAAAPUB felhom"},
|
|
Installer: inst,
|
|
Enabler: en,
|
|
MarkerPath: filepath.Join(t.TempDir(), "offbox", "applied_marker"),
|
|
Logger: log.New(&logbuf, "", 0),
|
|
}
|
|
return b, cons, inst, en, &logbuf
|
|
}
|
|
|
|
func goodOffsite() config.OffsiteConfig {
|
|
return config.OffsiteConfig{Enabled: true, Type: "shared", Host: "h", User: "u", Port: 23, RepoPath: "/home/felhom-repo", HostFingerprint: "SHA256:goodfp"}
|
|
}
|
|
|
|
// Scenario A — full apply: consume → verify-pin → install → configure offbox → marker persisted; pw not logged.
|
|
func TestBridge_AppliesEndToEnd(t *testing.T) {
|
|
b, cons, inst, en, logbuf := newBridge(t, goodOffsite())
|
|
if err := b.Reconcile(context.Background()); err != nil {
|
|
t.Fatalf("reconcile: %v", err)
|
|
}
|
|
if cons.calls != 1 {
|
|
t.Fatalf("consume calls = %d, want 1", cons.calls)
|
|
}
|
|
if inst.calls != 1 || inst.gotPw != "the-transient-pw" || inst.gotPub == "" {
|
|
t.Fatalf("installer not called with pw+pub: %+v", inst)
|
|
}
|
|
if inst.gotKnownHost != "[h]:23 ssh-ed25519 AAAAKEY" {
|
|
t.Fatalf("installer must receive the scanner-verified known_hosts to pin (no TOFU), got %q", inst.gotKnownHost)
|
|
}
|
|
if en.calls != 1 || en.gotHost != "h" || en.gotKnownHost != "[h]:23 ssh-ed25519 AAAAKEY" || en.gotPriv != "PRIVPEM" {
|
|
t.Fatalf("enabler not called with the pinned known_hosts + key: %+v", en)
|
|
}
|
|
if b.readMarker() != descriptorHash(b.Cfg.Offsite) {
|
|
t.Fatal("marker not persisted after a successful apply")
|
|
}
|
|
if strings.Contains(logbuf.String(), "the-transient-pw") {
|
|
t.Fatal("the one-time password LEAKED into a log line")
|
|
}
|
|
}
|
|
|
|
// Scenario B — host-key mismatch → refuse: no consume, no install, no configure, no marker.
|
|
func TestBridge_HostKeyMismatchRefuses(t *testing.T) {
|
|
b, cons, inst, en, _ := newBridge(t, goodOffsite())
|
|
b.Scanner = &fakeScanner{fp: "SHA256:ATTACKER", line: "[h]:23 ssh-ed25519 EVIL"}
|
|
err := b.Reconcile(context.Background())
|
|
if err == nil || !strings.Contains(err.Error(), "MISMATCH") {
|
|
t.Fatalf("mismatch must refuse, got %v", err)
|
|
}
|
|
if cons.calls != 0 || inst.calls != 0 || en.calls != 0 {
|
|
t.Fatalf("nothing may proceed on a host-key mismatch: cons=%d inst=%d en=%d", cons.calls, inst.calls, en.calls)
|
|
}
|
|
if b.readMarker() != "" {
|
|
t.Fatal("no marker may be written on a mismatch")
|
|
}
|
|
}
|
|
|
|
// Scenario C — idempotent: marker already matches → no-op, consume is NOT called.
|
|
func TestBridge_IdempotentNoReconsume(t *testing.T) {
|
|
b, cons, inst, en, _ := newBridge(t, goodOffsite())
|
|
cons.panics = true // Consume must not be called
|
|
// pre-seed the marker with the current descriptor hash
|
|
_ = os.MkdirAll(filepath.Dir(b.MarkerPath), 0o700)
|
|
if err := os.WriteFile(b.MarkerPath, []byte(descriptorHash(b.Cfg.Offsite)), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := b.Reconcile(context.Background()); err != nil {
|
|
t.Fatalf("idempotent reconcile must be a clean no-op, got %v", err)
|
|
}
|
|
if cons.calls != 0 || inst.calls != 0 || en.calls != 0 {
|
|
t.Fatal("an already-applied descriptor must be a full no-op")
|
|
}
|
|
}
|
|
|
|
// Scenario D — install fails → fail-safe: marker NOT persisted, offbox NOT configured, loud log.
|
|
func TestBridge_InstallFailIsFailSafe(t *testing.T) {
|
|
b, cons, inst, en, logbuf := newBridge(t, goodOffsite())
|
|
inst.err = errors.New("ssh-copy-id refused")
|
|
err := b.Reconcile(context.Background())
|
|
if err == nil {
|
|
t.Fatal("install failure must error")
|
|
}
|
|
if en.calls != 0 {
|
|
t.Fatal("offbox must NOT be configured when install fails")
|
|
}
|
|
if b.readMarker() != "" {
|
|
t.Fatal("marker must NOT be persisted on a failed apply (fail-safe)")
|
|
}
|
|
if cons.calls != 1 {
|
|
t.Fatal("the password was consumed (spent) before install")
|
|
}
|
|
if !strings.Contains(logbuf.String(), "password is spent") {
|
|
t.Fatal("a consumed-but-failed install must log the loud 'password is spent' signal")
|
|
}
|
|
}
|
|
|
|
// Disabled → no-op (no consume/install/configure).
|
|
func TestBridge_DisabledNoOp(t *testing.T) {
|
|
o := goodOffsite()
|
|
o.Enabled = false
|
|
b, cons, inst, en, _ := newBridge(t, o)
|
|
if err := b.Reconcile(context.Background()); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if cons.calls+inst.calls+en.calls != 0 {
|
|
t.Fatal("disabled offsite must be a no-op")
|
|
}
|
|
}
|