Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect Vault 1.11+ import, update default issuer (#15253) #15437

Merged
merged 4 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/15253.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
connect: Fixed issue where using Vault 1.11+ as CA provider would eventually break Intermediate CAs [[GH-15217](https://github.com/hashicorp/consul/issues/15217)]
```
97 changes: 87 additions & 10 deletions agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,8 @@ func (v *VaultProvider) GenerateIntermediateCSR() (string, error) {
"cannot generate an intermediate CSR")
}

return v.generateIntermediateCSR()
csr, _, err := v.generateIntermediateCSR()
return csr, err
}

func (v *VaultProvider) setupIntermediatePKIPath() error {
Expand Down Expand Up @@ -406,11 +407,13 @@ func (v *VaultProvider) setupIntermediatePKIPath() error {
return err
}

func (v *VaultProvider) generateIntermediateCSR() (string, error) {
// generateIntermediateCSR returns the CSR and key_id (only present in
// Vault 1.11+) or any errors encountered.
func (v *VaultProvider) generateIntermediateCSR() (string, string, error) {
// Generate a new intermediate CSR for the root to sign.
uid, err := connect.CompactUID()
if err != nil {
return "", err
return "", "", err
}
data, err := v.writeNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"intermediate/generate/internal", map[string]interface{}{
"common_name": connect.CACN("vault", uid, v.clusterID, v.isPrimary),
Expand All @@ -419,17 +422,26 @@ func (v *VaultProvider) generateIntermediateCSR() (string, error) {
"uri_sans": v.spiffeID.URI().String(),
})
if err != nil {
return "", err
return "", "", err
}
if data == nil || data.Data["csr"] == "" {
return "", fmt.Errorf("got empty value when generating intermediate CSR")
return "", "", fmt.Errorf("got empty value when generating intermediate CSR")
}
csr, ok := data.Data["csr"].(string)
if !ok {
return "", fmt.Errorf("csr result is not a string")
return "", "", fmt.Errorf("csr result is not a string")
}

return csr, nil
// Vault 1.11+ will return a "key_id" field which helps
// identify the correct issuer to set as default.
// https://github.com/hashicorp/vault/blob/e445c8b4f58dc20a0316a7fd1b5725b401c3b17a/builtin/logical/pki/path_intermediate.go#L154
if rawkeyId, ok := data.Data["key_id"]; ok {
keyId, ok := rawkeyId.(string)
if !ok {
return "", "", fmt.Errorf("key_id is not a string")
}
return csr, keyId, nil
}
return csr, "", nil
}

// SetIntermediate writes the incoming intermediate and root certificates to the
Expand Down Expand Up @@ -531,7 +543,7 @@ func (v *VaultProvider) getCAChain(namespace, path string) (string, error) {
// necessary, then generates and signs a new CA CSR using the root PKI backend
// and updates the intermediate backend to use that new certificate.
func (v *VaultProvider) GenerateIntermediate() (string, error) {
csr, err := v.generateIntermediateCSR()
csr, keyId, err := v.generateIntermediateCSR()
if err != nil {
return "", err
}
Expand All @@ -551,16 +563,81 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) {
}

// Set the intermediate backend to use the new certificate.
_, err = v.writeNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"intermediate/set-signed", map[string]interface{}{
importResp, err := v.writeNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"intermediate/set-signed", map[string]interface{}{
"certificate": intermediate.Data["certificate"],
})
if err != nil {
return "", err
}

// Vault 1.11+ will return a non-nil response from intermediate/set-signed
if importResp != nil {
err := v.setDefaultIntermediateIssuer(importResp, keyId)
if err != nil {
return "", fmt.Errorf("failed to update default intermediate issuer: %w", err)
}
}

return v.ActiveIntermediate()
}

// setDefaultIntermediateIssuer updates the default issuer for
// intermediate CA since Vault, as part of its 1.11+ support for
// multiple issuers, no longer overwrites the default issuer when
// generateIntermediateCSR (intermediate/generate/internal) is called.
//
// The response we get from calling [/intermediate/set-signed]
// should contain a "mapping" data field we can use to cross-reference
// with the keyId returned when calling [/intermediate/generate/internal].
//
// [/intermediate/set-signed]: https://developer.hashicorp.com/vault/api-docs/secret/pki#import-ca-certificates-and-keys
// [/intermediate/generate/internal]: https://developer.hashicorp.com/vault/api-docs/secret/pki#generate-intermediate-csr
func (v *VaultProvider) setDefaultIntermediateIssuer(vaultResp *vaultapi.Secret, keyId string) error {
if vaultResp.Data["mapping"] == nil {
return fmt.Errorf("expected Vault response data to have a 'mapping' key")
}
if keyId == "" {
return fmt.Errorf("expected non-empty keyId")
}

mapping, ok := vaultResp.Data["mapping"].(map[string]any)
if !ok {
return fmt.Errorf("unexpected type for 'mapping' value in Vault response")
}

var intermediateId string
// The value in this KV pair is called "key"
for issuer, key := range mapping {
if key == keyId {
// Expect to find the key_id we got from Vault when we
// generated the intermediate CSR.
intermediateId = issuer
break
}
}
if intermediateId == "" {
return fmt.Errorf("could not find key_id %q in response from vault", keyId)
}

// For Vault 1.11+ it is important to GET then POST to avoid clobbering fields
// like `default_follows_latest_issuer`.
// https://developer.hashicorp.com/vault/api-docs/secret/pki#default_follows_latest_issuer
resp, err := v.readNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"config/issuers")
if err != nil {
return fmt.Errorf("could not read from /config/issuers: %w", err)
}
issuersConf := resp.Data
// Overwrite the default issuer
issuersConf["default"] = intermediateId

_, err = v.writeNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"config/issuers", issuersConf)
if err != nil {
return fmt.Errorf("could not write default issuer to /config/issuers: %w", err)
}

return nil
}

// Sign calls the configured role in the intermediate PKI backend to issue
// a new leaf certificate based on the provided CSR, with the issuing
// intermediate CA cert attached.
Expand Down
22 changes: 22 additions & 0 deletions agent/connect/ca/provider_vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,28 @@ func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) {
require.Equal(t, 333*3600, mountConfig.MaxLeaseTTL)
}

func TestVaultCAProvider_GenerateIntermediate(t *testing.T) {

SkipIfVaultNotPresent(t)

provider, _ := testVaultProviderWithConfig(t, true, nil)

orig, err := provider.ActiveIntermediate()
require.NoError(t, err)

// This test was created to ensure that our calls to Vault
// returns a new Intermediate certificate and further calls
// to ActiveIntermediate return the same new cert.
new, err := provider.GenerateIntermediate()
require.NoError(t, err)

newActive, err := provider.ActiveIntermediate()
require.NoError(t, err)

require.Equal(t, new, newActive)
require.NotEqual(t, orig, new)
}

func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time.Duration {
t.Helper()

Expand Down