Make mount view read only until after mount persist (#3910)

logical/storage.go

@@ -14,6 +14,10 @@ import (
 // cluster operation.
 var ErrReadOnly = errors.New("Cannot write to readonly storage")
 
+// ErrSetupReadOnly is returned when a write operation is attempted on a
+// storage while the backend is still being setup.
+var ErrSetupReadOnly = errors.New("Cannot write to storage during setup")
+
 // Storage is the way that logical backends are able read/write data.
 type Storage interface {
 	List(context.Context, string) ([]string, error)

vault/audit.go

@@ -90,6 +90,12 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry) error {
 	viewPath := auditBarrierPrefix + entry.UUID + "/"
 	view := NewBarrierView(c.barrier, viewPath)
 
+	// Mark the view as read-only until the mounting is complete and
+	// ensure that it is reset after. This ensures that there will be no
+	// writes during the construction of the backend.
+	view.setReadOnlyErr(logical.ErrSetupReadOnly)
+	defer view.setReadOnlyErr(nil)
+
 	// Lookup the new backend
 	backend, err := c.newAuditBackend(ctx, entry, view, entry.Options)
 	if err != nil {
@@ -320,14 +326,21 @@ func (c *Core) setupAudits(ctx context.Context) error {
 		viewPath := auditBarrierPrefix + entry.UUID + "/"
 		view := NewBarrierView(c.barrier, viewPath)
 
+		// Mark the view as read-only until the mounting is complete and
+		// ensure that it is reset after. This ensures that there will be no
+		// writes during the construction of the backend.
+		view.setReadOnlyErr(logical.ErrSetupReadOnly)
+
 		// Initialize the backend
 		backend, err := c.newAuditBackend(ctx, entry, view, entry.Options)
 		if err != nil {
 			c.logger.Error("core: failed to create audit entry", "path", entry.Path, "error", err)
+			view.setReadOnlyErr(nil)
 			continue
 		}
 		if backend == nil {
 			c.logger.Error("core: created audit entry was nil", "path", entry.Path, "type", entry.Type)
+			view.setReadOnlyErr(nil)
 			continue
 		}
 
@@ -335,6 +348,8 @@ func (c *Core) setupAudits(ctx context.Context) error {
 		broker.Register(entry.Path, backend, view)
 
 		successCount += 1
+
+		view.setReadOnlyErr(nil)
 	}
 
 	if len(c.audit.Entries) > 0 && successCount == 0 {

vault/audit_test.go

@@ -94,6 +94,30 @@ func (n *NoopAudit) Invalidate(ctx context.Context) {
 	n.salt = nil
 }
 
+func TestAudit_ReadOnlyViewDuringMount(t *testing.T) {
+	c, _, _ := TestCoreUnsealed(t)
+	c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) {
+		err := config.SaltView.Put(ctx, &logical.StorageEntry{
+			Key:   "bar",
+			Value: []byte("baz"),
+		})
+		if err == nil || !strings.Contains(err.Error(), logical.ErrSetupReadOnly.Error()) {
+			t.Fatalf("expected a read-only error")
+		}
+		return &NoopAudit{}, nil
+	}
+
+	me := &MountEntry{
+		Table: auditTableType,
+		Path:  "foo",
+		Type:  "noop",
+	}
+	err := c.enableAudit(context.Background(), me)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestCore_EnableAudit(t *testing.T) {
 	c, keys, _ := TestCoreUnsealed(t)
 	c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) {

vault/auth.go

@@ -96,6 +96,13 @@ func (c *Core) enableCredential(ctx context.Context, entry *MountEntry) error {
 	}
 	viewPath := credentialBarrierPrefix + entry.UUID + "/"
 	view := NewBarrierView(c.barrier, viewPath)
+
+	// Mark the view as read-only until the mounting is complete and
+	// ensure that it is reset after. This ensures that there will be no
+	// writes during the construction of the backend.
+	view.setReadOnlyErr(logical.ErrSetupReadOnly)
+	defer view.setReadOnlyErr(nil)
+
 	var err error
 	var backend logical.Backend
 	sysView := c.mountEntrySysView(entry)
@@ -446,6 +453,12 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 		// Create a barrier view using the UUID
 		viewPath := credentialBarrierPrefix + entry.UUID + "/"
 		view = NewBarrierView(c.barrier, viewPath)
+
+		// Mark the view as read-only until the mounting is complete and
+		// ensure that it is reset after. This ensures that there will be no
+		// writes during the construction of the backend.
+		view.setReadOnlyErr(logical.ErrSetupReadOnly)
+
 		// Initialize the backend
 		sysView := c.mountEntrySysView(entry)
 		conf := make(map[string]string)
@@ -463,15 +476,18 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 				c.logger.Warn("core: skipping plugin-based credential entry", "path", entry.Path)
 				goto ROUTER_MOUNT
 			}
+			view.setReadOnlyErr(nil)
 			return errLoadAuthFailed
 		}
 		if backend == nil {
+			view.setReadOnlyErr(nil)
 			return fmt.Errorf("nil backend returned from %q factory", entry.Type)
 		}
 
 		// Check for the correct backend type
 		backendType = backend.Type()
 		if entry.Type == "plugin" && backendType != logical.TypeCredential {
+			view.setReadOnlyErr(nil)
 			return fmt.Errorf("cannot mount '%s' of type '%s' as an auth backend", entry.Config.PluginName, backendType)
 		}
 
@@ -480,6 +496,7 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 		path := credentialRoutePrefix + entry.Path
 		err = c.router.Mount(backend, path, entry, view)
 		if err != nil {
+			view.setReadOnlyErr(nil)
 			c.logger.Error("core: failed to mount auth entry", "path", entry.Path, "error", err)
 			return errLoadAuthFailed
 		}
@@ -497,6 +514,8 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 			c.router.tokenStoreSaltFunc = c.tokenStore.Salt
 			c.tokenStore.cubbyholeBackend = c.router.MatchingBackend("cubbyhole/").(*CubbyholeBackend)
 		}
+
+		view.setReadOnlyErr(nil)
 	}
 
 	if persistNeeded {

vault/auth_test.go

@@ -10,6 +10,30 @@ import (
 	"github.com/hashicorp/vault/logical"
 )
 
+func TestAuth_ReadOnlyViewDuringMount(t *testing.T) {
+	c, _, _ := TestCoreUnsealed(t)
+	c.credentialBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) {
+		err := config.StorageView.Put(ctx, &logical.StorageEntry{
+			Key:   "bar",
+			Value: []byte("baz"),
+		})
+		if err == nil || !strings.Contains(err.Error(), logical.ErrSetupReadOnly.Error()) {
+			t.Fatalf("expected a read-only error")
+		}
+		return &NoopBackend{}, nil
+	}
+
+	me := &MountEntry{
+		Table: credentialTableType,
+		Path:  "foo",
+		Type:  "noop",
+	}
+	err := c.enableCredential(context.Background(), me)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestCore_DefaultAuthTable(t *testing.T) {
 	c, keys, _ := TestCoreUnsealed(t)
 	verifyDefaultAuthTable(t, c.auth)

vault/barrier_view.go

@@ -18,7 +18,7 @@ import (
 type BarrierView struct {
 	barrier     BarrierStorage
 	prefix      string
-	readonly bool
+	readOnlyErr error
 }
 
 var (
@@ -34,6 +34,10 @@ func NewBarrierView(barrier BarrierStorage, prefix string) *BarrierView {
 	}
 }
 
+func (v *BarrierView) setReadOnlyErr(readOnlyErr error) {
+	v.readOnlyErr = readOnlyErr
+}
+
 // sanityCheck is used to perform a sanity check on a key
 func (v *BarrierView) sanityCheck(key string) error {
 	if strings.Contains(key, "..") {
@@ -81,8 +85,8 @@ func (v *BarrierView) Put(ctx context.Context, entry *logical.StorageEntry) erro
 
 	expandedKey := v.expandKey(entry.Key)
 
-	if v.readonly {
+	if v.readOnlyErr != nil {
-		return logical.ErrReadOnly
+		return v.readOnlyErr
 	}
 
 	nested := &Entry{
@@ -101,8 +105,8 @@ func (v *BarrierView) Delete(ctx context.Context, key string) error {
 
 	expandedKey := v.expandKey(key)
 
-	if v.readonly {
+	if v.readOnlyErr != nil {
-		return logical.ErrReadOnly
+		return v.readOnlyErr
 	}
 
 	return v.barrier.Delete(ctx, expandedKey)
@@ -111,7 +115,7 @@ func (v *BarrierView) Delete(ctx context.Context, key string) error {
 // 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, readonly: v.readonly}
+	return &BarrierView{barrier: v.barrier, prefix: sub, readOnlyErr: v.readOnlyErr}
 }
 
 // expandKey is used to expand to the full key path with the prefix

vault/barrier_view_test.go

@@ -294,7 +294,7 @@ func TestBarrierView_Readonly(t *testing.T) {
 	}
 
 	// Enable read only mode
-	view.readonly = true
+	view.readOnlyErr = logical.ErrReadOnly
 
 	// Put should fail in readonly mode
 	if err := view.Put(context.Background(), entry.Logical()); err != logical.ErrReadOnly {

vault/mount.go

@@ -247,6 +247,13 @@ func (c *Core) mountInternal(ctx context.Context, entry *MountEntry) error {
 	}
 	viewPath := backendBarrierPrefix + entry.UUID + "/"
 	view := NewBarrierView(c.barrier, viewPath)
+
+	// Mark the view as read-only until the mounting is complete and
+	// ensure that it is reset after. This ensures that there will be no
+	// writes during the construction of the backend.
+	view.setReadOnlyErr(logical.ErrSetupReadOnly)
+	defer view.setReadOnlyErr(nil)
+
 	var backend logical.Backend
 	var err error
 	sysView := c.mountEntrySysView(entry)
@@ -735,6 +742,11 @@ func (c *Core) setupMounts(ctx context.Context) error {
 		// Create a barrier view using the UUID
 		view = NewBarrierView(c.barrier, barrierPath)
 
+		// Mark the view as read-only until the mounting is complete and
+		// ensure that it is reset after. This ensures that there will be no
+		// writes during the construction of the backend.
+		view.setReadOnlyErr(logical.ErrSetupReadOnly)
+
 		var backend logical.Backend
 		var err error
 		sysView := c.mountEntrySysView(entry)
@@ -754,15 +766,18 @@ func (c *Core) setupMounts(ctx context.Context) error {
 				c.logger.Warn("core: skipping plugin-based mount entry", "path", entry.Path)
 				goto ROUTER_MOUNT
 			}
+			view.setReadOnlyErr(nil)
 			return errLoadMountsFailed
 		}
 		if backend == nil {
+			view.setReadOnlyErr(nil)
 			return fmt.Errorf("created mount entry of type %q is nil", entry.Type)
 		}
 
 		// Check for the correct backend type
 		backendType = backend.Type()
 		if entry.Type == "plugin" && backendType != logical.TypeLogical {
+			view.setReadOnlyErr(nil)
 			return fmt.Errorf("cannot mount '%s' of type '%s' as a logical backend", entry.Config.PluginName, backendType)
 		}
 
@@ -772,6 +787,7 @@ func (c *Core) setupMounts(ctx context.Context) error {
 		// Mount the backend
 		err = c.router.Mount(backend, entry.Path, entry, view)
 		if err != nil {
+			view.setReadOnlyErr(nil)
 			c.logger.Error("core: failed to mount entry", "path", entry.Path, "error", err)
 			return errLoadMountsFailed
 		}
@@ -784,6 +800,8 @@ func (c *Core) setupMounts(ctx context.Context) error {
 		if entry.Tainted {
 			c.router.Taint(entry.Path)
 		}
+
+		view.setReadOnlyErr(nil)
 	}
 	return nil
 }

vault/mount_test.go

@@ -14,6 +14,30 @@ import (
 	"github.com/hashicorp/vault/logical"
 )
 
+func TestMount_ReadOnlyViewDuringMount(t *testing.T) {
+	c, _, _ := TestCoreUnsealed(t)
+	c.logicalBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) {
+		err := config.StorageView.Put(ctx, &logical.StorageEntry{
+			Key:   "bar",
+			Value: []byte("baz"),
+		})
+		if err == nil || !strings.Contains(err.Error(), logical.ErrSetupReadOnly.Error()) {
+			t.Fatalf("expected a read-only error")
+		}
+		return &NoopBackend{}, nil
+	}
+
+	me := &MountEntry{
+		Table: mountTableType,
+		Path:  "foo",
+		Type:  "noop",
+	}
+	err := c.mount(context.Background(), me)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestCore_DefaultMountTable(t *testing.T) {
 	c, keys, _ := TestCoreUnsealed(t)
 	verifyDefaultTable(t, c.mounts)

vault/testing.go

@@ -343,9 +343,8 @@ func testTokenStore(t testing.T, c *Core) *TokenStore {
 // mounted, so that logical token functions can be used
 func TestCoreWithTokenStore(t testing.T) (*Core, *TokenStore, [][]byte, string) {
 	c, keys, root := TestCoreUnsealed(t)
-	ts := testTokenStore(t, c)
 
-	return c, ts, keys, root
+	return c, c.tokenStore, keys, root
 }
 
 // TestCoreWithBackendTokenStore returns a core that has a token store