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

Consul used to rely on implicit issuer selection when calling Vault endpoints to issue new CSRs. Vault 1.11+ changed that behavior, which caused Consul to check the wrong (previous) issuer when renewing its Intermediate CA. This patch allows Consul to explicitly set a default issuer when it detects that the response from Vault is 1.11+.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Chris S. Kim <ckim@hashicorp.com>

.changelog/15253.txt

@@ -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)]
+```

agent/connect/ca/provider_vault.go

@@ -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 {
@@ -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),
@@ -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
@@ -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
 	}
@@ -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.

agent/connect/ca/provider_vault_test.go

@@ -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()