backport of commit c5549cdac6 (#21271)

Co-authored-by: Nick Cabatoff <ncabatoff@hashicorp.com>

changelog/21260.txt

@@ -0,0 +1,4 @@
+```release-note:bug
+core: Change where we evaluate filtered paths as part of mount operations; this is part of an enterprise bugfix that will
+have its own changelog entry.  Fix wrong lock used in ListAuths link meta interface implementation.
+```

helper/testhelpers/teststorage/teststorage.go

@@ -8,9 +8,18 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-hclog"
+	logicalKv "github.com/hashicorp/vault-plugin-secrets-kv"
+	"github.com/hashicorp/vault/audit"
+	auditFile "github.com/hashicorp/vault/builtin/audit/file"
+	auditSocket "github.com/hashicorp/vault/builtin/audit/socket"
+	auditSyslog "github.com/hashicorp/vault/builtin/audit/syslog"
+	logicalDb "github.com/hashicorp/vault/builtin/logical/database"
+	"github.com/hashicorp/vault/builtin/plugin"
 	"github.com/hashicorp/vault/helper/testhelpers"
+	"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
 	vaulthttp "github.com/hashicorp/vault/http"
 	"github.com/hashicorp/vault/physical/raft"
+	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/sdk/physical"
 	physFile "github.com/hashicorp/vault/sdk/physical/file"
 	"github.com/hashicorp/vault/sdk/physical/inmem"
@@ -238,5 +247,28 @@ func ClusterSetup(conf *vault.CoreConfig, opts *vault.TestClusterOptions, setup
 		setup = InmemBackendSetup
 	}
 	setup(&localConf, &localOpts)
+	if localConf.CredentialBackends == nil {
+		localConf.CredentialBackends = map[string]logical.Factory{
+			"plugin": plugin.Factory,
+		}
+	}
+	if localConf.LogicalBackends == nil {
+		localConf.LogicalBackends = map[string]logical.Factory{
+			"plugin":   plugin.Factory,
+			"database": logicalDb.Factory,
+			// This is also available in the plugin catalog, but is here due to the need to
+			// automatically mount it.
+			"kv": logicalKv.Factory,
+		}
+	}
+	if localConf.AuditBackends == nil {
+		localConf.AuditBackends = map[string]audit.Factory{
+			"file":   auditFile.Factory,
+			"socket": auditSocket.Factory,
+			"syslog": auditSyslog.Factory,
+			"noop":   corehelpers.NoopAuditFactory(nil),
+		}
+	}
+
 	return &localConf, &localOpts
 }

vault/auth.go

@@ -61,16 +61,6 @@ func (c *Core) enableCredential(ctx context.Context, entry *MountEntry) error {
 		return err
 	}
 
-	// Re-evaluate filtered paths
-	if err := runFilteredPathsEvaluation(ctx, c); err != nil {
-		c.logger.Error("failed to evaluate filtered paths", "error", err)
-
-		// We failed to evaluate filtered paths so we are undoing the mount operation
-		if disableCredentialErr := c.disableCredentialInternal(ctx, entry.Path, MountTableUpdateStorage); disableCredentialErr != nil {
-			c.logger.Error("failed to disable credential", "error", disableCredentialErr)
-		}
-		return err
-	}
 	return nil
 }
 
@@ -86,8 +76,13 @@ func (c *Core) enableCredentialInternal(ctx context.Context, entry *MountEntry,
 		return fmt.Errorf("backend path must be specified")
 	}
 
+	c.mountsLock.Lock()
 	c.authLock.Lock()
-	defer c.authLock.Unlock()
+	unlock := func() {
+		c.authLock.Unlock()
+		c.mountsLock.Unlock()
+	}
+	defer unlock()
 
 	ns, err := namespace.FromContext(ctx)
 	if err != nil {
@@ -221,6 +216,19 @@ func (c *Core) enableCredentialInternal(ctx context.Context, entry *MountEntry,
 		return err
 	}
 
+	// Re-evaluate filtered paths
+	if err := runFilteredPathsEvaluation(ctx, c, false); err != nil {
+		c.logger.Error("failed to evaluate filtered paths", "error", err)
+
+		unlock()
+		unlock = func() {}
+		// We failed to evaluate filtered paths so we are undoing the mount operation
+		if disableCredentialErr := c.disableCredentialInternal(ctx, entry.Path, MountTableUpdateStorage); disableCredentialErr != nil {
+			c.logger.Error("failed to disable credential", "error", disableCredentialErr)
+		}
+		return err
+	}
+
 	if !nilMount {
 		// restore the original readOnlyErr, so we can write to the view in
 		// Initialize() if necessary
@@ -256,7 +264,7 @@ func (c *Core) disableCredential(ctx context.Context, path string) error {
 	}
 
 	// Re-evaluate filtered paths
-	if err := runFilteredPathsEvaluation(ctx, c); err != nil {
+	if err := runFilteredPathsEvaluation(ctx, c, true); err != nil {
 		// Even we failed to evaluate filtered paths, the unmount operation was still successful
 		c.logger.Error("failed to evaluate filtered paths", "error", err)
 	}
@@ -523,11 +531,6 @@ func (c *Core) remountCredEntryForceInternal(ctx context.Context, path string, u
 		return err
 	}
 
-	// Re-evaluate filtered paths
-	if err := runFilteredPathsEvaluation(ctx, c); err != nil {
-		c.logger.Error("failed to evaluate filtered paths", "error", err)
-		return err
-	}
 	return nil
 }
 

vault/core.go

@@ -3854,8 +3854,8 @@ func (c *Core) ListAuths() ([]*MountEntry, error) {
 		return nil, fmt.Errorf("vault is sealed")
 	}
 
-	c.mountsLock.RLock()
+	c.authLock.RLock()
-	defer c.mountsLock.RUnlock()
+	defer c.authLock.RUnlock()
 
 	var entries []*MountEntry
 

vault/mount.go

@@ -545,23 +545,17 @@ func (c *Core) mount(ctx context.Context, entry *MountEntry) error {
 		return err
 	}
 
-	// Re-evaluate filtered paths
-	if err := runFilteredPathsEvaluation(ctx, c); err != nil {
-		c.logger.Error("failed to evaluate filtered paths", "error", err)
-
-		// We failed to evaluate filtered paths so we are undoing the mount operation
-		if unmountInternalErr := c.unmountInternal(ctx, entry.Path, MountTableUpdateStorage); unmountInternalErr != nil {
-			c.logger.Error("failed to unmount", "error", unmountInternalErr)
-		}
-		return err
-	}
-
 	return nil
 }
 
 func (c *Core) mountInternal(ctx context.Context, entry *MountEntry, updateStorage bool) error {
 	c.mountsLock.Lock()
-	defer c.mountsLock.Unlock()
+	c.authLock.Lock()
+	unlock := func() {
+		c.authLock.Unlock()
+		c.mountsLock.Unlock()
+	}
+	defer unlock()
 
 	ns, err := namespace.FromContext(ctx)
 	if err != nil {
@@ -644,6 +638,7 @@ func (c *Core) mountInternal(ctx context.Context, entry *MountEntry, updateStora
 	if err != nil {
 		return err
 	}
+
 	origReadOnlyErr := view.getReadOnlyErr()
 
 	// Mark the view as read-only until the mounting is complete and
@@ -712,6 +707,19 @@ func (c *Core) mountInternal(ctx context.Context, entry *MountEntry, updateStora
 		return err
 	}
 
+	// Re-evaluate filtered paths
+	if err := runFilteredPathsEvaluation(ctx, c, false); err != nil {
+		c.logger.Error("failed to evaluate filtered paths", "error", err)
+
+		unlock()
+		unlock = func() {}
+		// We failed to evaluate filtered paths so we are undoing the mount operation
+		if unmountInternalErr := c.unmountInternal(ctx, entry.Path, MountTableUpdateStorage); unmountInternalErr != nil {
+			c.logger.Error("failed to unmount", "error", unmountInternalErr)
+		}
+		return err
+	}
+
 	if !nilMount {
 		// restore the original readOnlyErr, so we can write to the view in
 		// Initialize() if necessary
@@ -791,7 +799,7 @@ func (c *Core) unmount(ctx context.Context, path string) error {
 	}
 
 	// Re-evaluate filtered paths
-	if err := runFilteredPathsEvaluation(ctx, c); err != nil {
+	if err := runFilteredPathsEvaluation(ctx, c, true); err != nil {
 		// Even we failed to evaluate filtered paths, the unmount operation was still successful
 		c.logger.Error("failed to evaluate filtered paths", "error", err)
 	}
@@ -1040,11 +1048,6 @@ func (c *Core) remountForceInternal(ctx context.Context, path string, updateStor
 		return err
 	}
 
-	// Re-evaluate filtered paths
-	if err := runFilteredPathsEvaluation(ctx, c); err != nil {
-		c.logger.Error("failed to evaluate filtered paths", "error", err)
-		return err
-	}
 	return nil
 }
 

vault/mount_util.go

@@ -25,7 +25,7 @@ func addKnownPath(*Core, string)                                              {}
 func preprocessMount(*Core, *MountEntry, *BarrierView) (bool, error)          { return false, nil }
 func clearIgnoredPaths(context.Context, *Core, logical.Backend, string) error { return nil }
 func addLicenseCallback(*Core, logical.Backend)                               {}
-func runFilteredPathsEvaluation(context.Context, *Core) error                 { return nil }
+func runFilteredPathsEvaluation(context.Context, *Core, bool) error           { return nil }
 
 // ViewPath returns storage prefix for the view
 func (e *MountEntry) ViewPath() string {