diff --git a/CHANGELOG.md b/CHANGELOG.md index 79e0ab2..fa507ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +## Unreleased (2026-10-28 night) — R-1254 (the ceremony half, review round 2 HUNT-3 H2): the agent asks the hub which package it holds before it words a failed upload — commit only; the next agent release (needs the operator's vouch) + +- **The defect:** the claim's (R-978) and the confirm's (R-1151) package upload can lose its answer, or answer late + (30 s), AFTER the hub stored the new package. The agent then answered 502 „the previous code still works" — false: the + hub's current package opens only with the NEW code. +- **Now (`internal/localapi/escrow_ceremony.go`):** after a failed upload the agent asks the hub (`HubHolds`, 15 s, + outside the lock, the slot still marked claiming): the hub holds THIS package → a success (the code is handed out / + the confirm completes; a WARN line names the lost answer); the hub holds another package → 502 and the old sentence, + now true; the hub cannot be asked → **504** with a sentence that promises nothing about either code, and the wait goes + on (a retry decides). `cmd/felhom-agent`: `HubHolds` compares the pending package's identity blob with the one the hub + serves back (`GET /api/v1/hosts//escrow`, the read-back the agent already uses; ciphertext only, nothing logged); + a package without an identity blob cannot be compared → unknown. Tests `TestR1254_*` (localapi, red before the fix: + 502 while the hub held the new package) and `TestR1254_HubHoldsComparesTheIdentityBlob` (red-proved by mutation). +- Needs controller ≥ the R-1254 controller commit (2026-10-28 night) to word 504 for the household; an older controller + shows its generic „A kód lekérése nem sikerült." (claim) / its 502 sentence (confirm) for it. + ## v0.167.0 (2026-10-27) — the BYO SSH clean-up in the SAME update (R-1285); a reinstalled box's whole-guest tier is due at once (R-1232); Go 1.27.3 (R-972) — RELEASED (release-agent.sh from a clean clone; agent sha256 28bdd282…d826f0, bundle 9d034416…ed1268; signed updates committed on demo-felhom, Tester 1, demo-hp; VOUCHED by the operator 2026-10-27, Day-0 pin set) - **R-972: Go 1.27.3** (`go.mod`; released 2026-10-19 with a security fix to `crypto/x509`). `govulncheck` v1.8.0 before: diff --git a/cmd/felhom-agent/escrow_r1254_test.go b/cmd/felhom-agent/escrow_r1254_test.go new file mode 100644 index 0000000..47103f0 --- /dev/null +++ b/cmd/felhom-agent/escrow_r1254_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// R-1254 H2, the production question: does the hub's current package equal the pending upload body? Compared by the +// identity blob the hub serves back. A package without an identity, or a hub that cannot answer, is never „held". +// RED-PROOF: make hubHoldsEscrowPackage return true when the hub has any package → the „another package" case +// passes as held → FAILS. +func TestR1254_HubHoldsComparesTheIdentityBlob(t *testing.T) { + body := func(id string) []byte { + b, _ := json.Marshal(escrowUploadRequest{BlobB64: base64.StdEncoding.EncodeToString([]byte("k")), + IdentityBlobB64: base64.StdEncoding.EncodeToString([]byte(id))}) + return b + } + hubWith := func(id string, present bool, err error) func(context.Context) (*hub.IdentityEscrowResponse, error) { + return func(context.Context) (*hub.IdentityEscrowResponse, error) { + if err != nil { + return nil, err + } + return &hub.IdentityEscrowResponse{Present: present, IdentityEscrowB64: base64.StdEncoding.EncodeToString([]byte(id))}, nil + } + } + ctx := context.Background() + if held, err := hubHoldsEscrowPackage(ctx, hubWith("NEW", true, nil), body("NEW")); err != nil || !held { + t.Fatalf("the hub holds the new package: held=%v err=%v", held, err) + } + if held, err := hubHoldsEscrowPackage(ctx, hubWith("OLD", true, nil), body("NEW")); err != nil || held { + t.Fatalf("the hub holds another package: held=%v err=%v", held, err) + } + if held, err := hubHoldsEscrowPackage(ctx, hubWith("", false, nil), body("NEW")); err != nil || held { + t.Fatalf("the hub holds nothing: held=%v err=%v", held, err) + } + if _, err := hubHoldsEscrowPackage(ctx, hubWith("", false, errors.New("unreachable")), body("NEW")); err == nil { + t.Fatal("a hub that cannot answer must be an error (unknown), never „not held\"") + } + noID, _ := json.Marshal(escrowUploadRequest{BlobB64: "aw=="}) + if _, err := hubHoldsEscrowPackage(ctx, hubWith("NEW", true, nil), noID); err == nil { + t.Fatal("a package without an identity blob cannot be compared — an error") + } +} diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 11f6cb4..3c9e0cf 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -1250,6 +1250,14 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int // R-978: the ceremony hands its package back; the claim uploads it with the same PUT the root // subprocess used, under the same per-host key, before the code is handed out. Upload: func(ctx context.Context, pkg []byte) error { return putEscrowBody(ctx, cfg, pkg) }, + // R-1254 H2: after a failed upload, the hub is asked which package it holds before the household is told + // anything — the same per-host GET the identity read-back uses. + HubHolds: func(ctx context.Context, pkg []byte) (bool, error) { + if client == nil { + return false, fmt.Errorf("hub client not configured") + } + return hubHoldsEscrowPackage(ctx, client.FetchIdentityEscrow, pkg) + }, DRConfigured: func() bool { if drConfigured != nil { return drConfigured() @@ -3366,6 +3374,32 @@ func escrowUploadBody(res escrow.CreateResult, resticPwSHA256 string) []byte { // putEscrowBody PUTs an escrow upload body to this host's hub record (the per-host key). Used by the // text-mode ceremony at once, and by the daemon when the customer claims the code (R-978). +// hubHoldsEscrowPackage (R-1254 H2) reports whether the hub's CURRENT package is the upload body pkg, by its identity +// blob — the part the hub serves back (GET /api/v1/hosts//escrow). Ciphertext only; nothing is logged. A package +// without an identity blob cannot be compared: an error (the caller then promises nothing). +func hubHoldsEscrowPackage(ctx context.Context, fetch func(context.Context) (*hub.IdentityEscrowResponse, error), pkg []byte) (bool, error) { + var up escrowUploadRequest + if err := json.Unmarshal(pkg, &up); err != nil { + return false, fmt.Errorf("reading the pending package: %w", err) + } + mine, err := base64.StdEncoding.DecodeString(up.IdentityBlobB64) + if err != nil || len(mine) == 0 { + return false, fmt.Errorf("the pending package carries no identity blob to compare") + } + resp, err := fetch(ctx) + if err != nil { + return false, err + } + if !resp.Present || resp.IdentityEscrowB64 == "" { + return false, nil + } + theirs, err := base64.StdEncoding.DecodeString(resp.IdentityEscrowB64) + if err != nil { + return false, fmt.Errorf("decoding the hub's package: %w", err) + } + return bytes.Equal(mine, theirs), nil +} + func putEscrowBody(ctx context.Context, cfg config.Config, body []byte) error { if cfg.Hub.URL == "" || cfg.Hub.HostID == "" || cfg.Hub.APIKey == "" { return fmt.Errorf("hub not configured (url/host_id/api_key)") diff --git a/internal/localapi/escrow_ceremony.go b/internal/localapi/escrow_ceremony.go index 36f5d4e..e64e00e 100644 --- a/internal/localapi/escrow_ceremony.go +++ b/internal/localapi/escrow_ceremony.go @@ -72,12 +72,56 @@ type EscrowCeremonyConfig struct { // host's hub record — called by the claim, BEFORE the code is handed out (R-978). nil → a claim of // a pending package is refused (503): a code must never be handed out for a package the hub lacks. Upload func(ctx context.Context, pkg []byte) error + // HubHolds (R-1254 H2) asks the hub whether its CURRENT package is exactly pkg — asked after a failed upload, + // whose answer may have been lost AFTER the hub stored it. nil or an error → the outcome is unknown. + HubHolds func(ctx context.Context, pkg []byte) (bool, error) } // escrowUploadTimeout bounds the claim-time package upload (one small PUT; the subprocess's whole // ceremony incl. its upload was measured at ~2.4 s). const escrowUploadTimeout = 30 * time.Second +// escrowHoldTimeout bounds the hub question after a failed upload (R-1254 H2). +const escrowHoldTimeout = 15 * time.Second + +// uploadOutcome is what a claim or confirm upload did, as the HUB tells it (R-1254 H2): an upload whose answer was lost +// or late may have been stored all the same. +type uploadOutcome int + +const ( + uploadStored uploadOutcome = iota // the hub holds the new package + uploadNotStored // the hub answered and holds another package: the previous code still works + uploadUnknown // the hub cannot be asked: nothing may be promised about either code +) + +// escrowUnknownOutcomeMsg is the answer when the hub cannot say which package it holds — never „the previous code +// still works", which is false when the lost answer came after the store. +const escrowUnknownOutcomeMsg = "the hub did not answer in time — it is not known whether it stored the new recovery package; try again (until a try succeeds, do not rely on the previous code alone)" + +// outcomeAfterFailedUpload asks the hub whether it holds pkg after Upload returned an error. Called with the slot +// marked claiming (escrowClaiming), outside escrowMu — it is a network call. +func (s *Server) outcomeAfterFailedUpload(pkg []byte) uploadOutcome { + if s.escrowCeremony.HubHolds == nil { + return uploadUnknown + } + base := s.baseCtx + if base == nil { + base = context.Background() + } + ctx, cancel := context.WithTimeout(base, escrowHoldTimeout) + defer cancel() + held, err := s.escrowCeremony.HubHolds(ctx, pkg) + switch { + case err != nil: + s.logger.Warn("local-api: escrow — the hub cannot say which package it holds after a failed upload (R-1254)", "err", err) + return uploadUnknown + case held: + return uploadStored + default: + return uploadNotStored + } +} + // ceremonyRunner executes the fixed-argv sudo self-invocation. stdout is SECRET-BEARING until // parsed (it carries R inside the JSON object); the caller must zero it. Tests inject canned // spike-shaped output; production is runCeremonySubprocess. @@ -524,15 +568,30 @@ func (s *Server) handleEscrowCeremonyClaim(w http.ResponseWriter, r *http.Reques uctx, cancel := context.WithTimeout(base, escrowUploadTimeout) err := s.escrowCeremony.Upload(uctx, pkg) cancel() + outcome := uploadStored + if err != nil { + outcome = s.outcomeAfterFailedUpload(pkg) // R-1254 H2: the answer may be lost AFTER the store + } s.escrowMu.Lock() s.escrowClaiming = false - if err != nil { + switch outcome { + case uploadNotStored: s.escrowMu.Unlock() s.logger.Warn("local-api: escrow claim — package upload to the hub failed; code kept claimable, previous package stays current (R-978)", "vmid", vmid, "job_id", jobID, "err", err, "duration_ms", time.Since(start).Milliseconds()) writeErr(w, http.StatusBadGateway, "the new recovery package could not be stored on the hub — the previous code still works; try again") return + case uploadUnknown: + s.escrowMu.Unlock() + s.logger.Warn("local-api: escrow claim — upload failed and the hub cannot say which package it holds; code kept claimable (R-1254)", + "vmid", vmid, "job_id", jobID, "err", err, "duration_ms", time.Since(start).Milliseconds()) + writeErr(w, http.StatusGatewayTimeout, escrowUnknownOutcomeMsg) + return + } + if err != nil { + s.logger.Warn("local-api: escrow claim — the upload's answer was lost, but the hub holds the new package; handing out the code (R-1254)", + "vmid", vmid, "job_id", jobID, "err", err) } s.escrowPending = nil if s.escrowJob != nil && s.escrowJob.JobID == jobID { @@ -631,15 +690,30 @@ func (s *Server) handleEscrowCeremonyConfirm(w http.ResponseWriter, r *http.Requ uctx, cancel := context.WithTimeout(base, escrowUploadTimeout) err := s.escrowCeremony.Upload(uctx, pkg) cancel() + outcome := uploadStored + if err != nil { + outcome = s.outcomeAfterFailedUpload(pkg) // R-1254 H2 + } s.escrowMu.Lock() s.escrowClaiming = false - if err != nil { + switch outcome { + case uploadNotStored: s.escrowMu.Unlock() s.logger.Warn("local-api: escrow confirm — package upload to the hub failed; still waiting, previous package stays current (R-1151)", "vmid", vmid, "job_id", jobID, "err", err, "duration_ms", time.Since(start).Milliseconds()) writeErr(w, http.StatusBadGateway, "the new recovery package could not be stored on the hub — the previous code still works; try again") return + case uploadUnknown: + s.escrowMu.Unlock() + s.logger.Warn("local-api: escrow confirm — upload failed and the hub cannot say which package it holds; still waiting (R-1254)", + "vmid", vmid, "job_id", jobID, "err", err, "duration_ms", time.Since(start).Milliseconds()) + writeErr(w, http.StatusGatewayTimeout, escrowUnknownOutcomeMsg) + return + } + if err != nil { + s.logger.Warn("local-api: escrow confirm — the upload's answer was lost, but the hub holds the new package (R-1254)", + "vmid", vmid, "job_id", jobID, "err", err) } s.escrowPending = nil s.wipeEscrowConfirmLocked() diff --git a/internal/localapi/escrow_ceremony_test.go b/internal/localapi/escrow_ceremony_test.go index bb160cd..d5db683 100644 --- a/internal/localapi/escrow_ceremony_test.go +++ b/internal/localapi/escrow_ceremony_test.go @@ -41,9 +41,13 @@ type fakeEscrowHub struct { mu sync.Mutex current []byte uploads int - err error // non-nil → the upload fails, nothing stored - gate chan struct{} // non-nil → the upload blocks until closed - entered chan struct{} // non-nil → closed when an upload starts + err error // non-nil → the upload fails, nothing stored + // R-1254 H2: lost → the upload STORES the package and then fails (the answer is lost on the way back); + // holdsErr → the hub cannot be asked which package it holds. + lost bool + holdsErr error + gate chan struct{} // non-nil → the upload blocks until closed + entered chan struct{} // non-nil → closed when an upload starts } func (f *fakeEscrowHub) upload(ctx context.Context, pkg []byte) error { @@ -61,9 +65,23 @@ func (f *fakeEscrowHub) upload(ctx context.Context, pkg []byte) error { } f.current = append([]byte(nil), pkg...) f.uploads++ + if f.lost { + return context.DeadlineExceeded + } return nil } +// holds answers „is the hub's current package exactly pkg?" — the R-1254 question (production compares the identity +// blob the hub serves back). +func (f *fakeEscrowHub) holds(_ context.Context, pkg []byte) (bool, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.holdsErr != nil { + return false, f.holdsErr + } + return string(f.current) == string(pkg), nil +} + // opensWith reports whether the hub's CURRENT package opens with code r — the customer's question. func (f *fakeEscrowHub) opensWith(r string) bool { f.mu.Lock() @@ -100,6 +118,7 @@ func newEscrowTestServerHub(t *testing.T, run ceremonyRunner, hub *fakeEscrowHub HubConfigured: true, DRConfigured: func() bool { return true }, Upload: hub.upload, + HubHolds: hub.holds, } srv.ceremonyRun = run srv.escrowSudoCheck = func(context.Context) error { return nil } diff --git a/internal/localapi/escrow_r1254_test.go b/internal/localapi/escrow_r1254_test.go new file mode 100644 index 0000000..f9a5bc6 --- /dev/null +++ b/internal/localapi/escrow_r1254_test.go @@ -0,0 +1,98 @@ +package localapi + +import ( + "errors" + "net/http" + "strings" + "testing" +) + +// R-1254 (the ceremony half, review round 2 HUNT-3 H2): the upload's answer can be lost or late (30 s) AFTER the hub +// stored the new package. The household was then told „the previous code still works" — false: the hub now holds the +// package the NEW code opens. The agent asks the hub which package it holds before it words the result: +// - the hub holds the new package → success (the code is handed out / the confirm completes); +// - the hub holds another package → the old sentence, now true; +// - the hub cannot be asked → 504 and a sentence that does NOT promise the previous code; the wait goes on. + +// RED-PROOF: remove the hub question from the claim's failure branch → 502 „the previous code still works" while the +// hub holds the new package → FAILS. +func TestR1254_ALostAnswerAtTheClaimIsASuccessWhenTheHubHoldsTheNewPackage(t *testing.T) { + hub := &fakeEscrowHub{lost: true} + srv, _ := newEscrowTestServerHub(t, sequenceRunner(firstR), hub) + h := srv.Handler() + startAndWait(t, srv, h) + code, body := claimCode(t, h) + if !hub.opensWith(firstR) { + t.Fatal("fixture: the hub must hold the new package (the answer was lost after the store)") + } + if code != http.StatusOK || !strings.Contains(body, firstR) { + t.Fatalf("claim after a lost answer, the hub holding the new package: %d %s — want 200 with the code", code, body) + } + if strings.Contains(body, "previous code still works") { + t.Fatalf("the household was told the previous code still works: %s", body) + } + if st := do(t, h, "GET", "/escrow/ceremony/status", "A", "").Body.String(); !strings.Contains(st, `"uploaded":true`) { + t.Fatalf("the job must read uploaded: %s", st) + } +} + +// The hub cannot be asked: no promise about the previous code, 504, the code stays claimable (a retry decides). +// RED-PROOF: as above → 502 with „the previous code still works" → FAILS. +func TestR1254_AnUnknownOutcomeAtTheClaimPromisesNothing(t *testing.T) { + hub := &fakeEscrowHub{lost: true, holdsErr: errors.New("hub unreachable")} + srv, _ := newEscrowTestServerHub(t, sequenceRunner(firstR), hub) + h := srv.Handler() + startAndWait(t, srv, h) + code, body := claimCode(t, h) + if code != http.StatusGatewayTimeout { + t.Fatalf("claim with an unknown outcome: %d %s — want 504", code, body) + } + if strings.Contains(body, "previous code still works") { + t.Fatalf("an unknown outcome promised the previous code: %s", body) + } + assertNoRIn(t, "unknown-outcome claim payload", body, firstR) + if st := do(t, h, "GET", "/escrow/ceremony/status", "A", "").Body.String(); !strings.Contains(st, `"claimable":true`) { + t.Fatalf("the code must stay claimable: %s", st) + } + // The hub answers again: the retry finds the new package already stored and hands the code out. + hub.mu.Lock() + hub.holdsErr = nil + hub.mu.Unlock() + if code, body := claimCode(t, h); code != http.StatusOK || !strings.Contains(body, firstR) { + t.Fatalf("retry: %d %s", code, body) + } +} + +// The hub holds ANOTHER package (the upload really failed): the old sentence stays, and is now true. +func TestR1254_AFailedUploadTheHubDidNotStoreKeepsTheOldSentence(t *testing.T) { + hub := &fakeEscrowHub{err: errors.New("hub returned HTTP 502")} + srv, _ := newEscrowTestServerHub(t, sequenceRunner(firstR), hub) + h := srv.Handler() + startAndWait(t, srv, h) + if code, body := claimCode(t, h); code != http.StatusBadGateway || !strings.Contains(body, "previous code still works") { + t.Fatalf("a failed upload: %d %s — want 502 and the previous code named", code, body) + } +} + +// The same at the typed-back confirm (R-1151). +// RED-PROOF: remove the hub question from the confirm's failure branch → 502 → FAILS. +func TestR1254_ALostAnswerAtTheConfirmIsASuccessWhenTheHubHoldsTheNewPackage(t *testing.T) { + hub := &fakeEscrowHub{lost: true} + srv, _ := newEscrowTestServerHub(t, sequenceRunner(secondR), hub) + h := srv.Handler() + startAndWait(t, srv, h) + c := claimDeferred(t, h) + if code := confirm(t, h, c.typedBack(t)); code != http.StatusOK || !hub.opensWith(secondR) { + t.Fatalf("confirm after a lost answer: %d (opens: %v) — want 200", code, hub.opensWith(secondR)) + } + hub.mu.Lock() + hub.lost, hub.holdsErr = true, errors.New("hub unreachable") + hub.mu.Unlock() + srv2, _ := newEscrowTestServerHub(t, sequenceRunner(firstR), hub) + h2 := srv2.Handler() + startAndWait(t, srv2, h2) + c2 := claimDeferred(t, h2) + if code := confirm(t, h2, c2.typedBack(t)); code != http.StatusGatewayTimeout { + t.Fatalf("confirm with an unknown outcome: %d — want 504", code) + } +}