refactor(browser): simplify credential storage config (#593)

crypto/keyretriever/abe_windows.go

@@ -25,17 +25,15 @@ var errNoABEKey = errors.New("abe: Local State has no app_bound_encrypted_key")
 
 type ABERetriever struct{}
 
-func (r *ABERetriever) RetrieveKey(storage, localStatePath string) ([]byte, error) {
-	// Non-ABE Chromium forks (Opera/Vivaldi/Yandex/...) call this with an empty storage key; pre-v20
-	// Chrome profiles have no app_bound_encrypted_key in Local State. Both are "ABE not applicable" —
-	// return (nil, nil) so ChainRetriever falls through to DPAPI silently instead of emitting a Warnf
-	// for every non-ABE browser.
-	browserKey := strings.TrimSpace(storage)
+func (r *ABERetriever) RetrieveKey(hints Hints) ([]byte, error) {
+	// Non-ABE forks (Opera/Vivaldi/Yandex) supply no WindowsABEKey — treat as "not applicable".
+	// (Pre-v20 Chrome takes the errNoABEKey path below.)
+	browserKey := strings.TrimSpace(hints.WindowsABEKey)
 	if browserKey == "" {
 		return nil, nil
 	}
 
-	encKey, err := loadEncryptedKey(localStatePath)
+	encKey, err := loadEncryptedKey(hints.LocalStatePath)
 	if errors.Is(err, errNoABEKey) {
 		return nil, nil
 	}

crypto/keyretriever/keyretriever.go

@@ -20,13 +20,16 @@ import (
 // has no storage lookup.
 var errStorageNotFound = errors.New("not found in credential store") //nolint:unused // only used on darwin and linux
 
-// KeyRetriever retrieves the master encryption key for a Chromium-based browser. Each platform has
-// different implementations:
-//   - macOS: Keychain access (security command) or gcoredump exploit
-//   - Windows: DPAPI decryption of Local State file
-//   - Linux: D-Bus Secret Service or fallback to "peanuts" password
+// Hints bundles inputs for KeyRetriever; each retriever reads only the field that applies to it.
+type Hints struct {
+	KeychainLabel  string // macOS Keychain account / Linux D-Bus Secret Service label
+	WindowsABEKey  string // Windows ABE browser key (e.g. "chrome"); "" → ABE not applicable
+	LocalStatePath string // path to (temp-copied) Local State JSON; only used on Windows
+}
+
+// KeyRetriever retrieves the master encryption key for a Chromium-based browser.
 type KeyRetriever interface {
-	RetrieveKey(storage, localStatePath string) ([]byte, error)
+	RetrieveKey(hints Hints) ([]byte, error)
 }
 
 // ChainRetriever tries multiple retrievers in order, returning the first success. Used on macOS
@@ -40,10 +43,10 @@ func NewChain(retrievers ...KeyRetriever) KeyRetriever {
 	return &ChainRetriever{retrievers: retrievers}
 }
 
-func (c *ChainRetriever) RetrieveKey(storage, localStatePath string) ([]byte, error) {
+func (c *ChainRetriever) RetrieveKey(hints Hints) ([]byte, error) {
 	var errs []error
 	for _, r := range c.retrievers {
-		key, err := r.RetrieveKey(storage, localStatePath)
+		key, err := r.RetrieveKey(hints)
 		if err == nil && len(key) > 0 {
 			return key, nil
 		}

crypto/keyretriever/keyretriever_darwin.go

@@ -43,7 +43,7 @@ type GcoredumpRetriever struct {
 // through to the next retriever silently. The most common failure ("requires root privileges")
 // is documented expected behavior, not a warning-worthy condition; surfacing it on every profile
 // would drown out genuine warnings. The same pattern is used by ABERetriever (see abe_windows.go).
-func (r *GcoredumpRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
+func (r *GcoredumpRetriever) RetrieveKey(hints Hints) ([]byte, error) {
 	r.once.Do(func() {
 		r.records, r.err = DecryptKeychainRecords()
 	})
@@ -52,7 +52,7 @@ func (r *GcoredumpRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
 		return nil, nil //nolint:nilerr // intentional silent fallthrough
 	}
 
-	key, err := findStorageKey(r.records, storage)
+	key, err := findStorageKey(r.records, hints.KeychainLabel)
 	if err != nil {
 		log.Debugf("gcoredump: %v", err)
 		return nil, nil //nolint:nilerr // intentional silent fallthrough
@@ -96,7 +96,7 @@ type KeychainPasswordRetriever struct {
 	err     error
 }
 
-func (r *KeychainPasswordRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
+func (r *KeychainPasswordRetriever) RetrieveKey(hints Hints) ([]byte, error) {
 	if r.Password == "" {
 		return nil, fmt.Errorf("keychain password not provided")
 	}
@@ -108,7 +108,7 @@ func (r *KeychainPasswordRetriever) RetrieveKey(storage, _ string) ([]byte, erro
 		return nil, r.err
 	}
 
-	return findStorageKey(r.records, storage)
+	return findStorageKey(r.records, hints.KeychainLabel)
 }
 
 // SecurityCmdRetriever uses macOS `security` CLI to query Keychain.
@@ -124,7 +124,8 @@ type securityResult struct {
 	err error
 }
 
-func (r *SecurityCmdRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
+func (r *SecurityCmdRetriever) RetrieveKey(hints Hints) ([]byte, error) {
+	storage := hints.KeychainLabel
 	r.mu.Lock()
 	defer r.mu.Unlock()
 

crypto/keyretriever/keyretriever_darwin_test.go

@@ -34,7 +34,7 @@ func TestFindStorageKey_NotFound(t *testing.T) {
 
 func TestKeychainPasswordRetriever_EmptyPassword(t *testing.T) {
 	r := &KeychainPasswordRetriever{Password: ""}
-	key, err := r.RetrieveKey("Chrome", "")
+	key, err := r.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.Error(t, err)
 	assert.Nil(t, key)
 	assert.Contains(t, err.Error(), "keychain password not provided")

crypto/keyretriever/keyretriever_linux.go

@@ -21,7 +21,8 @@ var linuxParams = pbkdf2Params{
 // DBusRetriever queries GNOME Keyring / KDE Wallet via D-Bus Secret Service.
 type DBusRetriever struct{}
 
-func (r *DBusRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
+func (r *DBusRetriever) RetrieveKey(hints Hints) ([]byte, error) {
+	storage := hints.KeychainLabel
 	conn, err := dbus.SessionBus()
 	if err != nil {
 		return nil, fmt.Errorf("dbus session: %w", err)
@@ -74,7 +75,7 @@ func (r *DBusRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
 // the "v10" prefix when no keyring is available (headless servers, Docker, CI).
 type PosixRetriever struct{}
 
-func (r *PosixRetriever) RetrieveKey(_, _ string) ([]byte, error) {
+func (r *PosixRetriever) RetrieveKey(_ Hints) ([]byte, error) {
 	return linuxParams.deriveKey([]byte("peanuts")), nil
 }
 

crypto/keyretriever/keyretriever_linux_test.go

@@ -12,7 +12,7 @@ import (
 func TestPosixRetriever(t *testing.T) {
 	r := &PosixRetriever{}
 
-	key, err := r.RetrieveKey("Chrome", "")
+	key, err := r.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.NoError(t, err)
 	assert.Equal(t, linuxParams.deriveKey([]byte("peanuts")), key)
 	assert.Len(t, key, linuxParams.keySize)
@@ -27,9 +27,9 @@ func TestPosixRetriever(t *testing.T) {
 	}
 	assert.False(t, allZero, "derived key should not be all zeros")
 
-	// "peanuts" is a hardcoded password, so the result should be the same regardless of storage
-	// name or number of calls.
-	key2, err := r.RetrieveKey("Brave", "")
+	// "peanuts" is a hardcoded password, so the result should be the same regardless of the hints
+	// or number of calls.
+	key2, err := r.RetrieveKey(Hints{KeychainLabel: "Brave"})
 	require.NoError(t, err)
 	assert.Equal(t, key, key2, "kV10Key should be constant across any storage label")
 }
@@ -42,7 +42,7 @@ func TestPosixRetriever_MatchesChromiumKV10Key(t *testing.T) {
 		0x9d, 0xfa, 0x14, 0x7c, 0xa9, 0x27, 0x27, 0x78,
 	}
 	r := &PosixRetriever{}
-	key, err := r.RetrieveKey("", "")
+	key, err := r.RetrieveKey(Hints{})
 	require.NoError(t, err)
 	assert.Equal(t, want, key)
 }

crypto/keyretriever/keyretriever_test.go

@@ -13,7 +13,7 @@ type mockRetriever struct {
 	err error
 }
 
-func (m *mockRetriever) RetrieveKey(_, _ string) ([]byte, error) {
+func (m *mockRetriever) RetrieveKey(_ Hints) ([]byte, error) {
 	return m.key, m.err
 }
 
@@ -22,7 +22,7 @@ func TestChainRetriever_FirstSuccess(t *testing.T) {
 		&mockRetriever{key: []byte("first-key")},
 		&mockRetriever{key: []byte("second-key")},
 	)
-	key, err := chain.RetrieveKey("Chrome", "")
+	key, err := chain.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.NoError(t, err)
 	assert.Equal(t, []byte("first-key"), key)
 }
@@ -32,7 +32,7 @@ func TestChainRetriever_FallbackOnError(t *testing.T) {
 		&mockRetriever{err: errors.New("first failed")},
 		&mockRetriever{key: []byte("fallback-key")},
 	)
-	key, err := chain.RetrieveKey("Chrome", "")
+	key, err := chain.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.NoError(t, err)
 	assert.Equal(t, []byte("fallback-key"), key)
 }
@@ -42,7 +42,7 @@ func TestChainRetriever_AllFail(t *testing.T) {
 		&mockRetriever{err: errors.New("first failed")},
 		&mockRetriever{err: errors.New("second failed")},
 	)
-	key, err := chain.RetrieveKey("Chrome", "")
+	key, err := chain.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.Error(t, err)
 	assert.Nil(t, key)
 	assert.Contains(t, err.Error(), "all retrievers failed")
@@ -56,14 +56,14 @@ func TestChainRetriever_SkipEmptyKey(t *testing.T) {
 		&mockRetriever{key: nil, err: nil},
 		&mockRetriever{key: []byte("real-key")},
 	)
-	key, err := chain.RetrieveKey("Chrome", "")
+	key, err := chain.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.NoError(t, err)
 	assert.Equal(t, []byte("real-key"), key)
 }
 
 func TestChainRetriever_Empty(t *testing.T) {
 	chain := NewChain()
-	key, err := chain.RetrieveKey("Chrome", "")
+	key, err := chain.RetrieveKey(Hints{KeychainLabel: "Chrome"})
 	require.Error(t, err)
 	assert.Nil(t, key)
 }

crypto/keyretriever/keyretriever_windows.go

@@ -16,8 +16,8 @@ import (
 // and decrypts it using Windows DPAPI.
 type DPAPIRetriever struct{}
 
-func (r *DPAPIRetriever) RetrieveKey(_, localStatePath string) ([]byte, error) {
-	data, err := os.ReadFile(localStatePath)
+func (r *DPAPIRetriever) RetrieveKey(hints Hints) ([]byte, error) {
+	data, err := os.ReadFile(hints.LocalStatePath)
 	if err != nil {
 		return nil, fmt.Errorf("read Local State: %w", err)
 	}

crypto/keyretriever/masterkeys.go

@@ -5,47 +5,26 @@ import (
 	"fmt"
 )
 
-// MasterKeys bundles the per-cipher-version Chromium master keys used to decrypt data from a
-// single profile. decryptValue dispatches on the ciphertext's version prefix and picks the
-// matching key; a missing (nil) key for a tier means "that cipher version cannot be decrypted",
-// but the other tiers remain usable — a Chrome 127+ profile upgraded from pre-127 carries mixed
-// v10+v20 ciphertexts, and Linux profiles may carry mixed v10+v11 for analogous reasons.
-//
-//   - V10: Chrome 80+ key with "v10" cipher prefix.
-//   - Windows: os_crypt.encrypted_key decrypted by user-level DPAPI (AES-GCM ciphertexts).
-//   - macOS:   derived from Keychain via PBKDF2(1003, SHA-1) (AES-CBC ciphertexts).
-//   - Linux:   derived from "peanuts" hardcoded password (Chromium's kV10Key, AES-CBC).
-//   - V11: Chrome Linux key with "v11" cipher prefix, derived from D-Bus Secret Service
-//     (KWallet / GNOME Keyring) via PBKDF2. Nil on Windows and macOS (v11 prefix not used there).
-//   - V20: Chrome 127+ Windows key with "v20" cipher prefix, retrieved via reflective injection
-//     into the browser's elevation service. Nil on non-Windows platforms.
+// MasterKeys holds per-cipher-version Chromium master keys. A profile may carry mixed prefixes
+// (Chrome 127+ on Windows mixes v10+v20; Linux can mix v10+v11), so each tier must be populated
+// independently for lossless decryption. A nil tier means that cipher version cannot be decrypted.
 type MasterKeys struct {
 	V10 []byte
 	V11 []byte
 	V20 []byte
 }
 
-// Retrievers is the per-tier retriever configuration passed to NewMasterKeys. Each slot runs
-// independently — failure or absence of one tier does not affect others. Platform injectors set
-// only the slots that apply to their platform and leave the rest nil (e.g. Linux populates
-// V10+V11, leaves V20 nil).
+// Retrievers is the per-tier retriever configuration; unused slots are nil.
 type Retrievers struct {
 	V10 KeyRetriever
 	V11 KeyRetriever
 	V20 KeyRetriever
 }
 
-// NewMasterKeys fetches every configured tier in r independently and returns the assembled
-// MasterKeys together with any per-tier errors joined into one. Nil retrievers and retrievers
-// returning (nil, nil) (the "not applicable" signal — e.g. ABERetriever on a non-ABE fork)
-// contribute nil keys silently; only non-nil errors propagate.
-//
-// The returned error, when non-nil, is an errors.Join of per-tier failures formatted as
-// "<tier>: <err>" (e.g. "v10: dpapi decrypt: ..."). Callers are expected to log it at whatever
-// severity fits their context — this function itself never logs, leaving logging policy to its
-// callers. Other pieces of the keyretriever package (e.g. ChainRetriever) may still log on their
-// own failures; the "no-logging" guarantee is scoped to NewMasterKeys.
-func NewMasterKeys(r Retrievers, storage, localStatePath string) (MasterKeys, error) {
+// NewMasterKeys fetches each non-nil tier in r and returns the assembled MasterKeys with per-tier
+// errors joined. A retriever returning (nil, nil) signals "not applicable" and contributes no key
+// silently. This function never logs; the caller decides severity.
+func NewMasterKeys(r Retrievers, hints Hints) (MasterKeys, error) {
 	var keys MasterKeys
 	var errs []error
 
@@ -61,7 +40,7 @@ func NewMasterKeys(r Retrievers, storage, localStatePath string) (MasterKeys, er
 		if t.r == nil {
 			continue
 		}
-		k, err := t.r.RetrieveKey(storage, localStatePath)
+		k, err := t.r.RetrieveKey(hints)
 		if err != nil {
 			errs = append(errs, fmt.Errorf("%s: %w", t.name, err))
 			continue

crypto/keyretriever/masterkeys_test.go

@@ -10,20 +10,18 @@ import (
 )
 
 // recordingRetriever captures call count and arguments so tests can verify each tier's retriever
-// is invoked exactly once with the expected storage and localStatePath.
+// is invoked exactly once with the expected hints.
 type recordingRetriever struct {
 	key []byte
 	err error
 
-	calls      int
-	gotStorage string
-	gotPath    string
+	calls    int
+	gotHints Hints
 }
 
-func (r *recordingRetriever) RetrieveKey(storage, localStatePath string) ([]byte, error) {
+func (r *recordingRetriever) RetrieveKey(hints Hints) ([]byte, error) {
 	r.calls++
-	r.gotStorage = storage
-	r.gotPath = localStatePath
+	r.gotHints = hints
 	return r.key, r.err
 }
 
@@ -115,7 +113,7 @@ func TestNewMasterKeys_Matrix(t *testing.T) {
 				r.V20 = tt.v20
 			}
 
-			keys, err := NewMasterKeys(r, "chrome", "/tmp/Local State")
+			keys, err := NewMasterKeys(r, Hints{KeychainLabel: "chrome", LocalStatePath: "/tmp/Local State"})
 			assert.Equal(t, tt.wantV10, keys.V10)
 			assert.Equal(t, tt.wantV11, keys.V11)
 			assert.Equal(t, tt.wantV20, keys.V20)
@@ -136,8 +134,7 @@ func TestNewMasterKeys_Matrix(t *testing.T) {
 					continue
 				}
 				assert.Equal(t, 1, mock.calls, "%s retriever should be called exactly once", name)
-				assert.Equal(t, "chrome", mock.gotStorage)
-				assert.Equal(t, "/tmp/Local State", mock.gotPath)
+				assert.Equal(t, Hints{KeychainLabel: "chrome", LocalStatePath: "/tmp/Local State"}, mock.gotHints)
 			}
 		})
 	}
@@ -145,7 +142,7 @@ func TestNewMasterKeys_Matrix(t *testing.T) {
 
 func TestNewMasterKeys_AllNilRetrievers(t *testing.T) {
 	// All slots nil — macOS/Linux with no retriever wiring, or Windows with neither tier set up.
-	keys, err := NewMasterKeys(Retrievers{}, "chrome", "/tmp/Local State")
+	keys, err := NewMasterKeys(Retrievers{}, Hints{KeychainLabel: "chrome", LocalStatePath: "/tmp/Local State"})
 	require.NoError(t, err)
 	assert.Nil(t, keys.V10)
 	assert.Nil(t, keys.V11)
@@ -156,14 +153,14 @@ func TestNewMasterKeys_PartialNil(t *testing.T) {
 	// Only V10 wired — typical macOS shape. V11/V20 left nil.
 	k10 := []byte("v10-key-bytes-for-testing")
 	r := &recordingRetriever{key: k10}
-	keys, err := NewMasterKeys(Retrievers{V10: r}, "Chrome", "")
+	keys, err := NewMasterKeys(Retrievers{V10: r}, Hints{KeychainLabel: "Chrome"})
 
 	require.NoError(t, err)
 	assert.Equal(t, k10, keys.V10)
 	assert.Nil(t, keys.V11)
 	assert.Nil(t, keys.V20)
 	assert.Equal(t, 1, r.calls)
-	assert.Equal(t, "Chrome", r.gotStorage)
+	assert.Equal(t, Hints{KeychainLabel: "Chrome"}, r.gotHints)
 }
 
 func TestNewMasterKeys_ErrorWrapping(t *testing.T) {
@@ -172,7 +169,7 @@ func TestNewMasterKeys_ErrorWrapping(t *testing.T) {
 	sentinel := errors.New("sentinel")
 	r := Retrievers{V20: &recordingRetriever{err: sentinel}}
 
-	_, err := NewMasterKeys(r, "chrome", "")
+	_, err := NewMasterKeys(r, Hints{KeychainLabel: "chrome"})
 	require.Error(t, err)
 	assert.ErrorIs(t, err, sentinel, "errors.Is should find wrapped sentinel error")
 }