Skip to content

Commit

Permalink
Update intermediate pki mount/role when reconfiguring Vault provider
Browse files Browse the repository at this point in the history
  • Loading branch information
kyhavlov committed Sep 13, 2022
1 parent b6c0880 commit 40ec083
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 47 deletions.
3 changes: 3 additions & 0 deletions .changelog/14516.txt
Original file line number Diff line number Diff line change
@@ -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.
```
78 changes: 39 additions & 39 deletions agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -373,49 +377,36 @@ 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
}
} 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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
29 changes: 25 additions & 4 deletions agent/connect/ca/provider_vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -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{}{
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions agent/structs/connect_ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 40ec083

Please sign in to comment.