New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ca: fix storing the leaf signing cert with Vault provider #11671
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -520,12 +520,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 | ||
|
@@ -537,26 +551,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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unrelated, but something I noticed while working on this. I'm pretty sure that previously we would not log anything when a server initializes itself using an existing stored I made the log message slightly different, so that it's clear this initialization was from an existing |
||
return nil | ||
} | ||
|
||
if needsSigningKeyUpdate { | ||
rootCA.SigningKeyID = expectedSigningKeyID | ||
} | ||
Comment on lines
-556
to
-558
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line was moved up to the place where we set |
||
|
||
// Get the highest index | ||
idx, _, err := state.CARoots(nil) | ||
if err != nil { | ||
|
@@ -584,6 +593,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 | ||
|
@@ -1149,10 +1174,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() | ||
|
@@ -1536,3 +1559,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) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1