diff --git a/.changelog/14429.txt b/.changelog/14429.txt new file mode 100644 index 0000000000..4387d1ed40 --- /dev/null +++ b/.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. +`` \ No newline at end of file diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 5e22681645..fe780f7f22 100644 --- a/agent/consul/leader_connect_ca.go +++ b/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 diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index 37756eb204..91095be8e6 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/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, ""}, diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index d9b3863865..c8b361b03b 100644 --- a/agent/consul/leader_connect_test.go +++ b/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) {