amadan
#162: the branch review's five findings — tenancy rails, gate, channel door
B1 (P1). The reconcile pass and buildSiblings treated "the home record exists" as the whole check, where the console's create door enforces four cross-record rails. A hand-written bucket record in any account naming another account's host as its home was minted on the named box, had the victim's public host aliased to the attacker's process there, and was written into the VICTIM's own siblings.json as a vouched-for replica with a dial address and a live pair key. Both places now ask the door's four questions off the record map they already hold: same account, same app, a home rather than a replica, and this placement named in the home's Replicas. The pass protects and continues (never retires — the fail-safe direction for a home it cannot verify is to leave the existing row serving) and logs which condition failed. buildSiblings drops the "pair it anyway when the placement is not opted in" branch: its comment called that mid-edit, but no door produces it — only an old console or a hand edit can, which is the case being defended against. B2 (P2). handleAPIInstanceChannel accepted a replica: it wrote the record, audited a move that did not happen, bumped the serial, and the pass converged the route back every tick. It now refuses, naming the home. The proposals path asks the same question on the record as it is at execution time, ahead of the etag pin. B3 (P2). A home's password gate did not reach its replica, so a gated account's public host was served ungated from the replica box. The gate now travels the pair exactly as the channel does: stamped in routeFor's replica branch and converged in convergeOwned, which is where the gate converge now lives for every record. B12 (P3). docs/site/cli.md says --host and --channel are refused with --role replica; the CLI dropped them in silence. Refused in validateReplicaCreateArgs. B14 (P3). Two comments cited "#162 revision 2" for custom domains; the spec says revision 3.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
10 files changed,
+463
−32
instances_cli.go+14 −2instances_cli_test.go+25 −4internal/console/channelmove.go+21 −0internal/console/channelmove_test.go+94 −0internal/host/edgesfile.go+1 −1internal/host/edgesfile_test.go+1 −1internal/host/siblings.go+23 −12internal/host/siblings_test.go+76 −0internal/provision/reconcile.go+66 −12internal/provision/reconcile_test.go+142 −0
CI passed — run details
diff --git a/instances_cli.go b/instances_cli.go| index 6d3b3020e..f840e9046 100644 |
| --- a/instances_cli.go |
| +++ b/instances_cli.go |
| @@ -291,7 +291,15 @@ func requireAPISession(verb, consoleFlag string) *apiSession { |
| // --role replica with no --home has named nothing to mirror, and a bare |
| // --home with no --role replica would otherwise be silently ignored, leaving |
| // the caller believing they created a mirror when they did not. |
| -func validateReplicaCreateArgs(role, home string) error { |
| +// |
| +// --host and --channel are refused with --role replica for the same reason, |
| +// and because the documentation already says they are (docs/site/cli.md): |
| +// the console mints a replica's host itself and stamps the home's channel on |
| +// the record, so a CLI that accepted either flag and dropped it in silence |
| +// would leave the caller wrong about what their mirror is named and what it |
| +// runs. Refused rather than passed on, so the answer arrives before the |
| +// round trip and in the same words the docs use. |
| +func validateReplicaCreateArgs(role, home, host, channel string) error { |
| if !provision.ValidRole(role) { |
| return fmt.Errorf("instances create: role %q: want \"\" or \"replica\"", role) |
| } |
| @@ -300,6 +308,10 @@ func validateReplicaCreateArgs(role, home string) error { |
| return fmt.Errorf("instances create --role replica needs --home <the instance it mirrors>") |
| case role != "replica" && home != "": |
| return fmt.Errorf("instances create: --home names the instance a REPLICA mirrors — pass --role replica too, or drop --home") |
| + case role == "replica" && host != "": |
| + return fmt.Errorf("instances create: --host is refused with --role replica — the console mints a replica's own host and prints it") |
| + case role == "replica" && channel != "": |
| + return fmt.Errorf("instances create: --channel is refused with --role replica — a replica runs its home's channel, and follows it") |
| } |
| return nil |
| } |
| @@ -325,7 +337,7 @@ func runInstancesCreate(args []string) { |
| fs.Parse(args) |
| roleVal := strings.TrimSpace(*role) |
| homeVal := strings.TrimSpace(*home) |
| - if err := validateReplicaCreateArgs(roleVal, homeVal); err != nil { |
| + if err := validateReplicaCreateArgs(roleVal, homeVal, strings.TrimSpace(*host), strings.TrimSpace(*channel)); err != nil { |
| log.Fatal(err) |
| } |
| // A replica's host is minted by the console (it has no --host to give); |
diff --git a/instances_cli_test.go b/instances_cli_test.go| index db903e9dd..fa0ffa67e 100644 |
| --- a/instances_cli_test.go |
| +++ b/instances_cli_test.go |
| @@ -181,21 +181,42 @@ func TestInstancesListShowsRole(t *testing.T) { |
| // so the refusal it defers to is pulled out as a plain function a test can |
| // call directly, per the plan's revision 2 correction. |
| func TestInstancesCreateReplicaNeedsAHome(t *testing.T) { |
| - err := validateReplicaCreateArgs("replica", "") |
| + err := validateReplicaCreateArgs("replica", "", "", "") |
| if err == nil || !strings.Contains(err.Error(), "home") { |
| t.Fatalf("err = %v, want a local refusal naming --home before any request", err) |
| } |
| - if err := validateReplicaCreateArgs("replica", "acme.go.tito.io"); err != nil { |
| + if err := validateReplicaCreateArgs("replica", "acme.go.tito.io", "", ""); err != nil { |
| t.Fatalf("a well-formed replica request was refused: %v", err) |
| } |
| - if err := validateReplicaCreateArgs("", ""); err != nil { |
| + if err := validateReplicaCreateArgs("", "", "", ""); err != nil { |
| t.Fatalf("an ordinary (home) create was refused: %v", err) |
| } |
| - if err := validateReplicaCreateArgs("", "acme.go.tito.io"); err == nil { |
| + if err := validateReplicaCreateArgs("", "acme.go.tito.io", "", ""); err == nil { |
| t.Fatal("a --home with no --role replica was accepted — home means nothing without it") |
| } |
| } |
| +// The docs say "`--host` and `--channel` are refused alongside `--role |
| +// replica`" (docs/site/cli.md) because the console mints a replica's host |
| +// and stamps its home's channel on it. The CLI used to drop both flags in |
| +// silence, so `--channel canary --role replica` produced a replica on the |
| +// home's channel and said nothing — a caller who believed the flag would be |
| +// wrong about what their mirror is running. |
| +func TestInstancesCreateReplicaRefusesHostAndChannel(t *testing.T) { |
| + err := validateReplicaCreateArgs("replica", "acme.go.tito.io", "r7x2k9m4qa.go.tito.io", "") |
| + if err == nil || !strings.Contains(err.Error(), "--host") { |
| + t.Fatalf("err = %v, want a refusal naming --host: the console mints a replica's host", err) |
| + } |
| + err = validateReplicaCreateArgs("replica", "acme.go.tito.io", "", "canary") |
| + if err == nil || !strings.Contains(err.Error(), "--channel") { |
| + t.Fatalf("err = %v, want a refusal naming --channel: a replica runs its home's", err) |
| + } |
| + // Both flags stay ordinary on an ordinary create. |
| + if err := validateReplicaCreateArgs("", "", "web.go.tito.io", "canary"); err != nil { |
| + t.Fatalf("an ordinary create with --host and --channel was refused: %v", err) |
| + } |
| +} |
| + |
| func TestParseSeed(t *testing.T) { |
| cases := []struct { |
| in string |
diff --git a/internal/console/channelmove.go b/internal/console/channelmove.go| index be2ec0b3d..37725951d 100644 |
| --- a/internal/console/channelmove.go |
| +++ b/internal/console/channelmove.go |
| @@ -135,6 +135,17 @@ func (s *Server) handleAPIInstanceChannel(w http.ResponseWriter, r *http.Request |
| jsonError(w, http.StatusBadGateway, "could not read the instance") |
| return |
| } |
| + // A replica has no channel of its own to move: the reconcile pass takes |
| + // its route's channel from the HOME record every pass (spec §2), so |
| + // accepting this would write the record, audit a move that did not |
| + // happen and bump the serial for a route that converges straight back — |
| + // the console, the audit log and `carlos instances list` all saying |
| + // something the fleet never did. Name the host that CAN be moved. |
| + if rec.Role == "replica" { |
| + jsonError(w, http.StatusBadRequest, |
| + host+" is a replica and runs its home's channel; move "+rec.Home+" instead") |
| + return |
| + } |
| from := rec.Channel |
| if from == to { |
| // Nothing changed: no write, no serial bump, no audit line. A serial |
| @@ -213,6 +224,16 @@ func (s *Server) executePendingRouteChannel(ctx context.Context, acct *Account, |
| log.Printf("console: execute route-channel %s/%s/%s: %v", acct.Sqid, app, p.Host, err) |
| return "", errors.New("could not read the instance") |
| } |
| + // The door's replica refusal, asked again on the record as it is NOW — |
| + // before the etag pin, because "this is a replica" is a stronger answer |
| + // than "this moved": a confirmer told the proposal was merely stale |
| + // would propose it again, and the second one would be refused too. |
| + // Nothing in this binary can turn a home into a replica (Role is |
| + // create-only and protectedFieldsEqual holds it), so reaching this is a |
| + // hand-written record or an old console — the writers §6 warns about. |
| + if rec.Role == "replica" { |
| + return "", errors.New(p.Host + " is a replica and runs its home's channel; move " + rec.Home + " instead") |
| + } |
| if etag != p.ETag || rec.Channel != p.From { |
| return "", staleProposal("the instance is on " + rec.Channel + " now") |
| } |
diff --git a/internal/console/channelmove_test.go b/internal/console/channelmove_test.go| index f3165c061..933052b71 100644 |
| --- a/internal/console/channelmove_test.go |
| +++ b/internal/console/channelmove_test.go |
| @@ -128,6 +128,100 @@ func recordOn(t *testing.T, fx *consoleFixture, app, host, channel string) { |
| } |
| } |
| +// replicaRecordOn puts a REPLICA record in the bucket, homed on the given |
| +// host — what `carlos instances create --role replica` leaves behind. |
| +func replicaRecordOn(t *testing.T, fx *consoleFixture, app, host, home, channel string) { |
| + t.Helper() |
| + rec := provision.Record{ |
| + Schema: 1, Host: host, Slug: strings.SplitN(host, ".", 2)[0], App: app, |
| + Account: release.OpsAccount, Channel: channel, CreatedAt: time.Now().UTC(), |
| + Role: "replica", Home: home, Region: "uk", |
| + } |
| + if err := provision.CreateRecord(context.Background(), fx.rel, rec); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// A replica runs its home's channel (spec §2): the reconcile pass takes the |
| +// row's channel from the HOME record every pass. So a move accepted here |
| +// would write a record, audit a move that did not happen and bump the serial |
| +// for a route that converges straight back — the console saying one thing |
| +// and the fleet doing another. Refuse, and name the host that can be moved. |
| +func TestAPIInstanceChannelRefusesAReplica(t *testing.T) { |
| + fx, member := instanceFixture(t, "code-chan-replica") |
| + const home = "web.go.tito.io" |
| + const replica = "r7x2k9m4qa.go.tito.io" |
| + recordOn(t, fx, "titogo", home, "edge") |
| + replicaRecordOn(t, fx, "titogo", replica, home, "edge") |
| + before := serial(t, fx) |
| + |
| + status, _, out := rawReq(t, http.MethodPost, channelURL(fx, "bab", "titogo", replica), member, `{"channel":"beta"}`) |
| + if status != http.StatusBadRequest { |
| + t.Fatalf("move a replica = %d %s, want 400", status, out) |
| + } |
| + if !strings.Contains(out, home) { |
| + t.Fatalf("refusal = %s, want it to name the home %s", out, home) |
| + } |
| + if got := recordChannel(t, fx, "titogo", replica); got != "edge" { |
| + t.Fatalf("the replica record moved anyway: %q", got) |
| + } |
| + if got := serial(t, fx); got != before { |
| + t.Fatalf("serial = %d, want it unchanged at %d — nothing happened", got, before) |
| + } |
| +} |
| + |
| +// The proposals path reads the record again at execution time, and holds the |
| +// same rail there: a quorum cannot approve the move the door refuses. |
| +func TestExecutePendingRouteChannelRefusesAReplica(t *testing.T) { |
| + fx, member := instanceFixture(t, "code-chan-replica-exec") |
| + const host = "web.go.tito.io" |
| + recordOn(t, fx, "titogo", host, "production") |
| + setPipeline(t, fx, "titogo", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 1}}) |
| + |
| + status, _, out := rawReq(t, http.MethodPost, channelURL(fx, "bab", "titogo", host), member, `{"channel":"edge"}`) |
| + if status != http.StatusAccepted { |
| + t.Fatalf("propose = %d %s, want 202", status, out) |
| + } |
| + var body map[string]any |
| + if err := json.Unmarshal([]byte(out), &body); err != nil { |
| + t.Fatal(err) |
| + } |
| + id, _ := body["pending_id"].(string) |
| + pc, err := fx.st.PendingChange(id, time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + acct, err := fx.srv.store.AccountBySqid(release.OpsAccount) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // The record becomes a replica underneath the proposal. UpdateRecord's |
| + // protected-fields guard refuses that write, which is the point of the |
| + // guard — so this goes straight to the bucket, exactly as a hand edit or |
| + // a region box's IAM role would. |
| + rec, etag, err := provision.GetRecordETag(context.Background(), fx.rel, release.OpsAccount, "titogo", host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + rec.Role, rec.Home, rec.Region = "replica", "other.go.tito.io", "uk" |
| + raw, err := json.Marshal(rec) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.rel.Put(context.Background(), provision.RecordKey(release.OpsAccount, "titogo", host), raw, etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.srv.executePendingRouteChannel(context.Background(), acct, "titogo", "alice@keymail.dev", pc); err == nil || |
| + !strings.Contains(err.Error(), "other.go.tito.io") { |
| + t.Fatalf("execute err = %v, want a refusal naming the home", err) |
| + } |
| + if got := recordChannel(t, fx, "titogo", host); got != "production" { |
| + t.Fatalf("a refused execution wrote anyway: %q", got) |
| + } |
| +} |
| + |
| func recordChannel(t *testing.T, fx *consoleFixture, app, host string) string { |
| t.Helper() |
| rec, err := provision.GetRecord(context.Background(), fx.rel, release.OpsAccount, app, host) |
diff --git a/internal/host/edgesfile.go b/internal/host/edgesfile.go| index 2037f2674..b7cc951d5 100644 |
| --- a/internal/host/edgesfile.go |
| +++ b/internal/host/edgesfile.go |
| @@ -248,7 +248,7 @@ func edgesFilePass(ctx context.Context, s release.Store, reg *registry.Registry, |
| // byHost is the primary-route lookup the replica test below needs: |
| // whether an alias's PRIMARY route (a.Host) is itself a replica route |
| // (routes.replica_of set), never whether the alias's own NAME matches |
| - // ReplicaOf. With custom domains in scope (#162 revision 2), a replica |
| + // ReplicaOf. With custom domains in scope (#162 revision 3), a replica |
| // alias's name can be a tenant's own domain — the row is |
| // `claim.Host -> replica.Host` — while ReplicaOf always names the |
| // account's public host, so an equality test against a.Alias would |
diff --git a/internal/host/edgesfile_test.go b/internal/host/edgesfile_test.go| index f107b5772..d2dbbfba0 100644 |
| --- a/internal/host/edgesfile_test.go |
| +++ b/internal/host/edgesfile_test.go |
| @@ -786,7 +786,7 @@ func TestEdgesFilePublishesReplicaAliasWithoutADNSStamp(t *testing.T) { |
| } |
| } |
| -// #162 revision 2: with custom domains in scope, a replica alias's NAME can |
| +// #162 revision 3: with custom domains in scope, a replica alias's NAME can |
| // be a custom domain rather than the account host — the alias row is |
| // `claim.Host -> replica.Host`, and `ReplicaOf` names the account host, not |
| // the alias. The replica test must therefore key off the PRIMARY route's |
diff --git a/internal/host/siblings.go b/internal/host/siblings.go| index a2a078142..f3639826a 100644 |
| --- a/internal/host/siblings.go |
| +++ b/internal/host/siblings.go |
| @@ -17,6 +17,7 @@ import ( |
| "log" |
| "os" |
| "path/filepath" |
| + "slices" |
| "sort" |
| "sync/atomic" |
| @@ -125,24 +126,34 @@ func buildSiblings(all map[string]provision.Record, eipOf func(host string) (str |
| if rec.Role != "replica" || rec.Home == "" { |
| continue |
| } |
| + // The four CROSS-RECORD rails of the console's create door (spec §6), |
| + // asked again here because a bucket record is not a record the door |
| + // wrote: every region box's IAM role can put one there and an |
| + // operator can hand-write one. Pairing on the home record's mere |
| + // EXISTENCE would write the HOME's file — on the home's own box, in |
| + // the home's own account — naming a stranger's instance with a dial |
| + // address and a live pair key, telling the home's app to grant |
| + // leases to it and accept drains from it. Nothing to pair with is |
| + // the answer to all four: a file naming an address nobody vouched |
| + // for is worse than no file. |
| homeRec, ok := all[rec.Home] |
| - if !ok || homeRec.Role == "replica" { |
| - // No home record in the map (or a record pointing at another |
| - // replica): nothing to pair with, and a file naming an address |
| - // nobody vouched for is worse than no file. |
| + if !ok || homeRec.Account != rec.Account || homeRec.App != rec.App || |
| + homeRec.Role != "" || !slices.Contains(homeRec.Replicas, rec.Region) { |
| continue |
| } |
| label := pairLabel(homeRec.Account, rec.Home, host) |
| ks := derive(label) |
| - hf, ok := out[rec.Home] |
| - if !ok { |
| - // The replica record exists but its home's Replicas list does |
| - // not name its placement (mid-edit, or a stale record). Pair it |
| - // anyway: the replica is serving, and the home needs its |
| - // address to drain it. |
| - hf = siblingsFile{Role: "home", Host: rec.Home, Placement: homeRec.Region} |
| - } |
| + // The home's own file is always already there: the opt-in test above |
| + // is the same one the first loop wrote it under, so a replica that |
| + // reaches this line has a home with `Role == ""` and a Replicas list |
| + // naming this placement. (The branch that used to synthesize a file |
| + // here paired a replica the home had not opted into, calling it |
| + // "mid-edit" — but no door produces that state: the PATCH door |
| + // refuses withdrawing a placement under a live replica, so the only |
| + // writers who can are an old console and a hand edit, which is the |
| + // case being defended against.) |
| + hf := out[rec.Home] |
| if a, ok := addr(host); ok { |
| hf.Replicas = append(hf.Replicas, siblingEntry{ |
| Host: host, Placement: rec.Region, Addr: a, Keys: ks, |
diff --git a/internal/host/siblings_test.go b/internal/host/siblings_test.go| index 6c50afe1a..61b62baef 100644 |
| --- a/internal/host/siblings_test.go |
| +++ b/internal/host/siblings_test.go |
| @@ -99,6 +99,82 @@ func TestBuildSiblingsIgnoresAnAccountWithNoReplicas(t *testing.T) { |
| } |
| } |
| +// The four cross-record rails of the console's create door (spec §6), held |
| +// here too because a bucket record is not a record the door wrote: any |
| +// region box's IAM role can put one, and an operator can hand-write one. |
| +// Pairing an unvouched-for record would write the VICTIM's siblings file |
| +// naming a stranger's instance, with a dial address and a live pair key — |
| +// telling the victim's own app to grant leases to it and accept drains from |
| +// it. Each case below is a record that pairs today and must not. |
| +func TestBuildSiblingsRefusesAnUnvouchedForReplica(t *testing.T) { |
| + const ( |
| + victim = "acme.go.tito.io" |
| + home2 = "other.go.tito.io" |
| + ) |
| + homeRec := provision.Record{Host: victim, Account: "bab", App: "tito", Region: "ie", |
| + Steering: "latency", Replicas: []string{"uk"}} |
| + replicaRec := func(host, account, app, region, home string) provision.Record { |
| + return provision.Record{Host: host, Account: account, App: app, Region: region, |
| + Role: "replica", Home: home} |
| + } |
| + eip := func(string) (string, bool) { return "203.0.113.9", true } |
| + |
| + for _, tc := range []struct { |
| + name string |
| + replica provision.Record |
| + extra []provision.Record |
| + }{ |
| + { |
| + name: "another account's home", |
| + replica: replicaRec("r0000000000.go.tito.io", "mal", "tito", "uk", victim), |
| + }, |
| + { |
| + name: "another app's home", |
| + replica: replicaRec("r1111111111.go.tito.io", "bab", "other", "uk", victim), |
| + }, |
| + { |
| + name: "the home is itself a replica", |
| + replica: replicaRec("r2222222222.go.tito.io", "bab", "tito", "de", home2), |
| + extra: []provision.Record{replicaRec(home2, "bab", "tito", "uk", victim)}, |
| + }, |
| + { |
| + name: "a placement the home never opted into", |
| + replica: replicaRec("r3333333333.go.tito.io", "bab", "tito", "de", victim), |
| + }, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + all := map[string]provision.Record{victim: homeRec, tc.replica.Host: tc.replica} |
| + for _, r := range tc.extra { |
| + all[r.Host] = r |
| + } |
| + got := buildSiblings(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]) |
| + } |
| + for host, f := range got { |
| + for _, e := range f.Replicas { |
| + if e.Host == tc.replica.Host { |
| + t.Errorf("%s's file vouches for %s with addr %q and a pair key", host, e.Host, e.Addr) |
| + } |
| + } |
| + if f.Home != nil && f.Home.Host == tc.replica.Host { |
| + t.Errorf("%s's file names %s as its home", host, f.Home.Host) |
| + } |
| + } |
| + // The victim's own file is still written — a home that opted in |
| + // gets one even with no replica paired (spec §7), and it lists |
| + // only the replicas it did opt into. |
| + v, ok := got[victim] |
| + if !ok { |
| + t.Fatal("the victim's own home file went missing") |
| + } |
| + if len(v.Replicas) != len(tc.extra) { |
| + t.Errorf("the victim's file lists %+v, want only the %d it opted into", v.Replicas, len(tc.extra)) |
| + } |
| + }) |
| + } |
| +} |
| + |
| // --- the writer --- |
| const ( |
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go| index aa70ba875..508842a3d 100644 |
| --- a/internal/provision/reconcile.go |
| +++ b/internal/provision/reconcile.go |
| @@ -371,6 +371,13 @@ func routeFor(rec Record, home Record, dataDir, seed string) registry.Route { |
| // and the replica record carries none of its own (spec §2). |
| rt.ReplicaOf = rec.Home |
| rt.Channel = home.Channel |
| + // And the home's GATE, for the same reason and with more at stake: |
| + // the alias sends the account's PUBLIC host to this row, so a |
| + // replica carrying its own (always empty) gate would serve a |
| + // password-gated site ungated from the replica's edge while the |
| + // home's edge still asks. An account that gates its site must not |
| + // have an ungated mirror of it. |
| + rt.Gate = home.Gate |
| } |
| if rec.DerivesPaths() { |
| rt.Socket = DefaultSocket(rec.Host) |
| @@ -606,6 +613,31 @@ func convergeOwned(reg *registry.Registry, existing registry.Route, rec Record, |
| log.Printf("provision: converged %s channel %s -> %s", rec.Host, existing.Channel, channel) |
| } |
| } |
| + // The password gate follows the home across a pair for the same reason |
| + // the channel does, and converges here rather than at the caller so both |
| + // halves of "what this row serves" are derived in one place: the alias |
| + // hands the account's PUBLIC host to the replica's row, so a replica |
| + // carrying its own empty gate would answer a gated site with no password |
| + // at all (spec §6, routeFor's replica branch is the mint half). "" is as |
| + // much an answer as a hash — lifting the home's gate must reach the |
| + // mirror, or it keeps asking for a password the account has withdrawn. |
| + // |
| + // The log names the STATE, never the secret: a bcrypt hash is not a |
| + // password, but it is the material an offline cracker works on, and an |
| + // operations log travels further than a registry row. |
| + gate := rec.Gate |
| + if rec.Role == "replica" { |
| + gate = home.Gate |
| + } |
| + if existing.Gate != gate { |
| + if dryRun { |
| + log.Printf("provision: dry-run: would converge %s gate %s -> %s", rec.Host, gateWord(existing.Gate), gateWord(gate)) |
| + } else if err := reg.SetGate(rec.Host, gate); err != nil { |
| + log.Printf("provision: converge gate on %s: %v", rec.Host, err) |
| + } else { |
| + log.Printf("provision: converged %s gate %s -> %s", rec.Host, gateWord(existing.Gate), gateWord(gate)) |
| + } |
| + } |
| if existing.Environment != rec.Environment { |
| if dryRun { |
| log.Printf("provision: dry-run: would converge %s environment %q -> %q", rec.Host, existing.Environment, rec.Environment) |
| @@ -896,10 +928,41 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| // Protect rather than guess — the row keeps serving what it serves, |
| // and the removal phase leaves it alone (spec §2). Asked before |
| // trashedFor and seedFor, so a mirror of nothing costs no reads. |
| + // |
| + // The same four CROSS-RECORD rails the console's create door |
| + // enforces (spec §6) are asked here, because the door is not a |
| + // guarantee about what is in the bucket: every region box's IAM role |
| + // can PutObject into it and an operator can hand-write a record, and |
| + // this is the one place in the pass where the other record is |
| + // already in hand. Without them a record in ANY account naming |
| + // another account's host as its home mints here, has the domains |
| + // pass alias the victim's public host to this box, and is written |
| + // into the VICTIM's own siblings file as a vouched-for replica with |
| + // a live pair key. |
| + // |
| + // Protect rather than retire, in every one of these cases: a home |
| + // whose `replicas` an old console stripped keeps its existing row |
| + // serving, which is the fail-safe direction the spec chose for a |
| + // home that cannot be verified. The condition that failed is named |
| + // in the log, because "not minting" with no reason is how an |
| + // operator's typo becomes an afternoon. |
| if rec.Role == "replica" { |
| - if _, ok := all[rec.Home]; !ok { |
| - log.Printf("provision: not minting replica %s: its home record %s is absent or unreadable this pass", |
| - rec.Host, rec.Home) |
| + home, ok := all[rec.Home] |
| + var why string |
| + switch { |
| + case !ok: |
| + why = "its home record " + rec.Home + " is absent or unreadable this pass" |
| + case home.Account != rec.Account: |
| + why = "its home " + rec.Home + " belongs to another account" |
| + case home.App != rec.App: |
| + why = "its home " + rec.Home + " belongs to another app" |
| + case home.Role != "": |
| + why = "its home " + rec.Home + " is itself a replica" |
| + case !slices.Contains(home.Replicas, rec.Region): |
| + why = "its home " + rec.Home + " has not opted into placement " + rec.Region |
| + } |
| + if why != "" { |
| + log.Printf("provision: not minting replica %s: %s", rec.Host, why) |
| protect[rec.Host] = true |
| continue |
| } |
| @@ -980,15 +1043,6 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| log.Printf("provision: converged %s hibernate %v -> %v%s", rec.Host, existing.Hibernate, *rec.Hibernate, dropped) |
| } |
| } |
| - if existing.Gate != rec.Gate { |
| - if cfg.DryRun { |
| - log.Printf("provision: dry-run: would converge %s gate %s -> %s", rec.Host, gateWord(existing.Gate), gateWord(rec.Gate)) |
| - } else if err := reg.SetGate(rec.Host, rec.Gate); err != nil { |
| - log.Printf("provision: converge gate on %s: %v", rec.Host, err) |
| - } else { |
| - log.Printf("provision: converged %s gate %s -> %s", rec.Host, gateWord(existing.Gate), gateWord(rec.Gate)) |
| - } |
| - } |
| // seedIsKnown guards against stamping a transient manifest-read |
| // failure's "" fallback over a working seed — see seedFor. |
| if seedIsKnown && existing.Seed != seed { |
diff --git a/internal/provision/reconcile_test.go b/internal/provision/reconcile_test.go| index edf6991fb..3a84d8b2b 100644 |
| --- a/internal/provision/reconcile_test.go |
| +++ b/internal/provision/reconcile_test.go |
| @@ -2852,3 +2852,145 @@ func TestReconcileReportsEveryRecord(t *testing.T) { |
| t.Error("OnRecords did not carry the in-scope replica record") |
| } |
| } |
| + |
| +// The pass holds the same four cross-record rails the console's create door |
| +// does (spec §6), because the door is not a guarantee about what is in the |
| +// bucket: every region box's IAM role can PutObject there, and an operator |
| +// can hand-write a record. A replica record in the ATTACKER's account naming |
| +// the VICTIM's host as its home must not mint — minting it would alias the |
| +// victim's public host to the attacker's process on this box. |
| +func TestReconcileRefusesAReplicaWhoseHomeIsAnotherAccounts(t *testing.T) { |
| + s, _, home, _ := replicaSetup(t) |
| + mallory := writeAccountManifest(t, s, 3, "Mallory") |
| + if err := PutManifest(ctx, s, mallory, "tito", Manifest{ |
| + Schema: 1, App: "tito", Domain: "go.tito.io", Pool: "tito", Seed: []string{"seed"}, |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + evil := Record{Schema: 1, Host: "r0000000000.go.tito.io", Slug: "r0000000000", App: "tito", |
| + Account: mallory, 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) |
| + } |
| + uk := regWith(t) |
| + if err := Reconcile(ctx, s, uk, ReconcileConfig{Region: "uk", Pools: []string{"tito"}, DataDir: "/data"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + // Get reports a miss as (nil, nil), never an error. |
| + if rt, err := uk.Get(evil.Host); err != nil { |
| + t.Fatal(err) |
| + } else if rt != nil { |
| + t.Errorf("a replica record naming ANOTHER ACCOUNT's home was minted: %+v", rt) |
| + } |
| +} |
| + |
| +// Same rail, the app half: one account's own two apps are two tenancies for |
| +// this purpose, and a replica of app A's home minted under app B would carry |
| +// app B's records and app A's traffic. |
| +func TestReconcileRefusesAReplicaWhoseHomeIsInAnotherApp(t *testing.T) { |
| + s, sqid, home, _ := replicaSetup(t) |
| + if err := PutManifest(ctx, s, sqid, "other", Manifest{ |
| + Schema: 1, App: "other", Domain: "go.tito.io", Pool: "tito", Seed: []string{"seed"}, |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + cross := Record{Schema: 1, Host: "r1111111111.go.tito.io", Slug: "r1111111111", App: "other", |
| + Account: sqid, Pool: "tito", Region: "uk", Channel: "stable", CreatedAt: time.Now(), |
| + Role: "replica", Home: home.Host} |
| + if err := CreateRecord(ctx, s, cross); err != nil { |
| + t.Fatal(err) |
| + } |
| + uk := regWith(t) |
| + if err := Reconcile(ctx, s, uk, ReconcileConfig{Region: "uk", Pools: []string{"tito"}, DataDir: "/data"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt, err := uk.Get(cross.Host); err != nil { |
| + t.Fatal(err) |
| + } else if rt != nil { |
| + t.Errorf("a replica record naming a home in ANOTHER APP was minted: %+v", rt) |
| + } |
| +} |
| + |
| +// No chains: a replica of a replica is not a shape the platform carries |
| +// (spec §2), and the door says so. So does the pass. |
| +func TestReconcileRefusesAReplicaOfAReplica(t *testing.T) { |
| + s, sqid, _, replica := replicaSetup(t) |
| + chained := Record{Schema: 1, Host: "r2222222222.go.tito.io", Slug: "r2222222222", App: "tito", |
| + Account: sqid, Pool: "tito", Region: "de", Channel: "stable", CreatedAt: time.Now(), |
| + Role: "replica", Home: replica.Host} |
| + if err := CreateRecord(ctx, s, chained); err != nil { |
| + t.Fatal(err) |
| + } |
| + de := regWith(t) |
| + if err := Reconcile(ctx, s, de, ReconcileConfig{Region: "de", Pools: []string{"tito"}, DataDir: "/data"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt, err := de.Get(chained.Host); err != nil { |
| + t.Fatal(err) |
| + } else if rt != nil { |
| + t.Errorf("a replica whose home is itself a replica was minted: %+v", rt) |
| + } |
| +} |
| + |
| +// Replicas IS the residence gate (spec §2): a placement the home never opted |
| +// into is a decision about where order data lives that nobody made. |
| +func TestReconcileRefusesAReplicaInAPlacementTheHomeNeverOptedInto(t *testing.T) { |
| + s, sqid, home, _ := replicaSetup(t) // home opts into uk, and only uk |
| + elsewhere := Record{Schema: 1, Host: "r3333333333.go.tito.io", Slug: "r3333333333", App: "tito", |
| + Account: sqid, Pool: "tito", Region: "de", Channel: "stable", CreatedAt: time.Now(), |
| + Role: "replica", Home: home.Host} |
| + if err := CreateRecord(ctx, s, elsewhere); err != nil { |
| + t.Fatal(err) |
| + } |
| + de := regWith(t) |
| + if err := Reconcile(ctx, s, de, ReconcileConfig{Region: "de", Pools: []string{"tito"}, DataDir: "/data"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt, err := de.Get(elsewhere.Host); err != nil { |
| + t.Fatal(err) |
| + } else if rt != nil { |
| + t.Errorf("a replica minted in a placement the home never opted into: %+v", rt) |
| + } |
| +} |
| + |
| +// A gated home must not have an ungated mirror of itself: the alias sends the |
| +// account's public host to the replica's row, so that row carries the home's |
| +// gate or the password gate simply does not exist in that placement. The |
| +// gate is carried exactly as the channel is — stamped at the mint and |
| +// converged on every pass, so it follows a change. |
| +func TestReconcileReplicaCarriesItsHomesGate(t *testing.T) { |
| + s, sqid, home, replica := replicaSetup(t) |
| + const ( |
| + gateOne = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy" |
| + gateTwo = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWx" |
| + ) |
| + home.Gate = gateOne |
| + putReplace(t, s, RecordKey(sqid, "tito", home.Host), mustJSON(t, home)) |
| + 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 got := mustGet(t, uk, replica.Host).Gate; got != home.Gate { |
| + t.Errorf("minted replica gate = %q, want the home's %q — the mirror is serving the gated host ungated", got, home.Gate) |
| + } |
| + home.Gate = gateTwo |
| + putReplace(t, s, RecordKey(sqid, "tito", home.Host), mustJSON(t, home)) |
| + if err := Reconcile(ctx, s, uk, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := mustGet(t, uk, replica.Host).Gate; got != home.Gate { |
| + t.Errorf("converged replica gate = %q, want the home's %q — a gate change did not reach the replica", got, home.Gate) |
| + } |
| + // And when the home's gate is lifted the replica's must lift too, or the |
| + // mirror keeps asking for a password the account has withdrawn. |
| + home.Gate = "" |
| + putReplace(t, s, RecordKey(sqid, "tito", home.Host), mustJSON(t, home)) |
| + if err := Reconcile(ctx, s, uk, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := mustGet(t, uk, replica.Host).Gate; got != "" { |
| + t.Errorf("replica gate = %q after the home's was lifted, want empty", got) |
| + } |
| +} |