From efffcd56d0d2a9218b4b27cf7725bcaa2f59abdd Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Mon, 28 Nov 2022 16:17:58 -0500 Subject: [PATCH] Fix Vault managed intermediate PKI bug (#15525) --- .changelog/15525.txt | 3 + agent/connect/ca/provider_vault.go | 56 +++++++++++-- agent/connect/ca/provider_vault_test.go | 105 ++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 .changelog/15525.txt diff --git a/.changelog/15525.txt b/.changelog/15525.txt new file mode 100644 index 000000000..d920109e6 --- /dev/null +++ b/.changelog/15525.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: Fixed issue where using Vault as Connect CA with Vault-managed policies would error on start-up if the intermediate PKI mount existed but was empty +``` \ No newline at end of file diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 25eb07691..48eacb9da 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -70,6 +70,13 @@ type VaultProvider struct { clusterID string spiffeID *connect.SpiffeIDSigning logger hclog.Logger + + // isConsulMountedIntermediate is used to determine if we should tune the + // mount if the VaultProvider is ever reconfigured. This is at most a + // "best guess" to determine whether this instance of Consul created the + // intermediate mount but will not be able to tell if an existing mount + // was created by Consul (in a previous running instance) or was external. + isConsulMountedIntermediate bool } func NewVaultProvider(logger hclog.Logger) *VaultProvider { @@ -310,9 +317,10 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { }, }) if err != nil { - return RootResult{}, err + return RootResult{}, fmt.Errorf("failed to mount root CA backend: %w", err) } + // We want to initialize afterwards fallthrough case ErrBackendNotInitialized: uid, err := connect.CompactUID() @@ -326,7 +334,7 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { "key_bits": v.config.PrivateKeyBits, }) if err != nil { - return RootResult{}, err + return RootResult{}, fmt.Errorf("failed to initialize root CA: %w", err) } var ok bool rootPEM, ok = resp.Data["certificate"].(string) @@ -336,7 +344,7 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { default: if err != nil { - return RootResult{}, err + return RootResult{}, fmt.Errorf("unexpected error while setting root PKI backend: %w", err) } } @@ -381,19 +389,51 @@ func (v *VaultProvider) setupIntermediatePKIPath() error { Config: mountConfig, }) if err != nil { - return err + return fmt.Errorf("failed to mount intermediate PKI backend: %w", err) } + // Required to determine if we should tune the mount + // if the VaultProvider is ever reconfigured. + v.isConsulMountedIntermediate = true + + } else if err == ErrBackendNotInitialized { + // If this is the first time calling setupIntermediatePKIPath, the backend + // will not have been initialized. Since the mount is ready we can suppress + // this error. } else { - return err + return fmt.Errorf("unexpected error while fetching intermediate CA: %w", err) } } else { + v.logger.Info("Found existing Intermediate PKI path mount", + "namespace", v.config.IntermediatePKINamespace, + "path", v.config.IntermediatePKIPath, + ) + + // This codepath requires the Vault policy: + // + // path "/sys/mounts//tune" { + // capabilities = [ "update" ] + // } + // err := v.tuneMountNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath, &mountConfig) if err != nil { - v.logger.Warn("Could not update intermediate PKI mount settings", "path", v.config.IntermediatePKIPath, "error", err) + if v.isConsulMountedIntermediate { + v.logger.Warn("Intermediate PKI path was mounted by Consul but could not be tuned", + "namespace", v.config.IntermediatePKINamespace, + "path", v.config.IntermediatePKIPath, + "error", err, + ) + } else { + v.logger.Debug("Failed to tune Intermediate PKI mount. 403 Forbidden is expected if Consul does not have tune capabilities for the Intermediate PKI mount (i.e. using Vault-managed policies)", + "namespace", v.config.IntermediatePKINamespace, + "path", v.config.IntermediatePKIPath, + "error", err, + ) + } + } } - // Create the role for issuing leaf certs if it doesn't exist yet + // Create the role for issuing leaf certs rolePath := v.config.IntermediatePKIPath + "roles/" + VaultCALeafCertRole _, err = v.writeNamespaced(v.config.IntermediatePKINamespace, rolePath, map[string]interface{}{ "allow_any_name": true, @@ -710,7 +750,7 @@ func (v *VaultProvider) SignIntermediate(csr *x509.CertificateRequest) (string, func (v *VaultProvider) CrossSignCA(cert *x509.Certificate) (string, error) { rootPEM, err := v.getCA(v.config.RootPKINamespace, v.config.RootPKIPath) if err != nil { - return "", err + return "", fmt.Errorf("failed to get root CA: %w", err) } rootCert, err := connect.ParseCert(rootPEM) if err != nil { diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 797084c2e..9fbe9f337 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -924,6 +924,111 @@ func TestVaultCAProvider_GenerateIntermediate(t *testing.T) { require.NotEqual(t, orig, new) } +func TestVaultCAProvider_VaultManaged(t *testing.T) { + + SkipIfVaultNotPresent(t) + + const vaultManagedPKIPolicy = ` +path "/pki-root/" { + capabilities = [ "read" ] +} + +path "/pki-root/root/sign-intermediate" { + capabilities = [ "update" ] +} + +path "/pki-intermediate/*" { + capabilities = [ "create", "read", "update", "delete", "list" ] +} + +path "auth/token/renew-self" { + capabilities = [ "update" ] +} + +path "auth/token/lookup-self" { + capabilities = [ "read" ] +} +` + + testVault, err := runTestVault(t) + if err != nil { + t.Fatalf("err: %v", err) + } + + testVault.WaitUntilReady(t) + + client := testVault.Client() + + client.SetToken("root") + + // Mount pki root externally + require.NoError(t, client.Sys().Mount("pki-root", &vaultapi.MountInput{ + Type: "pki", + Description: "root CA backend for Consul Connect", + Config: vaultapi.MountConfigInput{ + MaxLeaseTTL: "12m", + }, + })) + _, err = client.Logical().Write("pki-root/root/generate/internal", map[string]interface{}{ + "common_name": "testconsul", + }) + require.NoError(t, err) + + // Mount pki intermediate externally + require.NoError(t, client.Sys().Mount("pki-intermediate", &vaultapi.MountInput{ + Type: "pki", + Description: "intermediate CA backend for Consul Connect", + Config: vaultapi.MountConfigInput{ + MaxLeaseTTL: "6m", + }, + })) + + // Generate a policy and token for the VaultProvider to use + require.NoError(t, client.Sys().PutPolicy("consul-ca", vaultManagedPKIPolicy)) + tcr := &vaultapi.TokenCreateRequest{ + Policies: []string{"consul-ca"}, + } + secret, err := testVault.client.Auth().Token().Create(tcr) + require.NoError(t, err) + providerToken := secret.Auth.ClientToken + + // We want to test the provider.Configure() step + _, err = createVaultProvider(t, true, testVault.Addr, providerToken, nil) + require.NoError(t, err) +} + +func TestVaultCAProvider_ConsulManaged(t *testing.T) { + + SkipIfVaultNotPresent(t) + + testVault, err := runTestVault(t) + if err != nil { + t.Fatalf("err: %v", err) + } + + testVault.WaitUntilReady(t) + + client := testVault.Client() + + client.SetToken("root") + + // We do not configure any mounts and instead let Consul + // be responsible for mounting root and intermediate PKI + + // Generate a policy and token for the VaultProvider to use + require.NoError(t, client.Sys().PutPolicy("consul-ca", pkiTestPolicy)) + tcr := &vaultapi.TokenCreateRequest{ + Policies: []string{"consul-ca"}, + } + secret, err := testVault.client.Auth().Token().Create(tcr) + require.NoError(t, err) + providerToken := secret.Auth.ClientToken + + // We want to test the provider.Configure() step + _, err = createVaultProvider(t, true, testVault.Addr, providerToken, nil) + require.NoError(t, err) +} + func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time.Duration { t.Helper()