amadan
#162: a refusal is not a withdrawal — the siblings half of the fail-safe rails
f0ea0386 made the cross-record rails fail-safe on the reconcile side: a host claimed by more than one RecordKey is contested, vouches for nothing, and a refusal never revokes. The siblings writer held only half of that. siblingsWriter.pass held a directory when that directory's own route host was contested. The OTHER side of a refused pairing has an uncontested host and nothing built for it, which is indistinguishable from 'the placement was withdrawn' — so on the replica's box a contested HOME removed the replica's siblings.json, and on the home's box a contested REPLICA rewrote the home's file with an empty replicas list, dropping a live sibling's address and key. Both on the say-so of one record in a prefix the victim cannot see. PairableHome now reports whether the condition that failed was a contested host, buildSiblings hands that reason out for both sides of the pairing, and the writer holds on it: no write, no removal. A genuine withdrawal still removes the file, and a home still opted in still gets the honest rewrite to an empty list. Two more, both about what a half-read pass may say. Reconcile no longer calls OnRecords at all when an account manifest or a record body failed to read: a map missing the victim's home beside a standing shadow of that host is not a stale map, it is one that pairs the shadow, and it wrote a poisoned file the next (properly contested) pass then preserved. And the contested set now travels with the record family instead of being recomputed from it — Reconcile counts claims off the bucket LISTING, buildSiblings recomputed from the DECODED map, so an undecodable or rail-failing shadow contested on one side and not the other. Spec §6 gains the vouching window (which ReconcileConfig.OnRecords already pointed at) and a plainly stated correction: a prefix-scoped writer CAN take a live pair's public host, through the mint path. A plain record in the attacker's own prefix carrying the victim's host and the replica's region mints a real route on that box, ReconcileAliases sweeps the replica alias as 'a real route owns that host', and the box serves the victim's public host from the attacker's process. No rail asks anything of a plain record and nothing checks a host against the manifest domain at mint time. Recorded as a known limit of the mint path, asserted as it stands so closing it is a test failure someone has to read. Tests adopt the third review's two reproductions in full, plus the withdrawal boundary and the blip-plus-shadow case.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8 files changed,
+865
−129
docs/superpowers/specs/2026-09-21-replica-placements-design.md+102 −10internal/host/agent.go+4 −2internal/host/replica_adversarial_test.go+292 −5internal/host/siblings.go+133 −56internal/host/siblings_test.go+42 −24internal/provision/reconcile.go+88 −31internal/provision/reconcile_test.go+1 −1internal/provision/replica_adversarial_test.go+203 −0
diff --git a/docs/superpowers/specs/2026-09-21-replica-placements-design.md b/docs/superpowers/specs/2026-09-21-replica-placements-design.md| index 44dc8192c..b445611ac 100644 |
| --- a/docs/superpowers/specs/2026-09-21-replica-placements-design.md |
| +++ b/docs/superpowers/specs/2026-09-21-replica-placements-design.md |
| @@ -1,6 +1,11 @@ |
| # Replica placements: an account served locally in a second placement |
| -Design record, 2026-09-21, revision 3 (2026-09-22). The platform half of amadan |
| +Design record, 2026-09-21, revision 4 (2026-09-22). Revision 4 corrects two |
| +claims in §6 that were false as written: that a prefix-scoped writer cannot |
| +take a live pair from its owner (it can, through the mint path, which these |
| +rails never touched), and the silence about what a pass that could not read |
| +everything may hand on. Both are now stated where the code points. The |
| +platform half of amadan |
| discussion carlos/platform #162, *A route can carry replica placements, |
| served locally instead of hairpinned*, filed by titogo for its edge-checkout |
| work (titogo `docs/superpowers/specs/2026-09-21-edge-checkout-design.md`, |
| @@ -436,17 +441,43 @@ link is left exactly as it stands, and neither side of the pairing gets a |
| claimants, because two records claiming one instance host is a thing an |
| operator has to see. **No path in either pass clears `replica_of`.** |
| -**What that defends** is an operator's typo and a **writer scoped to its |
| -own prefix**: neither can reach into another account's pairing, take a live |
| -pair away from its owner, or land in another account's `siblings.json` with |
| -a dial address and a pair key. What it is **not** is a tenancy boundary |
| +**What that defends is the PAIRING path, and only it.** An operator's typo |
| +and a writer scoped to its own prefix cannot reach into another account's |
| +*pairing*: they cannot have a replica minted against a home that is not |
| +theirs, cannot take a live `replica_of` away from its owner, and cannot land |
| +in another account's `siblings.json` with a dial address and a pair key. |
| +Every one of those goes through `PairableHome`, and every refusal there is |
| +fail-safe. |
| + |
| +**The mint path is not defended, and a prefix-scoped writer can take a live |
| +pair's public host with it.** This is a known, pre-existing limit — of |
| +instance minting generally, not of these rails — and it is stated here |
| +because it is the obvious thing to assume the rails above cover: |
| + |
| +> A **plain** record (no `role`, no `replicas`, nothing to do with pairing) |
| +> written in the **attacker's own prefix**, carrying the **victim's public |
| +> host** and the **replica's region**, mints a real route on the replica's |
| +> box. No rail asks anything of a plain record — the cross-record rails are |
| +> reached only by `role=replica` — and nothing anywhere checks a record's |
| +> host against its app's manifest domain at mint time. `ReconcileAliases` |
| +> then finds a real route owning that host, logs *"replica alias skipped: a |
| +> real route owns that host"*, and sweeps the victim's alias. The box serves |
| +> the victim's public host from the attacker's process. It happens in both |
| +> sort orders; the victim's `replica_of` survives untouched, which buys |
| +> nothing (`TestReconcileMintPathLetsAPlainShadowTakeTheVictimsHostOnTheReplicasBox`). |
| + |
| +Closing it means a host-against-manifest-domain check at mint time, or the |
| +grant narrowing below. Neither is this branch, and no arrangement of the |
| +pairing rails substitutes for either. |
| + |
| +What these rails are **not**, then, is a tenancy boundary — and least of all |
| against a writer holding bucket-wide `s3:PutObject`, which every region |
| box's IAM role does hold. Such a writer can author a record in the |
| -**victim's own prefix**, where every rail passes trivially, and can as |
| -easily write a plain instance record claiming the victim's host with no |
| -replica involved at all. They can also **deny service** by contesting a |
| -host: one record naming the victim's host freezes that host's pairings — |
| -nothing new mints, nothing converges — until it is removed. That is the |
| +**victim's own prefix**, where every rail passes trivially. They can also |
| +**deny service** by contesting a host: one record naming the victim's host |
| +freezes that host's pairings — nothing new mints, nothing converges, and |
| +neither side's `siblings.json` is written or removed — until it is removed. |
| +That is the |
| deliberate trade, because the alternative is picking a claimant, and every |
| rule for picking one hands the attacker the victim's public host instead of |
| a stalled pairing. **This pass cannot stop them.** The real fix is |
| @@ -457,6 +488,53 @@ console's create door (the manifest domain, the custom-domain claim) and by |
| who can write which bucket prefix; the pass can refuse only what it can |
| see, and what it sees is records, not the authority behind them. |
| +**The vouching window: what a pass that could not read everything may say.** |
| +A refusal is only fail-safe if the pass knows what it failed to see. Two |
| +rules make that true, and both are load-bearing enough that |
| +`ReconcileConfig.OnRecords` names this paragraph. |
| + |
| +*Claims are counted off the bucket LISTING, before any body is decoded.* A |
| +key is a claim on its host whether or not the object behind it reads. Count |
| +from decoded records instead and a claim vanishes the moment its body fails |
| +— which an attacker only has to arrange once: with the victim's record |
| +unreadable for one pass, the attacker's shadow of that host is the *only* |
| +claimant, so nothing is contested and every rail passes against the shadow. |
| +It follows that the contested set must also **travel with** the record map |
| +to any consumer that pairs records, never be recomputed from it: recomputing |
| +is counting from decoded records by another name, and it made the two box |
| +passes disagree about the same host — the reconcile pass refusing to mint |
| +while the siblings writer went on pairing |
| +(`TestReconcileHandsOverTheListingsContestedSet`). |
| + |
| +*A pass whose reads failed publishes no record family at all.* Contested |
| +counting protects the pass that does the counting; it does not make a |
| +half-read map safe to hand to somebody else. The map is what a pairing is |
| +built FROM, and a map missing the victim's home record beside a standing |
| +shadow of that host builds the attacker's pairing. So `OnRecords` is not |
| +called when an account manifest or a record body failed to read, and the |
| +consumer goes on working from the last family that was whole |
| +(`TestReconcileWithholdsTheRecordFamilyWhileAReadFails`). The cost is one |
| +tick of staleness on a bucket blip, which is the same price the removal |
| +phase already pays; without it, a blip plus a standing shadow wrote a |
| +poisoned `siblings.json` and the next pass — seeing a properly contested |
| +host — held the directory, so the poison stayed on disk until someone |
| +deleted a record the victim could not see. |
| + |
| +**And a refusal must never look like a withdrawal.** "Nothing was built for |
| +this host" is how the siblings writer learns a placement is gone and removes |
| +the file, so the two meanings have to stay apart. Holding only the side |
| +whose *own* host is contested is not enough: the other side of a refused |
| +pairing has an uncontested host and nothing built for it, which reads as a |
| +withdrawal. On the replica's box that removed the replica's file; on the |
| +home's box the home's file was rewritten with an empty `replicas` list, |
| +dropping a live sibling's address and key. `PairableHome` reports whether the |
| +condition that failed was a contested host, and the writer holds **both** |
| +sides of such a pairing: no write, no removal, the file left exactly as it |
| +is. A genuine withdrawal — the home dropped the placement, the record was |
| +deleted — still removes it, and a home that is still opted in still gets its |
| +file rewritten with an empty list when its replica record goes, because "no |
| +replicas" and "no file" are different answers (§7). |
| + |
| ## 7. Sibling addressing (Ask 3): no new channel |
| An instance reaches a sibling by dialling **that box's EIP with the real |
| @@ -688,6 +766,20 @@ Each of these fails before its change and passes after: |
| replicas still getting a file. |
| 10. An account with no replicas produces byte-identical edges-file, |
| registry-row and route-table output to before the change. |
| +11. The §6 rails, from the adversarial side, in both sort orders: a replica |
| + vouched for by a shadow never mints, a plain shadow never severs a live |
| + pair, and no path clears `replica_of`. On the siblings side a contested |
| + host gets no file built for it and — the half that came later — |
| + **neither side of a refused pairing loses the file it has**, on the |
| + replica's box when its home is contested and on the home's box when its |
| + replica is, while a genuine withdrawal still removes it. A pass whose |
| + reads failed hands over no record family, and the contested set the |
| + family carries is the listing's, not one recomputed from the decoded |
| + records. |
| +12. The mint-path limit §6 records is asserted as it stands |
| + (`TestReconcileMintPathLetsAPlainShadowTakeTheVictimsHostOnTheReplicasBox`), |
| + so that closing it is a test failure someone has to read rather than a |
| + silent change of posture. |
| ## 12. Scope of the first landing |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 9bb3312d3..84aec5d5b 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -2067,8 +2067,10 @@ func provisionTickConfig(getenv func(string) string) provision.ReconcileConfig { |
| // from this function, and a pass that ran without the callback |
| // would leave the siblings writer working from a stale map. |
| // |
| - // rememberRecords copies before publishing: the map belongs to the |
| - // pass, and the callback runs while reconcileMu is held. |
| + // rememberRecords copies before publishing: the maps belong to the |
| + // pass, and the callback runs while reconcileMu is held. A pass whose |
| + // reads failed does not call it at all (see OnRecords), so the writer |
| + // goes on working from the last family that was whole. |
| OnRecords: rememberRecords, |
| } |
| // The root release public key, same source and same parser as adoptLoop's |
diff --git a/internal/host/replica_adversarial_test.go b/internal/host/replica_adversarial_test.go| index f4b020705..1c191a0b5 100644 |
| --- a/internal/host/replica_adversarial_test.go |
| +++ b/internal/host/replica_adversarial_test.go |
| @@ -1,6 +1,8 @@ |
| package host |
| import ( |
| + "errors" |
| + "io/fs" |
| "os" |
| "path/filepath" |
| "testing" |
| @@ -57,7 +59,7 @@ func TestBuildSiblingsRefusesAContestedHostInEitherSortOrder(t *testing.T) { |
| provision.Record{Host: evilReplica, Account: tc.attacker, App: "tito", Region: "uk", |
| Role: "replica", Home: victim}, |
| ) |
| - got := buildSiblings(all, func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}) |
| + got, _ := buildSiblings(recordSet(all), func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}) |
| // The file the victim's own box would write for the victim's own |
| // instance must never name a stranger. |
| @@ -102,7 +104,7 @@ func TestBuildSiblingsRefusesAContestedReplicaHost(t *testing.T) { |
| provision.Record{Host: victimReplica, Account: "baaa", App: "other", Region: "uk", |
| Role: "replica", Home: "mal.go.tito.io"}, |
| ) |
| - got := buildSiblings(all, func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}) |
| + got, _ := buildSiblings(recordSet(all), func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}) |
| if f, ok := got[victimReplica]; ok { |
| t.Errorf("the contested replica host got a file: %+v — whichever claimant it was written for, "+ |
| @@ -136,7 +138,7 @@ func TestBuildSiblingsStillPairsWhenNoShadowExists(t *testing.T) { |
| provision.Record{Host: victimReplica, Account: "bac", App: "tito", Region: "uk", |
| Role: "replica", Home: victim}, |
| ) |
| - got := buildSiblings(all, func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}) |
| + got, _ := buildSiblings(recordSet(all), func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}) |
| hf, ok := got[victim] |
| if !ok || len(hf.Replicas) != 1 || hf.Replicas[0].Host != victimReplica { |
| @@ -160,7 +162,7 @@ func TestSiblingsPassLeavesTheFileAloneWhenTheHostIsContested(t *testing.T) { |
| reg := openSiblingsRegistry(t, dd) |
| addNestedRoute(t, reg, dd, siblingsHomeHost) |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("first pass: %v", err) |
| } |
| path := filepath.Join(provision.InstanceDir(dd, siblingsHomeHost), siblingsFileName) |
| @@ -175,7 +177,7 @@ func TestSiblingsPassLeavesTheFileAloneWhenTheHostIsContested(t *testing.T) { |
| shadow := provision.Record{Host: siblingsHomeHost, Account: "baaa", App: "tito", Region: "de", |
| Steering: "latency", Replicas: []string{"fr"}} |
| all[provision.RecordKey(shadow.Account, shadow.App, shadow.Host)] = shadow |
| - if err := w.pass(all, siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(all), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("second pass: %v", err) |
| } |
| @@ -188,3 +190,288 @@ func TestSiblingsPassLeavesTheFileAloneWhenTheHostIsContested(t *testing.T) { |
| t.Errorf("the file was rewritten under a contested host:\n before %s\n after %s", before, after) |
| } |
| } |
| + |
| +// The test above held only because the contested host was the one with a |
| +// route on THIS box. The other side of a refused pairing is the half that was |
| +// missed, and it is the half that loses a file: "nothing was built" reads as |
| +// "the placement went away", and the writer tidies up after a withdrawal that |
| +// never happened. |
| +// |
| +// On the REPLICA's box, with the HOME host contested. f0ea0386 removed the |
| +// replica's siblings.json outright — one record in a prefix the victim cannot |
| +// see, and the instance loses the address and key of the home it serves. |
| +func TestSiblingsPassHoldsTheReplicasFileWhenItsHomeIsContested(t *testing.T) { |
| + dd := t.TempDir() |
| + reg := openSiblingsRegistry(t, dd) |
| + addNestedRoute(t, reg, dd, siblingsReplicaHost) |
| + w := &siblingsWriter{reg: reg, dataDir: dd} |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + path := filepath.Join(provision.InstanceDir(dd, siblingsReplicaHost), siblingsFileName) |
| + before, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("first pass wrote no replica file to start from: %v", err) |
| + } |
| + |
| + all := siblingsPair() |
| + shadow := provision.Record{Host: siblingsHomeHost, Account: "baaa", App: "tito", Region: "de", |
| + Steering: "latency", Replicas: []string{"fr"}} |
| + all[provision.RecordKey(shadow.Account, shadow.App, shadow.Host)] = shadow |
| + if err := w.pass(recordSet(all), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + after, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("REVOKED: the replica's %s was removed because its HOME host is contested: %v", |
| + siblingsFileName, err) |
| + } |
| + if string(after) != string(before) { |
| + t.Errorf("the replica's file was rewritten under a contested home:\n before %s\n after %s", before, after) |
| + } |
| +} |
| + |
| +// The same miss from the other end. On the HOME's box, with the REPLICA host |
| +// contested, the home's own file is still BUILT — its own host is claimed by |
| +// one record, so the opt-in loop writes it — but with the refused pairing |
| +// missing from its replicas list. f0ea0386 wrote that file, and the home lost |
| +// the address and key of a replica that is up and serving. |
| +func TestSiblingsPassHoldsTheHomesFileWhenItsReplicaIsContested(t *testing.T) { |
| + dd := t.TempDir() |
| + reg := openSiblingsRegistry(t, dd) |
| + addNestedRoute(t, reg, dd, siblingsHomeHost) |
| + w := &siblingsWriter{reg: reg, dataDir: dd} |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + path := filepath.Join(provision.InstanceDir(dd, siblingsHomeHost), siblingsFileName) |
| + before, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("first pass wrote no home file to start from: %v", err) |
| + } |
| + |
| + // A pairing of the attacker's own, valid on its own terms except that its |
| + // replica host is the victim's. |
| + all := siblingsPair() |
| + for _, r := range []provision.Record{ |
| + {Host: "mal.go.tito.io", Account: "baaa", App: "other", Region: "ie", |
| + Steering: "latency", Replicas: []string{"uk"}}, |
| + {Host: siblingsReplicaHost, Account: "baaa", App: "other", Region: "uk", |
| + Role: "replica", Home: "mal.go.tito.io"}, |
| + } { |
| + all[provision.RecordKey(r.Account, r.App, r.Host)] = r |
| + } |
| + if err := w.pass(recordSet(all), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + after, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("REVOKED: the home's file was removed: %v", err) |
| + } |
| + if string(after) != string(before) { |
| + t.Errorf("REWRITTEN: the home's file lost its real replica because the REPLICA host is "+ |
| + "contested:\n before %s\n after %s", before, after) |
| + } |
| +} |
| + |
| +// Holding must not swallow the real thing it is standing in front of. A |
| +// placement that was genuinely withdrawn — the home dropped it, or the record |
| +// is gone — still takes the file with it, on both boxes. Without this, FIX 1 |
| +// would have turned "leave a contested directory alone" into "never clean up |
| +// anything", and a file naming a sibling that no longer exists is a file the |
| +// app goes on dialling. |
| +func TestSiblingsPassStillRemovesTheFileWhenThePlacementIsWithdrawn(t *testing.T) { |
| + for _, tc := range []struct { |
| + name string |
| + host string |
| + // gone is false for the one withdrawal that is not a removal: a home |
| + // that is still opted in keeps its file when a replica record goes, |
| + // because "no replicas" and "no file" are different answers (spec §7). |
| + // The file is REWRITTEN with an empty list, and that rewrite is a live |
| + // path FIX 1 must not have frozen either. |
| + gone bool |
| + next func() map[string]provision.Record |
| + }{ |
| + { |
| + name: "the home withdrew the placement, on the home's box", |
| + host: siblingsHomeHost, |
| + gone: true, |
| + next: func() map[string]provision.Record { |
| + all := siblingsPair() |
| + home := all[provision.RecordKey("bab", "tito", siblingsHomeHost)] |
| + home.Replicas, home.Steering = nil, "" |
| + all[provision.RecordKey("bab", "tito", siblingsHomeHost)] = home |
| + return all |
| + }, |
| + }, |
| + { |
| + name: "the home withdrew the placement, on the replica's box", |
| + host: siblingsReplicaHost, |
| + gone: true, |
| + next: func() map[string]provision.Record { |
| + all := siblingsPair() |
| + home := all[provision.RecordKey("bab", "tito", siblingsHomeHost)] |
| + home.Replicas, home.Steering = nil, "" |
| + all[provision.RecordKey("bab", "tito", siblingsHomeHost)] = home |
| + return all |
| + }, |
| + }, |
| + { |
| + name: "the replica record was deleted, on the home's box", |
| + host: siblingsHomeHost, |
| + next: func() map[string]provision.Record { |
| + all := siblingsPair() |
| + delete(all, provision.RecordKey("bab", "tito", siblingsReplicaHost)) |
| + return all |
| + }, |
| + }, |
| + { |
| + name: "the home record was deleted, on the replica's box", |
| + host: siblingsReplicaHost, |
| + gone: true, |
| + next: func() map[string]provision.Record { |
| + all := siblingsPair() |
| + delete(all, provision.RecordKey("bab", "tito", siblingsHomeHost)) |
| + return all |
| + }, |
| + }, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + dd := t.TempDir() |
| + reg := openSiblingsRegistry(t, dd) |
| + addNestedRoute(t, reg, dd, tc.host) |
| + w := &siblingsWriter{reg: reg, dataDir: dd} |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + path := filepath.Join(provision.InstanceDir(dd, tc.host), siblingsFileName) |
| + if _, err := os.ReadFile(path); err != nil { |
| + t.Fatalf("first pass wrote no file to withdraw: %v", err) |
| + } |
| + if err := w.pass(recordSet(tc.next()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if tc.gone { |
| + if _, err := os.Stat(path); !errors.Is(err, fs.ErrNotExist) { |
| + raw, _ := os.ReadFile(path) |
| + t.Errorf("a genuine withdrawal left %s behind (%v): %s — the guard that holds a "+ |
| + "contested directory must not hold an honest one", siblingsFileName, err, raw) |
| + } |
| + return |
| + } |
| + f := readSiblingsFile(t, path) |
| + if len(f.Replicas) != 0 { |
| + t.Errorf("the home's file still names %+v after the replica record went — the guard that "+ |
| + "holds a contested directory must not freeze an honest rewrite", f.Replicas) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// The vouching window, and the reason a pass that could not read everything |
| +// must not publish what it did read. |
| +// |
| +// A shadow of the victim's home host stands in the attacker's prefix, with a |
| +// replica of its own. Then the victim's OWN home record fails to read for one |
| +// pass. The decoded map now holds exactly one claimant of that host — the |
| +// attacker's — so every rail passes against it, and f0ea0386 wrote the |
| +// victim's own siblings.json naming the attacker's replica at the attacker's |
| +// address with a live pair key. Worse, the NEXT pass saw both records, called |
| +// the host contested and held the directory, so the poison stayed on disk |
| +// until someone deleted a record the victim cannot see. |
| +// |
| +// Two things now stop it, and this test pins the second. Reconcile no longer |
| +// publishes a family from a pass whose reads failed |
| +// (TestReconcileWithholdsTheRecordFamilyWhileAReadFails), so the writer never |
| +// sees this map at all; and if it somehow did, the contested set travels with |
| +// the map and is counted off the LISTING, where the victim's key is still a |
| +// claim on its host whether or not its body read. Either way the directory is |
| +// held and nothing poisoned is ever written. |
| +func TestSiblingsPassWritesNothingWhileAVictimsRecordBlipsBesideAShadow(t *testing.T) { |
| + const evilReplica = "r9z9.go.tito.io" |
| + dd := t.TempDir() |
| + reg := openSiblingsRegistry(t, dd) |
| + addNestedRoute(t, reg, dd, siblingsHomeHost) |
| + w := &siblingsWriter{reg: reg, dataDir: dd} |
| + eip := func(host string) (string, bool) { |
| + if host == evilReplica { |
| + return "192.0.2.66", true |
| + } |
| + return siblingsEIP(host) |
| + } |
| + if err := w.pass(recordSet(siblingsPair()), eip, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + path := filepath.Join(provision.InstanceDir(dd, siblingsHomeHost), siblingsFileName) |
| + healthy, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("first pass wrote no home file to start from: %v", err) |
| + } |
| + |
| + victimKey := provision.RecordKey("bab", "tito", siblingsHomeHost) |
| + shadow := provision.Record{Host: siblingsHomeHost, Account: "baaa", App: "tito", Region: "de", |
| + Steering: "latency", Replicas: []string{"uk"}} |
| + evil := provision.Record{Host: evilReplica, Account: "baaa", App: "tito", Region: "uk", |
| + Role: "replica", Home: siblingsHomeHost} |
| + // victimHomeRead=false is the blip: the record is still in the bucket and |
| + // the listing still names its key, so it is still a CLAIM on the host — |
| + // only its body is missing from the map. That is the whole distinction the |
| + // listing-counted contested set exists to keep. |
| + withShadow := func(victimHomeRead bool) provision.RecordSet { |
| + all := siblingsPair() |
| + if !victimHomeRead { |
| + delete(all, victimKey) |
| + } |
| + for _, r := range []provision.Record{shadow, evil} { |
| + all[provision.RecordKey(r.Account, r.App, r.Host)] = r |
| + } |
| + return provision.RecordSet{All: all, Contested: map[string][]string{ |
| + siblingsHomeHost: {provision.RecordKey("baaa", "tito", siblingsHomeHost), victimKey}, |
| + }} |
| + } |
| + |
| + // Pass 2: the blip. |
| + if err := w.pass(withShadow(false), eip, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + blip, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("the blip pass removed the home file: %v", err) |
| + } |
| + for _, e := range readSiblingsFile(t, path).Replicas { |
| + if e.Host == evilReplica { |
| + t.Errorf("VOUCHED: the victim's home file names %s at %s with %d key(s) — a stranger's "+ |
| + "process, with leases granted to it", e.Host, e.Addr, len(e.Keys)) |
| + } |
| + } |
| + if string(blip) != string(healthy) { |
| + t.Errorf("the blip pass rewrote the victim's file:\n before %s\n after %s", healthy, blip) |
| + } |
| + |
| + // Pass 3: reads are healthy again, the shadow still standing. The host is |
| + // plainly contested now, so the directory is held — and because the blip |
| + // wrote nothing, there is nothing poisoned left for holding to preserve. |
| + // (The reviewer's reproduction asserted this pass must CHANGE the file, |
| + // which was the right assertion while pass 2 poisoned it: holding a |
| + // poisoned file is how the poison outlived the blip. With nothing written |
| + // on the blip, the same assertion inverts — the file must not change.) |
| + if err := w.pass(withShadow(true), eip, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + held, _ := os.ReadFile(path) |
| + if string(held) != string(healthy) { |
| + t.Errorf("the healthy-with-shadow pass changed the victim's file:\n before %s\n after %s", healthy, held) |
| + } |
| + |
| + // Pass 4: the shadow is gone and the pair is ordinary again. Refusing to |
| + // vouch is not refusing to work. |
| + if err := w.pass(recordSet(siblingsPair()), eip, [][]byte{[]byte("mesh-A")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + restored, _ := os.ReadFile(path) |
| + if string(restored) != string(healthy) { |
| + t.Errorf("the pair did not come back once the shadow went:\n want %s\n got %s", healthy, restored) |
| + } |
| +} |
diff --git a/internal/host/siblings.go b/internal/host/siblings.go| index f25ff945c..64868ddcd 100644 |
| --- a/internal/host/siblings.go |
| +++ b/internal/host/siblings.go |
| @@ -91,32 +91,59 @@ func deriveSiblingKey(meshKey []byte, label string) string { |
| // this instance mine". (Filtering by region would be wrong anyway: a home in |
| // the deployment's home region carries Region "".) |
| // |
| -// `all` is the fleet-wide record map ReconcileConfig.OnRecords hands the |
| +// `rs` is the fleet-wide record family ReconcileConfig.OnRecords hands the |
| // pass — both sides of a pair are in it even though only one of them is in |
| -// this box's scope, which is the whole reason the callback exists. It is |
| +// this box's scope, which is the whole reason the callback exists. rs.All is |
| // keyed by the record's IDENTITY (provision.RecordKey), never by host, and |
| // nothing here may look a record up by host alone: one slot per host is |
| // exactly what let a record written in one account's prefix be read as |
| // another account's home. |
| // |
| -// The returned map IS keyed by host, because a file on a box is addressed by |
| -// host — so no CONTESTED host reaches it. A host two record identities claim |
| -// is a host this pass cannot write a file for: whichever claimant the file |
| -// was built from, the other one's instance is the process that reads it, and |
| -// the file hands out a dial address and a live pair key. Refusing both sides |
| -// is the only answer available, because there is no authority here to pick |
| -// between them (provision.ContestedHosts says why sort order is not one). |
| +// rs.Contested comes with it, counted off the reconcile pass's bucket |
| +// LISTING. It is deliberately NOT recomputed here with ContestedHosts: a key |
| +// is a claim on its host whether or not its body decodes, so a shadow that |
| +// fails to decode (or fails a per-record rail, like an empty channel) is a |
| +// claim the listing counts and a decoded map cannot. Recomputing made the two |
| +// halves of one defence disagree about the same host — the reconcile pass |
| +// refusing to mint while this pass went on pairing. |
| +// |
| +// The returned files map IS keyed by host, because a file on a box is |
| +// addressed by host — so no CONTESTED host reaches it. A host two record |
| +// identities claim is a host this pass cannot write a file for: whichever |
| +// claimant the file was built from, the other one's instance is the process |
| +// that reads it, and the file hands out a dial address and a live pair key. |
| +// Refusing both sides is the only answer available, because there is no |
| +// authority here to pick between them (provision.ContestedHosts says why sort |
| +// order is not one). |
| +// |
| +// The second return is the other half of that refusal, and it exists because |
| +// a refusal must never look like a withdrawal. It maps a host to the reason |
| +// NOTHING was built for it when that reason was a contested host on EITHER |
| +// side of a pairing — the host's own, or the host it pairs with. The writer |
| +// holds such a directory instead of removing or rewriting the file there. |
| +// Without it the two sides of a refused pairing were indistinguishable from |
| +// "the placement is gone": the replica's file was removed because its HOME |
| +// was contested, and the home's file was rewritten with an empty replicas |
| +// list because its REPLICA was — each of them a live pair's file destroyed by |
| +// one record planted in a prefix the victim cannot see. |
| // |
| // eipOf is the edge's home map: the sibling box's EIP, or false for a box |
| // that has published no edges file yet. keys is the mesh key list, current |
| // first. |
| -func buildSiblings(all map[string]provision.Record, eipOf func(host string) (string, bool), keys [][]byte) map[string]siblingsFile { |
| +func buildSiblings(rs provision.RecordSet, eipOf func(host string) (string, bool), keys [][]byte) (map[string]siblingsFile, map[string]string) { |
| out := map[string]siblingsFile{} |
| - // A pure function of `all`, so siblingsWriter.pass asking the same |
| - // question of the same map cannot land on a different answer — the two |
| - // derivations this file warns about elsewhere are two DIFFERENT lookups, |
| - // which this is not. |
| - contested := provision.ContestedHosts(all) |
| + all, contested := rs.All, rs.Contested |
| + // held is host -> why nothing was built for it, contested reasons only. |
| + // Deterministic under map iteration (the smallest reason wins) so a box |
| + // logs the same line pass after pass rather than churning between two |
| + // equally true ones. |
| + held := map[string]string{} |
| + hold := func(host, reason string) { |
| + if prev, ok := held[host]; ok && prev <= reason { |
| + return |
| + } |
| + held[host] = reason |
| + } |
| addr := func(host string) (string, bool) { |
| eip, ok := eipOf(host) |
| if !ok || eip == "" { |
| @@ -146,7 +173,11 @@ func buildSiblings(all map[string]provision.Record, eipOf func(host string) (str |
| // instance: it took the victim's placement, its replica list, and the |
| // account the pair key is derived from. |
| for _, rec := range all { |
| - if rec.Role == "replica" || len(rec.Replicas) == 0 || len(contested[rec.Host]) > 0 { |
| + if rec.Role == "replica" || len(rec.Replicas) == 0 { |
| + continue |
| + } |
| + if by := contested[rec.Host]; len(by) > 0 { |
| + hold(rec.Host, "it is claimed by more than one record ("+strings.Join(by, ", ")+")") |
| continue |
| } |
| out[rec.Host] = siblingsFile{Role: "home", Host: rec.Host, Placement: rec.Region} |
| @@ -173,8 +204,22 @@ func buildSiblings(all map[string]provision.Record, eipOf func(host string) (str |
| // replica's own host as much as its home's, since this file is |
| // written under the replica's host and read by whichever instance |
| // actually holds it. |
| - homeRec, why := provision.PairableHome(all, contested, rec) |
| + homeRec, why, byContest := provision.PairableHome(all, contested, rec) |
| if why != "" { |
| + if byContest { |
| + // BOTH sides, and that is the whole fix. The refused pairing |
| + // is the only thing either file would have been built from, so |
| + // on the replica's box "nothing built" would read as "the |
| + // placement was withdrawn" and remove the file, and on the |
| + // home's box the home file is still built — by the loop above, |
| + // from an uncontested home record — but with this replica |
| + // missing from its list, so writing it would quietly drop a |
| + // live sibling's address and key. Neither is a decision this |
| + // pass is entitled to make off a record it cannot attribute. |
| + reason := "the pairing of " + rec.Home + " with " + host + " was refused: " + why |
| + hold(host, reason) |
| + hold(rec.Home, reason) |
| + } |
| continue |
| } |
| label := pairLabel(homeRec.Account, rec.Home, host) |
| @@ -216,7 +261,7 @@ func buildSiblings(all map[string]provision.Record, eipOf func(host string) (str |
| sort.Slice(f.Replicas, func(i, j int) bool { return f.Replicas[i].Host < f.Replicas[j].Host }) |
| out[host] = f |
| } |
| - return out |
| + return out, held |
| } |
| // siblingsWriter is the plan's siblingsPass with the one piece of state it |
| @@ -241,13 +286,15 @@ type siblingsWriter struct { |
| } |
| // pass renders every siblings file this box owns and writes the ones that |
| -// changed. Called from instancesLoop right after the reconcile pass whose |
| -// OnRecords handed it `all` — never concurrently with itself. |
| -func (w *siblingsWriter) pass(all map[string]provision.Record, eipOf func(host string) (string, bool), keys [][]byte) error { |
| +// changed. Called from instancesLoop right after the reconcile pass, over the |
| +// last record family OnRecords published — never concurrently with itself. |
| +// "Last published", not "this pass's": a pass whose reads failed publishes |
| +// nothing, so what arrives here is always a family that was whole. |
| +func (w *siblingsWriter) pass(rs provision.RecordSet, eipOf func(host string) (string, bool), keys [][]byte) error { |
| if w == nil || w.reg == nil || eipOf == nil { |
| return nil |
| } |
| - if len(all) == 0 { |
| + if len(rs.All) == 0 { |
| // No records at all is a pass that learned nothing — before the |
| // first reconcile has run, or after one that could read nothing. |
| // Treating it as "every pair is gone" would remove every file on |
| @@ -261,7 +308,7 @@ func (w *siblingsWriter) pass(all map[string]provision.Record, eipOf func(host s |
| // edge takes towards an absent mesh key for hop proofs. |
| return nil |
| } |
| - built := buildSiblings(all, eipOf, keys) |
| + built, withheld := buildSiblings(rs, eipOf, keys) |
| w.republishRetired(built) |
| routes, err := w.reg.List() |
| @@ -276,14 +323,27 @@ func (w *siblingsWriter) pass(all map[string]provision.Record, eipOf func(host s |
| // keys turn by turn, with whichever app read last holding a key minted |
| // for someone else's pair. |
| // |
| - // held is the same decision for a CONTESTED host: buildSiblings writes |
| - // nothing for one, and "nothing built" is otherwise how this pass learns |
| - // a placement was withdrawn and removes the file. Removing it here would |
| - // hand a writer who plants one record the power to take a live pair's |
| - // file off the box — the revocation the rails exist to refuse. A |
| - // directory holding a contested host is left exactly as it is instead: |
| - // no write, no removal. |
| - contested := provision.ContestedHosts(all) |
| + // held is the same decision for a host nothing was built for BECAUSE some |
| + // host in its pairing is contested. "Nothing built" is otherwise how this |
| + // pass learns a placement was withdrawn and removes the file, and letting |
| + // a refusal share that meaning hands a writer who plants one record the |
| + // power to take a live pair's file off the box — the revocation the rails |
| + // exist to refuse. Such a directory is left exactly as it is instead: no |
| + // write, no removal. |
| + // |
| + // Two sources, and both are needed. buildSiblings' withheld map names the |
| + // hosts whose pairing it refused, INCLUDING the other side of the pairing, |
| + // which is not itself contested and so appears nowhere else: that is the |
| + // home whose replica was contested, whose file would otherwise be rewritten |
| + // with an empty replicas list. rs.Contested names the rest — a contested |
| + // host whose records this pass never even visited because neither body |
| + // decoded is in the listing's count and in no loop's reach. |
| + // |
| + // The cost is stated plainly: a home with one contested replica and one |
| + // good one publishes neither until the contest is resolved, because "held" |
| + // means the file is not touched at all. A stale file naming a real sibling |
| + // is a smaller harm than a fresh one that dropped it. |
| + contested := rs.Contested |
| var dirs []string |
| owners := map[string][]string{} |
| held := map[string][]string{} |
| @@ -296,8 +356,11 @@ func (w *siblingsWriter) pass(all map[string]provision.Record, eipOf func(host s |
| if _, ok := built[rt.Host]; ok { |
| owners[dir] = append(owners[dir], rt.Host) |
| } |
| - if len(contested[rt.Host]) > 0 { |
| - held[dir] = append(held[dir], rt.Host) |
| + switch by := contested[rt.Host]; { |
| + case len(by) > 0: |
| + held[dir] = append(held[dir], rt.Host+": it is claimed by more than one record ("+strings.Join(by, ", ")+")") |
| + case withheld[rt.Host] != "": |
| + held[dir] = append(held[dir], rt.Host+": "+withheld[rt.Host]) |
| } |
| } |
| sort.Strings(dirs) |
| @@ -311,10 +374,11 @@ func (w *siblingsWriter) pass(all map[string]provision.Record, eipOf func(host s |
| for _, dir := range dirs { |
| if h := held[dir]; len(h) > 0 { |
| sort.Strings(h) |
| - for _, host := range h { |
| - log.Printf("agent: siblings: %s is claimed by more than one record (%s), so %s in %s "+ |
| - "is left exactly as it is — this pass cannot tell which claimant a file there "+ |
| - "would belong to", host, strings.Join(contested[host], ", "), siblingsFileName, dir) |
| + for _, reason := range h { |
| + log.Printf("agent: siblings: nothing could be built for %s, so %s in %s is left exactly "+ |
| + "as it is — a contested host is one this pass cannot attribute a file to, and "+ |
| + "refusing to vouch must never remove what is already serving", |
| + reason, siblingsFileName, dir) |
| } |
| continue |
| } |
| @@ -504,8 +568,8 @@ func siblingInputs() (eipOf func(host string) (string, bool), keys [][]byte, ok |
| }, keys, true |
| } |
| -// lastRecords is the record family the most recent reconcile pass saw — |
| -// EVERY record, including the ones out of this box's scope, which is the |
| +// lastRecords is the record family the most recent CLEAN reconcile pass saw |
| +// — EVERY record, including the ones out of this box's scope, which is the |
| // half the siblings pass cannot do without: a replica's file names its home, |
| // and the home lives on another box. |
| // |
| @@ -513,26 +577,39 @@ func siblingInputs() (eipOf func(host string) (string, bool), keys [][]byte, ok |
| // both reconcile legs carry it) and read by instancesLoop after the pass |
| // returns. An atomic.Value rather than a field on the loop because the two |
| // legs share one callback and run under reconcileMu. |
| -var lastRecords atomic.Value // map[string]provision.Record, keyed by RecordKey |
| +// |
| +// "Clean" is the reconcile pass's own judgement and it is load-bearing: a |
| +// pass whose reads failed never calls the callback, so what stays here is the |
| +// last family that was whole. A family missing the record a shadow of its |
| +// host is standing in for is not a stale family, it is a family that pairs |
| +// the shadow. |
| +var lastRecords atomic.Value // provision.RecordSet |
| -// rememberRecords is the OnRecords callback. It COPIES: the map belongs to |
| -// the reconcile pass, the callback runs while reconcileMu is held, and the |
| -// reader is another goroutine's tick. The keys are carried through exactly |
| -// as the pass wrote them (provision.RecordKey — account, app and host), the |
| -// key being the property that keeps one account's record out of another's |
| -// slot. |
| -func rememberRecords(all map[string]provision.Record) { |
| - cp := make(map[string]provision.Record, len(all)) |
| - for key, rec := range all { |
| - cp[key] = rec |
| +// rememberRecords is the OnRecords callback. It COPIES both halves: they |
| +// belong to the reconcile pass, the callback runs while reconcileMu is held, |
| +// and the reader is another goroutine's tick. The keys are carried through |
| +// exactly as the pass wrote them (provision.RecordKey — account, app and |
| +// host), the key being the property that keeps one account's record out of |
| +// another's slot; the contested set is carried through rather than recomputed, |
| +// which is the point of its being handed over at all. |
| +func rememberRecords(rs provision.RecordSet) { |
| + cp := provision.RecordSet{ |
| + All: make(map[string]provision.Record, len(rs.All)), |
| + Contested: make(map[string][]string, len(rs.Contested)), |
| + } |
| + for key, rec := range rs.All { |
| + cp.All[key] = rec |
| + } |
| + for host, by := range rs.Contested { |
| + cp.Contested[host] = append([]string(nil), by...) |
| } |
| lastRecords.Store(cp) |
| } |
| -// latestRecords is the last pass's record family, or nil before any pass has |
| -// run — which the siblings pass reads as "nothing to write yet", never as |
| -// "every pair is gone". |
| -func latestRecords() map[string]provision.Record { |
| - all, _ := lastRecords.Load().(map[string]provision.Record) |
| - return all |
| +// latestRecords is the last clean pass's record family, or the zero RecordSet |
| +// before any pass has published one — which the siblings pass reads as |
| +// "nothing to write yet", never as "every pair is gone". |
| +func latestRecords() provision.RecordSet { |
| + rs, _ := lastRecords.Load().(provision.RecordSet) |
| + return rs |
| } |
diff --git a/internal/host/siblings_test.go b/internal/host/siblings_test.go| index 75d1edf2c..10cc38679 100644 |
| --- a/internal/host/siblings_test.go |
| +++ b/internal/host/siblings_test.go |
| @@ -24,6 +24,19 @@ func recordMap(recs ...provision.Record) map[string]provision.Record { |
| return all |
| } |
| +// recordSet wraps a hand-built map as the RecordSet a reconcile pass hands |
| +// over, deriving the contested set FROM THAT MAP — which is what every record |
| +// here decoding makes correct, and only that. |
| +// |
| +// A test about a claim whose body did NOT decode builds the RecordSet by hand |
| +// instead, with a Contested set the map alone could not produce. That is the |
| +// real shape: the reconcile pass counts claims off the bucket listing, before |
| +// any body is read, exactly so a body an attacker can make unreadable cannot |
| +// take its host's claim with it. |
| +func recordSet(all map[string]provision.Record) provision.RecordSet { |
| + return provision.RecordSet{All: all, Contested: provision.ContestedHosts(all)} |
| +} |
| + |
| // The pair a home and its replica make: each side's file names the other, |
| // with the other's dial address and the key of THIS pair (spec §7). The two |
| // files are written by two different boxes, from the same fleet-wide record |
| @@ -39,7 +52,7 @@ func TestBuildSiblingsPairsHomeAndReplica(t *testing.T) { |
| eip := func(host string) (string, bool) { |
| return map[string]string{"acme.go.tito.io": "198.51.100.4", "r7x2.go.tito.io": "203.0.113.9"}[host], true |
| } |
| - got := buildSiblings(all, eip, [][]byte{[]byte("mesh-current")}) |
| + got, _ := buildSiblings(recordSet(all), eip, [][]byte{[]byte("mesh-current")}) |
| home := got["acme.go.tito.io"] |
| if home.Role != "home" || len(home.Replicas) != 1 { |
| @@ -68,7 +81,7 @@ func TestBuildSiblingsKeysArePerPairAndRotate(t *testing.T) { |
| provision.Record{Host: "r8y3.go.tito.io", Account: "bab", App: "tito", Region: "de", Role: "replica", Home: "acme.go.tito.io"}, |
| ) |
| eip := func(host string) (string, bool) { return "203.0.113.9", true } |
| - got := buildSiblings(all, eip, [][]byte{[]byte("cur"), []byte("prev")}) |
| + got, _ := buildSiblings(recordSet(all), eip, [][]byte{[]byte("cur"), []byte("prev")}) |
| home := got["acme.go.tito.io"] |
| if len(home.Replicas) != 2 { |
| t.Fatalf("want two replicas, got %+v", home.Replicas) |
| @@ -90,7 +103,7 @@ func TestBuildSiblingsOmitsASiblingWithNoEIPAndStillWritesAnEmptyHome(t *testing |
| provision.Record{Host: "acme.go.tito.io", Account: "bab", App: "tito", Region: "ie", Steering: "latency", Replicas: []string{"uk"}}, |
| provision.Record{Host: "r7x2.go.tito.io", Account: "bab", App: "tito", Region: "uk", Role: "replica", Home: "acme.go.tito.io"}, |
| ) |
| - got := buildSiblings(all, func(string) (string, bool) { return "", false }, [][]byte{[]byte("cur")}) |
| + got, _ := buildSiblings(recordSet(all), func(string) (string, bool) { return "", false }, [][]byte{[]byte("cur")}) |
| home, ok := got["acme.go.tito.io"] |
| if !ok { |
| t.Fatal("a home with no reachable replica still gets a file, so the app can tell 'no replicas' from 'no file'") |
| @@ -105,7 +118,7 @@ func TestBuildSiblingsOmitsASiblingWithNoEIPAndStillWritesAnEmptyHome(t *testing |
| // only). |
| func TestBuildSiblingsIgnoresAnAccountWithNoReplicas(t *testing.T) { |
| all := recordMap(provision.Record{Host: "plain.go.tito.io", Account: "zed", App: "tito", Region: "ie"}) |
| - if got := buildSiblings(all, func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}); len(got) != 0 { |
| + if got, _ := buildSiblings(recordSet(all), func(string) (string, bool) { return "203.0.113.9", true }, [][]byte{[]byte("cur")}); len(got) != 0 { |
| t.Errorf("buildSiblings = %+v, want nothing for an account with no replicas", got) |
| } |
| } |
| @@ -160,7 +173,7 @@ func TestBuildSiblingsRefusesAnUnvouchedForReplica(t *testing.T) { |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| all := recordMap(append([]provision.Record{homeRec, tc.replica}, tc.extra...)...) |
| - got := buildSiblings(all, eip, [][]byte{[]byte("cur")}) |
| + got, _ := buildSiblings(recordSet(all), eip, [][]byte{[]byte("cur")}) |
| if _, ok := got[tc.replica.Host]; ok { |
| t.Errorf("the unvouched-for replica got a file of its own: %+v", got[tc.replica.Host]) |
| } |
| @@ -203,11 +216,16 @@ func TestBuildSiblingsRefusesAnUnvouchedForReplica(t *testing.T) { |
| // all, on either side. |
| // |
| // The cases live in replica_adversarial_test.go, in both sort orders: |
| -// TestBuildSiblingsRefusesAContestedHostInEitherSortOrder, |
| -// TestBuildSiblingsRefusesAContestedReplicaHost, and |
| -// TestSiblingsPassLeavesTheFileAloneWhenTheHostIsContested — which pins the |
| -// other half of fail-safe: a contested host does not cost an instance the |
| -// file it already has. |
| +// TestBuildSiblingsRefusesAContestedHostInEitherSortOrder and |
| +// TestBuildSiblingsRefusesAContestedReplicaHost for the refusal itself, and |
| +// four more for the other half of fail-safe — that a refusal costs no |
| +// instance the file it already has: |
| +// TestSiblingsPassLeavesTheFileAloneWhenTheHostIsContested, |
| +// TestSiblingsPassHoldsTheReplicasFileWhenItsHomeIsContested, |
| +// TestSiblingsPassHoldsTheHomesFileWhenItsReplicaIsContested, and |
| +// TestSiblingsPassWritesNothingWhileAVictimsRecordBlipsBesideAShadow. |
| +// TestSiblingsPassStillRemovesTheFileWhenThePlacementIsWithdrawn is the |
| +// boundary between the two: holding must not swallow a real withdrawal. |
| // --- the writer --- |
| @@ -301,7 +319,7 @@ func TestSiblingsPassWritesEachLayoutsDirectoryAt0600(t *testing.T) { |
| all[provision.RecordKey(other.Account, other.App, other.Host)] = other |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| - if err := w.pass(all, siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(all), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("pass: %v", err) |
| } |
| @@ -341,7 +359,7 @@ func TestSiblingsPassRewritesNothingWhenThePairIsUnchanged(t *testing.T) { |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| keys := [][]byte{[]byte("mesh-A")} |
| - if err := w.pass(siblingsPair(), siblingsEIP, keys); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, keys); err != nil { |
| t.Fatalf("first pass: %v", err) |
| } |
| path := filepath.Join(dd, siblingsFileName) |
| @@ -349,7 +367,7 @@ func TestSiblingsPassRewritesNothingWhenThePairIsUnchanged(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if err := w.pass(siblingsPair(), siblingsEIP, keys); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, keys); err != nil { |
| t.Fatalf("second pass: %v", err) |
| } |
| after, err := os.Stat(path) |
| @@ -370,7 +388,7 @@ func TestSiblingsPassRemovesAStaleFileWithoutFollowingASymlink(t *testing.T) { |
| addFlatRoute(t, reg, dd, siblingsHomeHost) |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("first pass: %v", err) |
| } |
| path := filepath.Join(dd, siblingsFileName) |
| @@ -381,7 +399,7 @@ func TestSiblingsPassRemovesAStaleFileWithoutFollowingASymlink(t *testing.T) { |
| // The account's replicas are withdrawn: the record no longer names a |
| // placement and the replica record is gone. |
| plain := recordMap(provision.Record{Host: siblingsHomeHost, Account: "bab", App: "tito", Region: "ie"}) |
| - if err := w.pass(plain, siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(plain), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("second pass: %v", err) |
| } |
| if _, err := os.Lstat(path); !os.IsNotExist(err) { |
| @@ -397,7 +415,7 @@ func TestSiblingsPassRemovesAStaleFileWithoutFollowingASymlink(t *testing.T) { |
| if err := os.Symlink(canary, path); err != nil { |
| t.Fatal(err) |
| } |
| - if err := w.pass(plain, siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(plain), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("third pass: %v", err) |
| } |
| if _, err := os.Stat(canary); err != nil { |
| @@ -416,7 +434,7 @@ func TestSiblingsPassRefusesTwoFlatHostsSharingOneDirectory(t *testing.T) { |
| reg := openSiblingsRegistry(t, dd) |
| addFlatRoute(t, reg, dd, siblingsHomeHost) |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("first pass: %v", err) |
| } |
| path := filepath.Join(dd, siblingsFileName) |
| @@ -431,7 +449,7 @@ func TestSiblingsPassRefusesTwoFlatHostsSharingOneDirectory(t *testing.T) { |
| rec := provision.Record{Host: other, Account: "zed", App: "tito", Region: "ie", |
| Steering: "latency", Replicas: []string{"uk"}} |
| all[provision.RecordKey(rec.Account, rec.App, rec.Host)] = rec |
| - if err := w.pass(all, siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(all), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("second pass: %v", err) |
| } |
| if _, err := os.Lstat(path); !os.IsNotExist(err) { |
| @@ -453,7 +471,7 @@ func TestSiblingsPassRepublishesTheRetiredKeyForOnePass(t *testing.T) { |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| path := filepath.Join(dd, siblingsFileName) |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("pass 1: %v", err) |
| } |
| fromA := readSiblingsFile(t, path).Replicas[0].Keys |
| @@ -462,7 +480,7 @@ func TestSiblingsPassRepublishesTheRetiredKeyForOnePass(t *testing.T) { |
| } |
| // The rotation promotes: meshKeys() now answers with B alone. |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-B")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-B")}); err != nil { |
| t.Fatalf("pass 2: %v", err) |
| } |
| fromB := readSiblingsFile(t, path).Replicas[0].Keys |
| @@ -474,7 +492,7 @@ func TestSiblingsPassRepublishesTheRetiredKeyForOnePass(t *testing.T) { |
| } |
| // One further pass and the retired derivative is gone. |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-B")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-B")}); err != nil { |
| t.Fatalf("pass 3: %v", err) |
| } |
| third := readSiblingsFile(t, path).Replicas[0].Keys |
| @@ -492,15 +510,15 @@ func TestSiblingsPassForgetsAPairThatIsGone(t *testing.T) { |
| addFlatRoute(t, reg, dd, siblingsHomeHost) |
| w := &siblingsWriter{reg: reg, dataDir: dd} |
| - if err := w.pass(siblingsPair(), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + if err := w.pass(recordSet(siblingsPair()), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("pass 1: %v", err) |
| } |
| if len(w.last) != 1 { |
| t.Fatalf("remembered %d derivatives, want one per pair", len(w.last)) |
| } |
| - if err := w.pass(recordMap( |
| + if err := w.pass(recordSet(recordMap( |
| provision.Record{Host: siblingsHomeHost, Account: "bab", App: "tito", Region: "ie"}, |
| - ), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| + )), siblingsEIP, [][]byte{[]byte("mesh-A")}); err != nil { |
| t.Fatalf("pass 2: %v", err) |
| } |
| if len(w.last) != 0 { |
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go| index d48a73a31..fbea85260 100644 |
| --- a/internal/provision/reconcile.go |
| +++ b/internal/provision/reconcile.go |
| @@ -76,29 +76,54 @@ type ReconcileConfig struct { |
| // path; the auto-purge that does still lives in domains, still refuses |
| // without a key. |
| ReleasePub ed25519.PublicKey |
| - // OnRecords, when set, is called once per pass with every record this |
| - // pass decoded — in scope or not. The siblings-file pass (spec §7) |
| - // needs both sides of a home/replica pair and only one is ever in |
| - // scope on a given box; this hands it the map the pass already built |
| - // rather than making it list apps/ a second time. |
| + // OnRecords, when set, is called with the record family this pass read — |
| + // in scope or not. The siblings-file pass (spec §7) needs both sides of |
| + // a home/replica pair and only one is ever in scope on a given box; this |
| + // hands it what the pass already built rather than making it list apps/ |
| + // a second time. |
| // |
| - // The map is keyed by the record's IDENTITY — RecordKey(account, app, |
| - // host), the bucket key it was read from — and that is not a detail of |
| - // convenience, it is the safety property the reader inherits. A map |
| - // keyed by host has one slot per host, so a record written in any |
| - // account's prefix can occupy another account's slot and be read as |
| - // that account's record. Look a host up the way this pass does: |
| - // all[RecordKey(acct, app, host)], inside the tenancy asking. |
| + // It is NOT called on a pass whose reads failed (§6, "the vouching |
| + // window"): the consumer then goes on working from the family the last |
| + // clean pass published. A record that did not read is a record missing |
| + // from RecordSet.All, and a map with the victim's home record missing |
| + // but an attacker's shadow of that host present is a map that pairs the |
| + // shadow with the attacker's replica and writes it into the victim's own |
| + // siblings.json with a dial address and a live key. The previous pass's |
| + // family is stale; this pass's is wrong, and wrong is what gets leases |
| + // granted to a stranger. |
| // |
| - // A reader that pairs records must also ask ContestedHosts of this map |
| - // before it believes any host in it: two identities can still claim one |
| - // host, and neither of them may be used to vouch for the other side of a |
| - // pairing. The one thing this map cannot show is a claim whose body did |
| - // not decode — the pass counts those off the bucket listing, which the |
| - // callback never sees — so a reader is fail-safe against a shadow, not |
| - // against a shadow plus a simultaneous read failure of the record it |
| - // shadows (spec §6). |
| - OnRecords func(all map[string]Record) |
| + // Skipping the call, rather than passing a flag for the consumer to act |
| + // on, because there is nothing a consumer could usefully do with such a |
| + // family that "keep the previous one" does not do — and a flag is a |
| + // thing a second consumer can forget to read, while a call that does not |
| + // happen cannot be misread. It costs a tick of freshness on a blip, |
| + // which is the same price the removal phase below already pays. |
| + OnRecords func(rs RecordSet) |
| +} |
| + |
| +// RecordSet is one reconcile pass's view of the record family: every record |
| +// it decoded, and the contested set counted off the bucket LISTING. |
| +// |
| +// The two travel together and are handed over together, because either one |
| +// alone is misleading. All is keyed by the record's IDENTITY — |
| +// RecordKey(account, app, host), the bucket key it was read from — and that |
| +// is not a detail of convenience, it is the safety property the reader |
| +// inherits: a map keyed by host has one slot per host, so a record written |
| +// in any account's prefix can occupy another account's slot and be read as |
| +// that account's record. Look a host up the way this pass does: |
| +// All[RecordKey(acct, app, host)], inside the tenancy asking. |
| +// |
| +// Contested is what a reader must consult before it believes any host in |
| +// All, and it is counted off the LISTING rather than recomputed from All |
| +// (which is what ContestedHosts would do) for one reason: a key is a claim |
| +// on its host whether or not its body decodes. Recomputing from All drops |
| +// the claim of any record that failed to decode or failed a per-record rail, |
| +// so an undecodable shadow contests on this pass's side and not on the |
| +// reader's — the two halves of one defence reaching opposite conclusions |
| +// about the same host. Handed over instead, they cannot diverge. |
| +type RecordSet struct { |
| + All map[string]Record |
| + Contested map[string][]string |
| } |
| // InboundAny and InboundNone are the closed vocabulary ReconcileConfig.Inbound |
| @@ -745,6 +770,14 @@ func convergeOwned(reg *registry.Registry, existing registry.Route, rec Record, |
| // revokes. A writer who can plant a record can therefore deny service by |
| // contesting a host, and no arrangement of this pass can stop them — the fix |
| // for that is narrowing the grant, which is infrastructure work (spec §6). |
| +// |
| +// This counts from DECODED records, which is strictly weaker than what |
| +// Reconcile itself does: that pass counts claims off the bucket LISTING, so a |
| +// body that will not read still holds its host's claim. Anything pairing |
| +// records must use the set that came with the map (RecordSet.Contested) and |
| +// not this — a contest only one side of a defence can see is the divergence |
| +// spec §6 describes. What is left here is the answer for a caller holding |
| +// nothing but a decoded map, and tests that need the weaker answer on purpose. |
| func ContestedHosts(all map[string]Record) map[string][]string { |
| claims := map[string][]string{} |
| for key, rec := range all { |
| @@ -802,12 +835,22 @@ func contestedOf(claims map[string][]string) map[string][]string { |
| // failed, for the log, because "not minting" with no reason is how an |
| // operator's typo becomes an afternoon. There is no second answer: EVERY |
| // failure here leaves the caller's existing state exactly as it is. |
| -func PairableHome(all map[string]Record, contested map[string][]string, rec Record) (home Record, why string) { |
| +// |
| +// byContest reports that the condition which failed was a CONTESTED host, on |
| +// one side or the other. Every caller refuses the pairing the same way, but |
| +// not every caller may then tidy up after it: "nothing to pair with" is also |
| +// how the siblings writer learns a placement was withdrawn and removes the |
| +// file, and a contested host must not be allowed to look like a withdrawal — |
| +// or one planted record takes a live pair's file off the box, which is the |
| +// revocation these rails exist to refuse. Reported here rather than |
| +// recomputed by the caller, so the answer the caller acts on is the answer |
| +// this function actually gave. |
| +func PairableHome(all map[string]Record, contested map[string][]string, rec Record) (home Record, why string, byContest bool) { |
| if by, dup := contested[rec.Host]; dup { |
| - return Record{}, "its own host is claimed by more than one record (" + strings.Join(by, ", ") + ")" |
| + return Record{}, "its own host is claimed by more than one record (" + strings.Join(by, ", ") + ")", true |
| } |
| if by, dup := contested[rec.Home]; dup { |
| - return Record{}, "its home " + rec.Home + " is claimed by more than one record (" + strings.Join(by, ", ") + ")" |
| + return Record{}, "its home " + rec.Home + " is claimed by more than one record (" + strings.Join(by, ", ") + ")", true |
| } |
| home, ok := all[RecordKey(rec.Account, rec.App, rec.Home)] |
| switch { |
| @@ -816,13 +859,13 @@ func PairableHome(all map[string]Record, contested map[string][]string, rec Reco |
| // answer, because the lookup cannot tell them apart and the pass must |
| // not guess. A mirror of nothing, and a bucket blip must never sweep |
| // a live placement's alias. |
| - return Record{}, "no record for its home " + rec.Home + " in this account and app" |
| + return Record{}, "no record for its home " + rec.Home + " in this account and app", false |
| case home.Role != "": |
| - return Record{}, "its home " + rec.Home + " is itself a replica" |
| + return Record{}, "its home " + rec.Home + " is itself a replica", false |
| case !slices.Contains(home.Replicas, rec.Region): |
| - return Record{}, "its home " + rec.Home + " has not opted into placement " + rec.Region |
| + return Record{}, "its home " + rec.Home + " has not opted into placement " + rec.Region, false |
| } |
| - return home, "" |
| + return home, "", false |
| } |
| // Reconcile materializes bucket instance records into registry routes for |
| @@ -986,8 +1029,22 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| "until one of them goes; existing rows are left exactly as they are", |
| host, strings.Join(contested[host], ", ")) |
| } |
| - if cfg.OnRecords != nil { |
| - cfg.OnRecords(all) |
| + // The record family goes out only when everything this pass tried to read |
| + // actually read (spec §6, "the vouching window"). readFailed here covers |
| + // exactly the two paths that can silently take a record OUT of `all`: an |
| + // account manifest that would not read (its whole prefix is then unparsed, |
| + // so its records are missing from the map AND from the claims above), and |
| + // a record body whose Get failed. Everything after this point — the seed |
| + // manifests, the trash markers — can set readFailed too, but none of them |
| + // can remove a record from the map, so waiting for the final value would |
| + // only mean a deployment with one permanently unreadable manifest never |
| + // publishes a family at all and writes no siblings files ever. |
| + // |
| + // A pass that decoded fewer records than the bucket holds is not a pass |
| + // with less news, it is a pass with WRONG news: the missing record is the |
| + // one a shadow of its host is waiting to stand in for. |
| + if cfg.OnRecords != nil && !readFailed { |
| + cfg.OnRecords(RecordSet{All: all, Contested: contested}) |
| } |
| // --- manifest seed cache, per (acct, app), lazily ------------------------- |
| @@ -1124,7 +1181,7 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| // afternoon. |
| if rec.Role == "replica" { |
| var why string |
| - home, why = PairableHome(all, contested, rec) |
| + home, why, _ = PairableHome(all, contested, rec) |
| if why != "" { |
| log.Printf("provision: not minting replica %s: %s", rec.Host, why) |
| protect[rec.Host] = true |
diff --git a/internal/provision/reconcile_test.go b/internal/provision/reconcile_test.go| index f1344c42e..78b324fc0 100644 |
| --- a/internal/provision/reconcile_test.go |
| +++ b/internal/provision/reconcile_test.go |
| @@ -2845,7 +2845,7 @@ func TestReconcileReportsEveryRecord(t *testing.T) { |
| s, sqid, home, replica := replicaSetup(t) |
| var seen map[string]Record |
| cfg := ReconcileConfig{Region: "uk", Pools: []string{"tito"}, DataDir: "/data", |
| - OnRecords: func(all map[string]Record) { seen = all }} |
| + OnRecords: func(rs RecordSet) { seen = rs.All }} |
| if err := Reconcile(ctx, s, regWith(t), cfg); err != nil { |
| t.Fatal(err) |
| } |
diff --git a/internal/provision/replica_adversarial_test.go b/internal/provision/replica_adversarial_test.go| index 9afa12a6c..62cbc784a 100644 |
| --- a/internal/provision/replica_adversarial_test.go |
| +++ b/internal/provision/replica_adversarial_test.go |
| @@ -356,6 +356,209 @@ func TestReconcileNeverClearsALiveReplicaLink(t *testing.T) { |
| } |
| } |
| +// The vouching window. A pass that could not read everything must not publish |
| +// the family it did read, because the record missing from it is the one a |
| +// shadow of its host is waiting to stand in for. |
| +// |
| +// The reconcile pass itself was already fail-safe here |
| +// (TestReconcileRefusesAShadowVouchedReplicaWhileTheVictimsRecordBlips): it |
| +// counts claims off the LISTING, so the victim's key contests the host even |
| +// when its body will not read. The siblings writer inherited none of that — |
| +// it was handed the decoded map alone, in which the attacker's shadow is the |
| +// only claimant, and it wrote the victim's own siblings.json naming the |
| +// attacker's replica with a live pair key. |
| +// |
| +// So the family is not handed over at all on such a pass, and the consumer |
| +// keeps the last one that was whole. |
| +func TestReconcileWithholdsTheRecordFamilyWhileAReadFails(t *testing.T) { |
| + s, sqid, home, _ := replicaSetup(t) |
| + eve := plantShadow(t, s, laterSortingID(t, domains.Sqid(acctID)), home.Host, []string{"uk"}) |
| + evil := Record{Schema: 1, Host: "r4444444444.go.tito.io", Slug: "r4444444444", App: "tito", |
| + Account: eve, Pool: "tito", Region: "uk", Channel: "stable", CreatedAt: time.Now(), |
| + Role: "replica", Home: home.Host} |
| + if err := CreateRecord(ctx, s, evil); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var published []RecordSet |
| + cfg := ReconcileConfig{Region: "uk", Pools: []string{"tito"}, DataDir: "/data", |
| + OnRecords: func(rs RecordSet) { published = append(published, rs) }} |
| + |
| + // The clean pass first, so the test can tell "withheld" from "never |
| + // published anything". |
| + if err := Reconcile(ctx, s, regWith(t), cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(published) != 1 { |
| + t.Fatalf("a clean pass published %d families, want one", len(published)) |
| + } |
| + |
| + fs := &failStore{Store: s} |
| + fs.failReadsFor(RecordKey(sqid, "tito", home.Host)) |
| + if err := Reconcile(ctx, fs, regWith(t), cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(published) != 1 { |
| + got := published[len(published)-1] |
| + _, victimPresent := got.All[RecordKey(sqid, "tito", home.Host)] |
| + _, shadowPresent := got.All[RecordKey(eve, "tito", home.Host)] |
| + t.Fatalf("PUBLISHED: a pass whose read of the victim's home record failed handed the family over "+ |
| + "anyway (victim record present: %v, attacker's shadow of the same host present: %v) — the "+ |
| + "siblings writer pairs the shadow and vouches for %s", victimPresent, shadowPresent, evil.Host) |
| + } |
| + |
| + // And it is a window, not a wall: the next clean pass publishes again. |
| + if err := Reconcile(ctx, s, regWith(t), cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(published) != 2 { |
| + t.Errorf("the pass after the blip published %d families in total, want two — a read failure must "+ |
| + "cost one tick of freshness, not the feature", len(published)) |
| + } |
| +} |
| + |
| +// The two "contested" derivations, made one. Reconcile counts claims off the |
| +// bucket LISTING, before any body is decoded; the siblings pass used to |
| +// recompute the set from the DECODED map it was handed. A record that decodes |
| +// but fails a per-record rail — here an empty channel, which no door writes — |
| +// is in the listing and not in the map, so the two halves of one defence |
| +// reached opposite conclusions about the same host: reconcile refused to mint |
| +// the victim's replica while the siblings writer went on pairing it. |
| +// |
| +// The set now travels with the map, so there is nothing left to diverge. |
| +func TestReconcileHandsOverTheListingsContestedSet(t *testing.T) { |
| + s, sqid, home, replica := replicaSetup(t) |
| + eve := writeAccountManifest(t, s, laterSortingID(t, domains.Sqid(acctID)), "Eve") |
| + if err := PutManifest(ctx, s, eve, "tito", Manifest{Schema: 1, App: "tito", Domain: "go.tito.io", Pool: "tito"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + // Written with Put rather than CreateRecord: the door would refuse it, and |
| + // a record no door wrote is the whole subject. |
| + bad := Record{Schema: 1, Host: home.Host, Slug: "acme", App: "tito", Account: eve, Pool: "tito", |
| + Region: "de", Channel: "", CreatedAt: time.Now()} |
| + if _, err := s.Put(ctx, RecordKey(eve, "tito", home.Host), mustJSON(t, bad), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var seen RecordSet |
| + uk := regWith(t) |
| + cfg := ReconcileConfig{Region: "uk", Pools: []string{"tito"}, DataDir: "/data", |
| + OnRecords: func(rs RecordSet) { seen = rs }} |
| + if err := Reconcile(ctx, s, uk, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // Reconcile's own answer: the host is contested, so the replica is not |
| + // minted. Nothing subtle — it is the baseline the other half must match. |
| + if rt, err := uk.Get(replica.Host); err != nil { |
| + t.Fatal(err) |
| + } else if rt != nil { |
| + t.Fatalf("fixture: the replica minted, so this pass does not think the host is contested: %+v", rt) |
| + } |
| + if by := seen.Contested[home.Host]; len(by) < 2 { |
| + t.Errorf("DIVERGENCE: the family handed to the siblings pass calls %s contested by %v, while this "+ |
| + "pass refused to mint off it — an undecodable or rail-failing claim must count on both sides", |
| + home.Host, by) |
| + } |
| + if _, ok := seen.All[RecordKey(eve, "tito", home.Host)]; ok { |
| + t.Error("a record that failed a per-record rail is in the decoded map: the map is what a pairing " + |
| + "is built FROM, and this one would have been") |
| + } |
| + // Recomputing from the map is exactly what must not be done, and this is |
| + // the assertion that says why: the map alone cannot see the claim. |
| + if by := ContestedHosts(seen.All)[home.Host]; len(by) > 0 { |
| + t.Errorf("the decoded map can see this contest after all (%v) — if that ever becomes true, the "+ |
| + "fixture stopped reproducing the divergence rather than the divergence being fixed", by) |
| + } |
| + if _, ok := seen.All[RecordKey(sqid, "tito", home.Host)]; !ok { |
| + t.Error("the victim's own home record is missing from the family") |
| + } |
| +} |
| + |
| +// A KNOWN, PRE-EXISTING LIMIT OF THE MINT PATH, recorded here because it is |
| +// the counterexample to a claim spec §6 used to make about these rails, and |
| +// because a limit nobody has written down is one the next reader assumes away. |
| +// |
| +// A PLAIN record — no role, no replicas, nothing to do with pairing — written |
| +// in the attacker's OWN prefix, carrying the victim's public host and the |
| +// REPLICA's region, mints a real route on the replica's box. No rail asks |
| +// anything of a plain record: the cross-record rails are reached only by a |
| +// record with role=replica, and nothing anywhere checks a host against the |
| +// app's manifest domain at mint time. domains.ReconcileAliases then finds a |
| +// real route owning that host, logs "replica alias skipped: a real route owns |
| +// that host", and sweeps the victim's alias — so the box serves the victim's |
| +// public host from the attacker's process. |
| +// |
| +// The contested rails do not stop this and were never able to: they govern |
| +// PAIRING, and there is no pairing here. Fixing it means narrowing the bucket |
| +// grant (spec §6's last paragraph) or checking hosts against the manifest |
| +// domain at mint time, neither of which is this branch. |
| +// |
| +// This test therefore asserts the BROKEN behaviour on purpose. If it starts |
| +// failing, someone has closed the hole: that is good news, and the thing to do |
| +// is delete this test and the paragraph in §6 that records it. |
| +func TestReconcileMintPathLetsAPlainShadowTakeTheVictimsHostOnTheReplicasBox(t *testing.T) { |
| + for _, order := range bothSortOrders(t) { |
| + t.Run(order.name, func(t *testing.T) { |
| + s, sqid, home, replica := replicaSetup(t) |
| + uk := regWith(t) |
| + cfg := ReconcileConfig{Region: "uk", Pools: []string{"tito"}, DataDir: "/data"} |
| + if err := Reconcile(ctx, s, uk, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := domains.ReconcileAliases(ctx, s, uk, time.Now, nil, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + aliases, _ := uk.ListAliases() |
| + if len(aliases) != 1 || aliases[0].Alias != home.Host || aliases[0].Host != replica.Host { |
| + t.Fatalf("fixture: want alias %s -> %s, got %+v", home.Host, replica.Host, aliases) |
| + } |
| + |
| + eve := writeAccountManifest(t, s, order.id, "Eve") |
| + if err := PutManifest(ctx, s, eve, "tito", Manifest{Schema: 1, App: "tito", Domain: "go.tito.io", Pool: "tito"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + // The attacker's own prefix, the attacker's own account and app, so |
| + // the record agrees with its own path and the create door itself |
| + // would have written it — for a host the door would have refused, |
| + // which is the part no box-side pass re-checks. |
| + plain := Record{Schema: 1, Host: home.Host, Slug: "acme", App: "tito", Account: eve, Pool: "tito", |
| + Region: "uk", Channel: "stable", CreatedAt: time.Now()} |
| + if err := CreateRecord(ctx, s, plain); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Reconcile(ctx, s, uk, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + rt, err := uk.Get(home.Host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt == nil { |
| + t.Fatalf("the attacker's plain record no longer mints %s on the replica's box — the mint "+ |
| + "path was fixed; delete this test and the limit it records in spec §6", home.Host) |
| + } |
| + if rt.Account != eve { |
| + t.Errorf("the route at %s belongs to %q, want the attacker's %q (victim is %q)", |
| + home.Host, rt.Account, eve, sqid) |
| + } |
| + // The pairing rails did hold their own half: the victim's live link |
| + // is untouched. It buys nothing here, which is the point. |
| + if got := mustGet(t, uk, replica.Host).ReplicaOf; got != home.Host { |
| + t.Errorf("the victim's replica link is %q, want %q", got, home.Host) |
| + } |
| + if err := domains.ReconcileAliases(ctx, s, uk, time.Now, nil, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if aliases, _ = uk.ListAliases(); len(aliases) != 0 { |
| + t.Errorf("the replica alias survived the attacker's route: %+v — if it now does, the "+ |
| + "alias pass learned to defend this and §6 should say so", aliases) |
| + } |
| + }) |
| + } |
| +} |
| + |
| // The rail refuses to VOUCH; it does not refuse to work. With no shadow |
| // anywhere the legitimate pair mints, links and converges exactly as it did |
| // before any of this — the assertion that stops a fail-safe from quietly |