Skip to content

Commit

Permalink
ca: make getLeafSigningCertFromRoot safer
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dnephin committed Dec 1, 2021
1 parent 6d18b30 commit b631479
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 23 deletions.
20 changes: 18 additions & 2 deletions agent/consul/leader_connect_ca.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1556,4 +1572,4 @@ func (c *CAManager) isIntermediateUsedToSignLeaf() bool {
}
provider, _ := c.getCAProvider()
return primaryUsesIntermediate(provider)
}
}
8 changes: 4 additions & 4 deletions agent/consul/leader_connect_test.go
Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down
17 changes: 0 additions & 17 deletions agent/structs/connect_ca.go
Expand Up @@ -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

Expand Down

0 comments on commit b631479

Please sign in to comment.