From 91b9405c81bd94f6f1067eed5ce866879363ba29 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 8 Oct 2026 07:53:01 +0200 Subject: [PATCH] R-304: no 'wrong code' when earlier sealed packages were not all checked (424 older_unchecked) Unreleased; ships with tomorrow's release. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- CHANGELOG.md | 11 ++- cmd/felhom-agent/main.go | 6 +- internal/escrow/recover.go | 60 +++++++++++--- internal/escrow/recover_retained_test.go | 78 ++++++++++++++++++- internal/localapi/escrow_recover.go | 16 ++++ .../localapi/escrow_recover_class_test.go | 7 ++ 6 files changed, 164 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 541a303..e1e9e33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## Unreleased (2026-10-08) — no OS leg after a household press; the after-boot kernel report carries the real ring (R-899) — ships with tomorrow's release +## Unreleased (2026-10-08) — no OS leg after a household press; the after-boot kernel report carries the real ring (R-899); no „wrong code" when older packages were not checked (R-304) — ships with tomorrow's release **Delivery: the agent binary only** — no root file changed. @@ -8,6 +8,15 @@ request without the parameter (an older controller, the scheduled path) behaves as before. `internal/localapi/server.go`; `TestAfterPrimaryBackup` gained two sub-cases (red-proved: ignoring `trigger`, the leg ran once after a press). +- **R-304 — „wrong code" only when every earlier package was tried.** `POST /escrow/recover-offsite-password` answered + 400 („the recovery code did not open the sealed bundle") whenever the current package and the TRIED earlier packages + refused the code — even when the hub withheld earlier packages (rows with no key material, rows over its serve cap), a + served package was malformed, the 6-attempt cap stopped the loop, or the retained list could not be read at all. Now + those cases answer **424** with `older_unchecked` (the count, -1 = unknown) and a sentence that does not call the code + wrong. `escrow.ErrRetainedUnchecked` / `RetainedUncheckedError`; the retained fetcher's second value is now every + withheld package (unopenable + truncated + malformed). Tests `TestR304_*` (real age crypto; red-proved: without the + check, four cases returned the wrong-code error) and a 424 row in `TestRecoverOffsitePassword_EachSituationGetsItsOwnStatus`. + An older controller maps the unknown 424 to its neutral „we do not know why" sentence. - **The „ring 1" label after a boot:** in the first second after a reboot the agent has not fetched the hub's block, and the after-boot kernel reports (`judging`, `good`, `revert`, `fell_back`…) said ring 1 on a ring-0 box (demo-felhom, 2026-10-08 night). Now they read the fetched block, else the block the daemon saved on disk before the reboot (R-866), diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 40e82cb..5bd1c84 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -1904,11 +1904,15 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St return nil, 0, ferr } out := make([]escrow.RetainedBlob, 0, len(resp.Packages)) + // R-304: every package the hub holds and the code will NOT be tried against — no key material, + // over the hub's cap, or malformed here. A refusal may call the code wrong only when this is 0. + withheld := resp.UnopenableCount + resp.TruncatedCount for _, p := range resp.Packages { blob, derr := base64.StdEncoding.DecodeString(p.IdentityEscrowB64) if derr != nil || len(blob) == 0 { // One malformed package must not sink the rest — the customer's code may open a // later one, and a skipped entry is strictly better than a refusal we cannot justify. + withheld++ continue } out = append(out, escrow.RetainedBlob{ @@ -1918,7 +1922,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St Index: p.Index, }) } - return out, resp.UnopenableCount, nil + return out, withheld, nil }, } srv, err := localapi.NewServer(localapi.Options{ diff --git a/internal/escrow/recover.go b/internal/escrow/recover.go index a259e6a..43575f3 100644 --- a/internal/escrow/recover.go +++ b/internal/escrow/recover.go @@ -59,8 +59,26 @@ var ( // It carries no material and no code: only WHICH earlier package opened, by its supersession date, // which is the one fact the customer needs to recognise it. ErrCodeOpensRetained = errors.New("escrow: the recovery code did not open the CURRENT sealed package, but it DID open a retained earlier one") + // ErrRetainedUnchecked — the code did NOT open the current package, and NOT every earlier package the hub + // holds for this host was tried (R-304, 2026-10-08): the retained list could not be fetched, the hub + // withheld rows (over its serve cap, or rows with no key material), a package was malformed, or the + // attempt cap stopped the loop. So „the code is wrong" is NOT known — it may be right for a package + // nobody tried. Distinct from a mistype for exactly the R-224 reason: never accuse the customer of + // something we did not check. + ErrRetainedUnchecked = errors.New("escrow: the recovery code did not open the current sealed package, and some earlier packages were NOT checked") ) +// RetainedUncheckedError wraps ErrRetainedUnchecked with how many earlier packages went unchecked +// (-1 = unknown: the retained list itself could not be read). No secret. +type RetainedUncheckedError struct { + Unchecked int +} + +func (e *RetainedUncheckedError) Error() string { + return fmt.Sprintf("%s (unchecked=%d)", ErrRetainedUnchecked.Error(), e.Unchecked) +} +func (e *RetainedUncheckedError) Unwrap() error { return ErrRetainedUnchecked } + // RetainedMatch says which retained package a code opened. Returned inside RetainedOpenedError; it // carries no secret — not the code, not the bundle, not the repository password. type RetainedMatch struct { @@ -102,8 +120,10 @@ type RetainedBlob struct { } // RetainedFetcher yields this host's RETAINED sealed packages, newest-superseded first. An empty -// slice is a clean "none". R-311. -type RetainedFetcher func(ctx context.Context) (blobs []RetainedBlob, unopenable int, err error) +// slice is a clean "none". R-311. `withheld` counts the earlier packages the hub holds that are NOT in +// blobs — rows with no key material, rows over the hub's serve cap, and packages dropped as malformed +// (R-304): each is a package the code was never tried against. +type RetainedFetcher func(ctx context.Context) (blobs []RetainedBlob, withheld int, err error) // OffsiteKeyRecoverer is the assembled links 6→8. Construct it with a fetcher; call it with R. type OffsiteKeyRecoverer struct { @@ -154,9 +174,14 @@ func (r OffsiteKeyRecoverer) RecoverOffsiteRepoPassword(ctx context.Context, rec // from the unwrap alone; the only way to tell is to try. Until this existed nobody tried, and // the screen said so out loud ("innen nem tudjuk megkülönböztetni őket") — a true sentence // about our own incuriosity, read by the customer as a statement about their code. - if m, ok := r.tryRetained(ctx, recoveryCode); ok { + m, ok, unchecked := r.tryRetained(ctx, recoveryCode) + if ok { return "", &RetainedOpenedError{Match: m} } + // R-304: only when EVERY earlier package the hub holds was tried may this stay a wrong code. + if unchecked != 0 { + return "", &RetainedUncheckedError{Unchecked: unchecked} + } return "", err // the fail-closed "the recovery code did not unwrap…" message; no secret in it } if bundle.ResticRepoPassword == "" { @@ -173,23 +198,38 @@ func (r OffsiteKeyRecoverer) RecoverOffsiteRepoPassword(ctx context.Context, rec // function breaking is the behaviour we had before it existed. // // NOTHING IS LOGGED HERE and no return value carries the code, a bundle or a password. -func (r OffsiteKeyRecoverer) tryRetained(ctx context.Context, recoveryCode string) (RetainedMatch, bool) { +// +// R-304 (2026-10-08): it also returns how many earlier packages were NOT tried — `withheld` from the hub, plus +// the ones past the attempt cap — or -1 when the retained list could not be read at all. A nil FetchRetained +// (an agent wired without the lookup) reports 0: the pre-R-311 refusal, unchanged. +func (r OffsiteKeyRecoverer) tryRetained(ctx context.Context, recoveryCode string) (RetainedMatch, bool, int) { if r.FetchRetained == nil { - return RetainedMatch{}, false + return RetainedMatch{}, false, 0 } - blobs, _, err := r.FetchRetained(ctx) - if err != nil || len(blobs) == 0 { - return RetainedMatch{}, false + blobs, withheld, err := r.FetchRetained(ctx) + if err != nil { + return RetainedMatch{}, false, -1 + } + if withheld < 0 { + withheld = 0 + } + if len(blobs) == 0 { + return RetainedMatch{}, false, withheld } limit := r.MaxRetainedTried if limit <= 0 { limit = defaultMaxRetainedTried } + unchecked := withheld + if len(blobs) > limit { + unchecked += len(blobs) - limit + } for i, rb := range blobs { if i >= limit { break } if len(rb.Blob) == 0 { + unchecked++ continue } bundle, uerr := UnwrapIdentityBundle(ctx, rb.Blob, recoveryCode) @@ -204,7 +244,7 @@ func (r OffsiteKeyRecoverer) tryRetained(ctx context.Context, recoveryCode strin // correct and must be told so — but the history behind it still cannot be reopened, and // saying otherwise would be a promise this path cannot keep. HasResticPassword: bundle.ResticRepoPassword != "", - }, true + }, true, 0 } - return RetainedMatch{}, false + return RetainedMatch{}, false, unchecked } diff --git a/internal/escrow/recover_retained_test.go b/internal/escrow/recover_retained_test.go index 07fb36a..998504b 100644 --- a/internal/escrow/recover_retained_test.go +++ b/internal/escrow/recover_retained_test.go @@ -115,8 +115,9 @@ func TestRecover_WrongCode_StaysAPlainRefusal(t *testing.T) { } } -// FAIL-SAFE — if the retained lookup itself fails, the original refusal must stand UNCHANGED. The -// worst outcome of this feature breaking is the behaviour we had before it. +// FAIL-SAFE — if the retained lookup itself fails, the lookup's error never reaches the customer and no +// retained package is claimed. R-304 (2026-10-08) changed what stands instead: not the wrong-code refusal (the +// earlier packages were never tried, so „wrong" is not known) but ErrRetainedUnchecked with Unchecked = -1. // // RED-PROOF: make tryRetained propagate the fetch error instead of returning false → the customer // gets a new, unexplained failure mode → this FAILS. @@ -140,6 +141,10 @@ func TestRecover_RetainedFetchFails_OriginalRefusalStands(t *testing.T) { if containsStr(err.Error(), "hub exploded") { t.Error("the retained-lookup failure leaked into the customer-facing refusal — it must be silent") } + var ue *RetainedUncheckedError + if !errors.As(err, &ue) || ue.Unchecked != -1 { + t.Errorf("err = %v, want RetainedUncheckedError{-1} — the earlier packages were never tried (R-304)", err) + } } // A nil FetchRetained keeps the pre-R-311 behaviour EXACTLY. An agent wired without it must be @@ -228,3 +233,72 @@ func containsStr(hay, needle string) bool { return false })() } + +// ── R-304 (2026-10-08) — „wrong code" only when every earlier package was tried ────────────────────── +// +// The consequence asserted: is the refusal a WRONG CODE (the only error the screen may answer with „check your +// typing")? It may be only when the hub withheld nothing and every served package was tried. +// +// RED-PROOF: make tryRetained return 0 for `unchecked` → the three „unchecked" cases return the plain refusal +// → they FAIL; the all-tried case keeps passing. +func TestR304_WrongCodeOnlyWhenEveryEarlierPackageWasTried(t *testing.T) { + ensureAge(t) + current := sealBundle(t, IdentityBundle{ResticRepoPassword: "4444567890abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}, testR) + other := sealBundle(t, IdentityBundle{ResticRepoPassword: "5555567890abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}, testR) + const code = "a code that opens nothing whatsoever in this test" + cases := []struct { + name string + blobs int + withheld int + limit int + unchecked int // 0 = must be a plain wrong code + }{ + {"all tried, nothing withheld", 2, 0, 6, 0}, + {"the hub withheld rows (no key material / over its cap)", 1, 3, 6, 3}, + {"only withheld rows, nothing served", 0, 2, 6, 2}, + {"the attempt cap stopped the loop", 5, 0, 2, 3}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + blobs := make([]RetainedBlob, 0, c.blobs) + for i := 0; i < c.blobs; i++ { + blobs = append(blobs, RetainedBlob{Blob: other, SupersededAt: "2026-08-01 00:00:00", Index: i}) + } + _, err := OffsiteKeyRecoverer{ + Fetch: fetcherFor(current), + FetchRetained: func(context.Context) ([]RetainedBlob, int, error) { + return blobs, c.withheld, nil + }, + MaxRetainedTried: c.limit, + }.RecoverOffsiteRepoPassword(context.Background(), code) + if err == nil { + t.Fatal("a code that opens nothing succeeded") + } + var ue *RetainedUncheckedError + isUnchecked := errors.As(err, &ue) + if c.unchecked == 0 { + if isUnchecked { + t.Fatalf("every earlier package was tried — this IS a wrong code, got %v", err) + } + return + } + if !isUnchecked || ue.Unchecked != c.unchecked || !errors.Is(err, ErrRetainedUnchecked) { + t.Fatalf("err = %v, want RetainedUncheckedError{%d} — never call the code wrong when packages were not tried", err, c.unchecked) + } + }) + } +} + +// A malformed (empty) served package counts as not tried. +func TestR304_EmptyServedPackageCountsAsUnchecked(t *testing.T) { + ensureAge(t) + current := sealBundle(t, IdentityBundle{ResticRepoPassword: "6666567890abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}, testR) + _, err := OffsiteKeyRecoverer{ + Fetch: fetcherFor(current), + FetchRetained: retainedFetcherFor(RetainedBlob{Blob: nil, SupersededAt: "2026-08-01 00:00:00"}), + }.RecoverOffsiteRepoPassword(context.Background(), testR2) + var ue *RetainedUncheckedError + if !errors.As(err, &ue) || ue.Unchecked != 1 { + t.Fatalf("err = %v, want RetainedUncheckedError{1}", err) + } +} diff --git a/internal/localapi/escrow_recover.go b/internal/localapi/escrow_recover.go index 9cca4e9..a548c89 100644 --- a/internal/localapi/escrow_recover.go +++ b/internal/localapi/escrow_recover.go @@ -127,6 +127,22 @@ func (s *Server) handleRecoverOffsitePassword(w http.ResponseWriter, r *http.Req "retained_has_restic_pw": match.HasResticPassword, }, "the recovery code is correct, but it belongs to an EARLIER sealed package (superseded "+match.SupersededAt+"), not the one currently held") + // ── R-304 (2026-10-08) — NOT EVERY EARLIER PACKAGE WAS CHECKED. ───────────────────────── + // + // The current package refused the code, no retained package opened it — and at least one earlier + // package the hub holds was never tried (or the list could not be read). Saying „the code is + // wrong" here would claim a check that did not happen. 424 (Failed Dependency): the verdict + // depends on packages we could not try. The controller classifies on the status, never on this + // sentence; an older controller maps an unknown status to its neutral „we do not know why". + case errors.Is(err, escrow.ErrRetainedUnchecked): + n := -1 + var ue *escrow.RetainedUncheckedError + if errors.As(err, &ue) { + n = ue.Unchecked + } + s.logger.Warn("local-api: offsite key recovery: the code did not open the current package and earlier packages were NOT all checked — not reported as a wrong code (R-304)", "vmid", vmid, "unchecked", n) + writeStatus(w, http.StatusFailedDependency, false, map[string]any{"older_unchecked": n}, + "the recovery code did not open the current sealed package, and earlier packages the hub holds were not all checked — the code may belong to one of them; nothing was written") case errors.Is(err, escrow.ErrNoResticPassword): s.logger.Warn("local-api: offsite key recovery: the bundle opened but predates the repository-password field", "vmid", vmid) writeErr(w, http.StatusConflict, "the recovery code opened the bundle, but it carries NO offsite repository password (sealed before that field existed; it cannot be retro-fitted)") diff --git a/internal/localapi/escrow_recover_class_test.go b/internal/localapi/escrow_recover_class_test.go index 861e479..f3978b5 100644 --- a/internal/localapi/escrow_recover_class_test.go +++ b/internal/localapi/escrow_recover_class_test.go @@ -54,6 +54,13 @@ func TestRecoverOffsitePassword_EachSituationGetsItsOwnStatus(t *testing.T) { wantStatus: 404, mustNotSay: []string{"did not open"}, }, + { + // R-304: earlier packages were not all tried — never „did not open the sealed bundle" (the wrong-code words). + name: "earlier packages not all checked — not a wrong code", + err: &escrow.RetainedUncheckedError{Unchecked: 2}, + wantStatus: 424, + mustNotSay: []string{"did not open the sealed bundle", "could not be fetched"}, + }, { name: "the bundle predates the repository-password field", err: escrow.ErrNoResticPassword,