From 9a34887acc06d223f267bbebe9979accbb2c3611 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 9 Jul 2026 19:23:39 +0200 Subject: [PATCH] harden offsite apply-bridge: pin verified host key on the install/verify sessions (no TOFU) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- CHANGELOG.md | 4 +++- REPORT.md | 11 ++++++++--- .../internal/offsiteapply/offsiteapply.go | 11 +++++++---- .../internal/offsiteapply/offsiteapply_test.go | 18 +++++++++++------- controller/internal/offsiteapply/seams.go | 16 +++++++++++++--- 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0139fe5..1a020af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ SLICE 3; soft-quota = SLICE 4.) → nothing persisted, retried next cycle). Flow: **scan + VERIFY the box host key against `host_fingerprint` (no blind TOFU)** → generate the controller keypair → **consume the one-time password** (`POST /api/v1/offsite/consume-password/{id}`, Bearer APIKey, single-use, never logged) → install the - pubkey (`sshpass -e ssh-copy-id -p 23 -s -f`) + verify key auth → configure the offbox target → + pubkey (`sshpass -e ssh-copy-id -p 23 -s -f`, **pinning the scanner-verified `known_hosts` with + `StrictHostKeyChecking=yes` — no `accept-new`/TOFU on the install or verify session**, so a MITM cannot + substitute a key in the gap between the scan and the install) + verify key auth → configure the offbox target → `EscrowState="pending"` (fork-4 enable path via `Manager.ApplyOffsiteTarget`) → persist the marker LAST. Seams (consume/scan/keygen/install/enable) so unit tests fake all I/O. A consumed-but-failed install logs a loud "password is spent — reset on the hub" signal. diff --git a/REPORT.md b/REPORT.md index 789bc46..e200b46 100644 --- a/REPORT.md +++ b/REPORT.md @@ -8,7 +8,9 @@ On startup the controller reconciles the hub-served `offsite:` descriptor into a target — the controller half of hub-driven offsite provisioning. - `internal/config.OffsiteConfig` — the `offsite:` section (mirrors the hub descriptor incl. `host_fingerprint`). - `internal/offsiteapply.Bridge.Reconcile` — **verify-pin (no blind TOFU)** → generate keypair → **consume - the one-time password** (single-use, never logged) → install pubkey (`sshpass -e ssh-copy-id -p 23 -s -f`) + the one-time password** (single-use, never logged) → install pubkey (`sshpass -e ssh-copy-id -p 23 -s -f`, + **pinning the scanner-verified `known_hosts` via `StrictHostKeyChecking=yes` — the install/verify sessions + refuse any key but the one the bridge already matched, closing the scan→install MITM gap**) + verify → configure offbox → `EscrowState="pending"` → persist the descriptor-hash marker LAST. **Idempotent** (marker prevents re-consuming a spent password) and **fail-safe** (any step fails → nothing persisted, retried next cycle; consumed-but-failed install logs a loud "password is spent — reset on the hub"). @@ -21,8 +23,11 @@ target — the controller half of hub-driven offsite provisioning. ## Tests + companion red-proofs Green gate `go build ./... && go vet ./... && go test ./...` — **ALL-GREEN** (both repos). -- `TestBridge_AppliesEndToEnd` — consume→verify-pin→install→configure→marker; asserts the enabler got the - pinned known_hosts + the private key, and **the one-time password never appears in a log line**. +- `TestBridge_AppliesEndToEnd` — consume→verify-pin→install→configure→marker; asserts **the installer AND + the enabler both receive the scanner-verified known_hosts** + the private key, and **the one-time password + never appears in a log line**. **Red-proof run:** passed an empty known_hosts to the installer (the pre-fix + TOFU shape) → test FAILED ("installer must receive the scanner-verified known_hosts to pin (no TOFU), got \"\""). Reverted. + Pinning-the-install-connection (not just the scan) is load-bearing — addresses the security-review TOFU-after-verify finding. - `TestBridge_HostKeyMismatchRefuses` — a scanned FP ≠ descriptor FP → refuse (no consume/install/configure/marker). **Red-proof run:** dropped the verify (`if false`) → the mismatch proceeded to install a wrong key → test FAILED ("mismatch must refuse, got "). Reverted. No-TOFU is load-bearing. diff --git a/controller/internal/offsiteapply/offsiteapply.go b/controller/internal/offsiteapply/offsiteapply.go index ec9fce5..c0e971e 100644 --- a/controller/internal/offsiteapply/offsiteapply.go +++ b/controller/internal/offsiteapply/offsiteapply.go @@ -33,9 +33,10 @@ type ( Generate() (privPEM, pubAuthorized string, err error) } // KeyInstaller installs the pub line on the box using the one-time password, then verifies passwordless - // key auth with the private key. Fails if the install or the verify fails. + // key auth with the private key. It MUST pin the VERIFIED knownHosts line (from the scan) on the + // connection — never blind-TOFU — so a MITM cannot swap the key between the scan and the install. KeyInstaller interface { - Install(ctx context.Context, host, user string, port int, password, privPEM, pubAuthorized string) error + Install(ctx context.Context, host, user string, port int, password, privPEM, pubAuthorized, knownHosts string) error } // OffboxEnabler configures the offbox target (key + known_hosts + target) and goes EscrowState="pending" // (the fork-4 enable path). @@ -132,8 +133,10 @@ func (b *Bridge) Reconcile(ctx context.Context) error { return fmt.Errorf("offsite-apply: consume one-time password: %w", err) } - // 4) Install the pubkey using the password (proven ssh-copy-id -s -f), verify key auth. - if err := b.Installer.Install(ctx, o.Host, o.User, port, password, privPEM, pubAuthorized); err != nil { + // 4) Install the pubkey using the password (proven ssh-copy-id -s -f), verify key auth. Pin the + // scanner-VERIFIED known_hosts line on the install/verify connections — never accept-new — so a MITM + // cannot substitute a different key in the gap between the scan and the install. + if err := b.Installer.Install(ctx, o.Host, o.User, port, password, privPEM, pubAuthorized, knownHostsLine); err != nil { // The password is now SPENT but install failed — a loud, distinct signal: the operator must reset // the box password on the hub and let the bridge retry. Do NOT mark applied. b.logf("[ERROR] [offsite-apply] key install FAILED after consuming the one-time password for %s@%s — the password is spent; reset it on the hub to retry: %v", o.User, o.Host, err) diff --git a/controller/internal/offsiteapply/offsiteapply_test.go b/controller/internal/offsiteapply/offsiteapply_test.go index 0a9327f..c0e3906 100644 --- a/controller/internal/offsiteapply/offsiteapply_test.go +++ b/controller/internal/offsiteapply/offsiteapply_test.go @@ -44,16 +44,17 @@ 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 + err error + calls int + gotPub string + gotPriv string + gotPw string + gotKnownHost string } -func (f *fakeInstaller) Install(_ context.Context, _, _ string, _ int, password, privPEM, pub string) error { +func (f *fakeInstaller) Install(_ context.Context, _, _ string, _ int, password, privPEM, pub, knownHosts string) error { f.calls++ - f.gotPub, f.gotPriv, f.gotPw = pub, privPEM, password + f.gotPub, f.gotPriv, f.gotPw, f.gotKnownHost = pub, privPEM, password, knownHosts return f.err } @@ -108,6 +109,9 @@ func TestBridge_AppliesEndToEnd(t *testing.T) { 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) } diff --git a/controller/internal/offsiteapply/seams.go b/controller/internal/offsiteapply/seams.go index 9ddf0d3..4064d3b 100644 --- a/controller/internal/offsiteapply/seams.go +++ b/controller/internal/offsiteapply/seams.go @@ -142,7 +142,10 @@ func (ED25519KeyGen) Generate() (string, string, error) { type SSHCopyIDInstaller struct{} -func (SSHCopyIDInstaller) Install(ctx context.Context, host, user string, port int, password, privPEM, pubAuthorized string) error { +func (SSHCopyIDInstaller) Install(ctx context.Context, host, user string, port int, password, privPEM, pubAuthorized, knownHosts string) error { + if strings.TrimSpace(knownHosts) == "" { + return fmt.Errorf("ssh-copy-id: empty known_hosts — refusing to install without a pinned host key") + } work, err := os.MkdirTemp("", "felhom-keyinstall-") if err != nil { return err @@ -150,22 +153,29 @@ func (SSHCopyIDInstaller) Install(ctx context.Context, host, user string, port i defer os.RemoveAll(work) pubPath := filepath.Join(work, "id.pub") privPath := filepath.Join(work, "id") + khPath := filepath.Join(work, "known_hosts") if err := os.WriteFile(pubPath, []byte(pubAuthorized), 0o600); err != nil { return err } if err := os.WriteFile(privPath, []byte(privPEM), 0o600); err != nil { return err } + // Pin the scanner-VERIFIED host key: StrictHostKeyChecking=yes against this known_hosts refuses any + // other key (no accept-new/TOFU) — the ssh-copy-id + verify sessions connect ONLY to the box whose + // fingerprint the bridge already matched against the hub descriptor. + if err := os.WriteFile(khPath, []byte(knownHosts+"\n"), 0o600); err != nil { + return err + } // Install (SSHPASS env is read by `sshpass -e`; the password never appears on argv). install := exec.CommandContext(ctx, "sshpass", "-e", "ssh-copy-id", "-p", strconv.Itoa(port), "-s", "-f", - "-i", pubPath, "-o", "StrictHostKeyChecking=accept-new", user+"@"+host) + "-i", pubPath, "-o", "StrictHostKeyChecking=yes", "-o", "UserKnownHostsFile="+khPath, user+"@"+host) install.Env = append(os.Environ(), "SSHPASS="+password) if out, err := install.CombinedOutput(); err != nil { return fmt.Errorf("ssh-copy-id: %w: %s", err, truncate(out)) } // Verify passwordless key auth (an SFTP no-op; the box's restricted shell only offers SFTP). verify := exec.CommandContext(ctx, "sftp", "-b", "-", "-P", strconv.Itoa(port), - "-i", privPath, "-oBatchMode=yes", "-oStrictHostKeyChecking=accept-new", user+"@"+host) + "-i", privPath, "-oBatchMode=yes", "-oStrictHostKeyChecking=yes", "-oUserKnownHostsFile="+khPath, user+"@"+host) verify.Stdin = strings.NewReader("pwd\n") if out, err := verify.CombinedOutput(); err != nil { return fmt.Errorf("key-auth verify failed after install: %w: %s", err, truncate(out))