Fix bug with Vault CA provider where updating

RootPKIPath but not IntermediatePKIPath would
not update leaf signing certs with the new root.

agent/connect/ca/provider.go

@@ -137,7 +137,7 @@ type PrimaryProvider interface {
 	// Depending on the provider and its configuration, GenerateCAChain may return
 	// a single root certificate or a chain of certs. The provider should return an
 	// existing CA chain if one exists or generate a new one and return it.
-	GenerateCAChain() (CAChainResult, error)
+	GenerateCAChain() (string, error)
 
 	// SignIntermediate will validate the CSR to ensure the trust domain in the
 	// URI SAN matches the local one and that basic constraints for a CA

agent/connect/ca/provider_aws.go

@@ -140,19 +140,19 @@ func (a *AWSProvider) State() (map[string]string, error) {
 }
 
 // GenerateCAChain implements Provider
-func (a *AWSProvider) GenerateCAChain() (CAChainResult, error) {
+func (a *AWSProvider) GenerateCAChain() (string, error) {
 	if !a.isPrimary {
-		return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
+		return "", fmt.Errorf("provider is not the root certificate authority")
 	}
 
 	if err := a.ensureCA(); err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
 	if a.rootPEM == "" {
-		return CAChainResult{}, fmt.Errorf("AWS CA provider not fully Initialized")
+		return "", fmt.Errorf("AWS CA provider not fully Initialized")
 	}
-	return CAChainResult{PEM: a.rootPEM}, nil
+	return a.rootPEM, nil
 }
 
 // ensureCA loads the CA resource to check it exists if configured by User or in

agent/connect/ca/provider_consul.go

@@ -155,25 +155,25 @@ func (c *ConsulProvider) State() (map[string]string, error) {
 }
 
 // GenerateCAChain initializes a new root certificate and private key if needed.
-func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
+func (c *ConsulProvider) GenerateCAChain() (string, error) {
 	providerState, err := c.getState()
 	if err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
 	if !c.isPrimary {
-		return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
+		return "", fmt.Errorf("provider is not the root certificate authority")
 	}
 	if providerState.RootCert != "" {
-		return CAChainResult{PEM: providerState.RootCert}, nil
+		return providerState.RootCert, nil
 	}
 
 	// Generate a private key if needed
 	newState := *providerState
 	if c.config.PrivateKey == "" {
 		_, pk, err := connect.GeneratePrivateKeyWithConfig(c.config.PrivateKeyType, c.config.PrivateKeyBits)
 		if err != nil {
-			return CAChainResult{}, err
+			return "", err
 		}
 		newState.PrivateKey = pk
 	} else {
@@ -184,12 +184,12 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
 	if c.config.RootCert == "" {
 		nextSerial, err := c.incrementAndGetNextSerialNumber()
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("error computing next serial number: %v", err)
+			return "", fmt.Errorf("error computing next serial number: %v", err)
 		}
 
 		ca, err := c.generateCA(newState.PrivateKey, nextSerial, c.config.RootCertTTL)
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("error generating CA: %v", err)
+			return "", fmt.Errorf("error generating CA: %v", err)
 		}
 		newState.RootCert = ca
 	} else {
@@ -202,10 +202,10 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
 		ProviderState: &newState,
 	}
 	if _, err := c.Delegate.ApplyCARequest(args); err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
-	return CAChainResult{PEM: newState.RootCert}, nil
+	return newState.RootCert, nil
 }
 
 // GenerateIntermediateCSR creates a private key and generates a CSR

agent/connect/ca/provider_vault.go

@@ -280,9 +280,9 @@ func (v *VaultProvider) State() (map[string]string, error) {
 }
 
 // GenerateCAChain mounts and initializes a new root PKI backend if needed.
-func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
+func (v *VaultProvider) GenerateCAChain() (string, error) {
 	if !v.isPrimary {
-		return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
+		return "", fmt.Errorf("provider is not the root certificate authority")
 	}
 
 	// Set up the root PKI backend if necessary.
@@ -302,15 +302,15 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
 			},
 		})
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("failed to mount root CA backend: %w", err)
+			return "", fmt.Errorf("failed to mount root CA backend: %w", err)
 		}
 
 		// We want to initialize afterwards
 		fallthrough
 	case ErrBackendNotInitialized:
 		uid, err := connect.CompactUID()
 		if err != nil {
-			return CAChainResult{}, err
+			return "", err
 		}
 		resp, err := v.writeNamespaced(v.config.RootPKINamespace, v.config.RootPKIPath+"root/generate/internal", map[string]interface{}{
 			"common_name": connect.CACN("vault", uid, v.clusterID, v.isPrimary),
@@ -319,23 +319,23 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
 			"key_bits":    v.config.PrivateKeyBits,
 		})
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("failed to initialize root CA: %w", err)
+			return "", fmt.Errorf("failed to initialize root CA: %w", err)
 		}
 		var ok bool
 		rootPEM, ok = resp.Data["certificate"].(string)
 		if !ok {
-			return CAChainResult{}, fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"])
+			return "", fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"])
 		}
 
 	default:
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("unexpected error while setting root PKI backend: %w", err)
+			return "", fmt.Errorf("unexpected error while setting root PKI backend: %w", err)
 		}
 	}
 
 	rootChain, err := v.getCAChain(v.config.RootPKINamespace, v.config.RootPKIPath)
 	if err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
 	// Workaround for a bug in the Vault PKI API.
@@ -344,18 +344,7 @@ func (v *VaultProvider) GenerateCAChain() (CAChainResult, error) {
 		rootChain = rootPEM
 	}
 
-	intermediate, err := v.ActiveLeafSigningCert()
-	if err != nil {
-		return CAChainResult{}, fmt.Errorf("error fetching active intermediate: %w", err)
-	}
-	if intermediate == "" {
-		intermediate, err = v.GenerateLeafSigningCert()
-		if err != nil {
-			return CAChainResult{}, fmt.Errorf("error generating intermediate: %w", err)
-		}
-	}
-
-	return CAChainResult{PEM: rootChain, IntermediatePEM: intermediate}, nil
+	return rootChain, nil
 }
 
 // GenerateIntermediateCSR creates a private key and generates a CSR
@@ -582,7 +571,7 @@ func (v *VaultProvider) getCAChain(namespace, path string) (string, error) {
 	return root, nil
 }
 
-// GenerateIntermediate mounts the configured intermediate PKI backend if
+// GenerateLeafSigningCert mounts the configured intermediate PKI backend if
 // 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) GenerateLeafSigningCert() (string, error) {

agent/consul/leader_connect_ca.go

@@ -258,8 +258,8 @@ func (c *CAManager) initializeCAConfig() (*structs.CAConfiguration, error) {
 }
 
 // newCARoot returns a filled-in structs.CARoot from a raw PEM value.
-func newCARoot(rootResult ca.CAChainResult, provider, clusterID string) (*structs.CARoot, error) {
-	primaryCert, err := connect.ParseCert(rootResult.PEM)
+func newCARoot(caPem, provider, clusterID string) (*structs.CARoot, error) {
+	primaryCert, err := connect.ParseCert(caPem)
 	if err != nil {
 		return nil, err
 	}
@@ -275,17 +275,12 @@ func newCARoot(rootResult ca.CAChainResult, provider, clusterID string) (*struct
 		ExternalTrustDomain: clusterID,
 		NotBefore:           primaryCert.NotBefore,
 		NotAfter:            primaryCert.NotAfter,
-		RootCert:            lib.EnsureTrailingNewline(rootResult.PEM),
+		RootCert:            lib.EnsureTrailingNewline(caPem),
 		PrivateKeyType:      keyType,
 		PrivateKeyBits:      keyBits,
 		Active:              true,
 	}
-	if rootResult.IntermediatePEM == "" {
-		return caRoot, nil
-	}
-	if err := setLeafSigningCert(caRoot, rootResult.IntermediatePEM); err != nil {
-		return nil, fmt.Errorf("error setting leaf signing cert: %w", err)
-	}
+
 	return caRoot, nil
 }
 
@@ -518,6 +513,19 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf
 		return err
 	}
 
+	// provider may use intermediates for leaf signing in which case
+	// we need to generate a leaf signing CA.
+	if usesIntermediate, ok := provider.(ca.PrimaryUsesIntermediate); ok {
+		leafPem, err := usesIntermediate.GenerateLeafSigningCert()
+		if err != nil {
+			return fmt.Errorf("error generating new leaf signing cert: %w", err)
+		}
+
+		if err := setLeafSigningCert(rootCA, leafPem); err != nil {
+			return fmt.Errorf("error setting leaf signing cert: %w", err)
+		}
+	}
+
 	var rootUpdateRequired bool
 	if len(rootCA.IntermediateCerts) > 0 {
 		rootUpdateRequired = true
@@ -764,7 +772,6 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
 		return err
 	}
 
-	// Exit early if it's a no-op change
 	state := c.delegate.State()
 	_, config, err := state.CAConfig(nil)
 	if err != nil {
@@ -780,6 +787,8 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
 
 	// Don't allow users to change the ClusterID.
 	args.Config.ClusterID = config.ClusterID
+
+	// Exit early if it's a no-op change
 	if args.Config.Provider == config.Provider && reflect.DeepEqual(args.Config.Config, config.Config) {
 		return nil
 	}
@@ -866,26 +875,52 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
 	}
 	args.Config.State = pState
 
-	providerRoot, err := newProvider.GenerateCAChain()
+	caPEM, err := newProvider.GenerateCAChain()
 	if err != nil {
 		return fmt.Errorf("error generating CA root certificate: %v", err)
 	}
 
-	newRootPEM := providerRoot.PEM
-	newActiveRoot, err := newCARoot(providerRoot, args.Config.Provider, args.Config.ClusterID)
+	newActiveRoot, err := newCARoot(caPEM, args.Config.Provider, args.Config.ClusterID)
 	if err != nil {
 		return err
 	}
 
+	// Fetch the existing root CA to compare with the current one.
 	state := c.delegate.State()
-	// Compare the new provider's root CA ID to the current one. If they
-	// match, just update the existing provider with the new config.
-	// If they don't match, begin the root rotation process.
 	_, root, err := state.CARootActive(nil)
 	if err != nil {
 		return err
 	}
 
+	// provider may use intermediates
+	if usesIntermediate, ok := newProvider.(ca.PrimaryUsesIntermediate); ok {
+		var leafPemFunc func() (string, error)
+		if root != nil && root.ID == newActiveRoot.ID {
+			// If Root ID is the same, we can reuse the existing leaf signing cert
+			leafPemFunc = newProvider.ActiveLeafSigningCert
+		} else {
+			// If Root ID is different, we need to generate a new leaf signing cert
+			// else the trust chain will break when the old root expires.
+			leafPemFunc = usesIntermediate.GenerateLeafSigningCert
+		}
+		leafPem, err := leafPemFunc()
+		if err != nil {
+			return fmt.Errorf("error fetching leaf signing cert: %w", err)
+		}
+		// newProvider.ActiveLeafSigningCert may return a blank leafPem so we
+		// fall back to generating a new one just in case.
+		if leafPem == "" {
+			leafPem, err = usesIntermediate.GenerateLeafSigningCert()
+			if err != nil {
+				return fmt.Errorf("error generating new leaf signing cert: %w", err)
+			}
+		}
+
+		if err := setLeafSigningCert(newActiveRoot, leafPem); err != nil {
+			return fmt.Errorf("error setting leaf signing cert: %w", err)
+		}
+	}
+
 	// If the root didn't change, just update the config and return.
 	if root != nil && root.ID == newActiveRoot.ID {
 		args.Op = structs.CAOpSetConfig
@@ -919,7 +954,7 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
 		// 3. Take the active root for the new provider and append the intermediate from step 2
 		// to its list of intermediates.
 		// TODO: this cert is already parsed once in newCARoot, could we remove the second parse?
-		newRoot, err := connect.ParseCert(newRootPEM)
+		newRoot, err := connect.ParseCert(caPEM)
 		if err != nil {
 			return err
 		}