perf(sync): overlap the PBKDF2 derive with the pull on keychain unlock (#103, Tier 2)

cmd/concurrent_unlock_test.go

@@ -80,3 +80,52 @@ func TestUnlockVaultWithSync_PasswordBranchConcurrent(t *testing.T) {
 		t.Error("expected vault to be unlocked after the concurrent password branch")
 	}
 }
+
+// Tier 2: unlockKeychainOverlappingPull runs the pull (goroutine) concurrently
+// with the PBKDF2 derivation (main), joins, then unlocks with the prepared key.
+// Driving it directly with the password (the value a keychain Retrieve would
+// return) exercises the full overlap under `go test -race` without needing a
+// keychain backend. Asserts a correct unlock and that masterPassword was retained
+// (a subsequent save works).
+func TestUnlockKeychainOverlappingPull_Concurrent(t *testing.T) {
+	const masterPassword = "TestPass!1234"
+
+	tmpDir := t.TempDir()
+	vaultPath := filepath.Join(tmpDir, "vault.enc")
+	cfgPath := filepath.Join(tmpDir, "config.yml")
+
+	cfg := "vault_path: " + vaultPath + "\nsync:\n  enabled: true\n  remote: \"mock-remote:bucket\"\n"
+	if err := os.WriteFile(cfgPath, []byte(cfg), 0600); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+	t.Setenv("PASS_CLI_CONFIG", cfgPath)
+
+	initVS, err := vault.New(vaultPath)
+	if err != nil {
+		t.Fatalf("vault.New (init): %v", err)
+	}
+	if err := initVS.Initialize([]byte(masterPassword), false, "", ""); err != nil {
+		t.Fatalf("Initialize: %v", err)
+	}
+
+	vs, err := vault.New(vaultPath)
+	if err != nil {
+		t.Fatalf("vault.New (locked): %v", err)
+	}
+	if !vs.IsSyncEnabled() {
+		t.Fatal("expected sync enabled")
+	}
+
+	setOffline(t, false)
+	setVerbose(t, false)
+
+	if err := unlockKeychainOverlappingPull(vs, []byte(masterPassword)); err != nil {
+		t.Fatalf("unlockKeychainOverlappingPull: %v", err)
+	}
+	if !vs.IsUnlocked() {
+		t.Fatal("expected vault unlocked after Tier 2 overlap")
+	}
+	if err := vs.AddCredential("svc", "user", []byte("secret"), "", "", ""); err != nil {
+		t.Errorf("AddCredential failed (masterPassword not retained?): %v", err)
+	}
+}

cmd/helpers.go

@@ -406,30 +406,25 @@ func unlockVault(vaultService *vault.VaultService) error {
 // Ordering constraint: Unlock reads and decrypts vault.enc in one step, and a pull
 // replaces that file — so the decrypt must run strictly AFTER the pull finishes.
 // Only work that does not touch vault.enc may overlap the pull:
-//   - the master-password prompt touches stdin only → safe to overlap (the win);
-//   - keychain unlock decrypts vault.enc, and keychain users have no prompt to hide
-//     behind, so that path stays sequential (Tier 2, overlapping the Argon2 KDF, is
-//     a deferred follow-up).
+//   - the master-password prompt touches stdin only → safe to overlap (Tier 1);
+//   - keychain unlock decrypts vault.enc, but its expensive PBKDF2 key DERIVATION
+//     touches only the password + pre-read key params, so it overlaps the pull and
+//     we decrypt after the join (Tier 2).
 func unlockVaultWithSync(vaultService *vault.VaultService) error {
 	// No pull to overlap (sync disabled or --offline): today's exact behavior.
 	if IsOffline() || !vaultService.IsSyncEnabled() {
 		syncPullBeforeUnlock(vaultService) // no-op in these cases
 		return unlockVault(vaultService)
 	}
 
-	// Keychain is resolved BEFORE the pull: retrieving the keychain password reads
-	// metadata + the OS keyring but never decrypts vault.enc. There's no human wait
-	// to hide behind, so pull then decrypt sequentially.
+	// Keychain path (Tier 2): retrieving the keychain password reads metadata + the
+	// OS keyring but never decrypts vault.enc. Read the key-derivation params first
+	// (cheap, pre-pull), then overlap the expensive PBKDF2 derivation with the pull,
+	// join, and unlock with the prepared key (which re-derives if the pull brought a
+	// re-keyed vault).
 	if password, err := vaultService.RetrieveKeychainPassword(); err == nil {
 		defer crypto.ClearBytes(password)
-		syncPullBeforeUnlock(vaultService)
-		if err := vaultService.Unlock(password); err != nil {
-			return fmt.Errorf("failed to unlock vault: %w", err)
-		}
-		if IsVerbose() {
-			fmt.Fprintln(os.Stderr, "🔓 Unlocked vault using keychain")
-		}
-		return nil
+		return unlockKeychainOverlappingPull(vaultService, password)
 	}
 
 	// Password path: run the pull concurrently with the prompt, then join before
@@ -474,6 +469,62 @@ func unlockVaultWithSync(vaultService *vault.VaultService) error {
 	return nil
 }
 
+// unlockKeychainOverlappingPull runs the keychain unlock (#103 Tier 2): it reads
+// the vault's key-derivation params, starts the sync pull in a goroutine, derives
+// the data key (the expensive PBKDF2 step) on this goroutine so it overlaps the
+// pull, joins, then unlocks with the prepared key — which itself falls back to a
+// full password unlock if the pull brought a re-keyed vault. The caller owns
+// (and clears) password.
+func unlockKeychainOverlappingPull(vaultService *vault.VaultService, password []byte) error {
+	prep, prepErr := vaultService.PrepareUnlock()
+	if prepErr != nil {
+		// Couldn't read key params → fall back to a sequential keychain unlock.
+		syncPullBeforeUnlock(vaultService)
+		if err := vaultService.Unlock(append([]byte(nil), password...)); err != nil {
+			return fmt.Errorf("failed to unlock vault: %w", err)
+		}
+		if IsVerbose() {
+			fmt.Fprintln(os.Stderr, "🔓 Unlocked vault using keychain")
+		}
+		return nil
+	}
+
+	pullDone := make(chan struct{})
+	go func() {
+		defer close(pullDone)
+		_ = vaultService.SyncPull()
+	}()
+
+	// A transient indicator is safe here — no password prompt to corrupt.
+	if IsVerbose() {
+		fmt.Fprintln(os.Stderr, "🔄 Checking remote for vault changes...")
+	} else {
+		fmt.Fprint(os.Stderr, "Checking remote...")
+	}
+	dataKey, deriveErr := prep.DeriveDataKey(password) // PBKDF2 overlaps the pull
+	<-pullDone                                         // join before any decrypt
+	if !IsVerbose() {
+		fmt.Fprint(os.Stderr, "\r\033[K")
+	}
+
+	if vaultService.SyncConflictDetected() {
+		fmt.Fprintln(os.Stderr, "Warning: sync conflict — local and remote vault both changed. Use `pass-cli sync resolve` to choose which version to keep.")
+	}
+
+	if deriveErr != nil {
+		// Derivation failed (e.g. malformed header) → full password unlock.
+		if err := vaultService.Unlock(append([]byte(nil), password...)); err != nil {
+			return fmt.Errorf("failed to unlock vault: %w", err)
+		}
+	} else if err := vaultService.UnlockWithPreparedKey(prep, dataKey, append([]byte(nil), password...)); err != nil {
+		return err
+	}
+	if IsVerbose() {
+		fmt.Fprintln(os.Stderr, "🔓 Unlocked vault using keychain")
+	}
+	return nil
+}
+
 // syncPullBeforeUnlock performs a smart sync pull before vault unlock.
 // This ensures we have the latest version from remote before reading.
 //

internal/storage/storage.go

@@ -1,6 +1,7 @@
 package storage
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -248,10 +249,12 @@ func (s *StorageService) loadVaultV2(encryptedVault *EncryptedVault, password st
 	return plaintext, nil
 }
 
-// LoadVaultWithKey loads and decrypts vault using a provided encryption key
-// Used for recovery when we have the vault recovery key instead of a password
-// Parameters: key (32-byte AES-256 key)
-// Returns: decrypted vault data, error
+// LoadVaultWithKey loads and decrypts the vault using a provided data-decryption
+// key, skipping password-to-key derivation. Used both for recovery (where the key
+// comes from a recovery secret) and for the prepared-key unlock path (#103 Tier 2),
+// where the key was derived ahead of time via DeriveDataKey. For v1 the key is the
+// password-derived key; for v2 it is the unwrapped DEK.
+// Parameters: key (32-byte AES-256 key). Returns: decrypted vault data, error.
 func (s *StorageService) LoadVaultWithKey(key []byte) ([]byte, error) {
 	encryptedVault, err := s.loadEncryptedVault()
 	if err != nil {
@@ -261,12 +264,91 @@ func (s *StorageService) LoadVaultWithKey(key []byte) ([]byte, error) {
 	// Decrypt vault data with provided key (skip password-to-key derivation)
 	plaintext, err := s.cryptoService.Decrypt(encryptedVault.Data, key)
 	if err != nil {
-		return nil, fmt.Errorf("failed to decrypt vault with recovery key: %w", err)
+		return nil, fmt.Errorf("failed to decrypt vault with provided key: %w", err)
 	}
 
 	return plaintext, nil
 }
 
+// PreparedKeyParams carries the key-derivation parameters read from the vault
+// header. They feed DeriveDataKey to compute the data-decryption key ahead of
+// time (concurrently with a sync pull) without touching the ciphertext, and they
+// are compared post-pull to detect a re-key (see vault.UnlockWithPreparedKey).
+type PreparedKeyParams struct {
+	Version         int
+	Salt            []byte
+	Iterations      int
+	WrappedDEK      []byte
+	WrappedDEKNonce []byte
+}
+
+// Equal reports whether two parameter sets would derive the same key against the
+// same ciphertext. Used to detect that the vault was re-keyed (password change or
+// re-key) between reading the params and decrypting.
+func (p PreparedKeyParams) Equal(o PreparedKeyParams) bool {
+	return p.Version == o.Version &&
+		p.Iterations == o.Iterations &&
+		bytes.Equal(p.Salt, o.Salt) &&
+		bytes.Equal(p.WrappedDEK, o.WrappedDEK) &&
+		bytes.Equal(p.WrappedDEKNonce, o.WrappedDEKNonce)
+}
+
+// ReadKeyParams reads the current on-disk key-derivation parameters without
+// decrypting the vault. Cheap; intended to run before a sync pull so the
+// expensive DeriveDataKey can overlap the pull.
+func (s *StorageService) ReadKeyParams() (PreparedKeyParams, error) {
+	encryptedVault, err := s.loadEncryptedVault()
+	if err != nil {
+		return PreparedKeyParams{}, err
+	}
+	m := encryptedVault.Metadata
+	return PreparedKeyParams{
+		Version:         m.Version,
+		Salt:            m.Salt,
+		Iterations:      m.Iterations,
+		WrappedDEK:      m.WrappedDEK,
+		WrappedDEKNonce: m.WrappedDEKNonce,
+	}, nil
+}
+
+// DeriveDataKey computes the vault's data-decryption key from the password and
+// pre-read key parameters WITHOUT reading the ciphertext — so it can run
+// concurrently with a sync pull. The expensive PBKDF2 step happens here.
+//   - v1: the password-derived key decrypts the vault directly.
+//   - v2: derive the password KEK, then unwrap the DEK; the DEK decrypts the vault.
+//
+// The returned key is what LoadVaultWithKey expects. Caller must ClearBytes it.
+func (s *StorageService) DeriveDataKey(password string, p PreparedKeyParams) ([]byte, error) {
+	if p.Version == 2 {
+		if len(p.WrappedDEK) != crypto.KeyLength+16 {
+			return nil, fmt.Errorf("invalid v2 vault: wrapped DEK length mismatch (expected %d, got %d)",
+				crypto.KeyLength+16, len(p.WrappedDEK))
+		}
+		if len(p.WrappedDEKNonce) != crypto.NonceLength {
+			return nil, fmt.Errorf("invalid v2 vault: nonce length mismatch")
+		}
+
+		passwordKEK, err := s.cryptoService.DeriveKey([]byte(password), p.Salt, p.Iterations)
+		if err != nil {
+			return nil, fmt.Errorf("failed to derive key: %w", err)
+		}
+		defer s.cryptoService.ClearKey(passwordKEK)
+
+		dek, err := crypto.UnwrapKey(crypto.WrappedKey{Ciphertext: p.WrappedDEK, Nonce: p.WrappedDEKNonce}, passwordKEK)
+		if err != nil {
+			return nil, fmt.Errorf("failed to unwrap DEK (invalid password?): %w", err)
+		}
+		return dek, nil
+	}
+
+	// v1: the password-derived key is the data key.
+	key, err := s.cryptoService.DeriveKey([]byte(password), p.Salt, p.Iterations)
+	if err != nil {
+		return nil, fmt.Errorf("failed to derive key: %w", err)
+	}
+	return key, nil
+}
+
 func (s *StorageService) SaveVault(data []byte, password string, callback ProgressCallback) error {
 	// T015: Notify audit logger of save operation start
 	if callback != nil {