amadan

carlos / platform Public

objstore: two-key rotation with explicit finish

Rotate mints a fresh key and delivers it while leaving the OLD key ACTIVE:
adoption swaps a symlink, it does not restart processes, so at the instant
new credentials land there are still hosts and in-flight requests holding
the old pair. RotateFinish is the member's explicit "nothing is using it
any more" — the only walk that destroys a credential an app might still be
using, and so a verb rather than a timer.

The overlap runs into AWS's hard two-keys-per-user ceiling, so the estate
is a two-state machine (one live key, or one live plus one pending) and
every step keeps it there: a pending key from an unfinished earlier
rotation is deleted by name, then every key the record does not name is
reaped — a crash between mint and reseal strands a key no record can name,
and without the reap the next mint wedges at the ceiling forever. Record
last, as in Grant: until the secret is delivered the record must keep
naming the key the app is actually holding.

Sentinels ErrNotProvisioned (rotation edits an estate, it never creates
one — the repair for that state is Grant) and ErrNoPendingFinish (a finish
with nothing to finish refuses rather than succeeding vacuously).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 68b4b348fc5da5a96c9717144a309159e96a3555 parent 68a688d
2 files changed, +403 −0
  • internal/objstore/walk.go +123 −0
  • internal/objstore/walk_test.go +280 −0
diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go
index a1011b798..034e0a3c7 100644
--- a/internal/objstore/walk.go
+++ b/internal/objstore/walk.go
@@ -28,6 +28,21 @@ var ErrNotDeclared = errors.New("objstore: no store is declared for this app")
// raises this; the grant walk itself never re-reads a prefix choice).
var ErrEnvPrefixLocked = errors.New("objstore: store already provisioned — env prefix is fixed")
+// ErrNotProvisioned is a rotation verb aimed at a store that was never
+// provisioned — declared-only, granted-but-half-walked, or absent entirely.
+// Rotation edits an existing credential estate; it never creates one, because
+// the coordinates it reseals (bucket, region, endpoint, IAM user) come from
+// the record, and there is no defensible guess for them. The caller's repair
+// for this state is Grant, not a rotate.
+var ErrNotProvisioned = errors.New("objstore: store is not provisioned")
+
+// ErrNoPendingFinish is a finish with nothing to finish. Finish is the one
+// destructive step in the rotation state machine — it kills a key that may
+// still be in use — so it refuses rather than succeeding vacuously: a member
+// who believes they just retired the old key, when in truth an earlier finish
+// already did, has a different picture of the estate than the estate does.
+var ErrNoPendingFinish = errors.New("objstore: no pending key to finish")
+
// UserPath is the IAM path every store principal lives under, so an operator
// (or a policy) can name the whole population in one place.
const UserPath = "/carlos-store/"
@@ -206,6 +221,114 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment,
return rec, nil
}
+// Rotate mints a fresh key, delivers it, and leaves the OLD key active (§8).
+// That overlap is the whole point: adoption swaps a symlink, it does not
+// restart processes, so at the instant the new credentials land there are
+// still hosts, in-flight requests and hibernating tenants holding the old
+// pair. Killing it here would break them; RotateFinish is the member's
+// explicit "nothing is using it any more".
+//
+// The overlap runs into AWS's hard ceiling of two access keys per user, so
+// the estate is a two-state machine — one live key, or one live plus one
+// pending — and every step below exists to keep it there:
+//
+// Step one deletes a pending key from an UNFINISHED earlier rotation.
+// Rotating twice without finishing is ordinary member behaviour, and the
+// second rotation is itself the statement that the oldest key is done.
+//
+// Step two reaps every key the record does not name. A crash between mint and
+// reseal (step four failing after step three) leaves a key no record can name;
+// without the reap the next mint wedges at the ceiling forever, and re-running
+// the walk — the platform's whole repair story — would stop working.
+//
+// Steps three and four mint, then reseal into the app's config. Coordinates
+// come from the record: a rotation changes credentials and nothing else, so
+// the bucket an app's data is in is never re-derived from a deployment that
+// may have moved on.
+//
+// Step five records, last, as in Grant: until the secret is delivered the
+// record must keep naming the key the app is actually holding. A record that
+// claimed the new key after a failed delivery would point every reader at a
+// key whose secret died with the write, and the re-run would then reap the ONE
+// key still working.
+func Rotate(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.PublicKey,
+ acct, app string, now time.Time) (*Record, error) {
+ rec, etag, err := Get(ctx, s, acct, app)
+ if err != nil {
+ return nil, err
+ }
+ if rec == nil || rec.Provisioned == nil {
+ return nil, ErrNotProvisioned
+ }
+ if rec.Declared == nil {
+ return nil, ErrNotDeclared // no prefix to deliver into
+ }
+ pr := rec.Provisioned
+ // 1. finish the pending rotation late. The reap below would collect this
+ // key too, but doing it by name first keeps the intent legible and means
+ // a backend that refuses the delete stops the walk here, before a mint.
+ if pr.PrevKeyID != "" {
+ if err := p.DeleteKey(ctx, pr.IAMUser, pr.PrevKeyID); err != nil {
+ return nil, fmt.Errorf("objstore: delete pending key %s for %s: %w", pr.PrevKeyID, pr.IAMUser, err)
+ }
+ }
+ // 2. collect anything no record names — only list-and-delete finds it.
+ if err := p.ReapKeys(ctx, pr.IAMUser, []string{pr.KeyID}); err != nil {
+ return nil, fmt.Errorf("objstore: reap keys for %s: %w", pr.IAMUser, err)
+ }
+ // 3. mint into the now-guaranteed free slot
+ id, secret, err := p.MintKey(ctx, pr.IAMUser)
+ if err != nil {
+ return nil, fmt.Errorf("objstore: mint key for %s: %w", pr.IAMUser, err)
+ }
+ // 4. deliver — same names, same single CAS write, same validation of the
+ // credential halves before sealing (writeEnv owns that rail).
+ if err := writeEnv(ctx, s, sealPub, acct, app, rec.Declared.EnvPrefix,
+ pr.Bucket, pr.Region, pr.Endpoint, id, secret); err != nil {
+ return nil, err
+ }
+ // 5. record: the key just retired from live becomes the pending one.
+ pr.PrevKeyID, pr.KeyID, pr.At = pr.KeyID, id, now
+ if _, err := Put(ctx, s, acct, app, rec, etag); err != nil {
+ return nil, err
+ }
+ return rec, nil
+}
+
+// RotateFinish closes the two-key window: the pending key is deleted and the
+// record stops naming it. Nothing is delivered — the app has been reading the
+// current key since the rotation, so there is no config change to make, and
+// making one would restart nothing and risk everything.
+//
+// This is the only walk that destroys a credential an app might still be
+// using, which is why it is a separate verb a member asks for rather than a
+// timer: only the member knows every host, worker and hibernating tenant has
+// picked the new pair up.
+func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app string) (*Record, error) {
+ rec, etag, err := Get(ctx, s, acct, app)
+ if err != nil {
+ return nil, err
+ }
+ if rec == nil || rec.Provisioned == nil {
+ return nil, ErrNotProvisioned
+ }
+ pr := rec.Provisioned
+ if pr.PrevKeyID == "" {
+ return nil, ErrNoPendingFinish
+ }
+ if err := p.DeleteKey(ctx, pr.IAMUser, pr.PrevKeyID); err != nil {
+ return nil, fmt.Errorf("objstore: delete pending key %s for %s: %w", pr.PrevKeyID, pr.IAMUser, err)
+ }
+ // Recorded after the delete: a record that forgot the key before the
+ // backend did would strand a live credential nothing names, which is the
+ // one thing the reap exists to clean up and the estate exists to avoid.
+ pr.PrevKeyID = ""
+ if _, err := Put(ctx, s, acct, app, rec, etag); err != nil {
+ return nil, err
+ }
+ return rec, nil
+}
+
// writeEnv delivers the whole coordinate set in ONE appconfig.Apply — one CAS
// write, so an app never reads a bucket name paired with the previous key.
// Both key halves are sealed for the deployment's secrets key; the plaintext
diff --git a/internal/objstore/walk_test.go b/internal/objstore/walk_test.go
index 669a4dc7c..ca0c716a1 100644
--- a/internal/objstore/walk_test.go
+++ b/internal/objstore/walk_test.go
@@ -564,3 +564,283 @@ func TestEnvDeliveredReportsMissing(t *testing.T) {
t.Fatalf("after unset: missing = %v, %v", missing, err)
}
}
+
+// rotateFixture grants a store and returns the provisioned record, so each
+// rotation test starts from the one state Rotate is defined against.
+func rotateFixture(t *testing.T, s release.Store, pub *ecdh.PublicKey, p *MemProvisioner) *Record {
+ t.Helper()
+ declare(t, s, "bab", "titogo", "TITO_S3")
+ rec, err := Grant(context.Background(), s, p, testDeployment(), pub, "bab", "titogo", "op", "", time.Now().UTC())
+ if err != nil {
+ t.Fatal(err)
+ }
+ return rec
+}
+
+// openVar decrypts a delivered sealed var, so a test asserts on what the app
+// would actually read rather than on the record alone.
+func openVar(t *testing.T, priv *ecdh.PrivateKey, s release.Store, acct, app, name string) string {
+ t.Helper()
+ v := mustVars(t, s, acct, app)[name]
+ if v.Kind != "secret" || v.Envelope == "" {
+ t.Fatalf("%s = %+v, want a sealed secret", name, v)
+ }
+ plain, err := appconfig.Open(priv, v.Envelope)
+ if err != nil {
+ t.Fatalf("open %s: %v", name, err)
+ }
+ return string(plain)
+}
+
+// The whole point of the two-key window: the app keeps running on the key it
+// already holds (in-flight requests, un-restarted processes) while the new one
+// is delivered. The old key stays ACTIVE until an explicit finish.
+func TestRotateKeepsOldKeyActive(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ priv, pub := testSealKeypair(t)
+ p := NewMemProvisioner("eu-west-1")
+ granted := rotateFixture(t, s, pub, p)
+ oldID := granted.Provisioned.KeyID
+
+ now := time.Now().UTC().Add(time.Hour)
+ rec, err := Rotate(ctx, s, p, pub, "bab", "titogo", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if rec.Provisioned.KeyID == oldID {
+ t.Fatalf("rotate did not mint a new key: still %q", rec.Provisioned.KeyID)
+ }
+ if rec.Provisioned.PrevKeyID != oldID {
+ t.Fatalf("PrevKeyID = %q, want the old key %q", rec.Provisioned.PrevKeyID, oldID)
+ }
+ if !rec.Provisioned.At.Equal(now) {
+ t.Fatalf("At = %v, want %v", rec.Provisioned.At, now)
+ }
+ // coordinates are carried, never re-derived
+ if rec.Provisioned.Bucket != granted.Provisioned.Bucket ||
+ rec.Provisioned.Region != granted.Provisioned.Region ||
+ rec.Provisioned.IAMUser != granted.Provisioned.IAMUser ||
+ rec.Provisioned.UserARN != granted.Provisioned.UserARN {
+ t.Fatalf("rotate moved the store: %+v", rec.Provisioned)
+ }
+
+ u := p.Users["carlos-store-bab-titogo"]
+ if len(u.Keys) != 2 {
+ t.Fatalf("keys after rotate = %v, want both old and new", u.Keys)
+ }
+ if _, ok := u.Keys[oldID]; !ok {
+ t.Fatalf("old key %q was deleted by rotate: %v", oldID, u.Keys)
+ }
+ if _, ok := u.Keys[rec.Provisioned.KeyID]; !ok {
+ t.Fatalf("new key %q is not live: %v", rec.Provisioned.KeyID, u.Keys)
+ }
+
+ // the app reads the NEW pair
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_ACCESS_KEY"); got != rec.Provisioned.KeyID {
+ t.Fatalf("delivered access key = %q, want %q", got, rec.Provisioned.KeyID)
+ }
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_SECRET_KEY"); got != u.Keys[rec.Provisioned.KeyID] {
+ t.Fatalf("delivered secret = %q, want %q", got, u.Keys[rec.Provisioned.KeyID])
+ }
+ // coordinates still delivered alongside, in the same config
+ if v := mustVars(t, s, "bab", "titogo")["TITO_S3_BUCKET"]; v.Kind != "env" || v.Value != granted.Provisioned.Bucket {
+ t.Fatalf("TITO_S3_BUCKET = %+v", v)
+ }
+
+ stored, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || stored == nil || stored.Provisioned == nil ||
+ stored.Provisioned.KeyID != rec.Provisioned.KeyID || stored.Provisioned.PrevKeyID != oldID {
+ t.Fatalf("stored record = %+v, %v", stored, err)
+ }
+}
+
+// AWS allows two access keys per user, full stop. A member who rotates twice
+// without finishing must not wedge at the ceiling: the second rotate finishes
+// the pending one first, so the estate is never more than two keys.
+func TestRotateTwiceReapsPending(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ priv, pub := testSealKeypair(t)
+ p := NewMemProvisioner("eu-west-1")
+ first := rotateFixture(t, s, pub, p).Provisioned.KeyID
+ user := "carlos-store-bab-titogo"
+
+ second, err := Rotate(ctx, s, p, pub, "bab", "titogo", time.Now().UTC())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(p.Users[user].Keys) != 2 {
+ t.Fatalf("keys after first rotate = %v", p.Users[user].Keys)
+ }
+
+ third, err := Rotate(ctx, s, p, pub, "bab", "titogo", time.Now().UTC())
+ if err != nil {
+ t.Fatal(err)
+ }
+ keys := p.Users[user].Keys
+ if len(keys) != 2 {
+ t.Fatalf("keys after second rotate = %v, want exactly 2", keys)
+ }
+ if _, ok := keys[first]; ok {
+ t.Fatalf("the finished key %q is still live: %v", first, keys)
+ }
+ if _, ok := keys[second.Provisioned.KeyID]; !ok {
+ t.Fatalf("pending key %q was deleted: %v", second.Provisioned.KeyID, keys)
+ }
+ if _, ok := keys[third.Provisioned.KeyID]; !ok {
+ t.Fatalf("new key %q is not live: %v", third.Provisioned.KeyID, keys)
+ }
+ if third.Provisioned.PrevKeyID != second.Provisioned.KeyID {
+ t.Fatalf("PrevKeyID = %q, want %q", third.Provisioned.PrevKeyID, second.Provisioned.KeyID)
+ }
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_ACCESS_KEY"); got != third.Provisioned.KeyID {
+ t.Fatalf("delivered access key = %q, want %q", got, third.Provisioned.KeyID)
+ }
+}
+
+// A rotate that mints and then fails to reseal leaves a key the record does
+// not name. Re-running must collect it (the reap) rather than mint into a full
+// estate, and the app must end up holding the key the record names.
+func TestRotateResumesAfterResealFailure(t *testing.T) {
+ ctx := context.Background()
+ priv, pub := testSealKeypair(t)
+ s := &failingStore{Store: release.NewDirStore(t.TempDir())}
+ p := NewMemProvisioner("eu-west-1")
+ granted := rotateFixture(t, s, pub, p)
+ oldID := granted.Provisioned.KeyID
+ user := "carlos-store-bab-titogo"
+
+ s.failKeyOnce = appconfig.Key("bab", "titogo")
+ if _, err := Rotate(ctx, s, p, pub, "bab", "titogo", time.Now().UTC()); err == nil {
+ t.Fatal("reseal failure: Rotate returned no error")
+ }
+ // The record must still name the key the app is holding — the new one was
+ // never delivered, so claiming it would strand the app on a dead name.
+ half, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || half == nil || half.Provisioned == nil ||
+ half.Provisioned.KeyID != oldID || half.Provisioned.PrevKeyID != "" {
+ t.Fatalf("record after failed reseal = %+v, %v", half, err)
+ }
+ if len(p.Users[user].Keys) > 2 {
+ t.Fatalf("ceiling breached mid-failure: %v", p.Users[user].Keys)
+ }
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_ACCESS_KEY"); got != oldID {
+ t.Fatalf("failed rotate moved the app's key to %q, want %q", got, oldID)
+ }
+
+ rec, err := Rotate(ctx, s, p, pub, "bab", "titogo", time.Now().UTC())
+ if err != nil {
+ t.Fatalf("resume after reseal failure: %v", err)
+ }
+ keys := p.Users[user].Keys
+ if len(keys) != 2 {
+ t.Fatalf("keys after resume = %v, want exactly 2 (stray reaped)", keys)
+ }
+ if rec.Provisioned.PrevKeyID != oldID {
+ t.Fatalf("PrevKeyID = %q, want %q", rec.Provisioned.PrevKeyID, oldID)
+ }
+ if _, ok := keys[rec.Provisioned.KeyID]; !ok {
+ t.Fatalf("recorded key %q is not live: %v", rec.Provisioned.KeyID, keys)
+ }
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_ACCESS_KEY"); got != rec.Provisioned.KeyID {
+ t.Fatalf("delivered access key = %q, want %q", got, rec.Provisioned.KeyID)
+ }
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_SECRET_KEY"); got != keys[rec.Provisioned.KeyID] {
+ t.Fatalf("delivered secret = %q, want %q", got, keys[rec.Provisioned.KeyID])
+ }
+}
+
+// Finish is the member saying "nothing is using the old key any more". It
+// deletes exactly that key and nothing else, delivers nothing, and is not
+// repeatable — a second finish has no key to name and must say so.
+func TestRotateFinishDeletesPrev(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ priv, pub := testSealKeypair(t)
+ p := NewMemProvisioner("eu-west-1")
+ oldID := rotateFixture(t, s, pub, p).Provisioned.KeyID
+ user := "carlos-store-bab-titogo"
+
+ rotated, err := Rotate(ctx, s, p, pub, "bab", "titogo", time.Now().UTC())
+ if err != nil {
+ t.Fatal(err)
+ }
+ before := mustVars(t, s, "bab", "titogo")
+
+ rec, err := RotateFinish(ctx, s, p, "bab", "titogo")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if rec.Provisioned.PrevKeyID != "" {
+ t.Fatalf("PrevKeyID after finish = %q", rec.Provisioned.PrevKeyID)
+ }
+ if rec.Provisioned.KeyID != rotated.Provisioned.KeyID {
+ t.Fatalf("finish changed the live key: %q", rec.Provisioned.KeyID)
+ }
+ keys := p.Users[user].Keys
+ if len(keys) != 1 {
+ t.Fatalf("keys after finish = %v, want just the live one", keys)
+ }
+ if _, ok := keys[oldID]; ok {
+ t.Fatalf("finished key %q still live: %v", oldID, keys)
+ }
+ if _, ok := keys[rotated.Provisioned.KeyID]; !ok {
+ t.Fatalf("live key %q deleted by finish: %v", rotated.Provisioned.KeyID, keys)
+ }
+ // Finish delivers nothing: the app's config is untouched.
+ after := mustVars(t, s, "bab", "titogo")
+ if len(before) != len(after) {
+ t.Fatalf("finish rewrote the var set: %v then %v", before, after)
+ }
+ for k, v := range before {
+ if after[k] != v {
+ t.Fatalf("finish rewrote %s: %+v then %+v", k, v, after[k])
+ }
+ }
+ if got := openVar(t, priv, s, "bab", "titogo", "TITO_S3_ACCESS_KEY"); got != rec.Provisioned.KeyID {
+ t.Fatalf("delivered access key = %q, want %q", got, rec.Provisioned.KeyID)
+ }
+
+ if _, err := RotateFinish(ctx, s, p, "bab", "titogo"); !errors.Is(err, ErrNoPendingFinish) {
+ t.Fatalf("second finish: %v, want ErrNoPendingFinish", err)
+ }
+ if len(p.Users[user].Keys) != 1 {
+ t.Fatalf("refused finish touched the backend: %v", p.Users[user].Keys)
+ }
+
+ stored, _, err := Get(ctx, s, "bab", "titogo")
+ if err != nil || stored == nil || stored.Provisioned == nil || stored.Provisioned.PrevKeyID != "" {
+ t.Fatalf("stored record = %+v, %v", stored, err)
+ }
+}
+
+// Neither walk invents a store: there is nothing to rotate until a grant has
+// provisioned one, and both must refuse without touching the backend.
+func TestRotateUnprovisionedRefused(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ _, pub := testSealKeypair(t)
+ p := NewMemProvisioner("eu-west-1")
+ declare(t, s, "bab", "titogo", "TITO_S3")
+
+ if _, err := Rotate(ctx, s, p, pub, "bab", "titogo", time.Now().UTC()); !errors.Is(err, ErrNotProvisioned) {
+ t.Fatalf("Rotate on a declared-only store: %v, want ErrNotProvisioned", err)
+ }
+ if _, err := RotateFinish(ctx, s, p, "bab", "titogo"); !errors.Is(err, ErrNotProvisioned) {
+ t.Fatalf("RotateFinish on a declared-only store: %v, want ErrNotProvisioned", err)
+ }
+ // and on an app with no record at all
+ if _, err := Rotate(ctx, s, p, pub, "bab", "nostore", time.Now().UTC()); !errors.Is(err, ErrNotProvisioned) {
+ t.Fatalf("Rotate on a store-less app: %v, want ErrNotProvisioned", err)
+ }
+ if _, err := RotateFinish(ctx, s, p, "bab", "nostore"); !errors.Is(err, ErrNotProvisioned) {
+ t.Fatalf("RotateFinish on a store-less app: %v, want ErrNotProvisioned", err)
+ }
+ if len(p.Buckets) != 0 || len(p.Users) != 0 {
+ t.Fatalf("refused rotation touched the backend: %v %v", p.Buckets, p.Users)
+ }
+ if v := mustVars(t, s, "bab", "titogo"); len(v) != 0 {
+ t.Fatalf("refused rotation wrote config: %v", v)
+ }
+}