From ea41e62e83393d2a2006d7b7f1bb6fed6acabf21 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Fri, 2 Dec 2022 13:16:31 -0500 Subject: [PATCH] plugins: Mount missing plugin entries and skip loading (#18189) * Skip plugin startup for missing plugins * Skip secrets startup for missing plugins * Add changelog for bugfix * Make plugin handling on unseal version-aware * Update plugin lazy-load logic/comments for readability * Add register/mount/deregister/seal/unseal go test * Consolidate lazy mount logic to prevent inconsistencies Co-authored-by: Tom Proctor --- changelog/18189.txt | 3 ++ vault/auth.go | 7 ++-- vault/core.go | 26 ++++++++++++++ vault/external_plugin_test.go | 65 +++++++++++++++++++++++++++++++++++ vault/mount.go | 6 ++-- 5 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 changelog/18189.txt diff --git a/changelog/18189.txt b/changelog/18189.txt new file mode 100644 index 000000000..503340480 --- /dev/null +++ b/changelog/18189.txt @@ -0,0 +1,3 @@ +```release-note:bug +plugins: Skip loading but still mount data associated with missing plugins on unseal. +``` diff --git a/vault/auth.go b/vault/auth.go index a9c4fd3a3..08e81e1ae 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -800,11 +800,8 @@ func (c *Core) setupCredentials(ctx context.Context) error { backend, entry.RunningSha256, err = c.newCredentialBackend(ctx, entry, sysView, view) if err != nil { c.logger.Error("failed to create credential entry", "path", entry.Path, "error", err) - plug, plugerr := c.pluginCatalog.Get(ctx, entry.Type, consts.PluginTypeCredential, "") - if plugerr == nil && plug != nil && !plug.Builtin { - // If we encounter an error instantiating the backend due to an error, - // skip backend initialization but register the entry to the mount table - // to preserve storage and path. + + if c.isMountable(ctx, entry, consts.PluginTypeCredential) { c.logger.Warn("skipping plugin-based credential entry", "path", entry.Path) goto ROUTER_MOUNT } diff --git a/vault/core.go b/vault/core.go index 69ba5837a..3fb008bc7 100644 --- a/vault/core.go +++ b/vault/core.go @@ -3042,6 +3042,32 @@ func (c *Core) readFeatureFlags(ctx context.Context) (*FeatureFlags, error) { return &flags, nil } +// isMountable tells us whether or not we can continue mounting a plugin-based +// mount entry after failing to instantiate a backend. We do this to preserve +// the storage and path when a plugin is missing or has otherwise been +// misconfigured. This allows users to recover from errors when starting Vault +// 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 { + // 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 + } + + plug, err := c.pluginCatalog.Get(ctx, t, pluginType, entry.Version) + if err != nil { + return false + } + + return plug == nil || !plug.Builtin +} + // MatchingMount returns the path of the mount that will be responsible for // handling the given request path. func (c *Core) MatchingMount(ctx context.Context, reqPath string) string { diff --git a/vault/external_plugin_test.go b/vault/external_plugin_test.go index 0f59a682d..9876856ff 100644 --- a/vault/external_plugin_test.go +++ b/vault/external_plugin_test.go @@ -277,6 +277,71 @@ func TestCore_EnableExternalPlugin_MultipleVersions(t *testing.T) { } } +func TestCore_EnableExternalPlugin_Deregister_SealUnseal(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 := "therug" + 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 plugin + registerPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "", plugin.sha256, plugin.fileName) + mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "") + plugct := len(c.pluginCatalog.externalPlugins) + if plugct != 1 { + t.Fatalf("expected a single external plugin entry after registering, got: %d", plugct) + } + + // Now pull the rug out from underneath us + deregisterPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "", "", "") + + 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") + } + } + + plugct = len(c.pluginCatalog.externalPlugins) + if plugct != 0 { + t.Fatalf("expected no plugin entries after unseal, got: %d", plugct) + } + + found := false + mounts, err := c.ListAuths() + if err != nil { + t.Fatal(err) + } + for _, mount := range mounts { + if mount.Type == pluginName { + found = true + break + } + } + + if !found { + t.Fatalf("expected to find %s mount, but got none", pluginName) + } +} + func TestCore_EnableExternalPlugin_ShadowBuiltin(t *testing.T) { pluginDir, cleanup := MakeTestPluginDir(t) t.Cleanup(func() { cleanup(t) }) diff --git a/vault/mount.go b/vault/mount.go index ca786465e..a70e7a942 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -1467,10 +1467,8 @@ func (c *Core) setupMounts(ctx context.Context) error { backend, entry.RunningSha256, err = c.newLogicalBackend(ctx, entry, sysView, view) if err != nil { c.logger.Error("failed to create mount entry", "path", entry.Path, "error", err) - if !c.builtinRegistry.Contains(entry.Type, consts.PluginTypeSecrets) { - // If we encounter an error instantiating the backend due to an error, - // skip backend initialization but register the entry to the mount table - // to preserve storage and path. + + if c.isMountable(ctx, entry, consts.PluginTypeSecrets) { c.logger.Warn("skipping plugin-based mount entry", "path", entry.Path) goto ROUTER_MOUNT }