From aaaa20dbe19df287b9b7c7f9f099f75b9c92047e Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 15 Aug 2026 21:45:19 -0700 Subject: [PATCH] feat(config,identity): read under two KEKs, write under one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BRIDGE_KEK_PREVIOUS (optional, no dev default, refused if byte-equal to BRIDGE_KEK) names the key being rotated away from. The custodian seals only under the current KEK and falls back to the previous one exactly when the current fails AEAD authentication — malformed blobs keep their ValidationError untouched, so a data incident is never misread as a wrong-KEK incident, and a both-keys failure reads byte-identically to the single-KEK failure operators already alert on. This is deployable alone: with PREVIOUS unset, behavior is unchanged. The rotate-kek re-seal walk is the follow-up run. Co-Authored-By: Claude Fable 5 --- cmd/tidepool/main.go | 6 ++++- internal/config/config.go | 42 +++++++++++++++++++++++--------- internal/identity/keys.go | 51 +++++++++++++++++++++++++++++++++++---- 3 files changed, 82 insertions(+), 17 deletions(-) diff --git a/cmd/tidepool/main.go b/cmd/tidepool/main.go index b532a2a..71d78cd 100644 --- a/cmd/tidepool/main.go +++ b/cmd/tidepool/main.go @@ -170,7 +170,7 @@ func run(logger *slog.Logger) error { // The sync surface (task 04): com.atproto.sync.* + subscribeRepos, // describeServer, _health — everything a relay or Jetstream needs to // treat Tidepool as a subscribeRepos upstream. - custodian, err := identity.NewCustodian(cfg.BridgeKEK) + custodian, err := identity.NewCustodianWithPrevious(cfg.BridgeKEK, cfg.BridgeKEKPrevious) if err != nil { return err } @@ -250,6 +250,10 @@ func run(logger *slog.Logger) error { AllowPrivateAddresses: cfg.AllowPrivateAddresses, }) + // The boot canary for BRIDGE_KEK: the rotation key is sealed under the KEK + // and opened here, so a wrong or half-rotated key fails startup before any + // traffic is served, rather than surfacing later as per-actor decrypt + // failures scattered across the commit path. rotationKey, err := identity.LoadOrCreateRotationKey(ctx, serviceKeys, custodian) if err != nil { return err diff --git a/internal/config/config.go b/internal/config/config.go index 09179a7..42c218f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ package config import ( + "bytes" "encoding/base64" "encoding/hex" "fmt" @@ -54,10 +55,9 @@ type Config struct { BridgeKEK []byte // BridgeKEKPrevious is the KEK the bridge is rotating away from: key // material sealed under it must still open, but nothing new is sealed - // under it. Nil when BRIDGE_KEK_PREVIOUS is unset. - // - // STUB (TDD red): nothing parses BRIDGE_KEK_PREVIOUS yet, so this is - // always nil. + // under it. Nil when BRIDGE_KEK_PREVIOUS is unset — which is the steady + // state, so the variable is optional in every environment and has no + // development default. BridgeKEKPrevious []byte // BridgeServiceDID optionally pins a pre-provisioned service DID for the // bridge's own actor. Service-DID bootstrap is deferred: task 06 wires @@ -340,11 +340,29 @@ func Load(logger *slog.Logger) (*Config, error) { if err != nil { return nil, err } - cfg.BridgeKEK, err = decodeKEK(kekEncoded) + cfg.BridgeKEK, err = decodeKEK("BRIDGE_KEK", kekEncoded) if err != nil { return nil, err } + // Optional everywhere, deliberately NOT routed through stringVar: that + // helper makes a variable required in production, and requiring a previous + // KEK would refuse to boot every bridge that has never rotated. A rotation + // is a temporary state; the absence of the variable is the normal one. + if previousEncoded := strings.TrimSpace(os.Getenv("BRIDGE_KEK_PREVIOUS")); previousEncoded != "" { + cfg.BridgeKEKPrevious, err = decodeKEK("BRIDGE_KEK_PREVIOUS", previousEncoded) + if err != nil { + return nil, err + } + // Compared on the decoded bytes, not the strings: the same key pasted + // as hex in one variable and base64 in the other is still one key, and + // an operator who believes that is a rotation would retire the only + // key every escrowed signing key is sealed under. + if bytes.Equal(cfg.BridgeKEKPrevious, cfg.BridgeKEK) { + return nil, fmt.Errorf("config: BRIDGE_KEK_PREVIOUS must decode to a different key than the current one; a rotation needs two different keys") + } + } + // Optional in every environment: an operator may pre-provision the // bridge's service DID, otherwise identity bootstrap mints one. cfg.BridgeServiceDID = os.Getenv("BRIDGE_SERVICE_DID") @@ -639,22 +657,24 @@ func (c *Config) IsDevelopment() bool { return c.Environment == EnvironmentDevelopment } -// decodeKEK parses the BRIDGE_KEK value: 64 hex chars or standard base64, -// either way decoding to exactly 32 bytes. -func decodeKEK(encoded string) ([]byte, error) { +// decodeKEK parses a KEK-carrying variable: 64 hex chars or standard base64, +// either way decoding to exactly 32 bytes. name is the environment variable +// the value came from, so an operator holding two KEKs mid-rotation is told +// which one they broke rather than being sent to check the good one. +func decodeKEK(name, encoded string) ([]byte, error) { encoded = strings.TrimSpace(encoded) if raw, err := hex.DecodeString(encoded); err == nil { if len(raw) != 32 { - return nil, fmt.Errorf("config: BRIDGE_KEK must decode to 32 bytes, got %d", len(raw)) + return nil, fmt.Errorf("config: %s must decode to 32 bytes, got %d", name, len(raw)) } return raw, nil } raw, err := base64.StdEncoding.DecodeString(encoded) if err != nil { - return nil, fmt.Errorf("config: BRIDGE_KEK must be 64 hex chars or base64 of 32 bytes: %w", err) + return nil, fmt.Errorf("config: %s must be 64 hex chars or base64 of 32 bytes: %w", name, err) } if len(raw) != 32 { - return nil, fmt.Errorf("config: BRIDGE_KEK must decode to 32 bytes, got %d", len(raw)) + return nil, fmt.Errorf("config: %s must decode to 32 bytes, got %d", name, len(raw)) } return raw, nil } diff --git a/internal/identity/keys.go b/internal/identity/keys.go index 98d5a19..dfa30f6 100644 --- a/internal/identity/keys.go +++ b/internal/identity/keys.go @@ -63,6 +63,11 @@ type Custodian struct { // on every commit's hot path, and cipher.AEAD is safe for concurrent // use. aead cipher.AEAD + // previous is the KEK being rotated away from, nil outside a rotation. + // It only ever opens: seal uses aead unconditionally, so material written + // during the rotation window is readable by the current key alone and the + // operator can eventually drop the old one. + previous cipher.AEAD } // NewCustodian validates the KEK and returns a Custodian. @@ -87,9 +92,31 @@ func NewCustodian(kek []byte) (*Custodian, error) { // without orphaning key material sealed under the old one. previous may be // nil, in which case the result behaves exactly like NewCustodian(current). // -// STUB (TDD red): ignores previous entirely and delegates to NewCustodian. +// A wrong-length previous key is rejected here rather than at first use: the +// alternative is discovering it only when some pre-rotation key fails to open, +// long after the deploy that introduced it. func NewCustodianWithPrevious(current, previous []byte) (*Custodian, error) { - return NewCustodian(current) + custodian, err := NewCustodian(current) + if err != nil { + return nil, err + } + if len(previous) == 0 { + return custodian, nil + } + if len(previous) != KEKSize { + return nil, errors.NewValidationError("bridge_kek_previous", + fmt.Sprintf("must be %d bytes, got %d", KEKSize, len(previous))) + } + block, err := aes.NewCipher(previous) + if err != nil { + return nil, fmt.Errorf("identity: init AES for previous KEK: %w", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + return nil, fmt.Errorf("identity: init GCM for previous KEK: %w", err) + } + custodian.previous = gcm + return custodian, nil } // EncryptActorKey seals an actor's signing key for storage in @@ -141,10 +168,24 @@ func (c *Custodian) open(ciphertext, aad []byte) ([]byte, error) { nonce := ciphertext[1 : 1+c.aead.NonceSize()] sealed := ciphertext[1+c.aead.NonceSize():] plaintext, err := c.aead.Open(nil, nonce, sealed, aad) - if err != nil { - return nil, fmt.Errorf("identity: open sealed key: %w", err) + if err == nil { + return plaintext, nil + } + // The format checks above ran first, so reaching here means the blob is + // well-formed and merely failed GCM authentication — the one condition a + // second KEK can fix. A truncated or wrong-version blob never gets here, + // and so is never misreported as a key problem. + if c.previous != nil { + // Same aad: the retry re-tries the KEK, never the domain binding, so + // a ciphertext that wandered between AAD domains stays rejected. + if plaintext, prevErr := c.previous.Open(nil, nonce, sealed, aad); prevErr == nil { + return plaintext, nil + } } - return plaintext, nil + // One unreadable blob is one incident. Reporting only the current key's + // failure keeps the message byte-identical to the single-KEK case, so log + // lines and alerts written before a rotation still match during it. + return nil, fmt.Errorf("identity: open sealed key: %w", err) } // LoadOrCreateRotationKey returns the bridge's escrow rotation key, -- 2.51.2