console: rotate refuses a re-rotate, reconcile compares issued digest, purge publishes the aggregate (review fix)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5 files changed,
+267
−34
internal/console/ownership.go+15 −0internal/console/relayops.go+81 −30internal/console/relayops_test.go+147 −1internal/relay/keys.go+8 −0internal/relay/record.go+16 −3
diff --git a/internal/console/ownership.go b/internal/console/ownership.go| index 9fc8450db..afef84e4a 100644 |
| --- a/internal/console/ownership.go |
| +++ b/internal/console/ownership.go |
| @@ -470,6 +470,21 @@ func (s *Server) handlePurgeApp(w http.ResponseWriter, r *http.Request) { |
| fail(purgeAdvice(err, app)) |
| return |
| } |
| + // The relay's authority record is DERIVED from the relay.json objects |
| + // under apps/<sqid>/<app>/, and the sweep above just deleted this app's. |
| + // Republish now so the app is out of every relay's authority at once |
| + // (relay spec §5, "app deletion") rather than keeping its credentials |
| + // valid until the five-minute reconcile. Only the lease holder may |
| + // publish, and a failure here is a log line, not a failed purge: the |
| + // bucket is already swept, and the next holder's unconditional rebuild — |
| + // which lists records rather than hunting for unfinished ones — is |
| + // exactly the repair for this case. |
| + if s.relayHold() { |
| + if err := s.relayPublishAggregate(r.Context()); err != nil { |
| + log.Printf("console: purge %s/%s: relay aggregate (the reconcile republishes within five minutes): %v", |
| + acct.Sqid, app, err) |
| + } |
| + } |
| // No bumpSerial here, deliberately: PurgeApp does delete box-parsed |
| // shapes (instance records, config objects) under apps/<sqid>/<app>/, |
| // but the fleet-visible teardown already happened, and was already |
diff --git a/internal/console/relayops.go b/internal/console/relayops.go| index a93a6765f..924219ef8 100644 |
| --- a/internal/console/relayops.go |
| +++ b/internal/console/relayops.go |
| @@ -37,6 +37,7 @@ package console |
| import ( |
| "context" |
| "crypto/rand" |
| + "crypto/sha256" |
| "encoding/base64" |
| "encoding/hex" |
| "encoding/json" |
| @@ -45,7 +46,6 @@ import ( |
| "log" |
| "maps" |
| "os" |
| - "regexp" |
| "slices" |
| "sort" |
| "strings" |
| @@ -123,13 +123,11 @@ var ( |
| "this console does not hold the relay writer lease — another console is driving the relay; the operation was not started") |
| errRelayNotEnabled = errors.New("the relay is not enabled for this app") |
| errRelayUndelivered = errors.New("a route has no config delivery") |
| + // errRelayUnknownHost is `carlos relay adopted --host` naming something |
| + // that is not a route of this app — the door's 404. |
| + errRelayUnknownHost = errors.New("no such route on this app") |
| ) |
| -// relayKidRe mirrors internal/relay's own kid alphabet (keys.go). Enforced |
| -// on the way OUT of the console's environment so a typo in a staged master |
| -// becomes a refusal here, not a username the relay silently declines. |
| -var relayKidRe = regexp.MustCompile(`^[a-z0-9]{1,8}$`) |
| - |
| // relayLease is control/relay-writer.json. |
| type relayLease struct { |
| Holder string `json:"holder"` |
| @@ -333,7 +331,7 @@ func relayStagedMasters(region string) ([]relayStagedMaster, error) { |
| return nil, fmt.Errorf("%s holds an entry that is not <kid>:<base64 key>", name) |
| } |
| kid = strings.TrimSpace(kid) |
| - if !relayKidRe.MatchString(kid) { |
| + if !relay.ValidKid(kid) { |
| return nil, fmt.Errorf("%s holds an invalid kid %q (1-8 lowercase letters or digits)", name, kid) |
| } |
| key, err := base64.StdEncoding.DecodeString(strings.TrimSpace(keyB64)) |
| @@ -536,6 +534,15 @@ func (s *Server) relayExpectedSuffixes(ctx context.Context, acct, app string, re |
| return out, nil |
| } |
| +// relayEnvelopeDigest identifies a sealed envelope without opening it: |
| +// hex(sha256) of the envelope string as it sits in the bundle. It is what |
| +// makes "the bundle still holds exactly what we wrote" checkable by a console |
| +// that holds no decryption key — see relay.Record.IssuedDigest. |
| +func relayEnvelopeDigest(envelope string) string { |
| + sum := sha256.Sum256([]byte(envelope)) |
| + return hex.EncodeToString(sum[:]) |
| +} |
| + |
| // relayConfigVar reads the app's delivered CARLOS_RELAY entry, if any. |
| func (s *Server) relayConfigVar(ctx context.Context, acct, app string) (appconfig.Var, bool, error) { |
| m, err := appconfig.Get(ctx, s.cfg.Releases, acct, app) |
| @@ -576,6 +583,7 @@ func (s *Server) relayApplyConfig(ctx context.Context, acct, app string, rec *re |
| return err |
| } |
| rec.IssuedSuffixes = suffixes |
| + rec.IssuedDigest = relayEnvelopeDigest(env) |
| return nil |
| } |
| @@ -608,6 +616,18 @@ func (s *Server) relayAppLock(acct, app string) *sync.Mutex { |
| return v.(*sync.Mutex) |
| } |
| +// relayPutRecord is relay.PutRecord behind the lease check. EVERY record |
| +// write in this file goes through it rather than calling PutRecord directly: |
| +// a walk is several writes long, the lease can lapse between any two of them, |
| +// and spec §5's rule is a check before each bucket write, not one per |
| +// operation. |
| +func (s *Server) relayPutRecord(ctx context.Context, acct, app string, rec *relay.Record, etag string) (string, error) { |
| + if err := s.requireRelayLease(); err != nil { |
| + return "", err |
| + } |
| + return relay.PutRecord(ctx, s.cfg.Releases, acct, app, rec, etag) |
| +} |
| + |
| // relayAppRecord is one app's record with the coordinates that name it and |
| // the etag it was read at — the unit both the publish listing and the |
| // reconcile walk over. |
| @@ -728,7 +748,7 @@ func (s *Server) relayPublishAggregate(ctx context.Context) error { |
| } |
| err = s.relayPublishOnce(ctx, etag) |
| if err == nil { |
| - if err := release.BumpSerial(ctx, s.cfg.Releases, time.Now()); err != nil { |
| + if err := release.BumpSerial(ctx, s.cfg.Releases, s.relayTime()); err != nil { |
| log.Printf("console: relay serial bump (relays converge on their own timer anyway): %v", err) |
| } |
| return nil |
| @@ -761,10 +781,19 @@ type relayRoute struct { |
| // Records first, registry second, deduped on host: a route that exists in |
| // both is one route, and the record is the authority on its shape. |
| func (s *Server) relayAppRoutes(ctx context.Context, acct, app string) ([]relayRoute, error) { |
| - recs, _, err := provision.ListRecords(ctx, s.cfg.Releases, acct, app, relayRouteMax) |
| + recs, truncated, err := provision.ListRecords(ctx, s.cfg.Releases, acct, app, relayRouteMax) |
| if err != nil { |
| return nil, err |
| } |
| + // A truncated listing is a REFUSAL, not a partial answer. The one caller |
| + // that matters is the delivery check, and a check that only saw the first |
| + // 500 routes would enable the relay over an undeliverable route it never |
| + // read — the exact state spec §5 refuses because it blocks retirement |
| + // forever, arrived at silently. |
| + if truncated { |
| + return nil, fmt.Errorf("%s/%s has more than %d instance records — the relay cannot check them all, so it will not act on a partial list", |
| + acct, app, relayRouteMax) |
| + } |
| var out []relayRoute |
| seen := map[string]bool{} |
| for _, rec := range recs { |
| @@ -928,21 +957,21 @@ func (s *Server) relayEnable(ctx context.Context, acct *Account, app string, reg |
| Op: op + 1, Phase: relayPhaseConfig, EnabledAt: now, |
| } |
| } |
| - if etag, err = relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + if etag, err = s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| if err := s.relayApplyConfig(ctx, acct.Sqid, app, rec); err != nil { |
| return nil, err |
| } |
| rec.Phase = relayPhaseAggregate |
| - if etag, err = relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + if etag, err = s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| if err := s.relayPublishAggregate(ctx); err != nil { |
| return nil, err |
| } |
| rec.Phase = relayPhaseDone |
| - if _, err = relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + if _, err = s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| log.Printf("console: relay enabled %s/%s in %s by %s (incarnation %s, op %d)", |
| @@ -982,7 +1011,8 @@ func (s *Server) relayDisable(ctx context.Context, acct *Account, app string, by |
| // Nothing is issued any more. Cutovers stay: they are a historical |
| // record of what a member acknowledged, and a re-enable clears them. |
| rec.IssuedSuffixes = nil |
| - if etag, err = relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + rec.IssuedDigest = "" |
| + if etag, err = s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| } |
| @@ -990,14 +1020,14 @@ func (s *Server) relayDisable(ctx context.Context, acct *Account, app string, by |
| return nil, err |
| } |
| rec.Phase = relayPhaseAggregate |
| - if etag, err = relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + if etag, err = s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| if err := s.relayPublishAggregate(ctx); err != nil { |
| return nil, err |
| } |
| rec.Phase = relayPhaseDone |
| - if _, err = relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + if _, err = s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| log.Printf("console: relay disabled %s/%s by %s (op %d)", acct.Sqid, app, by, rec.Op) |
| @@ -1030,6 +1060,17 @@ func (s *Server) relayAdopted(ctx context.Context, acct *Account, app, host, by |
| if rec == nil || !rec.Enabled { |
| return nil, errRelayNotEnabled |
| } |
| + // The host must be one of the app's own routes. Cheap, and it is what |
| + // keeps the record bounded: without it, an ack naming anything at all |
| + // appends a cutover forever, and `status` would report identities against |
| + // routes that do not exist. |
| + routes, err := s.relayAppRoutes(ctx, acct.Sqid, app) |
| + if err != nil { |
| + return nil, err |
| + } |
| + if !slices.ContainsFunc(routes, func(rt relayRoute) bool { return rt.Host == host }) { |
| + return nil, fmt.Errorf("%s is not a route of this app: %w", host, errRelayUnknownHost) |
| + } |
| cut := relay.RouteCutover{ |
| Host: host, |
| Suffixes: maps.Clone(rec.IssuedSuffixes), |
| @@ -1047,7 +1088,7 @@ func (s *Server) relayAdopted(ctx context.Context, acct *Account, app, host, by |
| if !replaced { |
| rec.Cutovers = append(rec.Cutovers, cut) |
| } |
| - if _, err := relay.PutRecord(ctx, s.cfg.Releases, acct.Sqid, app, rec, etag); err != nil { |
| + if _, err := s.relayPutRecord(ctx, acct.Sqid, app, rec, etag); err != nil { |
| return nil, err |
| } |
| return rec, nil |
| @@ -1130,14 +1171,15 @@ func (s *Server) relayRotate(ctx context.Context, region string) (relayRotateRes |
| if curKid == "" { |
| curKid = staged[0].Kid |
| } |
| - next := "" |
| - for _, m := range staged { |
| - if m.Kid != curKid { |
| - next = m.Kid |
| - } |
| - } |
| - if next == "" { |
| - return res, fmt.Errorf("no new master is staged for %s — add one to %s (and to /etc/carlos/relay-masters on the region's boxes) before rotating", |
| + // The new kid is the LAST staged entry, full stop — never "the last one |
| + // that is not the issuer". After a rotation the variable holds |
| + // "<old>,<new>" with the pointer on <new> for the 28 hours before the old |
| + // one may be retired, and a second rotate in that window must refuse, not |
| + // walk the pointer BACK onto the master about to be retired and re-issue |
| + // every app under it. |
| + next := staged[len(staged)-1].Kid |
| + if next == curKid { |
| + return res, fmt.Errorf("no new master is staged for %s: the last entry in %s is already the issuer", |
| region, relayMastersEnv(region)) |
| } |
| hbs, err := s.relayHeartbeats(ctx) |
| @@ -1220,21 +1262,21 @@ func (s *Server) relayReissue(ctx context.Context, r relayAppRecord) error { |
| } |
| rec.Op++ |
| rec.Phase = relayPhaseConfig |
| - if etag, err = relay.PutRecord(ctx, s.cfg.Releases, r.Account, r.App, rec, etag); err != nil { |
| + if etag, err = s.relayPutRecord(ctx, r.Account, r.App, rec, etag); err != nil { |
| return err |
| } |
| if err := s.relayApplyConfig(ctx, r.Account, r.App, rec); err != nil { |
| return err |
| } |
| rec.Phase = relayPhaseAggregate |
| - if etag, err = relay.PutRecord(ctx, s.cfg.Releases, r.Account, r.App, rec, etag); err != nil { |
| + if etag, err = s.relayPutRecord(ctx, r.Account, r.App, rec, etag); err != nil { |
| return err |
| } |
| if err := s.relayPublishAggregate(ctx); err != nil { |
| return err |
| } |
| rec.Phase = relayPhaseDone |
| - _, err = relay.PutRecord(ctx, s.cfg.Releases, r.Account, r.App, rec, etag) |
| + _, err = s.relayPutRecord(ctx, r.Account, r.App, rec, etag) |
| return err |
| } |
| @@ -1317,7 +1359,16 @@ func (s *Server) relayConverge(ctx context.Context, r relayAppRecord) (string, b |
| if err != nil { |
| return "", false, err |
| } |
| - current := present && v.Kind == "secret" && v.Envelope != "" && maps.Equal(rec.IssuedSuffixes, want) |
| + // Current means BOTH halves agree: the identities the record says it |
| + // issued are the ones the masters in force imply (a rotation, |
| + // re-enable or region change moves those), AND the bundle still holds |
| + // the exact envelope this console last wrote (a straggler write from |
| + // a superseded console, or a member setting CARLOS_RELAY themselves, |
| + // moves that). A record from before IssuedDigest existed has none, so |
| + // it re-applies once and then carries one. |
| + current := present && v.Kind == "secret" && v.Envelope != "" && |
| + maps.Equal(rec.IssuedSuffixes, want) && |
| + rec.IssuedDigest != "" && relayEnvelopeDigest(v.Envelope) == rec.IssuedDigest |
| if current && rec.Phase == relayPhaseDone { |
| return etag, false, nil |
| } |
| @@ -1335,7 +1386,7 @@ func (s *Server) relayConverge(ctx context.Context, r relayAppRecord) (string, b |
| } |
| } |
| rec.Phase = relayPhaseAggregate |
| - etag, err = relay.PutRecord(ctx, s.cfg.Releases, r.Account, r.App, rec, etag) |
| + etag, err = s.relayPutRecord(ctx, r.Account, r.App, rec, etag) |
| if err != nil { |
| return "", false, err |
| } |
| @@ -1359,6 +1410,6 @@ func (s *Server) relayMarkDone(ctx context.Context, r relayAppRecord, etag strin |
| return nil // moved on under us; its own walk owns the phase now |
| } |
| rec.Phase = relayPhaseDone |
| - _, err = relay.PutRecord(ctx, s.cfg.Releases, r.Account, r.App, rec, etag) |
| + _, err = s.relayPutRecord(ctx, r.Account, r.App, rec, etag) |
| return err |
| } |
diff --git a/internal/console/relayops_test.go b/internal/console/relayops_test.go| index 95080b14f..8044cfe01 100644 |
| --- a/internal/console/relayops_test.go |
| +++ b/internal/console/relayops_test.go |
| @@ -14,6 +14,8 @@ import ( |
| "encoding/base64" |
| "encoding/json" |
| "errors" |
| + "net/http" |
| + "net/url" |
| "strings" |
| "sync" |
| "testing" |
| @@ -37,6 +39,14 @@ import ( |
| func relayFixture(t *testing.T, code string) (*consoleFixture, string) { |
| t.Helper() |
| fx, _ := storeFixture(t, code) |
| + return fx, armRelay(t, fx) |
| +} |
| + |
| +// armRelay gives any console fixture a relay: one armed region, a sealing key |
| +// whose private half comes back to the caller, a staged master, and the |
| +// writer lease held. |
| +func armRelay(t *testing.T, fx *consoleFixture) string { |
| + t.Helper() |
| pub, priv, _ := sealedFor(t, "irrelevant") |
| fx.srv.cfg.SecretsPub = pub |
| fx.srv.cfg.RelayRegions = []string{"eu-west-1"} |
| @@ -45,7 +55,7 @@ func relayFixture(t *testing.T, code string) (*consoleFixture, string) { |
| if err := fx.srv.relayAcquire(context.Background()); err != nil { |
| t.Fatalf("acquire the writer lease: %v", err) |
| } |
| - return fx, priv |
| + return priv |
| } |
| // relayTestMaster is a deterministic 32-byte master, base64 as the console |
| @@ -412,6 +422,36 @@ func TestRelayReconcileRepairsDeletionWithoutRecord(t *testing.T) { |
| } |
| } |
| +// TestRelayPurgeRepublishesTheAggregate: permanently deleting an app takes it |
| +// out of every relay's authority AT ONCE, not within five minutes. The |
| +// deletion walk sweeps the record; publishing right after is what makes the |
| +// app's credentials stop verifying while the operator is still watching. |
| +func TestRelayPurgeRepublishesTheAggregate(t *testing.T) { |
| + fx, c, csrf := trashPageFixture(t) |
| + armRelay(t, fx) |
| + ctx := context.Background() |
| + |
| + if _, err := fx.srv.relayEnable(ctx, fx.account(t), "kass", nil, "paul@keymail.dev"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := relayAuthority(t, fx).Lookup(fx.sqid, "kass"); !ok { |
| + t.Fatal("the enable did not publish") |
| + } |
| + |
| + status, _, body := postGated(t, fx, c, fx.u("/trash/delete"), |
| + url.Values{"csrf": {csrf}, "app": {"kass"}, "confirm": {"kass"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("purge = %d:\n%s", status, body) |
| + } |
| + if keys := appKeys(t, fx, "kass"); len(keys) != 0 { |
| + t.Fatalf("the bucket still holds %v", keys) |
| + } |
| + // No reconcile anywhere in this test: the purge itself republished. |
| + if _, ok := relayAuthority(t, fx).Lookup(fx.sqid, "kass"); ok { |
| + t.Fatal("a purged app still holds relay authority") |
| + } |
| +} |
| + |
| // TestRelayEnableRefusesRouteWithoutDelivery: a TCP-address route this |
| // platform does not own, whose bundle has not opted into file delivery, could |
| // never pick up a rotated key — so it would block every future retirement |
| @@ -445,6 +485,41 @@ func TestRelayEnableRefusesRouteWithoutDelivery(t *testing.T) { |
| } |
| } |
| +// TestRelayAdoptedRecordsTheIssuedIdentitiesAndRefusesAStranger: a member's |
| +// ack stamps the identities issued AT THAT MOMENT against their own route, so |
| +// an ack given before a later rotation cannot count for it — and a host that |
| +// is not a route of this app is refused rather than appended, which is what |
| +// keeps the record bounded. |
| +func TestRelayAdoptedRecordsTheIssuedIdentitiesAndRefusesAStranger(t *testing.T) { |
| + fx, _ := relayFixture(t, "code-relay-adopted") |
| + ctx := context.Background() |
| + acct := fx.account(t) |
| + fx.addRoute(t, registry.Route{Host: "titogo.oncarlos.com", App: "titogo", |
| + Kind: "instance", Status: "ready", Addr: "10.0.0.9:8080"}) |
| + |
| + rec, err := fx.srv.relayEnable(ctx, acct, "titogo", nil, "alice@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.srv.relayAdopted(ctx, acct, "titogo", "somewhere.else.example", "alice@keymail.dev"); !errors.Is(err, errRelayUnknownHost) { |
| + t.Fatalf("adopted for a host that is not a route = %v, want a refusal", err) |
| + } |
| + after, err := fx.srv.relayAdopted(ctx, acct, "titogo", "titogo.oncarlos.com", "alice@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(after.Cutovers) != 1 || after.Cutovers[0].Host != "titogo.oncarlos.com" || |
| + after.Cutovers[0].Suffixes["eu-west-1"] != rec.IssuedSuffixes["eu-west-1"] { |
| + t.Fatalf("cutovers = %+v, want one naming the issued identity", after.Cutovers) |
| + } |
| + // A second ack for the same route replaces the first rather than growing |
| + // the record. |
| + again, err := fx.srv.relayAdopted(ctx, acct, "titogo", "titogo.oncarlos.com", "alice@keymail.dev") |
| + if err != nil || len(again.Cutovers) != 1 { |
| + t.Fatalf("second ack = %+v (%v), want one cutover still", again.Cutovers, err) |
| + } |
| +} |
| + |
| // TestRelayRotateRefusesUntilHeartbeatsCarryKid: rotation's whole safety is |
| // that the console will not issue from a master a live relay has not loaded. |
| // Until the region's heartbeat lists the new kid, rotate refuses and the |
| @@ -487,6 +562,17 @@ func TestRelayRotateRefusesUntilHeartbeatsCarryKid(t *testing.T) { |
| if res.Kid != "k2" || res.Apps != 1 { |
| t.Fatalf("rotate result = %+v, want kid k2 and one app re-issued", res) |
| } |
| + // A SECOND rotate in the same window must refuse. The staged variable |
| + // still holds "k1,k2" — the old master stays there for the 28 hours before |
| + // it may be retired — and rotating "to the entry that is not the issuer" |
| + // would walk the pointer back onto the master about to be retired. |
| + if _, err := fx.srv.relayRotate(ctx, "eu-west-1"); err == nil || |
| + !strings.Contains(err.Error(), "already the issuer") { |
| + t.Fatalf("re-rotate with nothing new staged = %v, want a refusal naming the last entry", err) |
| + } |
| + if issuers, _, err := fx.srv.relayIssuers(ctx); err != nil || issuers["eu-west-1"] != "k2" { |
| + t.Fatalf("issuer pointer = %v (%v) after a refused re-rotate, want it still on k2", issuers, err) |
| + } |
| if d := time.Until(res.RetireNoEarlierThan); d < 27*time.Hour || d > 29*time.Hour { |
| t.Fatalf("retire-no-earlier-than is %s away, want about 28h", d) |
| } |
| @@ -504,6 +590,66 @@ func TestRelayRotateRefusesUntilHeartbeatsCarryKid(t *testing.T) { |
| if after.Op != before.Op+1 || after.Phase != relayPhaseDone { |
| t.Fatalf("record = %+v, want one finished operation past %d", after, before.Op) |
| } |
| + |
| + // Staging a THIRD master, with the pointer on the middle one, rotates |
| + // forward to it — the last entry, not merely a different one. |
| + t.Setenv(relayMastersEnv("eu-west-1"), |
| + "k1:"+relayTestMaster(1)+",k2:"+relayTestMaster(2)+",k3:"+relayTestMaster(3)) |
| + hb.Kids = []string{"k1", "k2", "k3"} |
| + if err := relay.WriteHeartbeat(ctx, fx.rel, hb); err != nil { |
| + t.Fatal(err) |
| + } |
| + third, err := fx.srv.relayRotate(ctx, "eu-west-1") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if third.Kid != "k3" { |
| + t.Fatalf("rotate with k1,k2,k3 staged and the pointer on k2 = %q, want k3", third.Kid) |
| + } |
| + if !strings.HasSuffix(relayRecord(t, fx, "titogo").IssuedSuffixes["eu-west-1"], ":k3") { |
| + t.Fatal("the app was not re-issued under the newest kid") |
| + } |
| +} |
| + |
| +// TestRelayReconcileRepairsOverwrittenEnvelope: the bundle holding SOMETHING |
| +// under CARLOS_RELAY is not the same fact as it holding what this console |
| +// wrote. A straggler write from a superseded console, or a member setting the |
| +// variable themselves, leaves suffixes that still agree and a value that does |
| +// not — which is exactly what the record's issued digest catches. |
| +func TestRelayReconcileRepairsOverwrittenEnvelope(t *testing.T) { |
| + fx, priv := relayFixture(t, "code-relay-envelope") |
| + ctx := context.Background() |
| + |
| + rec, err := fx.srv.relayEnable(ctx, fx.account(t), "titogo", nil, "alice@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if rec.IssuedDigest == "" { |
| + t.Fatal("enable recorded no issued digest") |
| + } |
| + // Someone else's envelope, under the same name, with the record untouched. |
| + _, _, foreign := sealedFor(t, "not what the console issued") |
| + if _, err := appconfig.Apply(ctx, fx.rel, fx.sqid, "titogo", |
| + map[string]appconfig.Var{relayVar: {Kind: "secret", Envelope: foreign}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + if err := fx.srv.relayReconcile(ctx); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + v := appVars(t, fx, "titogo")[relayVar] |
| + if v.Envelope == foreign { |
| + t.Fatal("the reconcile left an envelope this console did not write") |
| + } |
| + after := relayRecord(t, fx, "titogo") |
| + if after.IssuedDigest != relayEnvelopeDigest(v.Envelope) { |
| + t.Fatalf("record digest %q does not describe the bundle's envelope", after.IssuedDigest) |
| + } |
| + entries, err := relay.ParseConfig(openSecret(t, priv, v)) |
| + if err != nil || len(entries) != 1 || entries[0].UsernameSuffix != after.IssuedSuffixes["eu-west-1"] { |
| + t.Fatalf("re-delivered value = %+v (%v), want the record's own identity", entries, err) |
| + } |
| } |
| // TestRelayWriterLeaseExcludesSecondHolder: two consoles over one bucket, one |
diff --git a/internal/relay/keys.go b/internal/relay/keys.go| index 81a881251..80be5c585 100644 |
| --- a/internal/relay/keys.go |
| +++ b/internal/relay/keys.go |
| @@ -41,6 +41,14 @@ func DeriveKey(master []byte, account, app, incarnation string) []byte { |
| return key |
| } |
| +// ValidKid reports whether s is usable as a key id: 1-8 lowercase letters or |
| +// digits, the same alphabet ParseUsername and ParseMasters hold a kid to. |
| +// Exported so a WRITER of masters — `carlos relay keygen`, and the console |
| +// parsing its own CARLOS_RELAY_MASTERS_<REGION> — refuses a typo where the |
| +// operator can still fix it, instead of minting usernames every relay |
| +// silently declines. |
| +func ValidKid(s string) bool { return kidRe.MatchString(s) } |
| + |
| func Suffix(account, app, incarnation, kid string) string { |
| return account + ":" + app + ":" + incarnation + ":" + kid |
| } |
diff --git a/internal/relay/record.go b/internal/relay/record.go| index 11db01a74..1d640e06e 100644 |
| --- a/internal/relay/record.go |
| +++ b/internal/relay/record.go |
| @@ -43,9 +43,22 @@ type Record struct { |
| Op int64 `json:"op"` |
| Phase string `json:"phase"` // "config"|"aggregate"|"done" |
| IssuedSuffixes map[string]string `json:"issued_suffixes"` // region -> username_suffix last issued |
| - Cutovers []RouteCutover `json:"cutovers"` // member acks for TCP exec routes |
| - EnabledAt time.Time `json:"enabled_at"` |
| - DisabledAt time.Time `json:"disabled_at"` |
| + // IssuedDigest is hex(sha256) of the sealed CARLOS_RELAY envelope the |
| + // console last WROTE into the app's config bundle. Additive (omitempty, |
| + // no schema bump): a record written before this field simply has none, |
| + // and the console's reconcile re-applies once to fill it in. |
| + // |
| + // It exists because the console cannot open the envelope it wrote — it |
| + // holds no decryption key — and two seals of identical plaintext differ |
| + // anyway. Without a digest the reconcile can only compare the record to |
| + // itself, and a bundle carrying a DIFFERENT envelope (a straggler write |
| + // from a superseded console, or a member's own `carlos secrets set |
| + // CARLOS_RELAY`) reads as converged forever. With it, "the bundle holds |
| + // exactly what we last wrote" is a fact the console can check. |
| + IssuedDigest string `json:"issued_digest,omitempty"` |
| + Cutovers []RouteCutover `json:"cutovers"` // member acks for TCP exec routes |
| + EnabledAt time.Time `json:"enabled_at"` |
| + DisabledAt time.Time `json:"disabled_at"` |
| } |
| // GetRecord reads the record. Absent is (nil, "", nil) — same idiom as |