crypto: bind AAD on master-key AEAD wrapping (CAP-57)#233
Merged
Conversation
encryptMasterKey/decryptMasterKey now bind Additional Authenticated Data so a wrapped master-key blob can't be verified under a different (user, org) or moved between the org and local-only keystores: - org wrapping (key.enc): AAD = masterKeyAAD(userId, orgId) - local-only keystore (key.local): AAD = LOCAL_MASTER_KEY_AAD (domain tag) decryptMasterKey verifies against the supplied AAD first, then falls back to a no-AAD decrypt for blobs written before AAD binding existed — a transparent grandfather. A blob written WITH an AAD never verifies under a different one (wrong-AAD attempt fails the GCM tag and the no-AAD fallback also fails), so cross-context substitution is rejected; only genuinely AAD-less legacy blobs reach the fallback. Every reader of a master-key blob (keyResolver, invite, transport, local unlock) now passes the matching AAD. The other AES-GCM call sites in packages/cli/src/crypto/ — value encryption (encryptor), invite/deploy wrapping (inviteCrypto, deployCrypto/deployRuntime) — keep no AAD by design: each derives its key via HKDF with the context already in the salt/info (projectId+orgId, orgId:email, deployId), so AAD would be redundant, and binding it on stored values would require re-encrypting everything. Each site is now commented to that effect. Service-side kms.ts already binds AAD via contextAAD / EncryptionContext (unchanged). Tests (tests/crypto/aeadAad.test.ts): round-trip under AAD; altered AAD fails; cross-context substitution fails; org/local domain separation; legacy no-AAD blobs still decrypt; AAD-bound blob refuses a reader that omits the AAD. Full suite green (413), typecheck + build clean.
cvince
added a commit
that referenced
this pull request
Jun 9, 2026
… legacy detection (#232) * crypto: versioned seed-phrase KDF (v2 = PBKDF2-600k/SHA-256) with trial-based legacy detection Upgrade the seed-phrase KDF from PBKDF2-2048/SHA-512 (v1, the BIP-39 default) to PBKDF2-600k/SHA-256 (v2, OWASP) for defense-in-depth. The 24-word mnemonic already carries 256-bit entropy, so v1 was never brute-forceable; v2 closes the gap for partially-leaked / non-uniform phrases and satisfies OWASP guidance. M is derived deterministically from the phrase with no stored salt (recovery must work offline from the 24 words alone after a machine wipe), so the KDF version cannot be persisted. New orgs/local setups derive M under the current version; existing orgs stay on their original version forever and are detected at the phrase->M boundaries by trial decryption against a known ciphertext — no data migration, no server change, no stored version marker. - keyManager: KdfVersion + CURRENT_KDF_VERSION/KDF_VERSIONS registry; seedPhraseToMasterKey(phrase, version?) defaults to current. - keyResolver: resolveProjectKeyByTrial() picks the version whose key decrypts a given ciphertext; resolveFromSeedPhrase takes a version. - decrypt: trials versions against the encrypted .env oracle. - recover: trials against a fetched ciphertext oracle, which also fixes the long-standing gap where a wrong phrase wrote a bad key.enc silently — it is now rejected before anything is written. - orgCreation: new orgs pinned to CURRENT_KDF_VERSION explicitly. - Tests: golden vectors pin v1+v2 params; migration tests prove a v1 org decrypts under the new code and the trial resolver picks the right version / refuses a wrong phrase (crypto + real-wiring recover). * crypto: bind AAD on master-key AEAD wrapping (CAP-57) (#233) encryptMasterKey/decryptMasterKey now bind Additional Authenticated Data so a wrapped master-key blob can't be verified under a different (user, org) or moved between the org and local-only keystores: - org wrapping (key.enc): AAD = masterKeyAAD(userId, orgId) - local-only keystore (key.local): AAD = LOCAL_MASTER_KEY_AAD (domain tag) decryptMasterKey verifies against the supplied AAD first, then falls back to a no-AAD decrypt for blobs written before AAD binding existed — a transparent grandfather. A blob written WITH an AAD never verifies under a different one (wrong-AAD attempt fails the GCM tag and the no-AAD fallback also fails), so cross-context substitution is rejected; only genuinely AAD-less legacy blobs reach the fallback. Every reader of a master-key blob (keyResolver, invite, transport, local unlock) now passes the matching AAD. The other AES-GCM call sites in packages/cli/src/crypto/ — value encryption (encryptor), invite/deploy wrapping (inviteCrypto, deployCrypto/deployRuntime) — keep no AAD by design: each derives its key via HKDF with the context already in the salt/info (projectId+orgId, orgId:email, deployId), so AAD would be redundant, and binding it on stored values would require re-encrypting everything. Each site is now commented to that effect. Service-side kms.ts already binds AAD via contextAAD / EncryptionContext (unchanged). Tests (tests/crypto/aeadAad.test.ts): round-trip under AAD; altered AAD fails; cross-context substitution fails; org/local domain separation; legacy no-AAD blobs still decrypt; AAD-bound blob refuses a reader that omits the AAD. Full suite green (413), typecheck + build clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements CAP-57 (Part 2 of crypto hardening). Stacked on #232 (base =
crypto/versioned-kdf) so this diff is AAD-only; it will retarget tomainonce #232 merges.What
encryptMasterKey/decryptMasterKeynow bind Additional Authenticated Data, so a wrapped master-key blob can't be verified under a different(user, org)or moved between the org and local-only keystores:key.enc):AAD = masterKeyAAD(userId, orgId)key.local):AAD = LOCAL_MASTER_KEY_AAD(a fixed domain tag — local has no user/org)Every reader of a master-key blob now passes the matching AAD:
keyResolver(both decrypt paths + both re-wrap writes),inviteCommand,transportCommand, and the local-mode unlock.Backward compatibility (the load-bearing bit)
decryptMasterKey(blob, key, aad)verifies againstaadfirst, then falls back to a no-AAD decrypt for blobs written before AAD existed — a transparent grandfather (existingkey.enc/key.localjust aren't context-bound). Crucially this does not weaken new blobs:aadA, read withaadB→ with-AAD attempt fails the GCM tag, no-AAD fallback also fails (the blob has an AAD) → rejected.Honest scope note
Given the inner wrapping key is
SHA256(userId:orgId)(already context-derived, and noted as "decorative" in the ticket), the AAD is defense-in-depth / correct AEAD hygiene + domain separation today rather than closing a live hole — but it becomes load-bearing if/when the inner key derivation is made secret-derived, and the domain tag does prevent org⇄local blob confusion now.Other AEAD sites — documented, intentionally no AAD
Per the acceptance criteria ("setAAD or a comment explaining why not"), every other
createCipheriv('aes-256-gcm', …)inpackages/cli/src/crypto/is now commented:encryptor(secret values) — key is the project keyHKDF(M, projectId, orgId); binding per-value AAD would mean re-encrypting every stored secret for marginal gain. Deferred.inviteCrypto/deployCrypto/deployRuntime— key isHKDF(token, salt, info)with context (orgId:email,deployId) already in the derivation; AAD would be redundant.kms.ts— already binds AAD viacontextAAD/EncryptionContext. Unchanged.Tests
tests/crypto/aeadAad.test.ts: round-trip under AAD; altered AAD fails; cross-context substitution (different user) fails; org⇄local domain separation; legacy no-AAD blobs still decrypt; an AAD-bound blob refuses a reader that omits the AAD; wrong key still fails. Full suite green (413 tests), typecheck + build clean.