From e9e897f64145fa1d97d30d2fc144f1094531973b Mon Sep 17 00:00:00 2001 From: Kira Bruneau Date: Tue, 23 Jul 2024 17:18:41 -0400 Subject: [PATCH 1/2] fix: Don't wipe vault on decryption errors --- internal/vault/vault.go | 4 ++++ internal/vault/vault_test.go | 14 ++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/internal/vault/vault.go b/internal/vault/vault.go index a5d0bebd..98561563 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -360,6 +360,10 @@ func newVault(path, gluonDir string, gcm cipher.AEAD) (*Vault, error, error) { var corrupt error if err := unmarshalFile(gcm, enc, new(Data)); err != nil { + if errors.Is(err, ErrDecryptFailed) { + return nil, nil, err + } + corrupt = err } diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index b84559b4..7ed2ea67 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -28,44 +28,50 @@ import ( "github.com/stretchr/testify/require" ) -func TestVault_Corrupt(t *testing.T) { +func TestVault_DecryptFailed(t *testing.T) { vaultDir, gluonDir := t.TempDir(), t.TempDir() { + // Create _, corrupt, err := vault.New(vaultDir, gluonDir, []byte("my secret key"), async.NoopPanicHandler{}) require.NoError(t, err) require.NoError(t, corrupt) } { + // Load _, corrupt, err := vault.New(vaultDir, gluonDir, []byte("my secret key"), async.NoopPanicHandler{}) require.NoError(t, err) require.NoError(t, corrupt) } { + // Load with bad key _, corrupt, err := vault.New(vaultDir, gluonDir, []byte("bad key"), async.NoopPanicHandler{}) - require.NoError(t, err) - require.ErrorIs(t, corrupt, vault.ErrDecryptFailed) + require.ErrorIs(t, err, vault.ErrDecryptFailed) + require.NoError(t, corrupt) } } -func TestVault_Corrupt_JunkData(t *testing.T) { +func TestVault_Corrupt(t *testing.T) { vaultDir, gluonDir := t.TempDir(), t.TempDir() { + // Create _, corrupt, err := vault.New(vaultDir, gluonDir, []byte("my secret key"), async.NoopPanicHandler{}) require.NoError(t, err) require.NoError(t, corrupt) } { + // Load _, corrupt, err := vault.New(vaultDir, gluonDir, []byte("my secret key"), async.NoopPanicHandler{}) require.NoError(t, err) require.NoError(t, corrupt) } { + // Load with junk data f, err := os.OpenFile(filepath.Join(vaultDir, "vault.enc"), os.O_WRONLY, 0o600) require.NoError(t, err) defer f.Close() //nolint:errcheck From 2fd0985a523784ec412de68ec5bec5bf4309e485 Mon Sep 17 00:00:00 2001 From: Kira Bruneau Date: Tue, 23 Jul 2024 17:37:06 -0400 Subject: [PATCH 2/2] fix: Double wrapping vault error with "could not create vault: " --- internal/app/vault.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/app/vault.go b/internal/app/vault.go index 8ec90f74..379c618e 100644 --- a/internal/app/vault.go +++ b/internal/app/vault.go @@ -37,7 +37,7 @@ func WithVault(locations *locations.Locations, keychains *keychain.List, panicHa // Create the encVault. encVault, insecure, corrupt, err := newVault(locations, keychains, panicHandler) if err != nil { - return fmt.Errorf("could not create vault: %w", err) + return fmt.Errorf("could not load/create vault: %w", err) } logrus.WithFields(logrus.Fields{ @@ -87,7 +87,7 @@ func newVault(locations *locations.Locations, keychains *keychain.List, panicHan vault, corrupt, err := vault.New(vaultDir, gluonCacheDir, vaultKey, panicHandler) if err != nil { - return nil, false, corrupt, fmt.Errorf("could not create vault: %w", err) + return nil, false, corrupt, err } return vault, insecure, corrupt, nil