Skip to content

Commit

Permalink
Revert "Detect Vault 1.11+ import, update default issuer (#15253) (#1…
Browse files Browse the repository at this point in the history
…5437)"

This reverts commit 84838e5.

It will not be part of the 1.14.1 patch release.
  • Loading branch information
hashi-derek committed Nov 21, 2022
1 parent 5ce0132 commit e297e3e
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 112 deletions.
3 changes: 0 additions & 3 deletions .changelog/15253.txt

This file was deleted.

97 changes: 10 additions & 87 deletions agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ func (v *VaultProvider) GenerateIntermediateCSR() (string, error) {
"cannot generate an intermediate CSR")
}

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

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

// generateIntermediateCSR returns the CSR and key_id (only present in
// Vault 1.11+) or any errors encountered.
func (v *VaultProvider) generateIntermediateCSR() (string, string, error) {
func (v *VaultProvider) generateIntermediateCSR() (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 @@ -422,26 +419,17 @@ func (v *VaultProvider) generateIntermediateCSR() (string, 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")
}
// 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 "", fmt.Errorf("csr result is not a string")
}
return csr, "", nil

return csr, nil
}

// SetIntermediate writes the incoming intermediate and root certificates to the
Expand Down Expand Up @@ -543,7 +531,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, keyId, err := v.generateIntermediateCSR()
csr, err := v.generateIntermediateCSR()
if err != nil {
return "", err
}
Expand All @@ -563,81 +551,16 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) {
}

// Set the intermediate backend to use the new certificate.
importResp, err := v.writeNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"intermediate/set-signed", map[string]interface{}{
_, 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: 0 additions & 22 deletions agent/connect/ca/provider_vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -902,28 +902,6 @@ 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

0 comments on commit e297e3e

Please sign in to comment.