From e9f20aee7a4cc704e0c3d9bf29ad6bc9c40fea80 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 22 Feb 2023 13:39:59 +0100 Subject: [PATCH] fix(GODT-2387): Ensure vault can be unlocked after factory reset When performing a factory reset, we don't want to wipe all keychain entries. The only keychain entry should be the vault's passphrase, and we need this to be able to decrypt the vault at next startup (to avoid it being reported as corrupt). --- internal/app/bridge.go | 2 -- internal/app/vault.go | 36 ++++++------------------- internal/bridge/settings.go | 21 ++++----------- internal/bridge/types.go | 2 +- internal/locations/locations.go | 5 ++-- internal/vault/helper.go | 48 +++++++++++++++++++++++++++++++++ internal/vault/vault.go | 4 +++ 7 files changed, 68 insertions(+), 50 deletions(-) diff --git a/internal/app/bridge.go b/internal/app/bridge.go index ef42697d..6705227a 100644 --- a/internal/app/bridge.go +++ b/internal/app/bridge.go @@ -41,8 +41,6 @@ import ( "github.com/urfave/cli/v2" ) -const vaultSecretName = "bridge-vault-key" - // deleteOldGoIMAPFiles Set with `-ldflags -X app.deleteOldGoIMAPFiles=true` to enable cleanup of old imap cache data. var deleteOldGoIMAPFiles bool //nolint:gochecknoglobals diff --git a/internal/app/vault.go b/internal/app/vault.go index 895e320d..55b583b1 100644 --- a/internal/app/vault.go +++ b/internal/app/vault.go @@ -18,18 +18,15 @@ package app import ( - "encoding/base64" "fmt" "path" - "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v3/internal/certs" "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/ProtonMail/proton-bridge/v3/internal/locations" "github.com/ProtonMail/proton-bridge/v3/internal/vault" "github.com/ProtonMail/proton-bridge/v3/pkg/keychain" "github.com/sirupsen/logrus" - "golang.org/x/exp/slices" ) func WithVault(locations *locations.Locations, fn func(*vault.Vault, bool, bool) error) error { @@ -82,7 +79,7 @@ func newVault(locations *locations.Locations) (*vault.Vault, bool, bool, error) insecure bool ) - if key, err := getVaultKey(vaultDir); err != nil { + if key, err := loadVaultKey(vaultDir); err != nil { insecure = true // We store the insecure vault in a separate directory @@ -104,42 +101,25 @@ func newVault(locations *locations.Locations) (*vault.Vault, bool, bool, error) return vault, insecure, corrupt, nil } -func getVaultKey(vaultDir string) ([]byte, error) { +func loadVaultKey(vaultDir string) ([]byte, error) { helper, err := vault.GetHelper(vaultDir) if err != nil { return nil, fmt.Errorf("could not get keychain helper: %w", err) } - keychain, err := keychain.NewKeychain(helper, constants.KeyChainName) + kc, err := keychain.NewKeychain(helper, constants.KeyChainName) if err != nil { return nil, fmt.Errorf("could not create keychain: %w", err) } - secrets, err := keychain.List() + has, err := vault.HasVaultKey(kc) if err != nil { - return nil, fmt.Errorf("could not list keychain: %w", err) + return nil, fmt.Errorf("could not check for vault key: %w", err) } - if !slices.Contains(secrets, vaultSecretName) { - tok, err := crypto.RandomToken(32) - if err != nil { - return nil, fmt.Errorf("could not generate random token: %w", err) - } - - if err := keychain.Put(vaultSecretName, base64.StdEncoding.EncodeToString(tok)); err != nil { - return nil, fmt.Errorf("could not put keychain item: %w", err) - } + if has { + return vault.GetVaultKey(kc) } - _, keyEnc, err := keychain.Get(vaultSecretName) - if err != nil { - return nil, fmt.Errorf("could not get keychain item: %w", err) - } - - keyDec, err := base64.StdEncoding.DecodeString(keyEnc) - if err != nil { - return nil, fmt.Errorf("could not decode keychain item: %w", err) - } - - return keyDec, nil + return vault.NewVaultKey(kc) } diff --git a/internal/bridge/settings.go b/internal/bridge/settings.go index 667406b2..0ab4c5a0 100644 --- a/internal/bridge/settings.go +++ b/internal/bridge/settings.go @@ -25,11 +25,9 @@ import ( "path/filepath" "github.com/Masterminds/semver/v3" - "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/ProtonMail/proton-bridge/v3/internal/safe" "github.com/ProtonMail/proton-bridge/v3/internal/updater" "github.com/ProtonMail/proton-bridge/v3/internal/vault" - "github.com/ProtonMail/proton-bridge/v3/pkg/keychain" "github.com/sirupsen/logrus" ) @@ -310,6 +308,9 @@ func (bridge *Bridge) SetColorScheme(colorScheme string) error { return bridge.vault.SetColorScheme(colorScheme) } +// FactoryReset deletes all users, wipes the vault, and deletes all files. +// Note: it does not clear the keychain. The only entry in the keychain is the vault password, +// which we need at next startup to decrypt the vault. func (bridge *Bridge) FactoryReset(ctx context.Context) { // Delete all the users. safe.Lock(func() { @@ -326,22 +327,10 @@ func (bridge *Bridge) FactoryReset(ctx context.Context) { logrus.WithError(err).Error("Failed to reset vault") } - // Then delete all files. - if err := bridge.locator.Clear(); err != nil { + // Lastly, delete all files except the vault. + if err := bridge.locator.Clear(bridge.vault.Path()); err != nil { logrus.WithError(err).Error("Failed to clear data paths") } - - // Lastly clear the keychain. - vaultDir, err := bridge.locator.ProvideSettingsPath() - if err != nil { - logrus.WithError(err).Error("Failed to get vault dir") - } else if helper, err := vault.GetHelper(vaultDir); err != nil { - logrus.WithError(err).Error("Failed to get keychain helper") - } else if keychain, err := keychain.NewKeychain(helper, constants.KeyChainName); err != nil { - logrus.WithError(err).Error("Failed to get keychain") - } else if err := keychain.Clear(); err != nil { - logrus.WithError(err).Error("Failed to clear keychain") - } } func getPort(addr net.Addr) int { diff --git a/internal/bridge/types.go b/internal/bridge/types.go index 4c79defc..3e779817 100644 --- a/internal/bridge/types.go +++ b/internal/bridge/types.go @@ -30,7 +30,7 @@ type Locator interface { ProvideGluonDataPath() (string, error) GetLicenseFilePath() string GetDependencyLicensesLink() string - Clear() error + Clear(...string) error } type Identifier interface { diff --git a/internal/locations/locations.go b/internal/locations/locations.go index 197549a4..8a5c14d5 100644 --- a/internal/locations/locations.go +++ b/internal/locations/locations.go @@ -217,14 +217,13 @@ func (l *Locations) getUpdatesPath() string { } // Clear removes everything except the lock and update files. -func (l *Locations) Clear() error { +func (l *Locations) Clear(except ...string) error { return files.Remove( l.userConfig, l.userData, l.userCache, ).Except( - l.GetGuiLockFile(), - l.getUpdatesPath(), + append(except, l.GetGuiLockFile(), l.getUpdatesPath())..., ).Do() } diff --git a/internal/vault/helper.go b/internal/vault/helper.go index 721c5ab0..dfc64878 100644 --- a/internal/vault/helper.go +++ b/internal/vault/helper.go @@ -18,13 +18,21 @@ package vault import ( + "encoding/base64" "encoding/json" "errors" + "fmt" "io/fs" "os" "path/filepath" + + "github.com/ProtonMail/gopenpgp/v2/crypto" + "github.com/ProtonMail/proton-bridge/v3/pkg/keychain" + "golang.org/x/exp/slices" ) +const vaultSecretName = "bridge-vault-key" + type Keychain struct { Helper string } @@ -60,3 +68,43 @@ func SetHelper(vaultDir, helper string) error { return os.WriteFile(getKeychainPrefPath(vaultDir), b, 0o600) } + +func HasVaultKey(kc *keychain.Keychain) (bool, error) { + secrets, err := kc.List() + if err != nil { + return false, fmt.Errorf("could not list keychain: %w", err) + } + + return slices.Contains(secrets, vaultSecretName), nil +} + +func GetVaultKey(kc *keychain.Keychain) ([]byte, error) { + _, keyEnc, err := kc.Get(vaultSecretName) + if err != nil { + return nil, fmt.Errorf("could not get keychain item: %w", err) + } + + keyDec, err := base64.StdEncoding.DecodeString(keyEnc) + if err != nil { + return nil, fmt.Errorf("could not decode keychain item: %w", err) + } + + return keyDec, nil +} + +func SetVaultKey(kc *keychain.Keychain, key []byte) error { + return kc.Put(vaultSecretName, base64.StdEncoding.EncodeToString(key)) +} + +func NewVaultKey(kc *keychain.Keychain) ([]byte, error) { + tok, err := crypto.RandomToken(32) + if err != nil { + return nil, fmt.Errorf("could not generate random token: %w", err) + } + + if err := kc.Put(vaultSecretName, base64.StdEncoding.EncodeToString(tok)); err != nil { + return nil, fmt.Errorf("could not put keychain item: %w", err) + } + + return tok, nil +} diff --git a/internal/vault/vault.go b/internal/vault/vault.go index 09f3b50e..4e605e3a 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -191,6 +191,10 @@ func (vault *Vault) Reset(gluonDir string) error { }) } +func (vault *Vault) Path() string { + return vault.path +} + func (vault *Vault) Close() error { vault.refLock.Lock() defer vault.refLock.Unlock()