From 27c960d8df9b6e1df7ae347bb0f26e2396fe8b06 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 31 Jan 2019 09:25:18 -0500 Subject: [PATCH] Split SubView functionality into logical.StorageView (#6141) This lets other parts of Vault that can't depend on the vault package take advantage of the subview functionality. This also allows getting rid of BarrierStorage and vault.Entry, two totally redundant abstractions. --- logical/storage_view.go | 114 ++++++++++++++++++++++++++++++++++ vault/audit.go | 4 +- vault/auth.go | 2 +- vault/barrier.go | 22 +------ vault/barrier_aes_gcm.go | 7 ++- vault/barrier_aes_gcm_test.go | 13 ++-- vault/barrier_test.go | 10 +-- vault/barrier_view.go | 88 +++++--------------------- vault/barrier_view_test.go | 10 +-- vault/cluster.go | 3 +- vault/core_test.go | 2 +- vault/ha.go | 2 +- vault/logical_system.go | 4 +- vault/mount.go | 2 +- vault/mount_test.go | 2 +- vault/rekey.go | 4 +- vault/router.go | 2 +- vault/wrapping.go | 2 +- 18 files changed, 171 insertions(+), 122 deletions(-) create mode 100644 logical/storage_view.go diff --git a/logical/storage_view.go b/logical/storage_view.go new file mode 100644 index 000000000..f0edc59f7 --- /dev/null +++ b/logical/storage_view.go @@ -0,0 +1,114 @@ +package logical + +import ( + "context" + "errors" + "strings" +) + +type StorageView struct { + storage Storage + prefix string +} + +var ( + ErrRelativePath = errors.New("relative paths not supported") +) + +func NewStorageView(storage Storage, prefix string) *StorageView { + return &StorageView{ + storage: storage, + prefix: prefix, + } +} + +// logical.Storage impl. +func (s *StorageView) List(ctx context.Context, prefix string) ([]string, error) { + if err := s.SanityCheck(prefix); err != nil { + return nil, err + } + return s.storage.List(ctx, s.ExpandKey(prefix)) +} + +// logical.Storage impl. +func (s *StorageView) Get(ctx context.Context, key string) (*StorageEntry, error) { + if err := s.SanityCheck(key); err != nil { + return nil, err + } + entry, err := s.storage.Get(ctx, s.ExpandKey(key)) + if err != nil { + return nil, err + } + if entry == nil { + return nil, nil + } + if entry != nil { + entry.Key = s.TruncateKey(entry.Key) + } + + return &StorageEntry{ + Key: entry.Key, + Value: entry.Value, + SealWrap: entry.SealWrap, + }, nil +} + +// logical.Storage impl. +func (s *StorageView) Put(ctx context.Context, entry *StorageEntry) error { + if entry == nil { + return errors.New("cannot write nil entry") + } + + if err := s.SanityCheck(entry.Key); err != nil { + return err + } + + expandedKey := s.ExpandKey(entry.Key) + + nested := &StorageEntry{ + Key: expandedKey, + Value: entry.Value, + SealWrap: entry.SealWrap, + } + + return s.storage.Put(ctx, nested) +} + +// logical.Storage impl. +func (s *StorageView) Delete(ctx context.Context, key string) error { + if err := s.SanityCheck(key); err != nil { + return err + } + + expandedKey := s.ExpandKey(key) + + return s.storage.Delete(ctx, expandedKey) +} + +func (s *StorageView) Prefix() string { + return s.prefix +} + +// SubView constructs a nested sub-view using the given prefix +func (s *StorageView) SubView(prefix string) *StorageView { + sub := s.ExpandKey(prefix) + return &StorageView{storage: s.storage, prefix: sub} +} + +// SanityCheck is used to perform a sanity check on a key +func (s *StorageView) SanityCheck(key string) error { + if strings.Contains(key, "..") { + return ErrRelativePath + } + return nil +} + +// ExpandKey is used to expand to the full key path with the prefix +func (s *StorageView) ExpandKey(suffix string) string { + return s.prefix + suffix +} + +// TruncateKey is used to remove the prefix of the key +func (s *StorageView) TruncateKey(full string) string { + return strings.TrimPrefix(full, s.prefix) +} diff --git a/vault/audit.go b/vault/audit.go index d23e8a9da..440a4c0c7 100644 --- a/vault/audit.go +++ b/vault/audit.go @@ -308,7 +308,7 @@ func (c *Core) persistAudit(ctx context.Context, table *MountTable, localOnly bo } // Create an entry - entry := &Entry{ + entry := &logical.StorageEntry{ Key: coreAuditConfigPath, Value: compressedBytes, } @@ -327,7 +327,7 @@ func (c *Core) persistAudit(ctx context.Context, table *MountTable, localOnly bo return err } - entry := &Entry{ + entry := &logical.StorageEntry{ Key: coreLocalAuditConfigPath, Value: compressedBytes, } diff --git a/vault/auth.go b/vault/auth.go index 38a77f828..5aafc1dca 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -517,7 +517,7 @@ func (c *Core) persistAuth(ctx context.Context, table *MountTable, local *bool) } // Create an entry - entry := &Entry{ + entry := &logical.StorageEntry{ Key: path, Value: compressedBytes, } diff --git a/vault/barrier.go b/vault/barrier.go index 7f8a31381..8de69e444 100644 --- a/vault/barrier.go +++ b/vault/barrier.go @@ -130,7 +130,7 @@ type SecurityBarrier interface { Keyring() (*Keyring, error) // SecurityBarrier must provide the storage APIs - BarrierStorage + logical.Storage // SecurityBarrier must provide the encryption APIs BarrierEncryptor @@ -139,10 +139,10 @@ type SecurityBarrier interface { // BarrierStorage is the storage only interface required for a Barrier. type BarrierStorage interface { // Put is used to insert or update an entry - Put(ctx context.Context, entry *Entry) error + Put(ctx context.Context, entry *logical.StorageEntry) error // Get is used to fetch an entry - Get(ctx context.Context, key string) (*Entry, error) + Get(ctx context.Context, key string) (*logical.StorageEntry, error) // Delete is used to permanently delete an entry Delete(ctx context.Context, key string) error @@ -160,22 +160,6 @@ type BarrierEncryptor interface { Decrypt(ctx context.Context, key string, ciphertext []byte) ([]byte, error) } -// Entry is used to represent data stored by the security barrier -type Entry struct { - Key string - Value []byte - SealWrap bool -} - -// Logical turns the Entry into a logical storage entry. -func (e *Entry) Logical() *logical.StorageEntry { - return &logical.StorageEntry{ - Key: e.Key, - Value: e.Value, - SealWrap: e.SealWrap, - } -} - // KeyInfo is used to convey information about the encryption key type KeyInfo struct { Term int diff --git a/vault/barrier_aes_gcm.go b/vault/barrier_aes_gcm.go index 39b1e05cd..ddddb42ca 100644 --- a/vault/barrier_aes_gcm.go +++ b/vault/barrier_aes_gcm.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/helper/jsonutil" "github.com/hashicorp/vault/helper/strutil" + "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/physical" ) @@ -662,7 +663,7 @@ func (b *AESGCMBarrier) updateMasterKeyCommon(key []byte) (*Keyring, error) { } // Put is used to insert or update an entry -func (b *AESGCMBarrier) Put(ctx context.Context, entry *Entry) error { +func (b *AESGCMBarrier) Put(ctx context.Context, entry *logical.StorageEntry) error { defer metrics.MeasureSince([]string{"barrier", "put"}, time.Now()) b.l.RLock() if b.sealed { @@ -690,7 +691,7 @@ func (b *AESGCMBarrier) Put(ctx context.Context, entry *Entry) error { } // Get is used to fetch an entry -func (b *AESGCMBarrier) Get(ctx context.Context, key string) (*Entry, error) { +func (b *AESGCMBarrier) Get(ctx context.Context, key string) (*logical.StorageEntry, error) { defer metrics.MeasureSince([]string{"barrier", "get"}, time.Now()) b.l.RLock() if b.sealed { @@ -735,7 +736,7 @@ func (b *AESGCMBarrier) Get(ctx context.Context, key string) (*Entry, error) { } // Wrap in a logical entry - entry := &Entry{ + entry := &logical.StorageEntry{ Key: key, Value: plain, SealWrap: pe.SealWrap, diff --git a/vault/barrier_aes_gcm_test.go b/vault/barrier_aes_gcm_test.go index 33a6a83c9..3d99523ef 100644 --- a/vault/barrier_aes_gcm_test.go +++ b/vault/barrier_aes_gcm_test.go @@ -8,6 +8,7 @@ import ( log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/logging" + "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/physical" "github.com/hashicorp/vault/physical/inmem" ) @@ -209,7 +210,7 @@ func TestAESGCMBarrier_Confidential(t *testing.T) { b.Unseal(context.Background(), key) // Put a logical entry - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} err = b.Put(context.Background(), entry) if err != nil { t.Fatalf("err: %v", err) @@ -249,7 +250,7 @@ func TestAESGCMBarrier_Integrity(t *testing.T) { b.Unseal(context.Background(), key) // Put a logical entry - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} err = b.Put(context.Background(), entry) if err != nil { t.Fatalf("err: %v", err) @@ -294,7 +295,7 @@ func TestAESGCMBarrier_MoveIntegrityV1(t *testing.T) { } // Put a logical entry - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} err = b.Put(context.Background(), entry) if err != nil { t.Fatalf("err: %v", err) @@ -338,7 +339,7 @@ func TestAESGCMBarrier_MoveIntegrityV2(t *testing.T) { } // Put a logical entry - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} err = b.Put(context.Background(), entry) if err != nil { t.Fatalf("err: %v", err) @@ -382,7 +383,7 @@ func TestAESGCMBarrier_UpgradeV1toV2(t *testing.T) { } // Put a logical entry - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} err = b.Put(context.Background(), entry) if err != nil { t.Fatalf("err: %v", err) @@ -432,7 +433,7 @@ func TestEncrypt_Unique(t *testing.T) { t.Fatalf("barrier is sealed") } - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} term := b.keyring.ActiveTerm() primary, _ := b.aeadForTerm(term) diff --git a/vault/barrier_test.go b/vault/barrier_test.go index a8bd2ac6f..1b77c20d3 100644 --- a/vault/barrier_test.go +++ b/vault/barrier_test.go @@ -5,6 +5,8 @@ import ( "reflect" "testing" "time" + + "github.com/hashicorp/vault/logical" ) func testBarrier(t *testing.T, b SecurityBarrier) { @@ -32,7 +34,7 @@ func testBarrier(t *testing.T, b SecurityBarrier) { } // All operations should fail - e := &Entry{Key: "test", Value: []byte("test")} + e := &logical.StorageEntry{Key: "test", Value: []byte("test")} if err := b.Put(context.Background(), e); err != ErrBarrierSealed { t.Fatalf("err: %v", err) } @@ -264,7 +266,7 @@ func testBarrier_Rotate(t *testing.T, b SecurityBarrier) { first := info.InstallTime // Write a key - e1 := &Entry{Key: "test", Value: []byte("test")} + e1 := &logical.StorageEntry{Key: "test", Value: []byte("test")} if err := b.Put(context.Background(), e1); err != nil { t.Fatalf("err: %v", err) } @@ -291,7 +293,7 @@ func testBarrier_Rotate(t *testing.T, b SecurityBarrier) { } // Write another key - e2 := &Entry{Key: "foo", Value: []byte("test")} + e2 := &logical.StorageEntry{Key: "foo", Value: []byte("test")} if err := b.Put(context.Background(), e2); err != nil { t.Fatalf("err: %v", err) } @@ -357,7 +359,7 @@ func testBarrier_Rekey(t *testing.T, b SecurityBarrier) { } // Write a key - e1 := &Entry{Key: "test", Value: []byte("test")} + e1 := &logical.StorageEntry{Key: "test", Value: []byte("test")} if err := b.Put(context.Background(), e1); err != nil { t.Fatalf("err: %v", err) } diff --git a/vault/barrier_view.go b/vault/barrier_view.go index 94fbac9a8..e89fd3aa3 100644 --- a/vault/barrier_view.go +++ b/vault/barrier_view.go @@ -3,7 +3,6 @@ package vault import ( "context" "errors" - "strings" "sync" "github.com/hashicorp/vault/logical" @@ -17,23 +16,17 @@ import ( // BarrierView implements logical.Storage so it can be passed in as the // durable storage mechanism for logical views. type BarrierView struct { - barrier BarrierStorage - prefix string + storage *logical.StorageView readOnlyErr error readOnlyErrLock sync.RWMutex iCheck interface{} } -var ( - ErrRelativePath = errors.New("relative paths not supported") -) - // NewBarrierView takes an underlying security barrier and returns // a view of it that can only operate with the given prefix. -func NewBarrierView(barrier BarrierStorage, prefix string) *BarrierView { +func NewBarrierView(barrier logical.Storage, prefix string) *BarrierView { return &BarrierView{ - barrier: barrier, - prefix: prefix, + storage: logical.NewStorageView(barrier, prefix), } } @@ -53,56 +46,25 @@ func (v *BarrierView) getReadOnlyErr() error { return v.readOnlyErr } -// sanityCheck is used to perform a sanity check on a key -func (v *BarrierView) sanityCheck(key string) error { - if strings.Contains(key, "..") { - return ErrRelativePath - } - return nil +func (v *BarrierView) Prefix() string { + return v.storage.Prefix() } -// logical.Storage impl. func (v *BarrierView) List(ctx context.Context, prefix string) ([]string, error) { - if err := v.sanityCheck(prefix); err != nil { - return nil, err - } - return v.barrier.List(ctx, v.expandKey(prefix)) + return v.storage.List(ctx, prefix) } -// logical.Storage impl. func (v *BarrierView) Get(ctx context.Context, key string) (*logical.StorageEntry, error) { - if err := v.sanityCheck(key); err != nil { - return nil, err - } - entry, err := v.barrier.Get(ctx, v.expandKey(key)) - if err != nil { - return nil, err - } - if entry == nil { - return nil, nil - } - if entry != nil { - entry.Key = v.truncateKey(entry.Key) - } - - return &logical.StorageEntry{ - Key: entry.Key, - Value: entry.Value, - SealWrap: entry.SealWrap, - }, nil + return v.storage.Get(ctx, key) } -// logical.Storage impl. +// Put differs from List/Get because it checks read-only errors func (v *BarrierView) Put(ctx context.Context, entry *logical.StorageEntry) error { if entry == nil { return errors.New("cannot write nil entry") } - if err := v.sanityCheck(entry.Key); err != nil { - return err - } - - expandedKey := v.expandKey(entry.Key) + expandedKey := v.storage.ExpandKey(entry.Key) roErr := v.getReadOnlyErr() if roErr != nil { @@ -111,21 +73,12 @@ func (v *BarrierView) Put(ctx context.Context, entry *logical.StorageEntry) erro } } - nested := &Entry{ - Key: expandedKey, - Value: entry.Value, - SealWrap: entry.SealWrap, - } - return v.barrier.Put(ctx, nested) + return v.storage.Put(ctx, entry) } // logical.Storage impl. func (v *BarrierView) Delete(ctx context.Context, key string) error { - if err := v.sanityCheck(key); err != nil { - return err - } - - expandedKey := v.expandKey(key) + expandedKey := v.storage.ExpandKey(key) roErr := v.getReadOnlyErr() if roErr != nil { @@ -134,21 +87,14 @@ func (v *BarrierView) Delete(ctx context.Context, key string) error { } } - return v.barrier.Delete(ctx, expandedKey) + return v.storage.Delete(ctx, key) } // SubView constructs a nested sub-view using the given prefix func (v *BarrierView) SubView(prefix string) *BarrierView { - sub := v.expandKey(prefix) - return &BarrierView{barrier: v.barrier, prefix: sub, readOnlyErr: v.getReadOnlyErr(), iCheck: v.iCheck} -} - -// expandKey is used to expand to the full key path with the prefix -func (v *BarrierView) expandKey(suffix string) string { - return v.prefix + suffix -} - -// truncateKey is used to remove the prefix of the key -func (v *BarrierView) truncateKey(full string) string { - return strings.TrimPrefix(full, v.prefix) + return &BarrierView{ + storage: v.storage.SubView(prefix), + readOnlyErr: v.getReadOnlyErr(), + iCheck: v.iCheck, + } } diff --git a/vault/barrier_view_test.go b/vault/barrier_view_test.go index bd91387c2..4b55b4b70 100644 --- a/vault/barrier_view_test.go +++ b/vault/barrier_view_test.go @@ -53,7 +53,7 @@ func TestBarrierView(t *testing.T) { view := NewBarrierView(barrier, "foo/") // Write a key outside of foo/ - entry := &Entry{Key: "test", Value: []byte("test")} + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} if err := barrier.Put(context.Background(), entry); err != nil { t.Fatalf("bad: %v", err) } @@ -77,7 +77,7 @@ func TestBarrierView(t *testing.T) { } // Try to put the same entry via the view - if err := view.Put(context.Background(), entry.Logical()); err != nil { + if err := view.Put(context.Background(), entry); err != nil { t.Fatalf("err: %v", err) } @@ -289,8 +289,8 @@ func TestBarrierView_Readonly(t *testing.T) { view := NewBarrierView(barrier, "foo/") // Add a key before enabling read-only - entry := &Entry{Key: "test", Value: []byte("test")} - if err := view.Put(context.Background(), entry.Logical()); err != nil { + entry := &logical.StorageEntry{Key: "test", Value: []byte("test")} + if err := view.Put(context.Background(), entry); err != nil { t.Fatalf("err: %v", err) } @@ -298,7 +298,7 @@ func TestBarrierView_Readonly(t *testing.T) { view.readOnlyErr = logical.ErrReadOnly // Put should fail in readonly mode - if err := view.Put(context.Background(), entry.Logical()); err != logical.ErrReadOnly { + if err := view.Put(context.Background(), entry); err != logical.ErrReadOnly { t.Fatalf("err: %v", err) } diff --git a/vault/cluster.go b/vault/cluster.go index 93b2dc731..d8497201b 100644 --- a/vault/cluster.go +++ b/vault/cluster.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/errwrap" uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/jsonutil" + "github.com/hashicorp/vault/logical" ) const ( @@ -270,7 +271,7 @@ func (c *Core) setupCluster(ctx context.Context) error { } // Store it - err = c.barrier.Put(ctx, &Entry{ + err = c.barrier.Put(ctx, &logical.StorageEntry{ Key: coreLocalClusterInfoPath, Value: rawCluster, }) diff --git a/vault/core_test.go b/vault/core_test.go index 934641ef3..96e673558 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1470,7 +1470,7 @@ func TestCore_CleanLeaderPrefix(t *testing.T) { if err != nil { t.Fatal(err) } - core.barrier.Put(namespace.RootContext(nil), &Entry{ + core.barrier.Put(namespace.RootContext(nil), &logical.StorageEntry{ Key: coreLeaderPrefix + keyUUID, Value: []byte(valueUUID), }) diff --git a/vault/ha.go b/vault/ha.go index 1fb70ada0..b1affc56f 100644 --- a/vault/ha.go +++ b/vault/ha.go @@ -813,7 +813,7 @@ func (c *Core) advertiseLeader(ctx context.Context, uuid string, leaderLostCh <- if err != nil { return err } - ent := &Entry{ + ent := &logical.StorageEntry{ Key: coreLeaderPrefix + uuid, Value: val, } diff --git a/vault/logical_system.go b/vault/logical_system.go index 5cc378407..69ef4f05c 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -2173,7 +2173,7 @@ func (b *SystemBackend) handleRawWrite(ctx context.Context, req *logical.Request } value := data.Get("value").(string) - entry := &Entry{ + entry := &logical.StorageEntry{ Key: path, Value: []byte(value), } @@ -2272,7 +2272,7 @@ func (b *SystemBackend) handleRotate(ctx context.Context, req *logical.Request, // Write to the canary path, which will force a synchronous truing during // replication - if err := b.Core.barrier.Put(ctx, &Entry{ + if err := b.Core.barrier.Put(ctx, &logical.StorageEntry{ Key: coreKeyringCanaryPath, Value: []byte(fmt.Sprintf("new-rotation-term-%d", newTerm)), }); err != nil { diff --git a/vault/mount.go b/vault/mount.go index 355b15054..f4a95ca9d 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -957,7 +957,7 @@ func (c *Core) persistMounts(ctx context.Context, table *MountTable, local *bool } // Create an entry - entry := &Entry{ + entry := &logical.StorageEntry{ Key: path, Value: compressedBytes, } diff --git a/vault/mount_test.go b/vault/mount_test.go index 0a28e68f7..731c9780c 100644 --- a/vault/mount_test.go +++ b/vault/mount_test.go @@ -558,7 +558,7 @@ func testCore_MountTable_UpgradeToTyped_Common( t.Fatalf("bad: values here should be different") } - entry := &Entry{ + entry := &logical.StorageEntry{ Key: path, Value: raw, } diff --git a/vault/rekey.go b/vault/rekey.go index 5152160e9..c38b653d1 100644 --- a/vault/rekey.go +++ b/vault/rekey.go @@ -519,7 +519,7 @@ func (c *Core) performBarrierRekey(ctx context.Context, newMasterKey []byte) log // Write to the canary path, which will force a synchronous truing during // replication - if err := c.barrier.Put(ctx, &Entry{ + if err := c.barrier.Put(ctx, &logical.StorageEntry{ Key: coreKeyringCanaryPath, Value: []byte(c.barrierRekeyConfig.Nonce), }); err != nil { @@ -721,7 +721,7 @@ func (c *Core) performRecoveryRekey(ctx context.Context, newMasterKey []byte) lo // Write to the canary path, which will force a synchronous truing during // replication - if err := c.barrier.Put(ctx, &Entry{ + if err := c.barrier.Put(ctx, &logical.StorageEntry{ Key: coreKeyringCanaryPath, Value: []byte(c.recoveryRekeyConfig.Nonce), }); err != nil { diff --git a/vault/router.go b/vault/router.go index b17661cab..b10180535 100644 --- a/vault/router.go +++ b/vault/router.go @@ -125,7 +125,7 @@ func (r *Router) Mount(backend logical.Backend, prefix string, mountEntry *Mount tainted: false, backend: backend, mountEntry: mountEntry, - storagePrefix: storageView.prefix, + storagePrefix: storageView.Prefix(), storageView: storageView, } re.rootPaths.Store(pathsToRadix(paths.Root)) diff --git a/vault/wrapping.go b/vault/wrapping.go index 81c750a07..b6ff5211c 100644 --- a/vault/wrapping.go +++ b/vault/wrapping.go @@ -46,7 +46,7 @@ func (c *Core) ensureWrappingKey(ctx context.Context) error { if err != nil { return errwrap.Wrapf("failed to encode wrapping key: {{err}}", err) } - entry = &Entry{ + entry = &logical.StorageEntry{ Key: coreWrappingJWTKeyPath, Value: val, }