From 05aeab27526987d09a88bf18d8add616c7381329 Mon Sep 17 00:00:00 2001
From: Tom Proctor <tomhjp@users.noreply.github.com>
Date: Thu, 1 Dec 2022 10:44:44 +0000
Subject: [PATCH] Fix plugin list API when audit logging enabled (#18173)

* Add test that fails due to audit log panic
* Rebuild VersionedPlugin as map of primitive types before adding to response
* Changelog
* Fix casting in external plugin tests
---
 changelog/18173.txt           |  3 +++
 vault/external_plugin_test.go |  8 ++++----
 vault/logical_system.go       | 23 ++++++++++++++++++++++-
 vault/logical_system_test.go  | 32 ++++++++++++++++++++++++++++++++
 vault/testing.go              |  4 ++++
 5 files changed, 65 insertions(+), 5 deletions(-)
 create mode 100644 changelog/18173.txt

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)
 }