Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new FxA DB migration level (195) introducing a passkeys_wraps table intended to store per-passkey key-wrapping “envelopes” (PRF-wrapped recovery keypair material and HPKE-sealed kB) and wiring it to passkeys via a composite ON DELETE CASCADE FK so passkey deletion also deletes its wrap.
Changes:
- Add forward migration
patch-194-195.sqlto createpasskeys_wrapswith(uid, credentialId)primary key and a composite cascading FK topasskeys(uid, credentialId). - Add rollback migration
patch-195-194.sql(commented-out rollback steps, consistent with existing rollback patch style). - Bump DB schema target level from 194 → 195.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/db-migrations/databases/fxa/patches/patch-194-195.sql | Creates the new wraps table and advances schema patch level to 195. |
| packages/db-migrations/databases/fxa/patches/patch-195-194.sql | Provides the rollback patch steps (commented), returning schema patch level to 194. |
| packages/db-migrations/databases/fxa/target-patch.json | Updates the target schema patch level to 195. |
| pkR VARBINARY(512) NOT NULL, -- v1: 133B, uncompressed P-521 point | ||
| prfWrappedSkR VARBINARY(512) NOT NULL, -- v1: 257B, AES-256-GCM(pkcs8 skR) + tag | ||
| keyWrapIv BINARY(12) NOT NULL, -- AES-GCM nonce | ||
| hpkeEncapsulatedSecret VARBINARY(512) NOT NULL, -- v1: 133B, RFC 9180 Nenc for DHKEM(P-521) | ||
| hpkeSealedKb VARBINARY(512) NOT NULL, -- v1: 48B, 32B kB + tag |
There was a problem hiding this comment.
My thinking was that we give a little bit of wiggle room so we're less likely to hit issues where the write is rejected because the size changes, but I'm unsure if that's even possible for the size to change. If it's not, or it shouldn't change, than I can make it more strict
There was a problem hiding this comment.
IUC, those sizes are fixed and based on the crypto libraries. If this is infact true, and we potentially have a bunch of records here, we should make them the exact size. Also use binary instead of varbinary. This is consistent with how columns that hold wrapped kbs work.
Because: * Passkey-based sync sign-in needs somewhere to store the PRF-wrapped recovery keypair and the HPKE-sealed kB, one envelope per passkey. This commit: * Adds patch-194-195.sql creating passkeyWraps, keyed on (uid, credentialId) with an ON DELETE CASCADE FK to passkeys so wraps are removed with their passkey. deleteAccount_24 needs no change. * Adds the patch-195-194.sql rollback and bumps target-patch to 195.
Because
keypair and the HPKE-sealed
kB, one envelope per passkey. This is the storagemodel from the technical spec §2.2.5.
builds on this table, so the schema needs to land first.
This pull request
packages/db-migrations/databases/fxa/patches/patch-194-195.sql, creatingpasskeyWrapskeyed on(uid, credentialId). Every payload column isfixed-length and sized from the v1 ciphersuite
(
DHKEM(P-521, HKDF-SHA512)/HKDF-SHA512/AES-256-GCM).ON DELETE CASCADEforeign key topasskeys(uid, credentialId),so deleting a passkey drops its envelope. Account deletion needs no procedure
change:
deleteAccount_24already deletes frompasskeysbeforeaccounts, andwraps cascade with that delete.
patch-195-194.sqlrollback and bumpstarget-patch.jsonto 195.Issue that this pull request solves
Closes: FXA-13138
Checklist
Put an
xin the boxes that applyHow to review (Optional)
patch-194-195.sql— specifically thatuid,credentialId, and the timestamp types matchpasskeysexactly, since thecomposite FK cannot be created otherwise.
lets this land without touching
deleteAccount_24.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Verified against a local DB.
packages/db-migrationshas no automated schemaassertions, so these checks are manual:
table and returns the level to 194; forward then re-applies cleanly.
wrap intact.
CALL deleteAccount_24(uid, reason)succeeded with a wrap present — account,passkeys, and wraps all removed,
deletionReasonrecorded — with no change to theprocedure.
Purely additive, and no application code reads the table yet, so this must deploy ahead
of the data model that consumes it.