perf(sync): overlap the PBKDF2 derive with the pull on keychain unlock (#103, Tier 2) - #110
Merged
Conversation
…m Unlock Pure no-behavior-change refactor, staged ahead of #103 Tier 2. Splits Unlock into three single-purpose pieces so the upcoming prepared-key unlock path can reuse the exact same state setup instead of duplicating it: - handleIncompleteMigration() — the vault.tmp auto-rollback block. - finishUnlock(data, masterPassword) — everything after decryption: unmarshal, set in-memory state (incl. the masterPassword copy, NOT recoveryDEK), restore audit logging, sync metadata, remove the post-migration backup, log success. - Unlock now calls handleIncompleteMigration → LoadVault → finishUnlock. The existing Unlock test coverage (unit + integration, all green) is what proves finishUnlock is correct, so the Tier 2 commit that follows only adds new surface. Refs #103. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
…ync pull (#103, Tier 2) Adds the primitives for keychain users to hide the pre-unlock probe latency behind the key derivation (PBKDF2-600k ≈ tens-to-hundreds of ms), the win Tier 1 couldn't give them (no prompt to overlap). The cmd wiring lands in a follow-up commit. storage: - PreparedKeyParams + ReadKeyParams(): read the vault header's key-derivation params (version, salt, iterations, wrappedDEK[,nonce]) WITHOUT decrypting — cheap, runs before the pull. - DeriveDataKey(password, params): the expensive PBKDF2 step, no file access, so it can run concurrently with the pull. v1 → password-derived key; v2 → derive KEK then unwrap the DEK. Returns the key LoadVaultWithKey consumes. - PreparedKeyParams.Equal: compare captured vs current params to detect a re-key. vault: - PrepareUnlock() → PreparedUnlock{DeriveDataKey()} → UnlockWithPreparedKey(). - UnlockWithPreparedKey decrypts with the prepared key ONLY if the current on-disk params still match what we derived against; otherwise it falls back to a full password unlock. This is deliberately param-comparison, not decrypt-failure: for v2 a remote password-only change keeps the DEK, so a stale prepared DEK would still decrypt and we'd unlock with a stale masterPassword (reads work, saves break). Param-comparison makes Tier 2 behave identically to a sequential unlock on every re-key — a stale keychain password fails cleanly at unlock. A decrypt failure is a belt-and-suspenders second fallback. - Reuses finishUnlock (retains masterPassword for saves, not recoveryDEK). Tests (-race): happy path (params match → prepared key used, save works, not a recovery unlock); re-key → fallback to current password succeeds; stale password after re-key → fails cleanly, vault stays locked. Refs #103 (Tier 2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
#103, Tier 2) Wires the Tier 2 primitives into the command path so keychain users — the common case, who have no password prompt to hide behind (Tier 1) — also get the pre-unlock probe latency hidden, here behind the PBKDF2 key derivation (600k iterations ≈ tens-to-hundreds of ms). cmd/unlockKeychainOverlappingPull (the keychain branch of unlockVaultWithSync): - PrepareUnlock() reads key params (cheap, pre-pull); - SyncPull runs in a goroutine while DeriveDataKey (PBKDF2) runs on the main goroutine — neither touches vault.enc, so they overlap safely; - join, then UnlockWithPreparedKey decrypts the now-current file, falling back to a full password unlock if the pull brought a re-keyed vault. - A transient "Checking remote..." indicator is shown here (no prompt to garble), and a sync conflict is surfaced cleanly after the join. Falls back to a sequential keychain unlock if the header can't be read. Tier 1 (password branch) is unchanged. Test (-race): drives the keychain overlap end-to-end against a real sync-enabled, initialized vault by calling the helper with the password directly (no keychain backend needed), asserting a correct unlock and a working save (masterPassword retained). Closes #103. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
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.
Completes #103. Tier 1 (#109) hid the pre-unlock probe latency behind the password prompt; this Tier 2 hides it for keychain users — the default/common case, who have no prompt to overlap — behind the PBKDF2 key derivation (600k iterations, ≈ tens-to-hundreds of ms; measured ~63ms on a fast desktop, more on a laptop). No staleness tradeoff.
How it works
Unlockcouples file-read + derive + decrypt, and a pull replaces the file, so the decrypt must run after the pull. But the expensive derivation only needs the password + the key params (salt/iterations/wrappedDEK) — not the ciphertext — so it can overlap the pull:PrepareUnlock()reads the key params (cheap, pre-pull).SyncPullruns in a goroutine whileDeriveDataKey(PBKDF2) runs on the main goroutine — neither touchesvault.enc.UnlockWithPreparedKeydecrypts the now-current file with the prepared key.A single goroutine (the pull); the derive is on the main thread — same shape as Tier 1.
The correctness crux: param-comparison, not decrypt-failure
If the pull brings a re-keyed vault, the prepared key is stale. The fallback gate compares the current on-disk key params against the captured ones — mismatch ⇒ discard the prepared key, fall back to a full password unlock.
This is deliberately not "try the key, fall back on decrypt failure". For a v2 vault a remote password-only change preserves the DEK, so a stale prepared DEK would still decrypt — and we'd unlock with a stale
masterPassword(reads work, saves break). Param-comparison makes Tier 2 behave identically to a sequential unlock on every re-key: a stale keychain password fails cleanly at unlock. (A decrypt failure is kept as a belt-and-suspenders second fallback.)Staged for safety
refactor(vault): extracthandleIncompleteMigration+finishUnlockfromUnlock— pure no-behavior-change, verified green by the existing Unlock tests, so the new path reuses proven state-setup (retainsmasterPassword, notrecoveryDEK).feat(vault): storageReadKeyParams/DeriveDataKey/Equal+ vaultPrepareUnlock/UnlockWithPreparedKey.perf(sync): wire the cmd keychain branch.Tests (all
-race)cmd/vault/storagepassgo test -race;golangci-lint v2.5clean.Closes #103.
🤖 Generated with Claude Code