From b631479df9d7ec760e3f26f398b3e4ab5a31df75 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 30 Nov 2021 19:26:36 -0500 Subject: [PATCH] ca: make getLeafSigningCertFromRoot safer As a method on the struct type this would not be safe to call without first checking c.isIntermediateUsedToSignLeaf. So for now, move this logic to the CAMananger, so that it is always correct. --- agent/consul/leader_connect_ca.go | 20 ++++++++++++++++++-- agent/consul/leader_connect_test.go | 8 ++++---- agent/structs/connect_ca.go | 17 ----------------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index e834df38f39c..98d2e362ae9a 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -534,7 +534,7 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf // Add the local leaf signing cert to the rootCA struct. This handles both // upgrades of existing state, and new rootCA. - if rootCA.LeafSigningCert() != interPEM { + if c.getLeafSigningCertFromRoot(rootCA) != interPEM { rootCA.IntermediateCerts = append(rootCA.IntermediateCerts, interPEM) rootUpdateRequired = true } @@ -594,6 +594,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 @@ -1556,4 +1572,4 @@ func (c *CAManager) isIntermediateUsedToSignLeaf() bool { } provider, _ := c.getCAProvider() return primaryUsesIntermediate(provider) -} \ No newline at end of file +} diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index 3c19edbe963d..a6716deaf6b4 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -403,7 +403,7 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { store := s1.caManager.delegate.State() _, activeRoot, err := store.CARootActive(nil) require.NoError(err) - require.Equal(intermediatePEM, activeRoot.LeafSigningCert()) + require.Equal(intermediatePEM, s1.caManager.getLeafSigningCertFromRoot(activeRoot)) require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) // Wait for dc1's intermediate to be refreshed. @@ -422,7 +422,7 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { _, activeRoot, err = store.CARootActive(nil) require.NoError(err) - require.Equal(intermediatePEM, activeRoot.LeafSigningCert()) + 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: @@ -548,7 +548,7 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { store := s2.fsm.State() _, activeRoot, err := store.CARootActive(nil) require.NoError(err) - require.Equal(intermediatePEM, activeRoot.LeafSigningCert()) + require.Equal(intermediatePEM, s2.caManager.getLeafSigningCertFromRoot(activeRoot)) require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) // Wait for dc2's intermediate to be refreshed. @@ -572,7 +572,7 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { _, activeRoot, err = store.CARootActive(nil) require.NoError(err) - require.Equal(intermediatePEM, activeRoot.LeafSigningCert()) + 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: diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 46c7e21327b8..9da766d75728 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -142,23 +142,6 @@ func (c *CARoot) Clone() *CARoot { return &newCopy } -// LeafSigningCert 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. -// -// This method has no knowledge of the provider, so it makes assumptions about -// which cert is used based on established convention. Generally you should -// check CAManager.isIntermediateUsedToSignLeaf first. -// -// If there are no IntermediateCerts the RootCert is returned. If there are -// IntermediateCerts the last one in the list is returned. -func (c *CARoot) LeafSigningCert() string { - if len(c.IntermediateCerts) == 0 { - return c.RootCert - } - return c.IntermediateCerts[len(c.IntermediateCerts)-1] -} - // CARoots is a list of CARoot structures. type CARoots []*CARoot