Merge pull request #14429 from hashicorp/ca-prune-intermediates

Prune old expired intermediate certs when appending a new one

.changelog/14429.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect: Fixed an issue where intermediate certificates could build up in the root CA because they were never being pruned after expiring.
+``

agent/consul/leader_connect_ca.go

@@ -1098,11 +1098,36 @@ func setLeafSigningCert(caRoot *structs.CARoot, pem string) error {
 		return fmt.Errorf("error parsing leaf signing cert: %w", err)
 	}
 
+	if err := pruneExpiredIntermediates(caRoot); err != nil {
+		return err
+	}
+
 	caRoot.IntermediateCerts = append(caRoot.IntermediateCerts, pem)
 	caRoot.SigningKeyID = connect.EncodeSigningKeyID(cert.SubjectKeyId)
 	return nil
 }
 
+// pruneExpiredIntermediates removes expired intermediate certificates
+// from the given CARoot.
+func pruneExpiredIntermediates(caRoot *structs.CARoot) error {
+	var newIntermediates []string
+	now := time.Now()
+	for _, intermediatePEM := range caRoot.IntermediateCerts {
+		cert, err := connect.ParseCert(intermediatePEM)
+		if err != nil {
+			return fmt.Errorf("error parsing leaf signing cert: %w", err)
+		}
+
+		// Only keep the intermediate cert if it's still valid.
+		if cert.NotAfter.After(now) {
+			newIntermediates = append(newIntermediates, intermediatePEM)
+		}
+	}
+
+	caRoot.IntermediateCerts = newIntermediates
+	return nil
+}
+
 // runRenewIntermediate periodically attempts to renew the intermediate cert.
 func (c *CAManager) runRenewIntermediate(ctx context.Context) error {
 	isPrimary := c.serverConf.Datacenter == c.serverConf.PrimaryDatacenter

agent/consul/leader_connect_ca_test.go

@@ -435,7 +435,6 @@ func TestCAManager_SignCertificate_WithExpiredCert(t *testing.T) {
 		errorMsg              string
 	}{
 		{"intermediate valid", time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2), time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2), false, ""},
-		{"intermediate expired", time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2), time.Now().AddDate(-2, 0, 0), time.Now().AddDate(0, 0, -1), true, "intermediate expired: certificate expired, expiration date"},
 		{"root expired", time.Now().AddDate(-2, 0, 0), time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2), true, "root expired: certificate expired, expiration date"},
 		// a cert that is not yet valid is ok, assume it will be valid soon enough
 		{"intermediate in the future", time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2), time.Now().AddDate(0, 0, 1), time.Now().AddDate(0, 0, 2), false, ""},

agent/consul/leader_connect_test.go

@@ -401,6 +401,18 @@ func TestCAManager_RenewIntermediate_Vault_Primary(t *testing.T) {
 	err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", &req, &cert)
 	require.NoError(t, err)
 	verifyLeafCert(t, activeRoot, cert.CertPEM)
+
+	// Wait for the primary's old intermediate to be pruned after expiring.
+	oldIntermediate := activeRoot.IntermediateCerts[0]
+	retry.Run(t, func(r *retry.R) {
+		store := s1.caManager.delegate.State()
+		_, storedRoot, err := store.CARootActive(nil)
+		r.Check(err)
+
+		if storedRoot.IntermediateCerts[0] == oldIntermediate {
+			r.Fatal("old intermediate should be gone")
+		}
+	})
 }
 
 func patchIntermediateCertRenewInterval(t *testing.T) {
@@ -516,6 +528,18 @@ func TestCAManager_RenewIntermediate_Secondary(t *testing.T) {
 	err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", &req, &cert)
 	require.NoError(t, err)
 	verifyLeafCert(t, activeRoot, cert.CertPEM)
+
+	// Wait for dc2's old intermediate to be pruned after expiring.
+	oldIntermediate := activeRoot.IntermediateCerts[0]
+	retry.Run(t, func(r *retry.R) {
+		store := s2.caManager.delegate.State()
+		_, storedRoot, err := store.CARootActive(nil)
+		r.Check(err)
+
+		if storedRoot.IntermediateCerts[0] == oldIntermediate {
+			r.Fatal("old intermediate should be gone")
+		}
+	})
 }
 
 func TestConnectCA_ConfigurationSet_RootRotation_Secondary(t *testing.T) {