feat(vault): phase 1 — crypto core, schema and API - #18
Merged
Conversation
First of four phases toward an end-to-end encrypted password manager. No
user-visible surface yet: this is the layer everything else sits on, so it
lands with its own tests and nothing depending on it.
Decision taken (section 01 of the plan): a SEPARATE vault password, not the
account password. MarkSyncr authenticates with signInWithPassword, so
Supabase receives the account password in plaintext and it therefore cannot
be an encryption key without giving up the end-to-end property. The
alternative — deriving an auth hash client-side so the real password is
never transmitted — is better UX and a stronger story, but it rewrites the
live login path and migrates every existing user, and that risk lands on
the bookmark sync that already works. It stays open: KDF parameters are
stored per user, so adopting it later invalidates nobody's vault.
packages/vault is shared by web and extension:
primitives.js Thin WebCrypto wrappers and nothing else — PBKDF2-SHA256,
HKDF-SHA256, AES-256-GCM, constant-time compare. Nothing in
the vault may implement a construction not in this file.
kdf.js master password -> masterKey -> {wrapKey, authHash} by HKDF
under distinct labels, so holding the auth hash does not
help decrypt anything. Labels are versioned and append-only:
editing one makes every existing vault unopenable.
vault-key.js A random user key encrypts items; the password only ever
encrypts that key. So a password change re-wraps 32 bytes
instead of re-encrypting the vault, and a second wrapped
copy under a recovery key survives a forgotten password
without the server learning anything.
items.js One record with three field groups — logins, cards,
identities are not three features. Password history is an
array inside the encrypted blob, so it is protected for
free rather than needing its own table.
PBKDF2 at 600k iterations rather than Argon2id: Argon2id is the better KDF
but is WASM in a browser, which means 'wasm-unsafe-eval' in the extension
CSP on a product already heading for review scrutiny. PBKDF2 is native, has
no dependency, and the per-user kdf/kdf_iterations columns mean the upgrade
later costs nothing. rewrapUserKey() already pulls a vault up to current
parameters on any password change.
Two attacks the code defends against explicitly, both tested:
KDF downgrade. Parameters arrive from the server, so a compromised server
could serve iterations: 1 and make captured auth hashes cheap to attack
offline. The client refuses to derive below its floor; the API and a CHECK
constraint refuse to store one.
Ciphertext relocation. Each item's id is bound in as AES-GCM additional
authenticated data, so a blob moved between rows fails to decrypt instead
of showing the wrong credential under the right name.
The migration stores base64 TEXT, not BYTEA — bytea round-trips through
PostgREST as an escaped hex string and invites encoding mistakes on exactly
the values that must not be corrupted. vault_items is one row per item,
unlike cloud_bookmarks' single blob, because two devices editing two
DIFFERENT passwords must not cost anyone a credential; a per-row revision
turns that into a 409 the client can merge.
Tests: 97 new (67 in the package, 30 on the API validation). The KDF and
HKDF cases are known-answer tests against RFC 7914 §11 and RFC 5869 A.1/A.3
rather than round-trips — a round-trip passes even when the parameters are
wrong in both directions. One test asserts the actual product claim: that
nothing the user typed appears anywhere in what is sent.
Not done here: the migration is written but NOT applied to any database,
and nothing calls this code yet. Phase 2 is the vault tab.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q3vvUS9Q7m4ASyESCL2C8D
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| it('travels inside the encrypted blob, so it is protected for free', async () => { | ||
| const key = userKey(); | ||
| let item = createItem('login', { login: { password: 'leaked-if-plaintext' } }); |
| }); | ||
|
|
||
| it('does not mutate the item it was given', () => { | ||
| const item = createItem('login', { login: { password: 'original' } }); |
ThreatCrush Security Scan11 finding(s) MEDIUM: 7 | LOW: 4
Snippets are redacted; ThreatCrush never prints matched credential material. |
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
2 participants
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.
Phase 1 of 4 toward an end-to-end encrypted password manager. No user-visible surface — this is the layer everything else sits on, landing with its own tests and nothing yet depending on it.
Plan: https://claude.ai/code/artifact/f8d564d9-23f6-408d-a34d-a4b3688b5118
The decision this bakes in
A separate vault password, not the account password. MarkSyncr authenticates with
signInWithPassword, so Supabase receives the account password in plaintext — it therefore cannot be an encryption key without giving up the end-to-end property entirely.The alternative (derive an auth hash client-side so the real password is never transmitted) is better UX and a stronger story, but it rewrites the live login path and migrates every existing user, and that risk lands on the bookmark sync that already works today. It stays open: KDF parameters are stored per-user, so adopting it later invalidates nobody's vault.
packages/vaultShared by web and extension.
primitives.jskdf.jsmasterKey→{wrapKey, authHash}via HKDF under distinct labels, so holding the auth hash does not help decrypt anything. Labels are versioned and append-only — editing one makes every existing vault unopenable.vault-key.jsitems.jsWhy PBKDF2 and not Argon2id
Argon2id is the better KDF, but in a browser it's WASM — which means adding
'wasm-unsafe-eval'to the extension CSP on a product already heading for review scrutiny. PBKDF2-SHA256 at 600k (the current OWASP floor) is native, dependency-free, and the per-userkdf/kdf_iterationscolumns make the upgrade later cost nothing.rewrapUserKey()already pulls a vault up to current parameters on any password change.Two attacks defended explicitly, both tested
iterations: 1and make captured auth hashes cheap to attack offline. The client refuses to derive below its floor; the API and aCHECKconstraint refuse to store one.Schema notes
TEXT, notBYTEA— bytea round-trips through PostgREST as an escaped hex string and invites encoding mistakes on exactly the values that must not be corrupted.cloud_bookmarks' single blob: two devices editing two different passwords must not cost anyone a credential. A per-rowrevisionturns that into a 409 the client can merge.user_settings, using(SELECT auth.uid())per migration 013's performance fix, and the new function setssearch_pathper migration 012's advisory fix.Testing
97 new tests (67 in the package, 30 on API validation), all passing; lint clean;
next buildcompiles and registers all three routes.The KDF and HKDF cases are known-answer tests against RFC 7914 §11 and RFC 5869 A.1/A.3, not round-trips — a round-trip passes even when the parameters are wrong in both directions. I verified those vectors against an independent implementation before hardcoding them.
One test asserts the actual product claim: that nothing the user typed appears anywhere in what gets sent.
__tests__/extension-auth.test.jshas 2 failures on this branch. They are pre-existing — they reproduce in isolation and that file imports nothing added here.Not done
🤖 Generated with Claude Code
https://claude.ai/code/session_01Q3vvUS9Q7m4ASyESCL2C8D