From 40ec0836589a274d5227f94fd29ec37ba27f8887 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 8 Sep 2022 01:11:19 -0700 Subject: [PATCH] Update intermediate pki mount/role when reconfiguring Vault provider --- .changelog/14516.txt | 3 + agent/connect/ca/provider_vault.go | 78 ++++++++++++------------- agent/connect/ca/provider_vault_test.go | 29 +++++++-- agent/structs/connect_ca.go | 4 -- 4 files changed, 67 insertions(+), 47 deletions(-) create mode 100644 .changelog/14516.txt diff --git a/.changelog/14516.txt b/.changelog/14516.txt new file mode 100644 index 0000000000000..9980ee20f4d85 --- /dev/null +++ b/.changelog/14516.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: Fixed a bug with the Vault CA provider where the intermediate PKI mount and leaf cert role were not being updated when the CA configuration was changed. +``` \ No newline at end of file diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 270d53a019fc1..fd57366bd8d97 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -66,11 +66,10 @@ type VaultProvider struct { stopWatcher func() - isPrimary bool - clusterID string - spiffeID *connect.SpiffeIDSigning - setupIntermediatePKIPathDone bool - logger hclog.Logger + isPrimary bool + clusterID string + spiffeID *connect.SpiffeIDSigning + logger hclog.Logger } func NewVaultProvider(logger hclog.Logger) *VaultProvider { @@ -174,6 +173,11 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error { go v.renewToken(ctx, lifetimeWatcher) } + // Update the intermediate (managed) PKI mount and role + if err := v.setupIntermediatePKIPath(); err != nil { + return err + } + return nil } @@ -363,8 +367,8 @@ func (v *VaultProvider) GenerateIntermediateCSR() (string, error) { } func (v *VaultProvider) setupIntermediatePKIPath() error { - if v.setupIntermediatePKIPathDone { - return nil + mountConfig := vaultapi.MountConfigInput{ + MaxLeaseTTL: v.config.IntermediateCertTTL.String(), } _, err := v.getCA(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath) @@ -373,9 +377,7 @@ func (v *VaultProvider) setupIntermediatePKIPath() error { err := v.mountNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath, &vaultapi.MountInput{ Type: "pki", Description: "intermediate CA backend for Consul Connect", - Config: vaultapi.MountConfigInput{ - MaxLeaseTTL: v.config.IntermediateCertTTL.String(), - }, + Config: mountConfig, }) if err != nil { return err @@ -383,39 +385,28 @@ func (v *VaultProvider) setupIntermediatePKIPath() error { } else { return err } + } else { + err := v.tuneMountNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath, &mountConfig) + if err != nil { + return err + } } // Create the role for issuing leaf certs if it doesn't exist yet rolePath := v.config.IntermediatePKIPath + "roles/" + VaultCALeafCertRole - role, err := v.readNamespaced(v.config.IntermediatePKINamespace, rolePath) - - if err != nil { - return err - } - if role == nil { - _, err := v.writeNamespaced(v.config.IntermediatePKINamespace, rolePath, map[string]interface{}{ - "allow_any_name": true, - "allowed_uri_sans": "spiffe://*", - "key_type": "any", - "max_ttl": v.config.LeafCertTTL.String(), - "no_store": true, - "require_cn": false, - }) + _, err = v.writeNamespaced(v.config.IntermediatePKINamespace, rolePath, map[string]interface{}{ + "allow_any_name": true, + "allowed_uri_sans": "spiffe://*", + "key_type": "any", + "max_ttl": v.config.LeafCertTTL.String(), + "no_store": true, + "require_cn": false, + }) - if err != nil { - return err - } - } - v.setupIntermediatePKIPathDone = true - return nil + return err } func (v *VaultProvider) generateIntermediateCSR() (string, error) { - err := v.setupIntermediatePKIPath() - if err != nil { - return "", err - } - // Generate a new intermediate CSR for the root to sign. uid, err := connect.CompactUID() if err != nil { @@ -465,10 +456,6 @@ func (v *VaultProvider) SetIntermediate(intermediatePEM, rootPEM string) error { // ActiveIntermediate returns the current intermediate certificate. func (v *VaultProvider) ActiveIntermediate() (string, error) { - if err := v.setupIntermediatePKIPath(); err != nil { - return "", err - } - cert, err := v.getCA(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath) // This error is expected when calling initializeSecondaryCA for the @@ -737,6 +724,19 @@ func (v *VaultProvider) mountNamespaced(namespace, path string, mountInfo *vault return err } +func (v *VaultProvider) tuneMountNamespaced(namespace, path string, mountConfig *vaultapi.MountConfigInput) error { + defer v.setNamespace(namespace)() + r := v.client.NewRequest("POST", fmt.Sprintf("/v1/sys/mounts/%s/tune", path)) + if err := r.SetJSONBody(mountConfig); err != nil { + return err + } + resp, err := v.client.RawRequest(r) + if resp != nil { + defer resp.Body.Close() + } + return err +} + func (v *VaultProvider) unmountNamespaced(namespace, path string) error { defer v.setNamespace(namespace)() r := v.client.NewRequest("DELETE", fmt.Sprintf("/v1/sys/mounts/%s", path)) diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 11689ae69f7c2..66b89ba9f5025 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -19,6 +19,16 @@ import ( "github.com/hashicorp/consul/sdk/testutil/retry" ) +const pkiTestPolicy = ` +path "sys/mounts/*" +{ + capabilities = ["create", "read", "update", "delete", "list", "sudo"] +} +path "pki-intermediate/*" +{ + capabilities = ["create", "read", "update", "delete", "list", "sudo"] +}` + func TestVaultCAProvider_ParseVaultCAConfig(t *testing.T) { cases := map[string]struct { rawConfig map[string]interface{} @@ -653,7 +663,7 @@ func TestVaultProvider_ConfigureWithAuthMethod(t *testing.T) { authMethodType: "userpass", configureAuthMethodFunc: func(t *testing.T, vaultClient *vaultapi.Client) map[string]interface{} { _, err := vaultClient.Logical().Write("/auth/userpass/users/test", - map[string]interface{}{"password": "foo", "policies": "admins"}) + map[string]interface{}{"password": "foo", "policies": "pki"}) require.NoError(t, err) return map[string]interface{}{ "Type": "userpass", @@ -667,7 +677,8 @@ func TestVaultProvider_ConfigureWithAuthMethod(t *testing.T) { { authMethodType: "approle", configureAuthMethodFunc: func(t *testing.T, vaultClient *vaultapi.Client) map[string]interface{} { - _, err := vaultClient.Logical().Write("auth/approle/role/my-role", nil) + _, err := vaultClient.Logical().Write("auth/approle/role/my-role", + map[string]interface{}{"token_policies": "pki"}) require.NoError(t, err) resp, err := vaultClient.Logical().Read("auth/approle/role/my-role/role-id") require.NoError(t, err) @@ -695,6 +706,9 @@ func TestVaultProvider_ConfigureWithAuthMethod(t *testing.T) { err := testVault.Client().Sys().EnableAuthWithOptions(c.authMethodType, &vaultapi.EnableAuthOptions{Type: c.authMethodType}) require.NoError(t, err) + err = testVault.Client().Sys().PutPolicy("pki", pkiTestPolicy) + require.NoError(t, err) + authMethodConf := c.configureAuthMethodFunc(t, testVault.Client()) conf := map[string]interface{}{ @@ -726,11 +740,18 @@ func TestVaultProvider_RotateAuthMethodToken(t *testing.T) { testVault := NewTestVaultServer(t) - err := testVault.Client().Sys().EnableAuthWithOptions("approle", &vaultapi.EnableAuthOptions{Type: "approle"}) + err := testVault.Client().Sys().PutPolicy("pki", pkiTestPolicy) + require.NoError(t, err) + + err = testVault.Client().Sys().EnableAuthWithOptions("approle", &vaultapi.EnableAuthOptions{Type: "approle"}) require.NoError(t, err) _, err = testVault.Client().Logical().Write("auth/approle/role/my-role", - map[string]interface{}{"token_ttl": "2s", "token_explicit_max_ttl": "2s"}) + map[string]interface{}{ + "token_ttl": "2s", + "token_explicit_max_ttl": "2s", + "token_policies": "pki", + }) require.NoError(t, err) resp, err := testVault.Client().Logical().Read("auth/approle/role/my-role/role-id") require.NoError(t, err) diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index b6be5c477f460..9f319ad09fee3 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -442,10 +442,6 @@ func (c CommonCAProviderConfig) Validate() error { return nil } - // todo(kyhavlov): should we output some kind of warning here (or in a Warnings() func) - // if the intermediate TTL is set in a secondary DC? allowing it to be set and do nothing - // seems bad. - // it's sufficient to check that the root cert ttl >= intermediate cert ttl // since intermediate cert ttl >= 3* leaf cert ttl; so root cert ttl >= 3 * leaf cert ttl > leaf cert ttl if c.RootCertTTL < c.IntermediateCertTTL {