diff --git a/changelog/18173.txt b/changelog/18173.txt new file mode 100644 index 000000000..04545ea39 --- /dev/null +++ b/changelog/18173.txt @@ -0,0 +1,3 @@ +```release-note:bug +plugins: Listing all plugins while audit logging is enabled will no longer result in an internal server error. +``` diff --git a/vault/external_plugin_test.go b/vault/external_plugin_test.go index cb4761091..0f59a682d 100644 --- a/vault/external_plugin_test.go +++ b/vault/external_plugin_test.go @@ -384,8 +384,8 @@ func TestCore_EnableExternalKv_MultipleVersions(t *testing.T) { t.Fatalf("%#v", resp) } found := false - for _, plugin := range resp.Data["detailed"].([]pluginutil.VersionedPlugin) { - if plugin.Name == pluginName && plugin.Version == "v1.2.3" { + for _, plugin := range resp.Data["detailed"].([]map[string]any) { + if plugin["name"] == pluginName && plugin["version"] == "v1.2.3" { found = true break } @@ -437,8 +437,8 @@ func TestCore_EnableExternalNoop_MultipleVersions(t *testing.T) { t.Fatalf("%#v", resp) } found := false - for _, plugin := range resp.Data["detailed"].([]pluginutil.VersionedPlugin) { - if plugin.Name == "noop" && plugin.Version == "v1.2.3" { + for _, plugin := range resp.Data["detailed"].([]map[string]any) { + if plugin["name"] == "noop" && plugin["version"] == "v1.2.3" { found = true break } diff --git a/vault/logical_system.go b/vault/logical_system.go index 8305383fe..994d4989c 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -432,7 +432,28 @@ func (b *SystemBackend) handlePluginCatalogUntypedList(ctx context.Context, _ *l } if len(versionedPlugins) != 0 { - data["detailed"] = versionedPlugins + // Audit logging uses reflection to HMAC the values of all fields in the + // response recursively, which panics if it comes across any unexported + // fields. Therefore, we have to rebuild the VersionedPlugin struct as + // a map of primitive types to avoid the panic that would happen when + // audit logging tries to HMAC the contents of the SemanticVersion field. + var detailed []map[string]any + for _, p := range versionedPlugins { + entry := map[string]any{ + "type": p.Type, + "name": p.Name, + "version": p.Version, + "builtin": p.Builtin, + } + if p.SHA256 != "" { + entry["sha256"] = p.SHA256 + } + if p.DeprecationStatus != "" { + entry["deprecation_status"] = p.DeprecationStatus + } + detailed = append(detailed, entry) + } + data["detailed"] = detailed } return &logical.Response{ diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 633b4c8a2..182e0f4c2 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -3130,6 +3130,38 @@ func TestSystemBackend_PluginCatalog_CRUD(t *testing.T) { } } +func TestSystemBackend_PluginCatalog_ListPlugins_SucceedsWithAuditLogEnabled(t *testing.T) { + core, b, root := testCoreSystemBackend(t) + + tempDir := t.TempDir() + f, err := os.CreateTemp(tempDir, "") + if err != nil { + t.Fatal(err) + } + + // Enable audit logging. + req := logical.TestRequest(t, logical.UpdateOperation, "audit/file") + req.Data = map[string]any{ + "type": "file", + "options": map[string]any{ + "file_path": f.Name(), + }, + } + ctx := namespace.RootContext(nil) + resp, err := b.HandleRequest(ctx, req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("resp: %#v, err: %v", resp, err) + } + + // List plugins + req = logical.TestRequest(t, logical.ReadOperation, "sys/plugins/catalog") + req.ClientToken = root + resp, err = core.HandleRequest(ctx, req) + if err != nil || resp == nil || resp.IsError() { + t.Fatalf("resp: %#v, err: %v", resp, err) + } +} + func TestSystemBackend_PluginCatalog_CannotRegisterBuiltinPlugins(t *testing.T) { c, b, _ := testCoreSystemBackend(t) // Bootstrap the pluginCatalog diff --git a/vault/testing.go b/vault/testing.go index 210739636..8d46fb0e5 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -34,6 +34,7 @@ import ( raftlib "github.com/hashicorp/raft" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/audit" + auditFile "github.com/hashicorp/vault/builtin/audit/file" "github.com/hashicorp/vault/builtin/credential/approle" "github.com/hashicorp/vault/command/server" "github.com/hashicorp/vault/helper/constants" @@ -129,6 +130,9 @@ func TestCoreWithSeal(t testing.T, testSeal Seal, enableRaw bool) *Core { EnableUI: false, EnableRaw: enableRaw, BuiltinRegistry: NewMockBuiltinRegistry(), + AuditBackends: map[string]audit.Factory{ + "file": auditFile.Factory, + }, } return TestCoreWithSealAndUI(t, conf) }