plugins: Handle mount/enable for shadowed builtins (#17879)

* Allow mounting external plugins with same name/type as deprecated builtins
* Add some go tests for deprecation status handling
* Move timestamp storage to post-unseal
* Add upgrade-aware deprecation shutdown and tests

changelog/17879.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+plugins: Allow running external plugins which override deprecated builtins.
+```
+```release-note:improvement
+plugins: Let Vault unseal and mount deprecated builtin plugins in a
+deactivated state if this is not the first unseal after an upgrade.
+```

command/auth_enable_test.go

@@ -2,7 +2,6 @@ package command
 
 import (
 	"io/ioutil"
-	"os"
 	"strings"
 	"testing"
 
@@ -53,13 +52,13 @@ func TestAuthEnableCommand_Run(t *testing.T) {
 		{
 			"deprecated builtin with standard mount",
 			[]string{"app-id"},
-			"",
+			"mount entry associated with pending removal builtin",
 			2,
 		},
 		{
 			"deprecated builtin with different mount",
 			[]string{"-path=/tmp", "app-id"},
-			"",
+			"mount entry associated with pending removal builtin",
 			2,
 		},
 	}
@@ -78,12 +77,12 @@ func TestAuthEnableCommand_Run(t *testing.T) {
 
 			code := cmd.Run(tc.args)
 			if code != tc.code {
-				t.Errorf("expected %d to be %d", code, tc.code)
+				t.Errorf("expected command return code to be %d, got %d", tc.code, code)
 			}
 
 			combined := ui.OutputWriter.String() + ui.ErrorWriter.String()
 			if !strings.Contains(combined, tc.out) {
-				t.Errorf("expected %q to contain %q", combined, tc.out)
+				t.Errorf("expected %q in response\n got: %+v", tc.out, combined)
 			}
 		})
 	}
@@ -225,13 +224,6 @@ func TestAuthEnableCommand_Run(t *testing.T) {
 
 		for _, b := range backends {
 			var expectedResult int = 0
-			status, _ := builtinplugins.Registry.DeprecationStatus(b, consts.PluginTypeCredential)
-			allowDeprecated := os.Getenv(consts.VaultAllowPendingRemovalMountsEnv)
-
-			// Need to handle deprecated builtins specially
-			if (status == consts.PendingRemoval && allowDeprecated == "") || status == consts.Removed {
-				expectedResult = 2
-			}
 
 			// Not a builtin
 			if b == "token" {
@@ -244,6 +236,13 @@ func TestAuthEnableCommand_Run(t *testing.T) {
 			actualResult := cmd.Run([]string{
 				b,
 			})
+
+			// Need to handle deprecated builtins specially
+			status, _ := builtinplugins.Registry.DeprecationStatus(b, consts.PluginTypeCredential)
+			if status == consts.PendingRemoval || status == consts.Removed {
+				expectedResult = 2
+			}
+
 			if actualResult != expectedResult {
 				t.Errorf("type: %s - got: %d, expected: %d - %s", b, actualResult, expectedResult, ui.OutputWriter.String()+ui.ErrorWriter.String())
 			}

command/path_map_upgrade_api_test.go

@@ -3,7 +3,6 @@ package command
 import (
 	"testing"
 
-	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/api"
 	vaulthttp "github.com/hashicorp/vault/http"
 	"github.com/hashicorp/vault/sdk/logical"
@@ -17,10 +16,10 @@ func TestPathMap_Upgrade_API(t *testing.T) {
 	coreConfig := &vault.CoreConfig{
 		DisableMlock: true,
 		DisableCache: true,
-		Logger:       log.NewNullLogger(),
 		CredentialBackends: map[string]logical.Factory{
 			"app-id": credAppId.Factory,
 		},
+		PendingRemovalMountsAllowed: true,
 	}
 
 	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{

command/secrets_enable_test.go

@@ -250,13 +250,6 @@ func TestSecretsEnableCommand_Run(t *testing.T) {
 
 		for _, b := range backends {
 			expectedResult := 0
-			status, _ := builtinplugins.Registry.DeprecationStatus(b, consts.PluginTypeSecrets)
-			allowDeprecated := os.Getenv(consts.VaultAllowPendingRemovalMountsEnv)
-
-			// Need to handle deprecated builtins specially
-			if (status == consts.PendingRemoval && allowDeprecated == "") || status == consts.Removed {
-				expectedResult = 2
-			}
 
 			ui, cmd := testSecretsEnableCommand(t)
 			cmd.client = client
@@ -264,6 +257,13 @@ func TestSecretsEnableCommand_Run(t *testing.T) {
 			actualResult := cmd.Run([]string{
 				b,
 			})
+
+			// Need to handle deprecated builtins specially
+			status, _ := builtinplugins.Registry.DeprecationStatus(b, consts.PluginTypeSecrets)
+			if status == consts.PendingRemoval || status == consts.Removed {
+				expectedResult = 2
+			}
+
 			if actualResult != expectedResult {
 				t.Errorf("type: %s - got: %d, expected: %d - %s", b, actualResult, expectedResult, ui.OutputWriter.String()+ui.ErrorWriter.String())
 			}

command/server.go

@@ -1105,16 +1105,6 @@ func (c *ServerCommand) Run(args []string) int {
 		}
 	}
 
-	if allowPendingRemoval := os.Getenv(consts.VaultAllowPendingRemovalMountsEnv); allowPendingRemoval != "" {
-		var err error
-		vault.PendingRemovalMountsAllowed, err = strconv.ParseBool(allowPendingRemoval)
-		if err != nil {
-			c.UI.Warn(wrapAtLength("WARNING! failed to parse " +
-				consts.VaultAllowPendingRemovalMountsEnv + " env var: " +
-				"defaulting to false."))
-		}
-	}
-
 	// If mlockall(2) isn't supported, show a warning. We disable this in dev
 	// because it is quite scary to see when first using Vault. We also disable
 	// this if the user has explicitly disabled mlock in configuration.
@@ -1227,6 +1217,16 @@ func (c *ServerCommand) Run(args []string) int {
 		return enableFourClusterDev(c, &coreConfig, info, infoKeys, c.flagDevListenAddr, os.Getenv("VAULT_DEV_TEMP_DIR"))
 	}
 
+	if allowPendingRemoval := os.Getenv(consts.EnvVaultAllowPendingRemovalMounts); allowPendingRemoval != "" {
+		var err error
+		coreConfig.PendingRemovalMountsAllowed, err = strconv.ParseBool(allowPendingRemoval)
+		if err != nil {
+			c.UI.Warn(wrapAtLength("WARNING! failed to parse " +
+				consts.EnvVaultAllowPendingRemovalMounts + " env var: " +
+				"defaulting to false."))
+		}
+	}
+
 	// Initialize the separate HA storage backend, if it exists
 	disableClustering, err := initHaBackend(c, config, &coreConfig, backend)
 	if err != nil {

sdk/helper/consts/deprecation_status.go

@@ -1,6 +1,9 @@
 package consts
 
-const VaultAllowPendingRemovalMountsEnv = "VAULT_ALLOW_PENDING_REMOVAL_MOUNTS"
+// EnvVaultAllowPendingRemovalMounts allows Pending Removal builtins to be
+// mounted as if they are Deprecated to facilitate migration to supported
+// builtin plugins.
+const EnvVaultAllowPendingRemovalMounts = "VAULT_ALLOW_PENDING_REMOVAL_MOUNTS"
 
 // DeprecationStatus represents the current deprecation state for builtins
 type DeprecationStatus uint32

vault/auth.go

@@ -802,7 +802,7 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 			c.logger.Error("failed to create credential entry", "path", entry.Path, "error", err)
 
 			if c.isMountable(ctx, entry, consts.PluginTypeCredential) {
-				c.logger.Warn("skipping plugin-based credential entry", "path", entry.Path)
+				c.logger.Warn("skipping plugin-based auth entry", "path", entry.Path)
 				goto ROUTER_MOUNT
 			}
 			return errLoadAuthFailed
@@ -820,6 +820,22 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 			}
 		}
 
+		// Do not start up deprecated builtin plugins. If this is a major
+		// upgrade, stop unsealing and shutdown. If we've already mounted this
+		// plugin, skip backend initialization and mount the data for posterity.
+		if versions.IsBuiltinVersion(entry.RunningVersion) {
+			_, err := c.handleDeprecatedMountEntry(ctx, entry, consts.PluginTypeCredential)
+			if c.isMajorVersionFirstMount(ctx) && err != nil {
+				go c.ShutdownCoreError(fmt.Errorf("could not mount %q: %w", entry.Type, err))
+				return errLoadAuthFailed
+			} else if err != nil {
+				c.logger.Error("skipping deprecated auth entry", "name", entry.Type, "path", entry.Path, "error", err)
+				backend.Cleanup(ctx)
+				backend = nil
+				goto ROUTER_MOUNT
+			}
+		}
+
 		{
 			// Check for the correct backend type
 			backendType := backend.Type()
@@ -848,7 +864,7 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 		}
 
 		if c.logger.IsInfo() {
-			c.logger.Info("successfully enabled credential backend", "type", entry.Type, "version", entry.Version, "path", entry.Path, "namespace", entry.Namespace())
+			c.logger.Info("successfully mounted", "type", entry.Type, "version", entry.RunningVersion, "path", entry.Path, "namespace", entry.Namespace())
 		}
 
 		// Ensure the path is tainted if set in the mount table
@@ -882,14 +898,15 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 			// Bind locally
 			localEntry := entry
 			c.postUnsealFuncs = append(c.postUnsealFuncs, func() {
+				postUnsealLogger := c.logger.With("type", localEntry.Type, "version", localEntry.RunningVersion, "path", localEntry.Path)
 				if backend == nil {
-					c.logger.Error("skipping initialization on nil backend", "path", localEntry.Path)
+					postUnsealLogger.Error("skipping initialization for nil auth backend")
 					return
 				}
 
 				err := backend.Initialize(ctx, &logical.InitializationRequest{Storage: view})
 				if err != nil {
-					c.logger.Error("failed to initialize auth entry", "path", localEntry.Path, "error", err)
+					postUnsealLogger.Error("failed to initialize auth backend", "error", err)
 				}
 			})
 		}

vault/core.go

@@ -652,6 +652,8 @@ type Core struct {
 	effectiveSDKVersion string
 
 	rollbackPeriod time.Duration
+
+	pendingRemovalMountsAllowed bool
 }
 
 func (c *Core) HAState() consts.HAState {
@@ -786,6 +788,8 @@ type CoreConfig struct {
 	EffectiveSDKVersion string
 
 	RollbackPeriod time.Duration
+
+	PendingRemovalMountsAllowed bool
 }
 
 // GetServiceRegistration returns the config's ServiceRegistration, or nil if it does
@@ -939,6 +943,7 @@ func CreateCore(conf *CoreConfig) (*Core, error) {
 		disableSSCTokens:               conf.DisableSSCTokens,
 		effectiveSDKVersion:            effectiveSDKVersion,
 		userFailedLoginInfo:            make(map[FailedLoginUser]*FailedLoginInfo),
+		pendingRemovalMountsAllowed:    conf.PendingRemovalMountsAllowed,
 	}
 
 	c.standbyStopCh.Store(make(chan struct{}))
@@ -1209,7 +1214,8 @@ func (c *Core) handleVersionTimeStamps(ctx context.Context) error {
 	if isUpdated {
 		c.logger.Info("Recorded vault version", "vault version", version.Version, "upgrade time", currentTime, "build date", version.BuildDate)
 	}
-	// Finally, populate the version history cache
+
+	// Finally, repopulate the version history cache
 	err = c.loadVersionHistory(ctx)
 	if err != nil {
 		return err
@@ -1234,6 +1240,14 @@ func (c *Core) DisableSSCTokens() bool {
 	return c.disableSSCTokens
 }
 
+// ShutdownCoreError logs a shutdown error and shuts down the Vault core.
+func (c *Core) ShutdownCoreError(err error) {
+	c.Logger().Error("shutting down core", "error", err)
+	if shutdownErr := c.ShutdownWait(); shutdownErr != nil {
+		c.Logger().Error("failed to shutdown core", "error", shutdownErr)
+	}
+}
+
 // Shutdown is invoked when the Vault instance is about to be terminated. It
 // should not be accessible as part of an API call as it will cause an availability
 // problem. It is only used to gracefully quit in the case of HA so that failover
@@ -2176,9 +2190,6 @@ func (s standardUnsealStrategy) unseal(ctx context.Context, logger log.Logger, c
 			return err
 		}
 	}
-	if err := c.handleVersionTimeStamps(ctx); err != nil {
-		return err
-	}
 	if err := c.setupPluginCatalog(ctx); err != nil {
 		return err
 	}
@@ -2282,6 +2293,11 @@ func (s standardUnsealStrategy) unseal(ctx context.Context, logger log.Logger, c
 	c.metricsCh = make(chan struct{})
 	go c.emitMetricsActiveNode(c.metricsCh)
 
+	// Establish version timestamps at the end of unseal on active nodes only.
+	if err := c.handleVersionTimeStamps(ctx); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -2320,6 +2336,12 @@ func (c *Core) postUnseal(ctx context.Context, ctxCancelFunc context.CancelFunc,
 		_ = c.seal.SetRecoveryConfig(ctx, nil)
 	}
 
+	// Load prior un-updated store into version history cache to compare
+	// previous state.
+	if err := c.loadVersionHistory(ctx); err != nil {
+		return err
+	}
+
 	if err := unsealer.unseal(ctx, c.logger, c); err != nil {
 		return err
 	}
@@ -3094,23 +3116,34 @@ func (c *Core) readFeatureFlags(ctx context.Context) (*FeatureFlags, error) {
 // with misconfigured plugins. It should not be possible for existing builtins
 // to be misconfigured, so that is a fatal error.
 func (c *Core) isMountable(ctx context.Context, entry *MountEntry, pluginType consts.PluginType) bool {
+	return !c.isMountEntryBuiltin(ctx, entry, pluginType)
+}
+
+// isMountEntryBuiltin determines whether a mount entry is associated with a
+// builtin of the specified plugin type.
+func (c *Core) isMountEntryBuiltin(ctx context.Context, entry *MountEntry, pluginType consts.PluginType) bool {
 	// Prevent a panic early on
 	if entry == nil || c.pluginCatalog == nil {
 		return false
 	}
 
-	// Handle aliases
-	t := entry.Type
-	if alias, ok := mountAliases[t]; ok {
-		t = alias
+	// Allow type to be determined from mount entry when not otherwise specified
+	if pluginType == consts.PluginTypeUnknown {
+		pluginType = c.builtinTypeFromMountEntry(ctx, entry)
 	}
 
-	plug, err := c.pluginCatalog.Get(ctx, t, pluginType, entry.Version)
-	if err != nil {
+	// Handle aliases
+	pluginName := entry.Type
+	if alias, ok := mountAliases[pluginName]; ok {
+		pluginName = alias
+	}
+
+	plug, err := c.pluginCatalog.Get(ctx, pluginName, pluginType, entry.Version)
+	if err != nil || plug == nil {
 		return false
 	}
 
-	return plug == nil || !plug.Builtin
+	return plug.Builtin
 }
 
 // MatchingMount returns the path of the mount that will be responsible for

vault/external_plugin_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -20,6 +21,7 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/sdk/plugin"
 	"github.com/hashicorp/vault/sdk/plugin/mock"
+	"github.com/hashicorp/vault/version"
 )
 
 const vaultTestingMockPluginEnv = "VAULT_TESTING_MOCK_PLUGIN"
@@ -342,6 +344,127 @@ func TestCore_EnableExternalPlugin_Deregister_SealUnseal(t *testing.T) {
 	}
 }
 
+// TestCore_Unseal_isMajorVersionFirstMount_PendingRemoval_Plugin tests the
+// behavior of deprecated builtins when attempting to unseal Vault after a major
+// version upgrade. It simulates this behavior by instantiating a Vault cluster,
+// registering a shadow plugin to mount a builtin, and deregistering the shadow
+// plugin. The first unseal should work. Before sealing and unsealing again, the
+// version store is cleared.  Vault sees the next unseal as a major upgrade and
+// should immediately shut down.
+func TestCore_Unseal_isMajorVersionFirstMount_PendingRemoval_Plugin(t *testing.T) {
+	pluginDir, cleanup := MakeTestPluginDir(t)
+	t.Cleanup(func() { cleanup(t) })
+
+	// create an external plugin to shadow the builtin "pending-removal-test-plugin"
+	pluginName := "pending-removal-test-plugin"
+	plugin := compilePlugin(t, consts.PluginTypeCredential, "", pluginDir)
+	err := os.Link(path.Join(pluginDir, plugin.fileName), path.Join(pluginDir, pluginName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	conf := &CoreConfig{
+		BuiltinRegistry: NewMockBuiltinRegistry(),
+		PluginDirectory: pluginDir,
+	}
+	c := TestCoreWithSealAndUI(t, conf)
+	c, keys, root := testCoreUnsealed(t, c)
+
+	// Register a shadow plugin
+	registerPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "", plugin.sha256, plugin.fileName)
+	mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+
+	// Deregister shadow plugin
+	deregisterPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "", plugin.sha256, plugin.fileName)
+
+	// Make sure this isn't the first mount for the current major version.
+	if c.isMajorVersionFirstMount(context.Background()) {
+		t.Fatalf("expected major version to register as mounted")
+	}
+
+	if err := c.Seal(root); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	for i, key := range keys {
+		unseal, err := TestCoreUnseal(c, key)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if i+1 == len(keys) && !unseal {
+			t.Fatalf("err: should be unsealed")
+		}
+	}
+
+	// Now remove version history and try again
+	vaultVersionPath := "core/versions/"
+	key := vaultVersionPath + version.Version
+	if err := c.barrier.Delete(context.Background(), key); err != nil {
+		t.Fatal(err)
+	}
+
+	// loadVersionHistory doesn't care about invalidating old entries, since
+	// they shouldn't really be deleted from the version store. It just updates
+	// the map, so we need to manually delete the current entry.
+	delete(c.versionHistory, version.Version)
+
+	// Make sure this appears to be the first mount for the current major
+	// version.
+	if !c.isMajorVersionFirstMount(context.Background()) {
+		t.Fatalf("expected major version first mount")
+	}
+
+	// Seal again and check for unseal failure.
+	if err := c.Seal(root); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	for i, key := range keys {
+		unseal, err := TestCoreUnseal(c, key)
+		if i+1 == len(keys) {
+			if !errors.Is(err, errLoadAuthFailed) {
+				t.Fatalf("expected error: %q, got: %q", errLoadAuthFailed, err)
+			}
+
+			if unseal {
+				t.Fatalf("err: should not be unsealed")
+			}
+		}
+	}
+}
+
+func TestCore_EnableExternalPlugin_PendingRemoval(t *testing.T) {
+	pluginDir, cleanup := MakeTestPluginDir(t)
+	t.Cleanup(func() { cleanup(t) })
+
+	// create an external plugin to shadow the builtin "pending-removal-test-plugin"
+	pluginName := "pending-removal-test-plugin"
+	plugin := compilePlugin(t, consts.PluginTypeCredential, "v1.2.3", pluginDir)
+	err := os.Link(path.Join(pluginDir, plugin.fileName), path.Join(pluginDir, pluginName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	conf := &CoreConfig{
+		BuiltinRegistry: NewMockBuiltinRegistry(),
+		PluginDirectory: pluginDir,
+	}
+
+	c := TestCoreWithSealAndUI(t, conf)
+	c, _, _ = testCoreUnsealed(t, c)
+
+	pendingRemovalString := "pending removal"
+
+	// Create a new auth method with builtin pending-removal-test-plugin
+	resp, err := mountPluginWithResponse(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+	if err == nil {
+		t.Fatalf("expected error when mounting deprecated backend")
+	}
+	if resp == nil || resp.Data == nil || !strings.Contains(resp.Data["error"].(string), pendingRemovalString) {
+		t.Fatalf("expected error response to contain %q but got %+v", pendingRemovalString, resp)
+	}
+
+	// Register a shadow plugin
+	registerPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "v1.2.3", plugin.sha256, plugin.fileName)
+	mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+}
+
 func TestCore_EnableExternalPlugin_ShadowBuiltin(t *testing.T) {
 	pluginDir, cleanup := MakeTestPluginDir(t)
 	t.Cleanup(func() { cleanup(t) })
@@ -880,7 +1003,7 @@ func registerPlugin(t *testing.T, sys *SystemBackend, pluginName, pluginType, ve
 	}
 }
 
-func mountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType consts.PluginType, version, path string) {
+func mountPluginWithResponse(t *testing.T, sys *SystemBackend, pluginName string, pluginType consts.PluginType, version, path string) (*logical.Response, error) {
 	t.Helper()
 	var mountPath string
 	if path == "" {
@@ -897,7 +1020,12 @@ func mountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType
 			"plugin_version": version,
 		}
 	}
-	resp, err := sys.HandleRequest(namespace.RootContext(nil), req)
+	return sys.HandleRequest(namespace.RootContext(nil), req)
+}
+
+func mountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType consts.PluginType, version, path string) {
+	t.Helper()
+	resp, err := mountPluginWithResponse(t, sys, pluginName, pluginType, version, path)
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("err:%v resp:%#v", err, resp)
 	}

vault/logical_system.go

@@ -1187,10 +1187,12 @@ func (b *SystemBackend) handleMount(ctx context.Context, req *logical.Request, d
 		Version:               pluginVersion,
 	}
 
-	// Detect and handle deprecated secrets engines
-	resp, err = b.Core.handleDeprecatedMountEntry(ctx, me, consts.PluginTypeSecrets)
-	if err != nil {
-		return handleError(err)
+	if b.Core.isMountEntryBuiltin(ctx, me, consts.PluginTypeSecrets) {
+		resp, err = b.Core.handleDeprecatedMountEntry(ctx, me, consts.PluginTypeSecrets)
+		if err != nil {
+			b.Core.logger.Error("could not mount builtin", "name", me.Type, "path", me.Path, "error", err)
+			return handleError(fmt.Errorf("could not mount %q: %w", me.Type, err))
+		}
 	}
 
 	// Attempt mount
@@ -2616,9 +2618,13 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque
 		Version:               pluginVersion,
 	}
 
-	resp, err := b.Core.handleDeprecatedMountEntry(ctx, me, consts.PluginTypeCredential)
-	if err != nil {
-		return handleError(err)
+	var resp *logical.Response
+	if b.Core.isMountEntryBuiltin(ctx, me, consts.PluginTypeCredential) {
+		resp, err = b.Core.handleDeprecatedMountEntry(ctx, me, consts.PluginTypeCredential)
+		if err != nil {
+			b.Core.logger.Error("could not mount builtin", "name", me.Type, "path", me.Path, "error", err)
+			return handleError(fmt.Errorf("could not mount %q: %w", me.Type, err))
+		}
 	}
 
 	// Attempt enabling

vault/mount.go

@@ -72,6 +72,13 @@ const (
 	MountTableNoUpdateStorage = false
 )
 
+// DeprecationStatus errors
+var (
+	errMountDeprecated     = errors.New("mount entry associated with deprecated builtin")
+	errMountPendingRemoval = errors.New("mount entry associated with pending removal builtin")
+	errMountRemoved        = errors.New("mount entry associated with removed builtin")
+)
+
 var (
 	// loadMountsFailed if loadMounts encounters an error
 	errLoadMountsFailed = errors.New("failed to setup mount table")
@@ -104,8 +111,6 @@ var (
 	// mountAliases maps old backend names to new backend names, allowing us
 	// to move/rename backends but maintain backwards compatibility
 	mountAliases = map[string]string{"generic": "kv"}
-
-	PendingRemovalMountsAllowed = false
 )
 
 func (c *Core) generateMountAccessor(entryType string) (string, error) {
@@ -503,12 +508,6 @@ func (c *Core) decodeMountTable(ctx context.Context, raw []byte) (*MountTable, e
 			continue
 		}
 
-		// Immediately shutdown the core if deprecated mounts are detected and VAULT_ALLOW_PENDING_REMOVAL_MOUNTS is unset
-		if _, err := c.handleDeprecatedMountEntry(ctx, entry, consts.PluginTypeUnknown); err != nil {
-			c.logger.Error("shutting down core", "error", err)
-			c.Shutdown()
-		}
-
 		entry.namespace = ns
 		mountEntries = append(mountEntries, entry)
 	}
@@ -964,14 +963,12 @@ func (c *Core) taintMountEntry(ctx context.Context, nsID, mountPath string, upda
 }
 
 // handleDeprecatedMountEntry handles the Deprecation Status of the specified
-// mount entry's builtin engine as follows:
-//
-// * Supported - do nothing
-// * Deprecated - log a warning about builtin deprecation
-// * PendingRemoval - log an error about builtin deprecation and return an error
-// if VAULT_ALLOW_PENDING_REMOVAL_MOUNTS is unset
-// * Removed - log an error about builtin deprecation and return an error
+// mount entry's builtin engine. Warnings are appended to the returned response
+// and logged. Errors are returned with a nil response to be processed by the
+// caller.
 func (c *Core) handleDeprecatedMountEntry(ctx context.Context, entry *MountEntry, pluginType consts.PluginType) (*logical.Response, error) {
+	resp := &logical.Response{}
+
 	if c.builtinRegistry == nil || entry == nil {
 		return nil, nil
 	}
@@ -989,28 +986,22 @@ func (c *Core) handleDeprecatedMountEntry(ctx context.Context, entry *MountEntry
 
 	status, ok := c.builtinRegistry.DeprecationStatus(t, pluginType)
 	if ok {
-		resp := &logical.Response{}
-		// Deprecation sublogger with some identifying information
-		dl := c.logger.With("name", t, "type", pluginType, "status", status, "path", entry.Path)
-		errDeprecatedMount := fmt.Errorf("mount entry associated with %s builtin", status)
-
 		switch status {
 		case consts.Deprecated:
-			dl.Warn(errDeprecatedMount.Error())
-			resp.AddWarning(errDeprecatedMount.Error())
+			c.logger.Warn("mounting deprecated builtin", "name", t, "type", pluginType, "path", entry.Path)
+			resp.AddWarning(errMountDeprecated.Error())
 			return resp, nil
 
 		case consts.PendingRemoval:
-			dl.Error(errDeprecatedMount.Error())
-			if !PendingRemovalMountsAllowed {
-				return nil, fmt.Errorf("could not mount %q: %w", t, errDeprecatedMount)
+			if c.pendingRemovalMountsAllowed {
+				c.Logger().Info("mount allowed by environment variable", "env", consts.EnvVaultAllowPendingRemovalMounts)
+				resp.AddWarning(errMountPendingRemoval.Error())
+				return resp, nil
 			}
-			resp.AddWarning(errDeprecatedMount.Error())
-			c.Logger().Info("mount allowed by environment variable", "env", consts.VaultAllowPendingRemovalMountsEnv)
-			return resp, nil
+			return nil, errMountPendingRemoval
 
 		case consts.Removed:
-			return nil, fmt.Errorf("could not mount %s: %w", t, errDeprecatedMount)
+			return nil, errMountRemoved
 		}
 	}
 	return nil, nil
@@ -1487,6 +1478,22 @@ func (c *Core) setupMounts(ctx context.Context) error {
 			}
 		}
 
+		// Do not start up deprecated builtin plugins. If this is a major
+		// upgrade, stop unsealing and shutdown. If we've already mounted this
+		// plugin, proceed with unsealing and skip backend initialization.
+		if versions.IsBuiltinVersion(entry.RunningVersion) {
+			_, err := c.handleDeprecatedMountEntry(ctx, entry, consts.PluginTypeSecrets)
+			if c.isMajorVersionFirstMount(ctx) && err != nil {
+				go c.ShutdownCoreError(fmt.Errorf("could not mount %q: %w", entry.Type, err))
+				return errLoadMountsFailed
+			} else if err != nil {
+				c.logger.Error("skipping deprecated mount entry", "name", entry.Type, "path", entry.Path, "error", err)
+				backend.Cleanup(ctx)
+				backend = nil
+				goto ROUTER_MOUNT
+			}
+		}
+
 		{
 			// Check for the correct backend type
 			backendType := backend.Type()
@@ -1523,20 +1530,21 @@ func (c *Core) setupMounts(ctx context.Context) error {
 			// Bind locally
 			localEntry := entry
 			c.postUnsealFuncs = append(c.postUnsealFuncs, func() {
+				postUnsealLogger := c.logger.With("type", localEntry.Type, "version", localEntry.RunningVersion, "path", localEntry.Path)
 				if backend == nil {
-					c.logger.Error("skipping initialization on nil backend", "path", localEntry.Path)
+					postUnsealLogger.Error("skipping initialization for nil backend", "path", localEntry.Path)
 					return
 				}
 
 				err := backend.Initialize(ctx, &logical.InitializationRequest{Storage: view})
 				if err != nil {
-					c.logger.Error("failed to initialize mount entry", "path", localEntry.Path, "error", err)
+					postUnsealLogger.Error("failed to initialize mount backend", "error", err)
 				}
 			})
 		}
 
 		if c.logger.IsInfo() {
-			c.logger.Info("successfully mounted backend", "type", entry.Type, "version", entry.Version, "path", entry.Path)
+			c.logger.Info("successfully mounted", "type", entry.Type, "version", entry.RunningVersion, "path", entry.Path, "namespace", entry.Namespace())
 		}
 
 		// Ensure the path is tainted if set in the mount table

vault/testing.go

@@ -1713,6 +1713,8 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te
 
 		coreConfig.RollbackPeriod = base.RollbackPeriod
 
+		coreConfig.PendingRemovalMountsAllowed = base.PendingRemovalMountsAllowed
+
 		testApplyEntBaseConfig(coreConfig, base)
 	}
 	if coreConfig.ClusterName == "" {
@@ -2310,31 +2312,41 @@ func toFunc(f logical.Factory) func() (interface{}, error) {
 
 func NewMockBuiltinRegistry() *mockBuiltinRegistry {
 	return &mockBuiltinRegistry{
-		forTesting: map[string]consts.PluginType{
-			"mysql-database-plugin":      consts.PluginTypeDatabase,
-			"postgresql-database-plugin": consts.PluginTypeDatabase,
-			"approle":                    consts.PluginTypeCredential,
-			"aws":                        consts.PluginTypeCredential,
-			"consul":                     consts.PluginTypeSecrets,
+		forTesting: map[string]mockBackend{
+			"mysql-database-plugin":      {PluginType: consts.PluginTypeDatabase},
+			"postgresql-database-plugin": {PluginType: consts.PluginTypeDatabase},
+			"approle":                    {PluginType: consts.PluginTypeCredential},
+			"pending-removal-test-plugin": {
+				PluginType:        consts.PluginTypeCredential,
+				DeprecationStatus: consts.PendingRemoval,
+			},
+			"aws":    {PluginType: consts.PluginTypeCredential},
+			"consul": {PluginType: consts.PluginTypeSecrets},
 		},
 	}
 }
 
+type mockBackend struct {
+	consts.PluginType
+	consts.DeprecationStatus
+}
+
 type mockBuiltinRegistry struct {
-	forTesting map[string]consts.PluginType
+	forTesting map[string]mockBackend
 }
 
 func (m *mockBuiltinRegistry) Get(name string, pluginType consts.PluginType) (func() (interface{}, error), bool) {
-	testPluginType, ok := m.forTesting[name]
+	testBackend, ok := m.forTesting[name]
 	if !ok {
 		return nil, false
 	}
+	testPluginType := testBackend.PluginType
 	if pluginType != testPluginType {
 		return nil, false
 	}
 
 	switch name {
-	case "approle":
+	case "approle", "pending-removal-test-plugin":
 		return toFunc(approle.Factory), true
 	case "aws":
 		return toFunc(func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) {
@@ -2394,6 +2406,7 @@ func (m *mockBuiltinRegistry) Keys(pluginType consts.PluginType) []string {
 		}
 	case consts.PluginTypeCredential:
 		return []string{
+			"pending-removal-test-plugin",
 			"approle",
 		}
 	}
@@ -2411,7 +2424,7 @@ func (m *mockBuiltinRegistry) Contains(name string, pluginType consts.PluginType
 
 func (m *mockBuiltinRegistry) DeprecationStatus(name string, pluginType consts.PluginType) (consts.DeprecationStatus, bool) {
 	if m.Contains(name, pluginType) {
-		return consts.Supported, true
+		return m.forTesting[name].DeprecationStatus, true
 	}
 
 	return consts.Unknown, false

vault/version_store.go

@@ -7,8 +7,10 @@ import (
 	"strings"
 	"time"
 
+	semver "github.com/hashicorp/go-version"
 	"github.com/hashicorp/vault/sdk/helper/consts"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/hashicorp/vault/version"
 )
 
 const (
@@ -149,6 +151,34 @@ func (c *Core) loadVersionHistory(ctx context.Context) error {
 	return nil
 }
 
+// isMajorVersionFirstMount checks the current running version of Vault against
+// the newest version in the version store to see if this major version has
+// already been mounted.
+func (c *Core) isMajorVersionFirstMount(ctx context.Context) bool {
+	// Grab the most recent previous version from the version store
+	prevMounted, _, err := c.FindNewestVersionTimestamp()
+	if err != nil {
+		return true
+	}
+
+	// Get versions into comparable form. Errors should be treated as the first
+	// unseal with this major version.
+	prev, err := semver.NewSemver(prevMounted)
+	if err != nil {
+		return true
+	}
+	curr, err := semver.NewSemver(version.Version)
+	if err != nil {
+		return true
+	}
+
+	// Check for milestone or major version upgrade
+	isMilestoneUpdate := curr.Segments()[0] > prev.Segments()[0]
+	isMajorVersionUpdate := curr.Segments()[1] > prev.Segments()[1]
+
+	return isMilestoneUpdate || isMajorVersionUpdate
+}
+
 func IsJWT(token string) bool {
 	return len(token) > 3 && strings.Count(token, ".") == 2 &&
 		(token[3] != '.' && token[1] != '.')