From b5656f6353d5664fe68343e6123bbf98198670a1 Mon Sep 17 00:00:00 2001 From: Tom Proctor Date: Fri, 23 Sep 2022 11:19:38 +0100 Subject: [PATCH] Plugins: Update running version everywhere running sha256 is set (#17292) --- builtin/plugin/backend_test.go | 5 +++-- http/sys_auth_test.go | 16 ++++++++-------- sdk/plugin/grpc_backend_test.go | 2 +- sdk/plugin/mock/backend.go | 2 +- vault/auth.go | 15 +++++++++++---- vault/external_plugin_test.go | 5 ++--- vault/logical_system_test.go | 6 +++--- vault/mount.go | 15 +++++++++++---- vault/plugin_catalog.go | 7 +++++++ vault/plugin_reload.go | 15 +++++++++++++++ 10 files changed, 62 insertions(+), 26 deletions(-) diff --git a/builtin/plugin/backend_test.go b/builtin/plugin/backend_test.go index 27533a5827..d7a678ba1f 100644 --- a/builtin/plugin/backend_test.go +++ b/builtin/plugin/backend_test.go @@ -126,8 +126,9 @@ func testConfig(t *testing.T, pluginCmd string) (*logical.BackendConfig, func()) Logger: logging.NewVaultLogger(log.Debug), System: sys, Config: map[string]string{ - "plugin_name": "mock-plugin", - "plugin_type": "secret", + "plugin_name": "mock-plugin", + "plugin_type": "secret", + "plugin_version": "v0.0.0+mock", }, } diff --git a/http/sys_auth_test.go b/http/sys_auth_test.go index f6b200a6f6..2d1fdf8144 100644 --- a/http/sys_auth_test.go +++ b/http/sys_auth_test.go @@ -45,7 +45,7 @@ func TestSysAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, }, "token/": map[string]interface{}{ @@ -63,7 +63,7 @@ func TestSysAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, } testResponseStatus(t, resp, 200) @@ -145,7 +145,7 @@ func TestSysEnableAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, }, "foo/": map[string]interface{}{ @@ -181,7 +181,7 @@ func TestSysEnableAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, } testResponseStatus(t, resp, 200) @@ -248,7 +248,7 @@ func TestSysDisableAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, }, "token/": map[string]interface{}{ @@ -266,7 +266,7 @@ func TestSysDisableAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, } testResponseStatus(t, resp, 200) @@ -542,7 +542,7 @@ func TestSysRemountAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, }, "bar/": map[string]interface{}{ @@ -577,7 +577,7 @@ func TestSysRemountAuth(t *testing.T) { "options": interface{}(nil), "plugin_version": "", "running_sha256": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), }, } testResponseStatus(t, resp, 200) diff --git a/sdk/plugin/grpc_backend_test.go b/sdk/plugin/grpc_backend_test.go index 5ab99fd42a..2f665beb04 100644 --- a/sdk/plugin/grpc_backend_test.go +++ b/sdk/plugin/grpc_backend_test.go @@ -157,7 +157,7 @@ func TestGRPCBackendPlugin_Version(t *testing.T) { } version := versioner.PluginVersion().Version - if version != "mock" { + if version != "v0.0.0+mock" { t.Fatalf("Got version %s, expected 'mock'", version) } } diff --git a/sdk/plugin/mock/backend.go b/sdk/plugin/mock/backend.go index fc840809cf..670e8950d7 100644 --- a/sdk/plugin/mock/backend.go +++ b/sdk/plugin/mock/backend.go @@ -59,7 +59,7 @@ func Backend() *backend { BackendType: logical.TypeLogical, } b.internal = "bar" - b.RunningVersion = "mock" + b.RunningVersion = "v0.0.0+mock" return &b } diff --git a/vault/auth.go b/vault/auth.go index 6353051f24..54ed9b18fb 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -184,10 +184,8 @@ func (c *Core) enableCredentialInternal(ctx context.Context, entry *MountEntry, if backendType != logical.TypeCredential { return fmt.Errorf("cannot mount %q of type %q as an auth backend", entry.Type, backendType) } - // update the entry running version with the backend's reported version - if versioner, ok := backend.(logical.PluginVersioner); ok { - entry.RunningVersion = versioner.PluginVersion().Version - } + // update the entry running version with the configured version, which was verified during registration. + entry.RunningVersion = entry.Version if entry.RunningVersion == "" { // don't set the running version to a builtin if it is running as an external plugin if externaler, ok := backend.(logical.Externaler); !ok || !externaler.IsExternal() { @@ -812,6 +810,15 @@ func (c *Core) setupCredentials(ctx context.Context) error { return fmt.Errorf("nil backend returned from %q factory", entry.Type) } + // update the entry running version with the configured version, which was verified during registration. + entry.RunningVersion = entry.Version + if entry.RunningVersion == "" { + // don't set the running version to a builtin if it is running as an external plugin + if externaler, ok := backend.(logical.Externaler); !ok || !externaler.IsExternal() { + entry.RunningVersion = versions.GetBuiltinVersion(consts.PluginTypeCredential, entry.Type) + } + } + { // Check for the correct backend type backendType := backend.Type() diff --git a/vault/external_plugin_test.go b/vault/external_plugin_test.go index 66f46e6841..086ad582a7 100644 --- a/vault/external_plugin_test.go +++ b/vault/external_plugin_test.go @@ -242,9 +242,8 @@ func TestCore_EnableExternalPlugin_MultipleVersions(t *testing.T) { t.Errorf("Expected mount to be version %s but got %s", tc.expectedVersion, raw.(*routeEntry).mountEntry.Version) } - // we don't override the running version of non-builtins, and they don't have the version set explicitly (yet) - if raw.(*routeEntry).mountEntry.RunningVersion != "" { - t.Errorf("Expected mount to have no running version but got %s", raw.(*routeEntry).mountEntry.RunningVersion) + if raw.(*routeEntry).mountEntry.RunningVersion != tc.expectedVersion { + t.Errorf("Expected mount running version to be %s but got %s", tc.expectedVersion, raw.(*routeEntry).mountEntry.RunningVersion) } if raw.(*routeEntry).mountEntry.RunningSha256 == "" { diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 1e5155623b..a13e1cdf17 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -1850,7 +1850,7 @@ func TestSystemBackend_authTable(t *testing.T) { "seal_wrap": false, "options": map[string]string(nil), "plugin_version": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), "running_sha256": "", }, } @@ -1936,7 +1936,7 @@ func TestSystemBackend_enableAuth(t *testing.T) { "seal_wrap": false, "options": map[string]string(nil), "plugin_version": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), "running_sha256": "", }, } @@ -3445,7 +3445,7 @@ func TestSystemBackend_InternalUIMounts(t *testing.T) { "local": false, "seal_wrap": false, "plugin_version": "", - "running_plugin_version": "", + "running_plugin_version": versions.GetBuiltinVersion(consts.PluginTypeCredential, "token"), "running_sha256": "", }, }, diff --git a/vault/mount.go b/vault/mount.go index b369716022..161b481de3 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -626,10 +626,8 @@ func (c *Core) mountInternal(ctx context.Context, entry *MountEntry, updateStora } } - // update the entry running version with the backend's reported version - if versioner, ok := backend.(logical.PluginVersioner); ok { - entry.RunningVersion = versioner.PluginVersion().Version - } + // update the entry running version with the configured version, which was verified during registration. + entry.RunningVersion = entry.Version if entry.RunningVersion == "" { // don't set the running version to a builtin if it is running as an external plugin if externaler, ok := backend.(logical.Externaler); !ok || !externaler.IsExternal() { @@ -1437,6 +1435,15 @@ func (c *Core) setupMounts(ctx context.Context) error { return fmt.Errorf("created mount entry of type %q is nil", entry.Type) } + // update the entry running version with the configured version, which was verified during registration. + entry.RunningVersion = entry.Version + if entry.RunningVersion == "" { + // don't set the running version to a builtin if it is running as an external plugin + if externaler, ok := backend.(logical.Externaler); !ok || !externaler.IsExternal() { + entry.RunningVersion = versions.GetBuiltinVersion(consts.PluginTypeSecrets, entry.Type) + } + } + { // Check for the correct backend type backendType := backend.Type() diff --git a/vault/plugin_catalog.go b/vault/plugin_catalog.go index c8a177bcaa..112756a27c 100644 --- a/vault/plugin_catalog.go +++ b/vault/plugin_catalog.go @@ -827,6 +827,13 @@ func (c *PluginCatalog) setInternal(ctx context.Context, name string, pluginType } else if version != "" && runningVersion.Version != "" && version != runningVersion.Version { c.logger.Warn("Plugin self-reported version did not match requested version", "plugin", name, "requestedVersion", version, "reportedVersion", runningVersion.Version) return nil, fmt.Errorf("plugin version mismatch: %s reported version (%s) did not match requested version (%s)", name, runningVersion.Version, version) + } else if version == "" && runningVersion.Version != "" { + version = runningVersion.Version + _, err := semver.NewVersion(version) + if err != nil { + return nil, fmt.Errorf("plugin self-reported version %q is not a valid semantic version: %w", version, err) + } + } entry := &pluginutil.PluginRunner{ diff --git a/vault/plugin_reload.go b/vault/plugin_reload.go index 0bcb42cdb9..2d5544de05 100644 --- a/vault/plugin_reload.go +++ b/vault/plugin_reload.go @@ -6,9 +6,11 @@ import ( "strings" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/helper/versions" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/strutil" + "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/plugin" ) @@ -188,6 +190,19 @@ func (c *Core) reloadBackendCommon(ctx context.Context, entry *MountEntry, isAut return fmt.Errorf("nil backend of type %q returned from creation function", entry.Type) } + // update the entry running version with the configured version, which was verified during registration. + entry.RunningVersion = entry.Version + if entry.RunningVersion == "" { + // don't set the running version to a builtin if it is running as an external plugin + if externaler, ok := backend.(logical.Externaler); !ok || !externaler.IsExternal() { + if isAuth { + entry.RunningVersion = versions.GetBuiltinVersion(consts.PluginTypeCredential, entry.Type) + } else { + entry.RunningVersion = versions.GetBuiltinVersion(consts.PluginTypeSecrets, entry.Type) + } + } + } + // update the mount table since we changed the runningSha if oldSha != entry.RunningSha256 && MountTableUpdateStorage { if isAuth {