From cb5d31995c59ce93e6940be2028792cef568df90 Mon Sep 17 00:00:00 2001
From: Theron Voran <tvoran@users.noreply.github.com>
Date: Fri, 7 Oct 2022 13:59:10 -0700
Subject: [PATCH] backport of #17340 (#17469)

Plugins: Fix file permissions check to always use the correct path

* Add failing test for when command != plugin name
* wrapFactoryCheckPerms uses pluginCatalog.Get to fetch the correct command
* Use filepath.Rel for consistency with plugin read API handler
---
 changelog/17340.txt           |  3 ++
 vault/external_plugin_test.go | 76 ++++++++++++++++++++++++++++++++++-
 vault/plugin_catalog.go       | 28 ++++++++++++-
 3 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 changelog/17340.txt

diff --git a/changelog/17340.txt b/changelog/17340.txt
new file mode 100644
index 000000000000..733c737a0a2a
--- /dev/null
+++ b/changelog/17340.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+plugins: Corrected the path to check permissions on when the registered plugin name does not match the plugin binary's filename.
+```
diff --git a/vault/external_plugin_test.go b/vault/external_plugin_test.go
index 99976ce772f4..0dd4ec5ed2f3 100644
--- a/vault/external_plugin_test.go
+++ b/vault/external_plugin_test.go
@@ -539,6 +539,80 @@ func TestExternalPlugin_getBackendTypeVersion(t *testing.T) {
 	}
 }
 
+func TestExternalPlugin_CheckFilePermissions(t *testing.T) {
+	// Turn on the check.
+	if err := os.Setenv(consts.VaultEnableFilePermissionsCheckEnv, "true"); err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := os.Unsetenv(consts.VaultEnableFilePermissionsCheckEnv); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	for name, tc := range map[string]struct {
+		pluginNameFmt string
+		pluginType    consts.PluginType
+		pluginVersion string
+	}{
+		"plugin name and file name match": {
+			pluginNameFmt: "%s",
+			pluginType:    consts.PluginTypeCredential,
+		},
+		"plugin name and file name mismatch": {
+			pluginNameFmt: "%s-foo",
+			pluginType:    consts.PluginTypeSecrets,
+		},
+		"plugin name has slash": {
+			pluginNameFmt: "%s/foo",
+			pluginType:    consts.PluginTypeCredential,
+		},
+		"plugin with version": {
+			pluginNameFmt: "%s/foo",
+			pluginType:    consts.PluginTypeCredential,
+			pluginVersion: "v1.2.3",
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			c, plugins := testCoreWithPlugins(t, tc.pluginType, tc.pluginVersion)
+			registeredPluginName := fmt.Sprintf(tc.pluginNameFmt, plugins[0].name)
+
+			// Permissions will be checked once during registration.
+			req := logical.TestRequest(t, logical.UpdateOperation, fmt.Sprintf("plugins/catalog/%s/%s", tc.pluginType.String(), registeredPluginName))
+			req.Data = map[string]interface{}{
+				"command": plugins[0].fileName,
+				"sha256":  plugins[0].sha256,
+				"version": tc.pluginVersion,
+			}
+			resp, err := c.systemBackend.HandleRequest(namespace.RootContext(nil), req)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if resp.Error() != nil {
+				t.Fatal(resp.Error())
+			}
+
+			// Now attempt to mount the plugin, which should trigger checking the permissions again.
+			req = logical.TestRequest(t, logical.UpdateOperation, mountTable(tc.pluginType))
+			req.Data = map[string]interface{}{
+				"type": registeredPluginName,
+			}
+			if tc.pluginVersion != "" {
+				req.Data["config"] = map[string]interface{}{
+					"plugin_version": tc.pluginVersion,
+				}
+			}
+			resp, err = c.systemBackend.HandleRequest(namespace.RootContext(nil), req)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if resp.Error() != nil {
+				t.Fatal(resp.Error())
+			}
+		})
+	}
+}
+
 func TestExternalPlugin_DifferentVersionsAndArgs_AreNotMultiplexed(t *testing.T) {
 	env := []string{fmt.Sprintf("%s=yes", vaultTestingMockPluginEnv)}
 	core, _, _ := TestCoreUnsealed(t)
@@ -696,6 +770,6 @@ func mountTableWithPath(pluginType consts.PluginType, path string) string {
 	case consts.PluginTypeSecrets:
 		return "mounts/" + path
 	default:
-		panic("test does not support plugin type yet")
+		panic("test does not support mounting plugin type yet: " + pluginType.String())
 	}
 }
diff --git a/vault/plugin_catalog.go b/vault/plugin_catalog.go
index ee5f92f8313e..288e9fe56d2a 100644
--- a/vault/plugin_catalog.go
+++ b/vault/plugin_catalog.go
@@ -128,7 +128,33 @@ type pluginClient struct {
 
 func wrapFactoryCheckPerms(core *Core, f logical.Factory) logical.Factory {
 	return func(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) {
-		if err := core.CheckPluginPerms(conf.Config["plugin_name"]); err != nil {
+		pluginName := conf.Config["plugin_name"]
+		pluginVersion := conf.Config["plugin_version"]
+		pluginTypeRaw := conf.Config["plugin_type"]
+		pluginType, err := consts.ParsePluginType(pluginTypeRaw)
+		if err != nil {
+			return nil, err
+		}
+
+		pluginDescription := fmt.Sprintf("%s plugin %s", pluginTypeRaw, pluginName)
+		if pluginVersion != "" {
+			pluginDescription += " version " + pluginVersion
+		}
+
+		plugin, err := core.pluginCatalog.Get(ctx, pluginName, pluginType, pluginVersion)
+		if err != nil {
+			return nil, fmt.Errorf("failed to find %s in plugin catalog: %w", pluginDescription, err)
+		}
+		if plugin == nil {
+			return nil, fmt.Errorf("failed to find %s in plugin catalog", pluginDescription)
+		}
+
+		command, err := filepath.Rel(core.pluginCatalog.directory, plugin.Command)
+		if err != nil {
+			return nil, fmt.Errorf("failed to compute plugin command: %w", err)
+		}
+
+		if err := core.CheckPluginPerms(command); err != nil {
 			return nil, err
 		}
 		return f(ctx, conf)