From a38c7439268ffaed33c8cecaf6dbe3c63aebed2a Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 9 Jul 2026 22:26:53 +0200 Subject: [PATCH] v0.107.0: key-auth-first bridge + staged-secret wipe on escrow confirm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Key-auth-first: a KeyAuthProber seam lets the bridge skip consume+install when the already-installed key still authenticates (pinned to the freshly verified host key) — descriptor changes on provisioned guests no longer loop on consume-404. Fingerprint verify still precedes everything. Wipe-on-escrowed: confirm-escrow now calls the agent's new DELETE /escrow/stage-secret (v0.78.0) best-effort, closing the hygiene gap where a ceremony-less confirm left the staged password file behind. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- CHANGELOG.md | 21 +++++++ controller/cmd/controller/main.go | 1 + controller/internal/agentapi/client.go | 23 +++++++ .../internal/offsiteapply/offsiteapply.go | 29 ++++++++- .../offsiteapply/offsiteapply_test.go | 61 +++++++++++++++++++ controller/internal/offsiteapply/seams.go | 39 ++++++++++++ controller/internal/web/offbox_escrow_test.go | 42 +++++++++++++ controller/internal/web/offbox_handlers.go | 21 +++++++ controller/internal/web/server.go | 5 ++ 9 files changed, 241 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cef1a42..02c763f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ ## Changelog +### v0.107.0 — offsite hardening: key-auth-first bridge + staged-secret wipe on confirm (2026-07-09) + +Part of the offsite-provisioning hardening bundle (pairs with hub v0.39.0 + agent v0.78.0). + +- **Key-auth-first (`internal/offsiteapply`):** new `KeyAuthProber` seam (`SFTPKeyAuthProber` — probes the + ALREADY-INSTALLED key against the descriptor target, pinned to the freshly-verified known_hosts). On a + descriptor change where the existing key still authenticates, the bridge **re-pins + reconfigures WITHOUT + consuming a one-time password** — kills the stale-descriptor consume-404 loop seen twice in the live e2e, + and shrinks the re-issue blast radius to genuinely-fresh guests. The probe NEVER bypasses the fingerprint + verify (scan+verify still precedes it; a mismatch refuses before any probe). Fresh guests (no key / auth + refused) fall through to the full verify→consume→install path unchanged. + Tests + red-proofs: probe-success with a panicking consumer (drop the skip → panic → FAIL); fresh-guest + fallthrough (early-return on probe-fail → nothing applies → FAIL); mismatch now also asserts the probe + never runs on a failed identity check. +- **Staged-secret wipe (`internal/web` + `internal/agentapi`):** `WipeStagedEscrowSecret` (DELETE + `/escrow/stage-secret`, agent ≥ v0.78.0); the confirm-escrow handler wipes the agent-staged repo password + whenever `EscrowState` flips to `escrowed` — best-effort (a wipe failure logs a loud ERROR but never fails + the confirm; re-confirm retries). Closes the fork-4 hygiene gap where a confirm without a fresh ceremony + left the staged 0600 file behind (observed live in the e2e's Option-A close). Test: confirm wipes exactly + once; a failing wipe still confirms + logs "NOT wiped". + ### v0.106.1 — offsite apply-bridge: ssh-copy-id -s needs ~/.ssh to exist (live finding F3) (2026-07-09) First supervised live apply: scan+verify passed, the one-time password was consumed, then `ssh-copy-id -s` diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 6c042cd..fe9d378 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -242,6 +242,7 @@ func main() { Scanner: offsiteapply.KeyscanScanner{}, KeyGen: offsiteapply.ED25519KeyGen{}, Installer: offsiteapply.SSHCopyIDInstaller{}, + Prober: offsiteapply.SFTPKeyAuthProber{KeyPath: filepath.Join(cfg.Paths.DataDir, "offbox", "ssh_key")}, Enabler: offsiteapply.EnablerFunc(func(ctx context.Context, host, user string, port int, repoPath, priv, kh string) error { tgt := &settings.OffboxTarget{Enabled: true, Host: host, User: user, Port: port, RepoPath: repoPath, Schedule: "daily"} stage := func(ctx context.Context, pw string) error { diff --git a/controller/internal/agentapi/client.go b/controller/internal/agentapi/client.go index 0f4b9f5..67f3711 100644 --- a/controller/internal/agentapi/client.go +++ b/controller/internal/agentapi/client.go @@ -474,6 +474,29 @@ func (c *Client) StageEscrowSecret(ctx context.Context, resticRepoPassword strin return refusalError("/escrow/stage-secret", status, env) } +// WipeStagedEscrowSecret removes the agent-staged offsite repo password (fork-4 hygiene) — called whenever +// EscrowState flips to escrowed, so the transient 0600 staging file doesn't outlive its purpose. Idempotent +// on the agent side (an absent file is a clean 200). Requires agent >= v0.78.0 (older agents 404 — the +// caller logs loudly and moves on). +func (c *Client) WipeStagedEscrowSecret(ctx context.Context) error { + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, c.baseURL+"/escrow/stage-secret", nil) + if err != nil { + return err + } + req.Header.Set("Authorization", "Bearer "+c.token) + resp, err := c.hc.Do(req) + if err != nil { + return fmt.Errorf("agentapi: DELETE /escrow/stage-secret: %w", err) + } + defer resp.Body.Close() + raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + var env apiResponse + if err := json.Unmarshal(raw, &env); err != nil { + return fmt.Errorf("agentapi: DELETE /escrow/stage-secret: HTTP %d, bad envelope: %w", resp.StatusCode, err) + } + return refusalError("/escrow/stage-secret", resp.StatusCode, env) +} + func (c *Client) EjectDisk(ctx context.Context, where string) (EjectResult, error) { var out EjectResult env, status, err := c.postWithStatus(ctx, "/disks/eject", map[string]string{"where": where}) diff --git a/controller/internal/offsiteapply/offsiteapply.go b/controller/internal/offsiteapply/offsiteapply.go index c0e971e..fdfe251 100644 --- a/controller/internal/offsiteapply/offsiteapply.go +++ b/controller/internal/offsiteapply/offsiteapply.go @@ -43,6 +43,14 @@ type ( OffboxEnabler interface { ConfigureOffbox(ctx context.Context, host, user string, port int, repoPath, privPEM, knownHosts string) error } + // KeyAuthProber checks whether an ALREADY-INSTALLED key authenticates to the target (pinned to the + // freshly-scanned knownHosts). ok=true returns that key's PEM so the descriptor change is applied by + // re-pinning + reconfiguring WITHOUT consuming a one-time password (key-auth-first — kills the + // stale-descriptor consume-404 loop and shrinks the re-issue blast radius to genuinely-fresh guests). + // ok=false (no key / auth refused) → the caller falls through to the full consume+install path. + KeyAuthProber interface { + Probe(ctx context.Context, host, user string, port int, knownHosts string) (privPEM string, ok bool) + } ) // Bridge reconciles the offsite descriptor into a configured offbox target. @@ -53,7 +61,8 @@ type Bridge struct { KeyGen KeyGenerator Installer KeyInstaller Enabler OffboxEnabler - MarkerPath string // where the applied-descriptor-hash is persisted (e.g. /offbox/applied_marker) + Prober KeyAuthProber // optional: key-auth-first (nil → always the full consume+install path) + MarkerPath string // where the applied-descriptor-hash is persisted (e.g. /offbox/applied_marker) Logger *log.Logger } @@ -121,6 +130,24 @@ func (b *Bridge) Reconcile(ctx context.Context) error { return fmt.Errorf("offsite-apply: host-key MISMATCH for %s (got %s, want %s) — refusing to pin/install (possible MITM)", o.Host, scannedFP, o.HostFingerprint) } + // 1b) Key-auth-first: if an already-installed key still authenticates (pinned to the key we JUST + // verified — the probe never weakens the identity check), the descriptor change is applied by + // re-pinning + reconfiguring alone. NO one-time password is consumed — a stale/re-scanned descriptor + // on an already-provisioned guest no longer loops on consume-404. + if b.Prober != nil { + if privPEM, ok := b.Prober.Probe(ctx, o.Host, o.User, port, knownHostsLine); ok { + if err := b.Enabler.ConfigureOffbox(ctx, o.Host, o.User, port, o.RepoPath, privPEM, knownHostsLine); err != nil { + return fmt.Errorf("offsite-apply: reconfigure (key-auth-first): %w", err) + } + if err := b.writeMarker(h); err != nil { + b.logf("[WARN] [offsite-apply] key-auth-first applied for %s but failed to persist the marker: %v", o.Host, err) + return err + } + b.logf("[INFO] [offsite-apply] existing key still authenticates to %s@%s — re-pinned + reconfigured without consuming a password", o.User, o.Host) + return nil + } + } + // 2) Generate the controller keypair. privPEM, pubAuthorized, err := b.KeyGen.Generate() if err != nil { diff --git a/controller/internal/offsiteapply/offsiteapply_test.go b/controller/internal/offsiteapply/offsiteapply_test.go index c0e3906..1fa5dd2 100644 --- a/controller/internal/offsiteapply/offsiteapply_test.go +++ b/controller/internal/offsiteapply/offsiteapply_test.go @@ -58,6 +58,23 @@ func (f *fakeInstaller) Install(_ context.Context, _, _ string, _ int, password, return f.err } +type fakeProber struct { + pem string + ok bool + panics bool + calls int + gotKH string +} + +func (f *fakeProber) Probe(_ context.Context, _, _ string, _ int, kh string) (string, bool) { + if f.panics { + panic("prober must NOT be called (verify must precede the probe)") + } + f.calls++ + f.gotKH = kh + return f.pem, f.ok +} + type fakeEnabler struct { err error calls int @@ -123,10 +140,54 @@ func TestBridge_AppliesEndToEnd(t *testing.T) { } } +// Key-auth-first (Scenario B) — the existing key still works: NO consume, NO install; re-verify + re-pin + +// reconfigure with the EXISTING key, marker updated. +func TestBridge_KeyAuthFirstSkipsConsume(t *testing.T) { + b, cons, inst, en, _ := newBridge(t, goodOffsite()) + cons.panics = true // the whole point: a working key must NEVER consume the one-time password + prober := &fakeProber{pem: "EXISTINGPEM", ok: true} + b.Prober = prober + if err := b.Reconcile(context.Background()); err != nil { + t.Fatalf("key-auth-first reconcile: %v", err) + } + if prober.calls != 1 || prober.gotKH != "[h]:23 ssh-ed25519 AAAAKEY" { + t.Fatalf("probe must run once with the freshly-scanned pinned known_hosts: %+v", prober) + } + if inst.calls != 0 { + t.Fatal("installer must NOT run when the existing key authenticates") + } + if en.calls != 1 || en.gotPriv != "EXISTINGPEM" || en.gotKnownHost != "[h]:23 ssh-ed25519 AAAAKEY" { + t.Fatalf("enabler must reconfigure with the EXISTING key + fresh pin: %+v", en) + } + if b.readMarker() != descriptorHash(b.Cfg.Offsite) { + t.Fatal("marker must be updated after a key-auth-first apply") + } +} + +// Scenario C — key-auth-first must NOT weaken the fresh path: probe fails → the full +// verify→consume→install path runs unchanged (with the freshly GENERATED key). +func TestBridge_FreshGuestFallsThroughToFullPath(t *testing.T) { + b, cons, inst, en, _ := newBridge(t, goodOffsite()) + b.Prober = &fakeProber{ok: false} // fresh guest: no key / auth refused + if err := b.Reconcile(context.Background()); err != nil { + t.Fatalf("fresh-guest reconcile: %v", err) + } + if cons.calls != 1 || inst.calls != 1 { + t.Fatalf("fresh guest must consume+install exactly once: cons=%d inst=%d", cons.calls, inst.calls) + } + if en.calls != 1 || en.gotPriv != "PRIVPEM" { + t.Fatalf("fresh guest must configure with the GENERATED key: %+v", en) + } + if b.readMarker() != descriptorHash(b.Cfg.Offsite) { + t.Fatal("marker must be persisted after a full-path apply") + } +} + // 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"} + b.Prober = &fakeProber{panics: true} // the probe must NEVER run when the identity check failed err := b.Reconcile(context.Background()) if err == nil || !strings.Contains(err.Error(), "MISMATCH") { t.Fatalf("mismatch must refuse, got %v", err) diff --git a/controller/internal/offsiteapply/seams.go b/controller/internal/offsiteapply/seams.go index 8760b94..bb143d4 100644 --- a/controller/internal/offsiteapply/seams.go +++ b/controller/internal/offsiteapply/seams.go @@ -191,6 +191,45 @@ func (SSHCopyIDInstaller) Install(ctx context.Context, host, user string, port i return nil } +// --- SFTPKeyAuthProber: does the ALREADY-INSTALLED key still authenticate? (key-auth-first) --- + +// SFTPKeyAuthProber probes passwordless auth with the existing installed key (KeyPath), pinned to the +// freshly-verified knownHosts line. No key file → ok=false (fresh guest). The probe never logs secrets. +type SFTPKeyAuthProber struct { + KeyPath string // the installed key, e.g. /offbox/ssh_key + Timeout time.Duration // per-probe budget; 0 → 20s +} + +func (p SFTPKeyAuthProber) Probe(ctx context.Context, host, user string, port int, knownHosts string) (string, bool) { + pem, err := os.ReadFile(p.KeyPath) + if err != nil { + return "", false // no existing key — a fresh guest; take the full path + } + timeout := p.Timeout + if timeout == 0 { + timeout = 20 * time.Second + } + pctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + work, err := os.MkdirTemp("", "felhom-keyprobe-") + if err != nil { + return "", false + } + defer os.RemoveAll(work) + khPath := filepath.Join(work, "known_hosts") + if err := os.WriteFile(khPath, []byte(knownHosts+"\n"), 0o600); err != nil { + return "", false + } + probe := exec.CommandContext(pctx, "sftp", "-b", "-", "-P", strconv.Itoa(port), + "-i", p.KeyPath, "-oBatchMode=yes", "-oConnectTimeout=10", + "-oStrictHostKeyChecking=yes", "-oUserKnownHostsFile="+khPath, user+"@"+host) + probe.Stdin = strings.NewReader("pwd\n") + if err := probe.Run(); err != nil { + return "", false // auth refused / unreachable — fall through to the full path + } + return string(pem), true +} + func truncate(b []byte) string { s := strings.TrimSpace(string(b)) if len(s) > 300 { diff --git a/controller/internal/web/offbox_escrow_test.go b/controller/internal/web/offbox_escrow_test.go index 35eb141..8f19681 100644 --- a/controller/internal/web/offbox_escrow_test.go +++ b/controller/internal/web/offbox_escrow_test.go @@ -1,6 +1,9 @@ package web import ( + "bytes" + "context" + "errors" "io" "log" "net/http/httptest" @@ -67,6 +70,45 @@ func TestOffboxWeb_RunGatedUntilConfirm(t *testing.T) { } } +// Scenario E — the confirm flip wipes the agent-staged secret; a wipe failure is logged loudly but does +// NOT fail the confirm (the state flip is the primary effect). +func TestOffboxWeb_ConfirmWipesStagedSecret(t *testing.T) { + s, sett, m := newOffboxWebServer(t) + if err := m.WriteOffboxSecrets("KEYMATERIAL", "nas.local ssh-ed25519 HOSTKEY"); err != nil { + t.Fatal(err) + } + if err := sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", Schedule: "daily", + EscrowState: "pending", + }); err != nil { + t.Fatal(err) + } + wipes := 0 + s.wipeStagedEscrowFn = func(context.Context) error { wipes++; return nil } + w := httptest.NewRecorder() + s.offboxConfirmEscrowHandler(w, httptest.NewRequest("POST", "/backup/offbox/confirm-escrow", nil)) + if w.Code != 302 || sett.GetOffboxTarget().EscrowState != "escrowed" { + t.Fatalf("confirm failed: code=%d state=%q", w.Code, sett.GetOffboxTarget().EscrowState) + } + if wipes != 1 { + t.Fatalf("confirm must wipe the staged secret exactly once, got %d", wipes) + } + + // wipe failure → confirm still succeeds (best-effort), loud ERROR logged + var logbuf bytes.Buffer + s.logger = log.New(&logbuf, "", 0) + _ = sett.UpdateOffboxStatus(func(o *settings.OffboxTarget) { o.EscrowState = "pending" }) + s.wipeStagedEscrowFn = func(context.Context) error { return errors.New("agent unreachable") } + w2 := httptest.NewRecorder() + s.offboxConfirmEscrowHandler(w2, httptest.NewRequest("POST", "/backup/offbox/confirm-escrow", nil)) + if w2.Code != 302 || sett.GetOffboxTarget().EscrowState != "escrowed" { + t.Fatal("a wipe failure must NOT fail the confirm (state flip is primary)") + } + if !strings.Contains(logbuf.String(), "NOT wiped") { + t.Fatal("a failed wipe must log the loud NOT-wiped signal") + } +} + // The inject endpoint pre-places a recovered password (DR seam). func TestOffboxWeb_InjectPassword(t *testing.T) { s, _, _ := newOffboxWebServer(t) diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index c7df056..4810038 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -122,9 +122,30 @@ func (s *Server) offboxConfirmEscrowHandler(w http.ResponseWriter, r *http.Reque return } s.logger.Printf("[INFO] [web] off-box escrow confirmed — offsite runs enabled") + // Fork-4 hygiene: the staged copy on the agent has served its purpose — wipe it. Best-effort: a wipe + // failure is logged LOUDLY but does not fail the confirm (the state flip is the primary effect; a + // lingering file is a hygiene gap, not a correctness one — re-confirm retries the wipe). + if err := s.wipeStagedEscrow(r.Context()); err != nil { + s.logger.Printf("[ERROR] [web] escrow confirmed but the agent-staged secret was NOT wiped (re-confirm to retry): %v", err) + } offboxRedirect(w, r, "A kulcs letétbe helyezése megerősítve — a NAS-mentés mostantól futhat.", false) } +// wipeStagedEscrow calls the injected seam (tests), else the agent's DELETE /escrow/stage-secret over the +// pinned local-API channel (agent >= v0.78.0). +func (s *Server) wipeStagedEscrow(ctx context.Context) error { + if s.wipeStagedEscrowFn != nil { + return s.wipeStagedEscrowFn(ctx) + } + client, err := s.agentClient() + if err != nil { + return err + } + wctx, cancel := context.WithTimeout(ctx, 15*time.Second) + defer cancel() + return client.WipeStagedEscrowSecret(wctx) +} + // offboxInjectPasswordHandler pre-places a RECOVERED repo password at the offbox password path (fork-4 DR // seam) so a subsequent configure uses it and the existing offsite repo opens. Operator/DR only; the value // is never logged. Body: {password, force?}. diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index fde8eae..a26caa8 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -2,6 +2,7 @@ package web import ( "bytes" + "context" "fmt" "html/template" "log" @@ -63,6 +64,10 @@ type Server struct { // Hub push status callback — set via SetHubPushStatus for monitoring page hubPushStatusFn func() HubPushStatusData + // Fork-4 hygiene seam: wipes the agent-staged offsite repo password when EscrowState flips to + // escrowed (DELETE /escrow/stage-secret). nil → the default agentClient()-backed impl; tests inject. + wipeStagedEscrowFn func(ctx context.Context) error + // Asset syncer for Hub-managed assets (optional) assetsSyncer *assets.Syncer