From bd76fdeaeb469b107a97e5ffd944816e6b30e531 Mon Sep 17 00:00:00 2001
From: John Eikenberry <jae@zhar.net>
Date: Wed, 3 May 2023 20:30:37 +0000
Subject: [PATCH] enable auto-tidy expired issuers in vault (as CA)

When using vault as a CA and generating the local signing cert, try to
enable the PKI endpoint's auto-tidy feature with it set to tidy expired
issuers.
---
 .changelog/17138.txt                    |  4 +++
 agent/connect/ca/provider_vault.go      | 36 ++++++++++++++++++++++
 agent/connect/ca/provider_vault_test.go | 41 +++++++++++++++++++++++++
 agent/connect/ca/testing.go             |  2 ++
 4 files changed, 83 insertions(+)
 create mode 100644 .changelog/17138.txt

diff --git a/.changelog/17138.txt b/.changelog/17138.txt
new file mode 100644
index 0000000000..63843690b0
--- /dev/null
+++ b/.changelog/17138.txt
@@ -0,0 +1,4 @@
+```release-note:improvement
+ca: automatically set up Vault's auto-tidy setting for tidy_expired_issuers when using Vault as a CA provider.
+```
+
diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go
index 9bb8f40a04..00a598d92d 100644
--- a/agent/connect/ca/provider_vault.go
+++ b/agent/connect/ca/provider_vault.go
@@ -438,6 +438,9 @@ func (v *VaultProvider) setupIntermediatePKIPath() error {
 		"require_cn":       false,
 	})
 
+	// enable auto-tidy with tidy_expired_issuers
+	v.autotidyIssuers(v.config.IntermediatePKIPath)
+
 	return err
 }
 
@@ -864,6 +867,39 @@ func (v *VaultProvider) setNamespace(namespace string) func() {
 	}
 }
 
+// autotidyIssuers sets Vault's auto-tidy to remove expired issuers
+// Returns a boolean on success for testing (as there is no post-facto way of
+// checking if it is set). Logs at info level on failure to set and why,
+// returning the log message for test purposes as well.
+func (v *VaultProvider) autotidyIssuers(path string) (bool, string) {
+	s, err := v.client.Logical().Write(path+"/config/auto-tidy",
+		map[string]interface{}{
+			"enabled":              true,
+			"tidy_expired_issuers": true,
+		})
+	var errStr string
+	if err != nil {
+		errStr = err.Error()
+		switch {
+		case strings.Contains(errStr, "404"):
+			errStr = "vault versions < 1.12 don't support auto-tidy"
+		case strings.Contains(errStr, "400"):
+			errStr = "vault versions < 1.13 don't support the tidy_expired_issuers field"
+		case strings.Contains(errStr, "403"):
+			errStr = "permission denied on auto-tidy path in vault"
+		}
+		v.logger.Info("Unable to enable Vault's auto-tidy feature for expired issuers", "reason", errStr, "path", path)
+	}
+	// return values for tests
+	tidySet := false
+	if s != nil {
+		if tei, ok := s.Data["tidy_expired_issuers"]; ok {
+			tidySet, _ = tei.(bool)
+		}
+	}
+	return tidySet, errStr
+}
+
 func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderConfig, error) {
 	config := structs.VaultCAProviderConfig{
 		CommonCAProviderConfig: defaultCommonConfig(),
diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go
index 31932b030b..b0e341fe91 100644
--- a/agent/connect/ca/provider_vault_test.go
+++ b/agent/connect/ca/provider_vault_test.go
@@ -8,6 +8,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"strconv"
+	"strings"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -1143,6 +1145,45 @@ func TestVaultCAProvider_GenerateIntermediate(t *testing.T) {
 	require.NotEqual(t, orig, new)
 }
 
+func TestVaultCAProvider_AutoTidyExpiredIssuers(t *testing.T) {
+	SkipIfVaultNotPresent(t)
+	t.Parallel()
+
+	testVault := NewTestVaultServer(t)
+	attr := &VaultTokenAttributes{
+		RootPath:         "pki-root",
+		IntermediatePath: "pki-intermediate",
+		ConsulManaged:    true,
+	}
+	token := CreateVaultTokenWithAttrs(t, testVault.client, attr)
+	provider := createVaultProvider(t, true, testVault.Addr, token,
+		map[string]any{
+			"RootPKIPath":         "pki-root/",
+			"IntermediatePKIPath": "pki-intermediate/",
+		})
+
+	version := strings.Split(vaultTestVersion, ".")
+	require.Len(t, version, 3)
+	minorVersion, err := strconv.Atoi(version[1])
+	require.NoError(t, err)
+	expIssSet, errStr := provider.autotidyIssuers("pki-intermediate/")
+	switch {
+	case minorVersion <= 11:
+		require.False(t, expIssSet)
+		require.Contains(t, errStr, "auto-tidy")
+	case minorVersion == 12:
+		require.False(t, expIssSet)
+		require.Contains(t, errStr, "tidy_expired_issuers")
+	default: // Consul 1.13+
+		require.True(t, expIssSet)
+	}
+
+	// check permission denied
+	expIssSet, errStr = provider.autotidyIssuers("pki-bad/")
+	require.False(t, expIssSet)
+	require.Contains(t, errStr, "permission denied")
+}
+
 func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) {
 	SkipIfVaultNotPresent(t)
 
diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go
index cfe4697d70..d7458bcda8 100644
--- a/agent/connect/ca/testing.go
+++ b/agent/connect/ca/testing.go
@@ -184,6 +184,7 @@ type TestVaultServer struct {
 }
 
 var printedVaultVersion sync.Once
+var vaultTestVersion string
 
 func (v *TestVaultServer) Client() *vaultapi.Client {
 	return v.client
@@ -205,6 +206,7 @@ func (v *TestVaultServer) WaitUntilReady(t testing.T) {
 		version = resp.Version
 	})
 	printedVaultVersion.Do(func() {
+		vaultTestVersion = version
 		fmt.Fprintf(os.Stderr, "[INFO] agent/connect/ca: testing with vault server version: %s\n", version)
 	})
 }