Plugins: Fix file permissions check to always use the correct path (#17340)

* 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

@@ -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.
+```

vault/external_plugin_test.go

@@ -509,11 +509,84 @@ 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, pluginName, pluginSHA256 := testCoreWithPlugin(t, tc.pluginType, tc.pluginVersion)
+			registeredPluginName := fmt.Sprintf(tc.pluginNameFmt, pluginName)
+
+			// 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": pluginName,
+				"sha256":  pluginSHA256,
+				"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 registerPlugin(t *testing.T, sys *SystemBackend, pluginName, pluginType, version, sha string) {
 	t.Helper()
 	req := logical.TestRequest(t, logical.UpdateOperation, fmt.Sprintf("plugins/catalog/%s/%s", pluginType, pluginName))
 	req.Data = map[string]interface{}{
-		"name":    pluginName,
 		"command": pluginName,
 		"sha256":  sha,
 		"version": version,
@@ -523,7 +596,7 @@ func registerPlugin(t *testing.T, sys *SystemBackend, pluginName, pluginType, ve
 		t.Fatal(err)
 	}
 	if resp.Error() != nil {
-		t.Fatalf("%#v", resp)
+		t.Fatal(resp.Error())
 	}
 }
 
@@ -543,7 +616,7 @@ func mountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType
 		t.Fatal(err)
 	}
 	if resp.Error() != nil {
-		t.Fatalf("%#v", resp)
+		t.Fatal(resp.Error())
 	}
 }
 
@@ -554,6 +627,6 @@ func mountTable(pluginType consts.PluginType) string {
 	case consts.PluginTypeSecrets:
 		return "mounts/foo"
 	default:
-		panic("test does not support plugin type yet")
+		panic("test does not support mounting plugin type yet: " + pluginType.String())
 	}
 }

vault/plugin_catalog.go

@@ -89,7 +89,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)