R-196 / R-204 item 2: a re-issue no longer marks a healthy escrow stale (hub v0.95.0)
ReissueCredentials marked the escrow stale on every re-issue, on precautionary grounds — the box's re-apply MIGHT mint a fresh repository password. It usually does not. A stale flag withholds restic_pw_sha256 from the ACK, which stops the controller's auto-confirm, which leaves EscrowState pending, which makes OffboxRunnable false: every off-site backup refused on a box whose key was never in doubt — and the customer told to re-run the one ceremony that would have superseded the key just recovered. The case it guessed at is measured elsewhere: the controller's Scenario-F re-check compares the sealed hash against the live repo password on every ACK (and the mark was BLINDING it by emptying that hash), and R-197's offsite_repo_key_changed fires on a proven difference across a supersession. offsite_reissued is unchanged. MarkEscrowStale is kept without a caller so a future EVIDENTIAL writer has the mechanism, with a test pinning it live. TestReissue_InvalidatesEscrow is replaced by its exact inverse.
This commit is contained in:
@@ -204,43 +204,51 @@ func (p *Provisioner) ReissueCredentials(ctx context.Context, customerID, typ st
|
||||
// nobody checked the CAUSE — and it sent two separate investigations the wrong way in one day
|
||||
// (audits/SPIKE-offsite-credential-recovery-2026-08-04.md Q4).
|
||||
//
|
||||
// WHY THE MARK IS STILL MADE, on honest grounds: a re-issue is the operator's response to a
|
||||
// broken delivery, and the box's re-apply may mint a fresh repository password (it does exactly
|
||||
// that whenever `<DataDir>/offbox/repo_password` is absent — the guest-rebuild shape). The mark is
|
||||
// therefore PRECAUTIONARY, not evidential. The evidential signal is R-197's
|
||||
// `offsite_repo_key_changed`, which fires on a measured hash difference at the next ceremony.
|
||||
// KNOWN CONSEQUENCE, left in place deliberately: on the ordinary re-issue shape — a box that still
|
||||
// holds its repo_password file — the password does NOT change, so this marks a healthy escrow
|
||||
// stale and asks the customer for an unnecessary ceremony. R-196 stays OPEN for that; it is a
|
||||
// behaviour change, not a comment fix, and it must not ride a comment-correction commit.
|
||||
// ⚠ THE MARK IS GONE (R-196 / R-204 item 2, hub v0.95.0, 2026-08-05). What used to stand here —
|
||||
// a `MarkEscrowStale` on every re-issue that found an escrow row, plus an `escrow_stale` customer
|
||||
// event — was PRECAUTIONARY, not evidential: it guessed that the box's re-apply MIGHT mint a fresh
|
||||
// repository password. On the ordinary re-issue shape (a box that still holds its
|
||||
// `<DataDir>/offbox/repo_password`) the password does not change, so it marked a HEALTHY escrow
|
||||
// stale. The 2026-08-04 recovery drill (R-201) is what promoted this from a nit to a blocker.
|
||||
//
|
||||
// Mark the escrow stale so the hub stops advertising "ceremony done" and the customer's escrow
|
||||
// wizard is offered again; a fresh ceremony seals the current password and clears the flag. Every
|
||||
// credential change also emits a visible customer event (offsite_reissued always; escrow_stale
|
||||
// only when a blob was invalidated). Best-effort: the password reset already succeeded — a
|
||||
// bookkeeping failure here must not fail it.
|
||||
escrowStaled := false
|
||||
if host, herr := p.Store.GetHostByCustomer(customerID); herr == nil && host != nil {
|
||||
if esc, eerr := p.Store.GetHostEscrow(host.HostID); eerr == nil && esc != nil {
|
||||
if serr := p.Store.MarkEscrowStale(host.HostID); serr != nil {
|
||||
p.logf("[offsite] WARN mark-escrow-stale for %s: %v", customerID, serr)
|
||||
} else {
|
||||
escrowStaled = true
|
||||
}
|
||||
}
|
||||
}
|
||||
// WHAT THE MARK ACTUALLY DID, mechanically, because "it asked for an unnecessary ceremony"
|
||||
// understates it by a lot:
|
||||
// 1. `stale_at` set → `GetEscrowStatusForCustomer` WITHHOLDS `restic_pw_sha256` from the report
|
||||
// ACK (store.go, the v0.57.0 rule).
|
||||
// 2. With no hash, the controller's SLICE-3 auto-confirm cannot flip pending→escrowed
|
||||
// (report.EscrowAutoConfirmer.Reconcile returns early on an empty hash).
|
||||
// 3. `OffboxRunnable() = OffboxConfigured() && EscrowState=="escrowed"` → EVERY off-site backup
|
||||
// is refused, indefinitely, on a box whose key was never in doubt.
|
||||
// 4. The customer is told to re-run the recovery ceremony — which mints a NEW recovery code and
|
||||
// supersedes the sealed blob. During a recovery that is the one act that would have destroyed
|
||||
// the key just recovered.
|
||||
// A precautionary flag that stops the data-protection it is guarding is not conservative.
|
||||
//
|
||||
// WHY REMOVING IT LEAVES NO GAP — the case it guessed at is MEASURED elsewhere, and better:
|
||||
// • Continuous, box-side: the controller compares the ACK's sealed hash against its CURRENT
|
||||
// local repo password on EVERY report ACK (`reconcileEscrowed`, the Scenario-F re-check). In
|
||||
// the guest-rebuild shape — the only shape where a re-issue is followed by a fresh repository
|
||||
// password — that comparison mismatches within one report cycle and raises the stale card plus
|
||||
// the „create a new recovery code" CTA. It is a measurement, not a guess.
|
||||
// AND THE MARK WAS BLINDING IT: by emptying the hash (step 1 above) it removed the very value
|
||||
// that comparison needs, so the box could only report the hash-LESS reason, which is false.
|
||||
// • Edge-triggered, hub-side: R-197's `offsite_repo_key_changed` fires on a proven hash
|
||||
// difference across a supersession (api.maybeEmitRepoKeyChanged) and pages the operator.
|
||||
//
|
||||
// DISAGREEMENT RECORDED, per the R-96 standing rule: the task's Scenario D asks that a real key
|
||||
// change "marks the escrow stale". It must NOT, and nothing here was changed to make it: the hub
|
||||
// learns of a real change at the moment a supersession SEALS THE NEW PASSWORD, i.e. when the escrow
|
||||
// is freshest. Marking it stale there would ask for a ceremony to fix the ceremony that just ran.
|
||||
// The correct consequence at that instant is the operator alarm, which is what R-197 does.
|
||||
//
|
||||
// `offsite_reissued` is UNCHANGED and still always fires — the customer must still learn that the
|
||||
// credential moved. Best-effort: the password reset already succeeded, so a bookkeeping failure
|
||||
// here must not fail it.
|
||||
if _, serr := p.Store.SaveEvent(customerID, "offsite_reissued", "info",
|
||||
"Az offsite (házon kívüli) mentési hozzáférést újra kiadtuk — az új egyszeri jelszót a vezérlő a következő frissítéskor átveszi.",
|
||||
"", "hub"); serr != nil {
|
||||
p.logf("[offsite] WARN save offsite_reissued event for %s: %v", customerID, serr)
|
||||
}
|
||||
if escrowStaled {
|
||||
if _, serr := p.Store.SaveEvent(customerID, "escrow_stale", "warning",
|
||||
"A helyreállítási kulcs-letét elavult az offsite jelszó cseréje miatt — futtasd le újra a helyreállítási szertartást (Biztonsági mentés → Helyreállítás).",
|
||||
"", "hub"); serr != nil {
|
||||
p.logf("[offsite] WARN save escrow_stale event for %s: %v", customerID, serr)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -211,12 +211,27 @@ func TestReissue_RefusesAmbiguousLookup(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// v0.57.0 (2.3, escrow honesty) — re-issuing offsite credentials INVALIDATES the key-escrow blob:
|
||||
// the blob sealed the OLD repo password, so a recovery code minted against it would decrypt a
|
||||
// password that no longer opens the repo. RED-PROOF (Scenario C): on pre-fix code (no MarkEscrowStale
|
||||
// in ReissueCredentials + no stale plumbing) the hub keeps advertising the escrow as current after a
|
||||
// re-issue and keeps serving its restic-hash for auto-confirm — this test asserts it does NEITHER.
|
||||
func TestReissue_InvalidatesEscrow(t *testing.T) {
|
||||
// SCENARIO C (R-196 / R-204 item 2, hub v0.95.0) — re-issuing the offsite PROVIDER credential must
|
||||
// LEAVE A HEALTHY ESCROW ALONE.
|
||||
//
|
||||
// THIS TEST REPLACES `TestReissue_InvalidatesEscrow`, which asserted the opposite and was wrong on a
|
||||
// false premise: a re-issue resets the storage-provider account password and cannot touch the restic
|
||||
// REPOSITORY password, which is generated on the box and never leaves it except sealed under R. The
|
||||
// old assertion looked right because the EFFECT it checked (a stale escrow) was real — nobody checked
|
||||
// the CAUSE. Kept as one test rather than deleted so the inversion is visible in `git log -L`.
|
||||
//
|
||||
// WHAT IT ASSERTS IS THE CONSEQUENCE, not the mechanism: not "MarkEscrowStale was not called" but
|
||||
// "the ACK still carries the hash", because the hash is what the controller's auto-confirm needs and
|
||||
// its absence is what blocked every off-site backup during the 2026-08-04 drill.
|
||||
//
|
||||
// RED-PROOF: restore the marking — put back
|
||||
//
|
||||
// if host, _ := p.Store.GetHostByCustomer(customerID); host != nil {
|
||||
// if esc, _ := p.Store.GetHostEscrow(host.HostID); esc != nil { p.Store.MarkEscrowStale(host.HostID) }
|
||||
// }
|
||||
//
|
||||
// in ReissueCredentials → es.Stale becomes true, the hash is withheld, and this test fails on both.
|
||||
func TestReissue_DoesNotMarkAHealthyEscrowStale(t *testing.T) {
|
||||
p, _, st := newTestProvisioner(t)
|
||||
const cust = "cust-esc"
|
||||
if _, err := p.ProvisionOffsite(context.Background(), cust, Input{Enabled: true, Type: "shared", QuotaGB: 10}); err != nil {
|
||||
@@ -229,7 +244,6 @@ func TestReissue_InvalidatesEscrow(t *testing.T) {
|
||||
if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("opaque-blob"), "SHA256:fp", "zero_knowledge", "2026-07-16T00:00:00Z", "OLDHASH"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Before re-issue: current escrow — the hub serves the sealed hash and is NOT stale.
|
||||
es, err := st.GetEscrowStatusForCustomer(cust)
|
||||
if err != nil || es == nil {
|
||||
t.Fatalf("escrow status (before): %v", err)
|
||||
@@ -238,25 +252,56 @@ func TestReissue_InvalidatesEscrow(t *testing.T) {
|
||||
t.Fatalf("pre-reissue escrow must be current: stale=%v hash=%q", es.Stale, es.ResticPwSHA256)
|
||||
}
|
||||
|
||||
// Re-issue the offsite credential — the repo password just changed under the sealed blob.
|
||||
// Re-issue the offsite PROVIDER credential. The box still holds its repo_password file, so the
|
||||
// repository password does NOT change — this is the ordinary re-issue shape.
|
||||
if err := p.ReissueCredentials(context.Background(), cust, "shared"); err != nil {
|
||||
t.Fatalf("reissue: %v", err)
|
||||
}
|
||||
|
||||
// After: the escrow is STALE and the restic-hash is WITHHELD (no auto-confirm against a dead key).
|
||||
es, err = st.GetEscrowStatusForCustomer(cust)
|
||||
if err != nil || es == nil {
|
||||
t.Fatalf("escrow status (after): %v", err)
|
||||
}
|
||||
if !es.Stale {
|
||||
t.Fatal("RED-PROOF: escrow must be STALE after an offsite re-issue (the hub was advertising ceremony-done against a key the repo no longer accepts)")
|
||||
if es.Stale {
|
||||
t.Fatal("a re-issue marked a HEALTHY escrow stale — this blocks every off-site backup and asks the customer for a ceremony that would supersede a good key (R-196)")
|
||||
}
|
||||
if es.ResticPwSHA256 != "" {
|
||||
t.Fatalf("a stale escrow must WITHHOLD the restic hash to inhibit auto-confirm, got %q", es.ResticPwSHA256)
|
||||
if es.ResticPwSHA256 != "OLDHASH" {
|
||||
t.Fatalf("the sealed hash must keep flowing to the ACK so the controller can auto-confirm; got %q", es.ResticPwSHA256)
|
||||
}
|
||||
|
||||
// The customer is still TOLD the credential moved — removing the false alarm must not remove the
|
||||
// true notice. `offsite_reissued` is untouched by this change.
|
||||
ev, eerr := st.GetLatestEventByType(cust, "offsite_reissued")
|
||||
if eerr != nil || ev == nil {
|
||||
t.Fatalf("offsite_reissued must still fire on every re-issue (err=%v ev=%+v)", eerr, ev)
|
||||
}
|
||||
// …and the false alarm is gone.
|
||||
if bad, _ := st.GetLatestEventByType(cust, "escrow_stale"); bad != nil {
|
||||
t.Fatalf("a re-issue must not raise escrow_stale on a healthy escrow: %+v", bad)
|
||||
}
|
||||
}
|
||||
|
||||
// The other half of Scenario C: the stale MECHANISM is intact and still does its job when something
|
||||
// legitimately sets it. Without this, "nothing marks stale any more" could decay into "stale no longer
|
||||
// works", and the next evidential caller would ship inert (the seam-built-but-never-wired shape).
|
||||
func TestEscrowStaleMechanism_StillWithholdsAndClears(t *testing.T) {
|
||||
_, _, st := newTestProvisioner(t)
|
||||
const cust = "cust-stale-mech"
|
||||
if err := st.UpsertHost(&store.Host{HostID: cust + "-01", CustomerID: cust, APIKey: "k"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("blob"), "SHA256:fp", "zero_knowledge", "2026-07-16T00:00:00Z", "OLDHASH"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := st.MarkEscrowStale(cust + "-01"); err != nil {
|
||||
t.Fatalf("MarkEscrowStale: %v", err)
|
||||
}
|
||||
es, _ := st.GetEscrowStatusForCustomer(cust)
|
||||
if es == nil || !es.Stale || es.ResticPwSHA256 != "" {
|
||||
t.Fatalf("a marked escrow must read stale AND withhold the hash: %+v", es)
|
||||
}
|
||||
// A fresh ceremony (new blob sealing the new password) clears stale + serves the new hash.
|
||||
if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("opaque-blob-2"), "SHA256:fp", "zero_knowledge", "2026-07-16T01:00:00Z", "NEWHASH"); err != nil {
|
||||
if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("blob-2"), "SHA256:fp", "zero_knowledge", "2026-07-16T01:00:00Z", "NEWHASH"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
es, _ = st.GetEscrowStatusForCustomer(cust)
|
||||
|
||||
Reference in New Issue
Block a user