feat(federation): seal federation peer client keys at rest (FED-M2-05) #495
Reference in New Issue
Block a user
Delete Branch "feat/federation-m2-key-sealing-v2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Closes FED-M2-05
Test plan
Generated with Claude Code
Independent Code Review — APPROVE
Reviewed by Opus 4.7 (independent agent, no shared context with author).
Summary
seal/unsealis byte-identical to the oldencrypt/decryptinprovider-credentials.service.ts(sameaes-256-gcm, 12-byte IV, 16-byte tag,base64(iv || tag || ct)layout, SHA-256(BETTER_AUTH_SECRET) derivation). Existingprovider_credentials.encrypted_valuerows will continue to decrypt cleanly. No data corruption risk.crypto.randomBytes(12). AuthTag set/get order correct. No AAD (acceptable here; see MEDIUM #1).any. Buffer/string boundaries explicit.MEDIUM (defer to follow-ups; not blockers)
packages/auth/src/seal.ts:11-17— Domain separation. Single shared key across all callers means a sealedprovider_credentials.encrypted_valueciphertext could be lifted and pasted intofederation_peers.client_key_pem(or vice versa) and wouldunseal()cleanly. Mitigation: accept optionalcontext: stringparameter and either derive a sub-key via HKDF or feed it tocipher.setAAD().packages/auth/src/seal.ts:11— Use HKDF instead of raw SHA-256.BETTER_AUTH_SECRETis recommended asopenssl rand -base64 32(high-entropy), which masks the weakness today, but HKDF-Extract+Expand is the standard practice.apps/gateway/src/federation/peer-key.util.ts:1-9— Wiring not in this PR. Wrappers introduced but no production caller seals/unsealsfederation_peers.client_key_pemin this PR. The "encrypted at rest" claim indocs/federation/SETUP.md:123is aspirational until a follow-up wires the peer service. Either land that wiring or update docs to say "scaffolding for FED-M2-06".apps/gateway/src/federation/__tests__/peer-key.spec.ts:48-56— Tamper coverage. Midpoint byte-flip lands in ciphertext body (good), but doesn't verify that flipping a byte inside the IV (offset 0-11) or authTag (offset 12-27) also throws. Add those two cases.apps/gateway/src/federation/__tests__/peer-key.spec.ts:58-62— Symmetric "missing secret" assertion. Onlyunsealis covered; pin the contract forsealClientKeytoo.docs/federation/SETUP.md:121-125— Rotation procedure. Warns operators but provides no procedure. Document the standard envelope-encryption upgrade path (addBETTER_AUTH_SECRET_PREVIOUS, try-new-then-old on unseal, re-seal on next write).Verdict: APPROVE
The refactor is correct and backward-compatible. MEDIUMs are real but appropriate for follow-up tasks rather than blockers on this scaffolding PR.