From 773f0d58adc255cddd8ca196478e21ea5cf40e04 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Fri, 11 Nov 2022 14:51:37 -0500 Subject: [PATCH] plugins: Filter builtins by RunningVersion (#17816) This commit adds some logic to handle the case where a mount entry has a non-builtin RunningVersion. This ensures that we only report deprecation status for builtins. --- changelog/17816.txt | 3 + vault/external_plugin_test.go | 116 ++++++++++++++++++++++++++++++++++ vault/mount.go | 7 +- 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 changelog/17816.txt diff --git a/changelog/17816.txt b/changelog/17816.txt new file mode 100644 index 000000000..4ad3bc862 --- /dev/null +++ b/changelog/17816.txt @@ -0,0 +1,3 @@ +```release-note:bug +plugins: Only report deprecation status for builtin plugins. +``` diff --git a/vault/external_plugin_test.go b/vault/external_plugin_test.go index 0dd4ec5ed..cb4761091 100644 --- a/vault/external_plugin_test.go +++ b/vault/external_plugin_test.go @@ -277,6 +277,85 @@ func TestCore_EnableExternalPlugin_MultipleVersions(t *testing.T) { } } +func TestCore_EnableExternalPlugin_ShadowBuiltin(t *testing.T) { + pluginDir, cleanup := MakeTestPluginDir(t) + t.Cleanup(func() { cleanup(t) }) + + // create an external plugin to shadow the builtin "approle" + plugin := compilePlugin(t, consts.PluginTypeCredential, "v1.2.3", pluginDir) + err := os.Link(path.Join(pluginDir, plugin.fileName), path.Join(pluginDir, "approle")) + if err != nil { + t.Fatal(err) + } + pluginName := "approle" + conf := &CoreConfig{ + BuiltinRegistry: NewMockBuiltinRegistry(), + PluginDirectory: pluginDir, + } + c := TestCoreWithSealAndUI(t, conf) + c, _, _ = testCoreUnsealed(t, c) + + verifyAuthListDeprecationStatus := func(authName string, checkExists bool) error { + req := logical.TestRequest(t, logical.ReadOperation, mountTable(consts.PluginTypeCredential)) + req.Data = map[string]interface{}{ + "type": authName, + } + resp, err := c.systemBackend.HandleRequest(namespace.RootContext(nil), req) + if err != nil { + return err + } + status := resp.Data["deprecation_status"] + if checkExists && status == nil { + return fmt.Errorf("expected deprecation status but found none") + } else if !checkExists && status != nil { + return fmt.Errorf("expected nil deprecation status but found %q", status) + } + return nil + } + + // Create a new auth method with builtin approle + mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "") + + // Read the auth table to verify deprecation status + if err := verifyAuthListDeprecationStatus(pluginName, true); err != nil { + t.Fatal(err) + } + + // Register a shadow plugin + registerPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "v1.2.3", plugin.sha256, plugin.fileName) + + // Verify auth table hasn't changed + if err := verifyAuthListDeprecationStatus(pluginName, true); err != nil { + t.Fatal(err) + } + + // Remount auth method using registered shadow plugin + unmountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "") + mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "") + + // Verify auth table has changed + if err := verifyAuthListDeprecationStatus(pluginName, false); err != nil { + t.Fatal(err) + } + + // Deregister shadow plugin + deregisterPlugin(t, c.systemBackend, pluginName, consts.PluginTypeSecrets.String(), "v1.2.3", plugin.sha256, plugin.fileName) + + // Verify auth table hasn't changed + if err := verifyAuthListDeprecationStatus(pluginName, false); err != nil { + t.Fatal(err) + } + + // Remount auth method + unmountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "") + mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "") + + // Verify auth table has changed + if err := verifyAuthListDeprecationStatus(pluginName, false); err != nil { + t.Fatal(err) + } +} + func TestCore_EnableExternalKv_MultipleVersions(t *testing.T) { pluginDir, cleanup := MakeTestPluginDir(t) t.Cleanup(func() { cleanup(t) }) @@ -759,6 +838,43 @@ func mountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType } } +func unmountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType consts.PluginType, version, path string) { + t.Helper() + var mountPath string + if path == "" { + mountPath = mountTable(pluginType) + } else { + mountPath = mountTableWithPath(consts.PluginTypeSecrets, path) + } + req := logical.TestRequest(t, logical.DeleteOperation, mountPath) + req.Data = map[string]interface{}{ + "type": pluginName, + } + if version != "" { + req.Data["config"] = map[string]interface{}{ + "plugin_version": version, + } + } + resp, err := sys.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } +} + +func deregisterPlugin(t *testing.T, sys *SystemBackend, pluginName, pluginType, version, sha, command string) { + t.Helper() + req := logical.TestRequest(t, logical.DeleteOperation, fmt.Sprintf("plugins/catalog/%s/%s", pluginType, pluginName)) + req.Data = map[string]interface{}{ + "command": command, + "sha256": sha, + "version": version, + } + resp, err := sys.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } +} + func mountTable(pluginType consts.PluginType) string { return mountTableWithPath(pluginType, "foo") } diff --git a/vault/mount.go b/vault/mount.go index b3b76396a..a2b5bc521 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -717,8 +717,13 @@ func (c *Core) builtinTypeFromMountEntry(ctx context.Context, entry *MountEntry) return consts.PluginTypeUnknown } + // Builtin plugins should contain the "builtin" string in their RunningVersion + if !strings.Contains(entry.RunningVersion, "builtin") { + return consts.PluginTypeUnknown + } + builtinPluginType := func(name string, pluginType consts.PluginType) (consts.PluginType, bool) { - plugin, err := c.pluginCatalog.Get(ctx, name, pluginType, "") + plugin, err := c.pluginCatalog.Get(ctx, name, pluginType, entry.RunningVersion) if err == nil && plugin != nil && plugin.Builtin { return plugin.Type, true }