Skip to content

Commit

Permalink
backport of #17340 (#17469)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tvoran committed Oct 7, 2022
1 parent eb5f980 commit cb5d319
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 2 deletions.
3 changes: 3 additions & 0 deletions changelog/17340.txt
Original file line number Diff line number Diff line change
@@ -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.
```
76 changes: 75 additions & 1 deletion vault/external_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
}
}
28 changes: 27 additions & 1 deletion vault/plugin_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit cb5d319

Please sign in to comment.