From cb3406b1ebac978217d91285cccb53e717594498 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Wed, 14 Dec 2022 13:06:33 -0500 Subject: [PATCH] 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 | 7 ++ command/auth_enable_test.go | 23 ++--- command/path_map_upgrade_api_test.go | 3 +- command/secrets_enable_test.go | 14 +-- command/server.go | 20 ++-- sdk/helper/consts/deprecation_status.go | 5 +- vault/auth.go | 25 ++++- vault/core.go | 55 ++++++++-- vault/external_plugin_test.go | 132 +++++++++++++++++++++++- vault/logical_system.go | 20 ++-- vault/mount.go | 72 +++++++------ vault/testing.go | 33 ++++-- vault/version_store.go | 30 ++++++ 13 files changed, 341 insertions(+), 98 deletions(-) create mode 100644 changelog/17879.txt diff --git a/changelog/17879.txt b/changelog/17879.txt new file mode 100644 index 000000000..efb71b898 --- /dev/null +++ b/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. +``` diff --git a/command/auth_enable_test.go b/command/auth_enable_test.go index 0ba9621e3..211c1da7d 100644 --- a/command/auth_enable_test.go +++ b/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()) } diff --git a/command/path_map_upgrade_api_test.go b/command/path_map_upgrade_api_test.go index 57c1f7734..b9e573b27 100644 --- a/command/path_map_upgrade_api_test.go +++ b/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{ diff --git a/command/secrets_enable_test.go b/command/secrets_enable_test.go index d0f41cb08..127e54a6a 100644 --- a/command/secrets_enable_test.go +++ b/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()) } diff --git a/command/server.go b/command/server.go index df63cd1d6..6612b7e21 100644 --- a/command/server.go +++ b/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 { diff --git a/sdk/helper/consts/deprecation_status.go b/sdk/helper/consts/deprecation_status.go index 5591924a7..656d6cc99 100644 --- a/sdk/helper/consts/deprecation_status.go +++ b/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 diff --git a/vault/auth.go b/vault/auth.go index 08e81e1ae..9115196d9 100644 --- a/vault/auth.go +++ b/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) } }) } diff --git a/vault/core.go b/vault/core.go index 32299302f..b4a0200b7 100644 --- a/vault/core.go +++ b/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 diff --git a/vault/external_plugin_test.go b/vault/external_plugin_test.go index 9876856ff..a1e8b5316 100644 --- a/vault/external_plugin_test.go +++ b/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) } diff --git a/vault/logical_system.go b/vault/logical_system.go index 5c467b1da..742dc1fdc 100644 --- a/vault/logical_system.go +++ b/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 diff --git a/vault/mount.go b/vault/mount.go index a70e7a942..7b7d5087b 100644 --- a/vault/mount.go +++ b/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 diff --git a/vault/testing.go b/vault/testing.go index caede6a41..47103e350 100644 --- a/vault/testing.go +++ b/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 diff --git a/vault/version_store.go b/vault/version_store.go index 5d78b0b53..1eb7e1e1a 100644 --- a/vault/version_store.go +++ b/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] != '.')