From 6e111d92fea827747a44e209a6a557eb66d414cd Mon Sep 17 00:00:00 2001 From: Tom Proctor Date: Tue, 30 Jan 2024 13:10:23 +0000 Subject: [PATCH] Support setting plugin TMPDIR in config as well as env (#24978) --- changelog/24978.txt | 3 + command/commands.go | 3 + command/server.go | 5 ++ command/server/config.go | 7 +++ command/server/config_test_helpers.go | 4 ++ command/server/test-fixtures/config.hcl | 4 +- http/sys_config_state_test.go | 1 + sdk/helper/pluginutil/run_config.go | 4 +- sdk/helper/pluginutil/runner.go | 1 + vault/core.go | 20 ++++++- vault/external_plugin_container_test.go | 36 +++++++++-- vault/plugincatalog/plugin_catalog.go | 60 ++++++++++++------- vault/plugincatalog/plugin_catalog_test.go | 14 +++-- vault/plugincatalog/plugin_runtime_catalog.go | 4 +- vault/testing.go | 1 + website/content/docs/configuration/index.mdx | 10 ++++ 16 files changed, 139 insertions(+), 38 deletions(-) create mode 100644 changelog/24978.txt diff --git a/changelog/24978.txt b/changelog/24978.txt new file mode 100644 index 0000000000..8bad557e31 --- /dev/null +++ b/changelog/24978.txt @@ -0,0 +1,3 @@ +```release-note:improvement +core: Added new `plugin_tmpdir` config option for containerized plugins, in addition to the existing `VAULT_PLUGIN_TMPDIR` environment variable. +``` diff --git a/command/commands.go b/command/commands.go index a055b15f4b..2d9047d55d 100644 --- a/command/commands.go +++ b/command/commands.go @@ -97,6 +97,9 @@ const ( // logged at startup _per node_. This was initially introduced for the events // system being developed over multiple release cycles. EnvVaultExperiments = "VAULT_EXPERIMENTS" + // EnvVaultPluginTmpdir sets the folder to use for Unix sockets when setting + // up containerized plugins. + EnvVaultPluginTmpdir = "VAULT_PLUGIN_TMPDIR" // flagNameAddress is the flag used in the base command to read in the // address of the Vault server. diff --git a/command/server.go b/command/server.go index 383e4aca66..fb2d15f596 100644 --- a/command/server.go +++ b/command/server.go @@ -1153,6 +1153,10 @@ func (c *ServerCommand) Run(args []string) int { config.License = envLicense } + if envPluginTmpdir := os.Getenv(EnvVaultPluginTmpdir); envPluginTmpdir != "" { + config.PluginTmpdir = envPluginTmpdir + } + if err := server.ExperimentsFromEnvAndCLI(config, EnvVaultExperiments, c.flagExperiments); err != nil { c.UI.Error(err.Error()) return 1 @@ -3082,6 +3086,7 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical. ClusterName: config.ClusterName, CacheSize: config.CacheSize, PluginDirectory: config.PluginDirectory, + PluginTmpdir: config.PluginTmpdir, PluginFileUid: config.PluginFileUid, PluginFilePermissions: config.PluginFilePermissions, EnableUI: config.EnableUI, diff --git a/command/server/config.go b/command/server/config.go index 7b7d73f912..938683d6a7 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -69,6 +69,7 @@ type Config struct { ClusterCipherSuites string `hcl:"cluster_cipher_suites"` PluginDirectory string `hcl:"plugin_directory"` + PluginTmpdir string `hcl:"plugin_tmpdir"` PluginFileUid int `hcl:"plugin_file_uid"` @@ -363,6 +364,11 @@ func (c *Config) Merge(c2 *Config) *Config { result.PluginDirectory = c2.PluginDirectory } + result.PluginTmpdir = c.PluginTmpdir + if c2.PluginTmpdir != "" { + result.PluginTmpdir = c2.PluginTmpdir + } + result.PluginFileUid = c.PluginFileUid if c2.PluginFileUid != 0 { result.PluginFileUid = c2.PluginFileUid @@ -1114,6 +1120,7 @@ func (c *Config) Sanitized() map[string]interface{} { "cluster_cipher_suites": c.ClusterCipherSuites, "plugin_directory": c.PluginDirectory, + "plugin_tmpdir": c.PluginTmpdir, "plugin_file_uid": c.PluginFileUid, diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index 8da927e2be..cd5e161001 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -476,6 +476,9 @@ func testLoadConfigFile(t *testing.T) { EnableResponseHeaderRaftNodeIDRaw: true, LicensePath: "/path/to/license", + + PluginDirectory: "/path/to/plugins", + PluginTmpdir: "/tmp/plugins", } addExpectedEntConfig(expected, []string{}) @@ -802,6 +805,7 @@ func testConfig_Sanitized(t *testing.T) { "max_lease_ttl": (30 * 24 * time.Hour) / time.Second, "pid_file": "./pidfile", "plugin_directory": "", + "plugin_tmpdir": "", "seals": []interface{}{ map[string]interface{}{ "disabled": false, diff --git a/command/server/test-fixtures/config.hcl b/command/server/test-fixtures/config.hcl index b20d6d3fe8..7750e5e656 100644 --- a/command/server/test-fixtures/config.hcl +++ b/command/server/test-fixtures/config.hcl @@ -51,4 +51,6 @@ disable_sealwrap = true disable_printable_check = true enable_response_header_hostname = true enable_response_header_raft_node_id = true -license_path = "/path/to/license" \ No newline at end of file +license_path = "/path/to/license" +plugin_directory = "/path/to/plugins" +plugin_tmpdir = "/tmp/plugins" \ No newline at end of file diff --git a/http/sys_config_state_test.go b/http/sys_config_state_test.go index fc55b948b0..8081aaf642 100644 --- a/http/sys_config_state_test.go +++ b/http/sys_config_state_test.go @@ -162,6 +162,7 @@ func TestSysConfigState_Sanitized(t *testing.T) { "max_lease_ttl": json.Number("0"), "pid_file": "", "plugin_directory": "", + "plugin_tmpdir": "", "plugin_file_uid": json.Number("0"), "plugin_file_permissions": json.Number("0"), "enable_response_header_hostname": false, diff --git a/sdk/helper/pluginutil/run_config.go b/sdk/helper/pluginutil/run_config.go index 3ce94cb49a..9b44e9c4f8 100644 --- a/sdk/helper/pluginutil/run_config.go +++ b/sdk/helper/pluginutil/run_config.go @@ -56,6 +56,7 @@ type runConfig struct { runtimeConfig *pluginruntimeutil.PluginRuntimeConfig PluginClientConfig + tmpdir string } func (rc runConfig) mlockEnabled() bool { @@ -144,7 +145,7 @@ func (rc runConfig) makeConfig(ctx context.Context) (*plugin.ClientConfig, error clientConfig.RunnerFunc = containerCfg.NewContainerRunner clientConfig.UnixSocketConfig = &plugin.UnixSocketConfig{ Group: strconv.Itoa(containerCfg.GroupAdd), - TempDir: os.Getenv("VAULT_PLUGIN_TMPDIR"), + TempDir: rc.tmpdir, } clientConfig.GRPCBrokerMultiplex = true } @@ -271,6 +272,7 @@ func (r *PluginRunner) RunConfig(ctx context.Context, opts ...RunOpt) (*plugin.C sha256: r.Sha256, env: r.Env, runtimeConfig: r.RuntimeConfig, + tmpdir: r.Tmpdir, PluginClientConfig: PluginClientConfig{ Name: r.Name, PluginType: r.Type, diff --git a/sdk/helper/pluginutil/runner.go b/sdk/helper/pluginutil/runner.go index dc0432ef98..ebbe110c34 100644 --- a/sdk/helper/pluginutil/runner.go +++ b/sdk/helper/pluginutil/runner.go @@ -69,6 +69,7 @@ type PluginRunner struct { Builtin bool `json:"builtin" structs:"builtin"` BuiltinFactory func() (interface{}, error) `json:"-" structs:"-"` RuntimeConfig *prutil.PluginRuntimeConfig `json:"-" structs:"-"` + Tmpdir string `json:"-" structs:"-"` } // BinaryReference returns either the OCI image reference if it's a container diff --git a/vault/core.go b/vault/core.go index dfa1c89468..101af9cd49 100644 --- a/vault/core.go +++ b/vault/core.go @@ -535,6 +535,9 @@ type Core struct { // pluginDirectory is the location vault will look for plugin binaries pluginDirectory string + // pluginTmpdir is the location vault will use for containerized plugin + // temporary files + pluginTmpdir string // pluginFileUid is the uid of the plugin files and directory pluginFileUid int @@ -822,6 +825,7 @@ type CoreConfig struct { EnableIntrospection bool PluginDirectory string + PluginTmpdir string PluginFileUid int @@ -1240,6 +1244,12 @@ func NewCore(conf *CoreConfig) (*Core, error) { return nil, fmt.Errorf("core setup failed, could not verify plugin directory: %w", err) } } + if conf.PluginTmpdir != "" { + c.pluginTmpdir, err = filepath.Abs(conf.PluginTmpdir) + if err != nil { + return nil, fmt.Errorf("core setup failed, could not verify plugin tmpdir: %w", err) + } + } if conf.PluginFileUid != 0 { c.pluginFileUid = conf.PluginFileUid @@ -2517,7 +2527,15 @@ func (c *Core) setupPluginRuntimeCatalog(ctx context.Context) error { // this method can be included in the slice of functions returned by the // buildUnsealSetupFunctionsSlice function. func (c *Core) setupPluginCatalog(ctx context.Context) error { - pluginCatalog, err := plugincatalog.SetupPluginCatalog(ctx, c.logger, c.builtinRegistry, NewBarrierView(c.barrier, pluginCatalogPath), c.pluginDirectory, c.enableMlock, c.pluginRuntimeCatalog) + pluginCatalog, err := plugincatalog.SetupPluginCatalog(ctx, &plugincatalog.PluginCatalogInput{ + Logger: c.logger, + BuiltinRegistry: c.builtinRegistry, + CatalogView: NewBarrierView(c.barrier, pluginCatalogPath), + PluginDirectory: c.pluginDirectory, + Tmpdir: c.pluginTmpdir, + EnableMlock: c.enableMlock, + PluginRuntimeCatalog: c.pluginRuntimeCatalog, + }) if err != nil { return err } diff --git a/vault/external_plugin_container_test.go b/vault/external_plugin_container_test.go index b77545372c..5bca8b9eb9 100644 --- a/vault/external_plugin_container_test.go +++ b/vault/external_plugin_container_test.go @@ -17,7 +17,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -func testClusterWithContainerPlugins(t *testing.T, types []consts.PluginType) (*Core, []pluginhelpers.TestPlugin) { +func testClusterWithContainerPlugins(t *testing.T, types []consts.PluginType) (*TestClusterCore, []pluginhelpers.TestPlugin) { var plugins []*TestPluginConfig for _, typ := range types { plugins = append(plugins, &TestPluginConfig{ @@ -26,15 +26,28 @@ func testClusterWithContainerPlugins(t *testing.T, types []consts.PluginType) (* Container: true, }) } - cluster := NewTestCluster(t, &CoreConfig{}, &TestClusterOptions{ + // Use os.MkdirTemp because t.TempDir() exceeds the Unix socket length limit. + // See https://www.man7.org/linux/man-pages/man7/unix.7.html for details. + tmpdir, err := os.MkdirTemp("", "") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.RemoveAll(tmpdir); err != nil { + t.Fatal(err) + } + }) + cluster := NewTestCluster(t, &CoreConfig{ + PluginTmpdir: tmpdir, + }, &TestClusterOptions{ Plugins: plugins, }) cluster.Start() t.Cleanup(cluster.Cleanup) - core := cluster.Cores[0].Core - TestWaitActive(t, core) + core := cluster.Cores[0] + TestWaitActive(t, core.Core) return core, cluster.Plugins } @@ -81,7 +94,7 @@ func TestExternalPluginInContainer_MountAndUnmount(t *testing.T) { }) } -func mountAndUnmountContainerPlugin_WithRuntime(t *testing.T, c *Core, plugin pluginhelpers.TestPlugin, ociRuntime string, rootless bool) { +func mountAndUnmountContainerPlugin_WithRuntime(t *testing.T, c *TestClusterCore, plugin pluginhelpers.TestPlugin, ociRuntime string, rootless bool) { if ociRuntime != "" { registerPluginRuntime(t, c.systemBackend, ociRuntime, rootless) } @@ -89,6 +102,18 @@ func mountAndUnmountContainerPlugin_WithRuntime(t *testing.T, c *Core, plugin pl mountPlugin(t, c.systemBackend, plugin.Name, plugin.Typ, "v1.0.0", "") + expectTmpdirEntries := func(expected int) { + t.Helper() + entries, err := os.ReadDir(c.CoreConfig.PluginTmpdir) + if err != nil { + t.Fatal(err) + } + if len(entries) != expected { + t.Fatalf("expected %d in tmpdir, got %v", expected, entries) + } + } + expectTmpdirEntries(1) + routeRequest := func(expectMatch bool) { pluginPath := "foo/bar" if plugin.Typ == consts.PluginTypeCredential { @@ -106,6 +131,7 @@ func mountAndUnmountContainerPlugin_WithRuntime(t *testing.T, c *Core, plugin pl routeRequest(true) unmountPlugin(t, c.systemBackend, plugin.Typ, "foo") routeRequest(false) + expectTmpdirEntries(0) } func registerContainerPlugin(t *testing.T, sys *SystemBackend, pluginName, pluginType, version, sha, image, runtime string) { diff --git a/vault/plugincatalog/plugin_catalog.go b/vault/plugincatalog/plugin_catalog.go index b136f47510..03c958b68c 100644 --- a/vault/plugincatalog/plugin_catalog.go +++ b/vault/plugincatalog/plugin_catalog.go @@ -48,6 +48,7 @@ type PluginCatalog struct { builtinRegistry BuiltinRegistry catalogView logical.Storage directory string + tmpdir string logger log.Logger // externalPlugins holds plugin process connections by a key which is @@ -138,37 +139,42 @@ type pluginClient struct { plugin.ClientProtocol } -func SetupPluginCatalog( - ctx context.Context, - logger log.Logger, - builtinRegistry BuiltinRegistry, - catalogView logical.Storage, - pluginDirectory string, - enableMlock bool, - pluginRuntimeCatalog *PluginRuntimeCatalog, -) (*PluginCatalog, error) { - pluginCatalog := &PluginCatalog{ - builtinRegistry: builtinRegistry, - catalogView: catalogView, - directory: pluginDirectory, +type PluginCatalogInput struct { + Logger log.Logger + BuiltinRegistry BuiltinRegistry + CatalogView logical.Storage + PluginDirectory string + Tmpdir string + EnableMlock bool + PluginRuntimeCatalog *PluginRuntimeCatalog +} + +func SetupPluginCatalog(ctx context.Context, in *PluginCatalogInput) (*PluginCatalog, error) { + logger := in.Logger + catalog := &PluginCatalog{ + builtinRegistry: in.BuiltinRegistry, + catalogView: in.CatalogView, + directory: in.PluginDirectory, + tmpdir: in.Tmpdir, logger: logger, - mlockPlugins: enableMlock, + mlockPlugins: in.EnableMlock, wrapper: logical.StaticSystemView{VersionString: version.GetVersion().Version}, - runtimeCatalog: pluginRuntimeCatalog, + runtimeCatalog: in.PluginRuntimeCatalog, } // Run upgrade if untyped plugins exist - err := pluginCatalog.UpgradePlugins(ctx, logger) + err := catalog.upgradePlugins(ctx, logger) if err != nil { logger.Error("error while upgrading plugin storage", "error", err) return nil, err } - if logger.IsInfo() { - logger.Info("successfully setup plugin catalog", "plugin-directory", pluginDirectory) + logger.Info("successfully setup plugin catalog", "plugin-directory", catalog.directory) + if catalog.tmpdir != "" { + logger.Debug("plugin temporary directory configured", "tmpdir", catalog.tmpdir) } - return pluginCatalog, nil + return catalog, nil } type pluginClientConn struct { @@ -723,9 +729,9 @@ func (c *PluginCatalog) isDatabasePlugin(ctx context.Context, pluginRunner *plug return merr.ErrorOrNil() } -// UpgradePlugins will loop over all the plugins of unknown type and attempt to +// upgradePlugins will loop over all the plugins of unknown type and attempt to // upgrade them to typed plugins -func (c *PluginCatalog) UpgradePlugins(ctx context.Context, logger log.Logger) error { +func (c *PluginCatalog) upgradePlugins(ctx context.Context, logger log.Logger) error { c.lock.Lock() defer c.lock.Unlock() @@ -739,6 +745,10 @@ func (c *PluginCatalog) UpgradePlugins(ctx context.Context, logger log.Logger) e if err != nil { return err } + if len(pluginsRaw) == 0 { + return nil + } + plugins := make([]string, 0, len(pluginsRaw)) for _, p := range pluginsRaw { if !strings.HasSuffix(p, "/") { @@ -838,6 +848,7 @@ func (c *PluginCatalog) get(ctx context.Context, name string, pluginType consts. // If none of the cases are satisfied, we'll search for a builtin plugin below. switch { case entry.OCIImage != "": + entry.Tmpdir = c.tmpdir if entry.Runtime != "" { entry.RuntimeConfig, err = c.runtimeCatalog.Get(ctx, entry.Runtime, consts.PluginRuntimeTypeContainer) if err != nil { @@ -1085,6 +1096,9 @@ func (c *PluginCatalog) ListPluginsWithRuntime(ctx context.Context, runtime stri if plugin.Runtime == runtime { ret = append(ret, plugin.Name) } + if plugin.OCIImage != "" { + plugin.Tmpdir = c.tmpdir + } } return ret, nil } @@ -1145,6 +1159,10 @@ func (c *PluginCatalog) listInternal(ctx context.Context, pluginType consts.Plug continue } + if plugin.OCIImage != "" { + plugin.Tmpdir = c.tmpdir + } + result = append(result, pluginutil.VersionedPlugin{ Name: plugin.Name, Type: plugin.Type.String(), diff --git a/vault/plugincatalog/plugin_catalog_test.go b/vault/plugincatalog/plugin_catalog_test.go index 6c44872c46..ac889f7c10 100644 --- a/vault/plugincatalog/plugin_catalog_test.go +++ b/vault/plugincatalog/plugin_catalog_test.go @@ -54,12 +54,14 @@ func testPluginCatalog(t *testing.T) *PluginCatalog { pluginRuntimeCatalog := testPluginRuntimeCatalog(t) pluginCatalog, err := SetupPluginCatalog( context.Background(), - logger, - corehelpers.NewMockBuiltinRegistry(), - logical.NewLogicalStorage(storage), - testDir, - false, - pluginRuntimeCatalog, + &PluginCatalogInput{ + Logger: logger, + BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(), + CatalogView: logical.NewLogicalStorage(storage), + PluginDirectory: testDir, + EnableMlock: false, + PluginRuntimeCatalog: pluginRuntimeCatalog, + }, ) if err != nil { t.Fatal(err) diff --git a/vault/plugincatalog/plugin_runtime_catalog.go b/vault/plugincatalog/plugin_runtime_catalog.go index e82c52ccd9..ed64a67f32 100644 --- a/vault/plugincatalog/plugin_runtime_catalog.go +++ b/vault/plugincatalog/plugin_runtime_catalog.go @@ -40,9 +40,7 @@ func SetupPluginRuntimeCatalog(ctx context.Context, logger log.Logger, catalogVi logger: logger, } - if logger.IsInfo() { - logger.Info("successfully setup plugin runtime catalog") - } + logger.Info("successfully setup plugin runtime catalog") return pluginRuntimeCatalog, nil } diff --git a/vault/testing.go b/vault/testing.go index d993950daa..dafc910e46 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -1435,6 +1435,7 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te coreConfig.MaxLeaseTTL = base.MaxLeaseTTL coreConfig.CacheSize = base.CacheSize coreConfig.PluginDirectory = base.PluginDirectory + coreConfig.PluginTmpdir = base.PluginTmpdir coreConfig.Seal = base.Seal coreConfig.UnwrapSeal = base.UnwrapSeal coreConfig.DevToken = base.DevToken diff --git a/website/content/docs/configuration/index.mdx b/website/content/docs/configuration/index.mdx index e601c650ed..bb5bb2ab2f 100644 --- a/website/content/docs/configuration/index.mdx +++ b/website/content/docs/configuration/index.mdx @@ -130,6 +130,16 @@ to specify where the configuration is. allowed to be loaded. Vault must have permission to read files in this directory to successfully load plugins, and the value cannot be a symbolic link. +- `plugin_tmpdir` `(string: "")` - A directory that Vault can create temporary + files in to support Unix socket communication with containerized plugins. If + not set, Vault will use the system's default directory for temporary files. + Generally not necessary unless you are using + [containerized plugins](/vault/docs/plugins/containerized-plugins) and Vault + does not share a temporary folder with other processes, such as if using + systemd's [PrivateTmp](https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#PrivateTmp=) + setting. This can also be specified via the `VAULT_PLUGIN_TMPDIR` environment + variable. + @include 'plugin-file-permissions-check.mdx' - `plugin_file_uid` `(integer: 0)` – Uid of the plugin directories and plugin binaries if they