diff --git a/.changelog/11671.txt b/.changelog/11671.txt new file mode 100644 index 0000000000..0f97366fb5 --- /dev/null +++ b/.changelog/11671.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: fixes a bug that caused the intermediate cert used to sign leaf certs to be missing from the /connect/ca/roots API response when the Vault provider was used. +``` diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index e208694213..9b1477de65 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -517,12 +517,26 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf } } + var rootUpdateRequired bool + // Versions prior to 1.9.3, 1.8.8, and 1.7.12 incorrectly used the primary // rootCA's subjectKeyID here instead of the intermediate. For // provider=consul this didn't matter since there are no intermediates in // the primaryDC, but for vault it does matter. expectedSigningKeyID := connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId) - needsSigningKeyUpdate := (rootCA.SigningKeyID != expectedSigningKeyID) + if rootCA.SigningKeyID != expectedSigningKeyID { + c.logger.Info("Correcting stored CARoot values", + "previous-signing-key", rootCA.SigningKeyID, "updated-signing-key", expectedSigningKeyID) + rootCA.SigningKeyID = expectedSigningKeyID + rootUpdateRequired = true + } + + // Add the local leaf signing cert to the rootCA struct. This handles both + // upgrades of existing state, and new rootCA. + if c.getLeafSigningCertFromRoot(rootCA) != interPEM { + rootCA.IntermediateCerts = append(rootCA.IntermediateCerts, interPEM) + rootUpdateRequired = true + } // Check if the CA root is already initialized and exit if it is, // adding on any existing intermediate certs since they aren't directly @@ -534,26 +548,21 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf if err != nil { return err } - if activeRoot != nil && needsSigningKeyUpdate { - c.logger.Info("Correcting stored SigningKeyID value", "previous", rootCA.SigningKeyID, "updated", expectedSigningKeyID) - - } else if activeRoot != nil && !needsSigningKeyUpdate { + if activeRoot != nil && !rootUpdateRequired { // This state shouldn't be possible to get into because we update the root and // CA config in the same FSM operation. if activeRoot.ID != rootCA.ID { return fmt.Errorf("stored CA root %q is not the active root (%s)", rootCA.ID, activeRoot.ID) } + // TODO: why doesn't this c.setCAProvider(provider, activeRoot) ? rootCA.IntermediateCerts = activeRoot.IntermediateCerts c.setCAProvider(provider, rootCA) + c.logger.Info("initialized primary datacenter CA from existing CARoot with provider", "provider", conf.Provider) return nil } - if needsSigningKeyUpdate { - rootCA.SigningKeyID = expectedSigningKeyID - } - // Get the highest index idx, _, err := state.CARoots(nil) if err != nil { @@ -577,6 +586,22 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf return nil } +// getLeafSigningCertFromRoot returns the PEM encoded certificate that should be used to +// sign leaf certificates in the local datacenter. The SubjectKeyId of the +// returned cert should always match the SigningKeyID of the CARoot. +// +// TODO: fix the data model so that we don't need this complicated lookup to +// find the leaf signing cert. See github.com/hashicorp/consul/issues/11347. +func (c *CAManager) getLeafSigningCertFromRoot(root *structs.CARoot) string { + if !c.isIntermediateUsedToSignLeaf() { + return root.RootCert + } + if len(root.IntermediateCerts) == 0 { + return "" + } + return root.IntermediateCerts[len(root.IntermediateCerts)-1] +} + // secondaryInitializeIntermediateCA runs the routine for generating an intermediate CA CSR and getting // it signed by the primary DC if the root CA of the primary DC has changed since the last // intermediate. It should only be called while the state lock is held by setting the state @@ -1133,10 +1158,8 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error // If this is the primary, check if this is a provider that uses an intermediate cert. If // it isn't, we don't need to check for a renewal. - if isPrimary { - if _, ok := provider.(ca.PrimaryUsesIntermediate); !ok { - return nil - } + if isPrimary && !primaryUsesIntermediate(provider) { + return nil } activeIntermediate, err := provider.ActiveIntermediate() @@ -1520,3 +1543,16 @@ func (c *CAManager) checkExpired(pem string) error { } return nil } + +func primaryUsesIntermediate(provider ca.Provider) bool { + _, ok := provider.(ca.PrimaryUsesIntermediate) + return ok +} + +func (c *CAManager) isIntermediateUsedToSignLeaf() bool { + if c.serverConf.Datacenter != c.serverConf.PrimaryDatacenter { + return true + } + provider, _ := c.getCAProvider() + return primaryUsesIntermediate(provider) +} diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index f4a65bf9e3..a6716deaf6 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -396,23 +396,34 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { provider, _ := getCAProviderWithLock(s1) intermediatePEM, err := provider.ActiveIntermediate() require.NoError(err) - _, err = connect.ParseCert(intermediatePEM) + intermediateCert, err := connect.ParseCert(intermediatePEM) require.NoError(err) + // Check that the state store has the correct intermediate + store := s1.caManager.delegate.State() + _, activeRoot, err := store.CARootActive(nil) + require.NoError(err) + require.Equal(intermediatePEM, s1.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) + // Wait for dc1's intermediate to be refreshed. // It is possible that test fails when the blocking query doesn't return. retry.Run(t, func(r *retry.R) { provider, _ = getCAProviderWithLock(s1) newIntermediatePEM, err := provider.ActiveIntermediate() r.Check(err) - _, err = connect.ParseCert(intermediatePEM) - r.Check(err) if newIntermediatePEM == intermediatePEM { r.Fatal("not a renewed intermediate") } + intermediateCert, err = connect.ParseCert(newIntermediatePEM) + r.Check(err) intermediatePEM = newIntermediatePEM }) + + _, activeRoot, err = store.CARootActive(nil) require.NoError(err) + require.Equal(intermediatePEM, s1.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) // Get the root from dc1 and validate a chain of: // dc1 leaf -> dc1 intermediate -> dc1 root @@ -439,6 +450,8 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { // Check that the leaf signed by the new intermediate can be verified using the // returned cert chain (signed intermediate + remote root). intermediatePool := x509.NewCertPool() + // TODO: do not explicitly add the intermediatePEM, we should have it available + // from leafPEM. Use connect.ParseLeafCerts to do the right thing. intermediatePool.AppendCertsFromPEM([]byte(intermediatePEM)) rootPool := x509.NewCertPool() rootPool.AppendCertsFromPEM([]byte(caRoot.RootCert)) @@ -515,10 +528,10 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { secondaryProvider, _ := getCAProviderWithLock(s2) intermediatePEM, err := secondaryProvider.ActiveIntermediate() require.NoError(err) - cert, err := connect.ParseCert(intermediatePEM) + intermediateCert, err := connect.ParseCert(intermediatePEM) require.NoError(err) - currentCertSerialNumber := cert.SerialNumber - currentCertAuthorityKeyId := cert.AuthorityKeyId + currentCertSerialNumber := intermediateCert.SerialNumber + currentCertAuthorityKeyId := intermediateCert.AuthorityKeyId // Capture the current root var originalRoot *structs.CARoot @@ -532,6 +545,12 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { waitForActiveCARoot(t, s1, originalRoot) waitForActiveCARoot(t, s2, originalRoot) + store := s2.fsm.State() + _, activeRoot, err := store.CARootActive(nil) + require.NoError(err) + require.Equal(intermediatePEM, s2.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) + // Wait for dc2's intermediate to be refreshed. // It is possible that test fails when the blocking query doesn't return. // When https://github.com/hashicorp/consul/pull/3777 is merged @@ -548,8 +567,13 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { currentCertAuthorityKeyId = cert.AuthorityKeyId r.Fatal("not a renewed intermediate") } + intermediateCert = cert }) + + _, activeRoot, err = store.CARootActive(nil) require.NoError(err) + require.Equal(intermediatePEM, s2.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) // Get the root from dc1 and validate a chain of: // dc2 leaf -> dc2 intermediate -> dc1 root @@ -570,17 +594,19 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { leafPEM, err := secondaryProvider.Sign(leafCsr) require.NoError(err) - cert, err = connect.ParseCert(leafPEM) + intermediateCert, err = connect.ParseCert(leafPEM) require.NoError(err) // Check that the leaf signed by the new intermediate can be verified using the // returned cert chain (signed intermediate + remote root). intermediatePool := x509.NewCertPool() + // TODO: do not explicitly add the intermediatePEM, we should have it available + // from leafPEM. Use connect.ParseLeafCerts to do the right thing. intermediatePool.AppendCertsFromPEM([]byte(intermediatePEM)) rootPool := x509.NewCertPool() rootPool.AppendCertsFromPEM([]byte(caRoot.RootCert)) - _, err = cert.Verify(x509.VerifyOptions{ + _, err = intermediateCert.Verify(x509.VerifyOptions{ Intermediates: intermediatePool, Roots: rootPool, }) diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 91898e666b..9da766d757 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -86,11 +86,20 @@ type CARoot struct { NotBefore time.Time NotAfter time.Time - // RootCert is the PEM-encoded public certificate. + // RootCert is the PEM-encoded public certificate for the root CA. The + // certificate is the same for all federated clusters. RootCert string // IntermediateCerts is a list of PEM-encoded intermediate certs to - // attach to any leaf certs signed by this CA. + // attach to any leaf certs signed by this CA. The list may include a + // certificate cross-signed by an old root CA, any subordinate CAs below the + // root CA, and the intermediate CA used to sign leaf certificates in the + // local Datacenter. + // + // If the provider which created this root uses an intermediate to sign + // leaf certificates (Vault provider), or this is a secondary Datacenter then + // the intermediate used to sign leaf certificates will be the last in the + // list. IntermediateCerts []string // SigningCert is the PEM-encoded signing certificate and SigningKey