From ccc8643d86b32796c42ccc6aaf3f9d80ac1d5f7f Mon Sep 17 00:00:00 2001 From: Roger Date: Mon, 6 Apr 2026 01:27:13 +0800 Subject: [PATCH] feat: add interactive terminal password prompt for keychain unlock (#558) * feat(darwin): add interactive terminal password prompt for keychain unlock (#556) * test: add unit tests for keyretriever and address review feedback - Add errStorageNotFound sentinel error for precise error matching - Non-TTY TerminalPasswordRetriever returns nil silently (review #558) - Add darwin tests: findStorageKey, empty password, non-TTY skip - Add linux tests: FallbackRetriever peanuts key, DefaultRetriever chain * fix: add nolint:unused for errStorageNotFound on Windows, clean up error message errStorageNotFound is only used on darwin/linux; Windows lint flagged it as unused. Also simplify error format to avoid "storage" duplication. * fix: add nolint:unused for errStorageNotFound, simplify error message errStorageNotFound is only referenced on darwin and linux; Windows lint flags it as unused. Also remove redundant "storage" prefix from the error format string. --- .github/dependabot.yml | 2 + crypto/keyretriever/keyretriever.go | 5 ++ crypto/keyretriever/keyretriever_darwin.go | 90 ++++++++++++++----- .../keyretriever/keyretriever_darwin_test.go | 50 +++++++++++ crypto/keyretriever/keyretriever_linux.go | 2 +- .../keyretriever/keyretriever_linux_test.go | 45 ++++++++++ go.mod | 5 +- go.sum | 9 +- 8 files changed, 181 insertions(+), 27 deletions(-) create mode 100644 crypto/keyretriever/keyretriever_darwin_test.go create mode 100644 crypto/keyretriever/keyretriever_linux_test.go diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 9a308dd..47f81dd 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -11,6 +11,8 @@ updates: versions: [">=1.32.0"] # v1.32+ requires Go 1.21, project is pinned to Go 1.20 - dependency-name: "golang.org/x/text" # indirect dep, newer versions may require Go 1.21+ - dependency-name: "golang.org/x/sys" # newer versions require Go 1.21+ + - dependency-name: "golang.org/x/term" + versions: [">=0.30.0"] # v0.30.0+ requires Go 1.23, project is pinned to Go 1.20 - package-ecosystem: "github-actions" directory: "/" schedule: diff --git a/crypto/keyretriever/keyretriever.go b/crypto/keyretriever/keyretriever.go index fe74152..7b9da2d 100644 --- a/crypto/keyretriever/keyretriever.go +++ b/crypto/keyretriever/keyretriever.go @@ -5,6 +5,11 @@ import ( "fmt" ) +// errStorageNotFound is returned when the requested browser storage +// account is not found in the credential store (keychain, keyring, etc.). +// Only used on darwin and linux; Windows uses DPAPI which 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 diff --git a/crypto/keyretriever/keyretriever_darwin.go b/crypto/keyretriever/keyretriever_darwin.go index 3720563..1e6405a 100644 --- a/crypto/keyretriever/keyretriever_darwin.go +++ b/crypto/keyretriever/keyretriever_darwin.go @@ -8,12 +8,14 @@ import ( "crypto/sha1" "errors" "fmt" + "os" "os/exec" "strings" "sync" "time" "github.com/moond4rk/keychainbreaker" + "golang.org/x/term" ) // https://source.chromium.org/chromium/chromium/src/+/master:components/os_crypt/os_crypt_mac.mm;l=157 @@ -55,6 +57,30 @@ func (r *GcoredumpRetriever) retrieveKeyOnce(storage string) ([]byte, error) { return darwinParams.deriveKey([]byte(secret)), nil } +// loadKeychainRecords opens login.keychain-db and unlocks it with the given +// password, returning all generic password records. +func loadKeychainRecords(password string) ([]keychainbreaker.GenericPassword, error) { + kc, err := keychainbreaker.Open() + if err != nil { + return nil, fmt.Errorf("open keychain: %w", err) + } + if err := kc.Unlock(keychainbreaker.WithPassword(password)); err != nil { + return nil, fmt.Errorf("unlock keychain: %w", err) + } + return kc.GenericPasswords() +} + +// findStorageKey searches keychain records for the given storage account +// and derives the encryption key. +func findStorageKey(records []keychainbreaker.GenericPassword, storage string) ([]byte, error) { + for _, rec := range records { + if rec.Account == storage { + return darwinParams.deriveKey(rec.Password), nil + } + } + return nil, fmt.Errorf("%q: %w", storage, errStorageNotFound) +} + // KeychainPasswordRetriever unlocks login.keychain-db directly using the // user's macOS login password. No root privileges required. // The keychain is opened and decrypted only once; subsequent calls @@ -67,35 +93,50 @@ type KeychainPasswordRetriever struct { err error } -func (r *KeychainPasswordRetriever) loadRecords() { - kc, err := keychainbreaker.Open() - if err != nil { - r.err = fmt.Errorf("open keychain: %w", err) - return - } - if err := kc.Unlock(keychainbreaker.WithPassword(r.Password)); err != nil { - r.err = fmt.Errorf("unlock keychain: %w", err) - return - } - r.records, r.err = kc.GenericPasswords() -} - func (r *KeychainPasswordRetriever) RetrieveKey(storage, _ string) ([]byte, error) { if r.Password == "" { return nil, fmt.Errorf("keychain password not provided") } - r.once.Do(r.loadRecords) + r.once.Do(func() { + r.records, r.err = loadKeychainRecords(r.Password) + }) if r.err != nil { return nil, r.err } - for _, rec := range r.records { - if rec.Account == storage { - return darwinParams.deriveKey(rec.Password), nil - } + return findStorageKey(r.records, storage) +} + +// TerminalPasswordRetriever prompts for the keychain password interactively +// via the terminal using golang.org/x/term (with echo disabled). +// Automatically skipped when stdin is not a TTY. +type TerminalPasswordRetriever struct { + once sync.Once + records []keychainbreaker.GenericPassword + err error +} + +func (r *TerminalPasswordRetriever) RetrieveKey(storage, _ string) ([]byte, error) { + if !term.IsTerminal(int(os.Stdin.Fd())) { + return nil, nil } - return nil, fmt.Errorf("storage %q not found in keychain", storage) + + r.once.Do(func() { + fmt.Fprintf(os.Stderr, "Enter macOS login password for %s: ", storage) + pwd, err := term.ReadPassword(int(os.Stdin.Fd())) + fmt.Fprintln(os.Stderr) + if err != nil { + r.err = fmt.Errorf("terminal: read password: %w", err) + return + } + r.records, r.err = loadKeychainRecords(string(pwd)) + }) + if r.err != nil { + return nil, r.err + } + + return findStorageKey(r.records, storage) } // SecurityCmdRetriever uses macOS `security` CLI to query Keychain. @@ -143,7 +184,11 @@ func (r *SecurityCmdRetriever) retrieveKeyOnce(storage string) ([]byte, error) { } // DefaultRetriever returns the macOS retriever chain. -// If keychainPassword is provided, the password-based retriever is included. +// The chain tries each method in order until one succeeds: +// 1. GcoredumpRetriever — CVE-2025-24204 exploit (root only, non-interactive) +// 2. KeychainPasswordRetriever — direct unlock with --keychain-pw flag +// 3. TerminalPasswordRetriever — interactive password prompt via terminal +// 4. SecurityCmdRetriever — security CLI fallback (may trigger system dialog) func DefaultRetriever(keychainPassword string) KeyRetriever { retrievers := []KeyRetriever{ &GcoredumpRetriever{}, @@ -151,6 +196,9 @@ func DefaultRetriever(keychainPassword string) KeyRetriever { if keychainPassword != "" { retrievers = append(retrievers, &KeychainPasswordRetriever{Password: keychainPassword}) } - retrievers = append(retrievers, &SecurityCmdRetriever{}) + retrievers = append(retrievers, + &TerminalPasswordRetriever{}, + &SecurityCmdRetriever{}, + ) return NewChain(retrievers...) } diff --git a/crypto/keyretriever/keyretriever_darwin_test.go b/crypto/keyretriever/keyretriever_darwin_test.go new file mode 100644 index 0000000..b7075ff --- /dev/null +++ b/crypto/keyretriever/keyretriever_darwin_test.go @@ -0,0 +1,50 @@ +//go:build darwin + +package keyretriever + +import ( + "testing" + + "github.com/moond4rk/keychainbreaker" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindStorageKey_Found(t *testing.T) { + records := []keychainbreaker.GenericPassword{ + {Account: "Chrome", Password: []byte("mock-secret")}, + {Account: "Brave", Password: []byte("brave-secret")}, + } + + key, err := findStorageKey(records, "Chrome") + require.NoError(t, err) + assert.Equal(t, darwinParams.deriveKey([]byte("mock-secret")), key) +} + +func TestFindStorageKey_NotFound(t *testing.T) { + records := []keychainbreaker.GenericPassword{ + {Account: "Chrome", Password: []byte("mock-secret")}, + } + + key, err := findStorageKey(records, "Firefox") + require.Error(t, err) + assert.Nil(t, key) + assert.ErrorIs(t, err, errStorageNotFound) +} + +func TestKeychainPasswordRetriever_EmptyPassword(t *testing.T) { + r := &KeychainPasswordRetriever{Password: ""} + key, err := r.RetrieveKey("Chrome", "") + require.Error(t, err) + assert.Nil(t, key) + assert.Contains(t, err.Error(), "keychain password not provided") +} + +func TestTerminalPasswordRetriever_NonTTY(t *testing.T) { + // In CI/test environments, stdin is not a TTY. + // The retriever should silently return nil, nil to let the chain continue. + r := &TerminalPasswordRetriever{} + key, err := r.RetrieveKey("Chrome", "") + require.NoError(t, err) + assert.Nil(t, key) +} diff --git a/crypto/keyretriever/keyretriever_linux.go b/crypto/keyretriever/keyretriever_linux.go index dc4463f..e934fc8 100644 --- a/crypto/keyretriever/keyretriever_linux.go +++ b/crypto/keyretriever/keyretriever_linux.go @@ -65,7 +65,7 @@ func (r *DBusRetriever) RetrieveKey(storage, _ string) ([]byte, error) { } } - return nil, fmt.Errorf("secret %q not found in keyring", storage) + return nil, fmt.Errorf("%q: %w", storage, errStorageNotFound) } // FallbackRetriever uses the hardcoded "peanuts" password when D-Bus is unavailable. diff --git a/crypto/keyretriever/keyretriever_linux_test.go b/crypto/keyretriever/keyretriever_linux_test.go new file mode 100644 index 0000000..4748e26 --- /dev/null +++ b/crypto/keyretriever/keyretriever_linux_test.go @@ -0,0 +1,45 @@ +//go:build linux + +package keyretriever + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFallbackRetriever(t *testing.T) { + r := &FallbackRetriever{} + + key, err := r.RetrieveKey("Chrome", "") + require.NoError(t, err) + assert.Equal(t, linuxParams.deriveKey([]byte("peanuts")), key) + assert.Len(t, key, linuxParams.keySize) + + // The key should not be all zeros. + allZero := true + for _, b := range key { + if b != 0 { + allZero = false + break + } + } + assert.False(t, allZero, "derived key should not be all zeros") + + // "peanuts" is a fixed fallback password, so the result should be + // the same regardless of storage name or number of calls. + key2, err := r.RetrieveKey("Brave", "") + require.NoError(t, err) + assert.Equal(t, key, key2, "fallback key should be the same for any storage") +} + +func TestDefaultRetriever_Linux(t *testing.T) { + r := DefaultRetriever("") + chain, ok := r.(*ChainRetriever) + require.True(t, ok, "DefaultRetriever should return a *ChainRetriever") + + assert.Len(t, chain.retrievers, 2, "chain should have 2 retrievers") + assert.IsType(t, &DBusRetriever{}, chain.retrievers[0], "first retriever should be DBusRetriever") + assert.IsType(t, &FallbackRetriever{}, chain.retrievers[1], "second retriever should be FallbackRetriever") +} diff --git a/go.mod b/go.mod index 7af2562..5ae978f 100644 --- a/go.mod +++ b/go.mod @@ -8,10 +8,12 @@ require ( github.com/otiai10/copy v1.14.1 github.com/ppacher/go-dbus-keyring v1.0.1 github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 github.com/syndtr/goleveldb v1.0.0 github.com/tidwall/gjson v1.18.0 - golang.org/x/sys v0.27.0 + golang.org/x/sys v0.30.0 + golang.org/x/term v0.29.0 modernc.org/sqlite v1.31.1 ) @@ -27,7 +29,6 @@ require ( github.com/otiai10/mint v1.6.3 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect - github.com/spf13/pflag v1.0.9 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.0 // indirect golang.org/x/sync v0.8.0 // indirect diff --git a/go.sum b/go.sum index 1668aa4..d37d7a9 100644 --- a/go.sum +++ b/go.sum @@ -43,8 +43,9 @@ github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qq github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= -github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= +github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/syndtr/goleveldb v1.0.0 h1:fBdIW9lB4Iz0n9khmH8w27SJ3QEJ7+IgjPEwGSZiFdE= @@ -64,8 +65,10 @@ golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s= -golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.29.0 h1:L6pJp37ocefwRRtYPKSWOWzOtWSxVajvz2ldH/xi3iU= +golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM= golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=