diff --git a/.changelog/11672.txt b/.changelog/11672.txt new file mode 100644 index 000000000..d14f74b16 --- /dev/null +++ b/.changelog/11672.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: fixes a bug that caused the SigningKeyID to be wrong in the primary DC, when the Vault provider is used, after a CA config creates a new root. +``` diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index c27e55c3f..dc712a9e5 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -1005,7 +1005,9 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C return err } if intermediate != newRootPEM { - newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediate) + if err := setLeafSigningCert(newActiveRoot, intermediate); err != nil { + return err + } } // Update the roots and CA config in the state store at the same time @@ -1060,16 +1062,10 @@ func (c *CAManager) primaryRenewIntermediate(provider ca.Provider, newActiveRoot return fmt.Errorf("error generating new intermediate cert: %v", err) } - intermediateCert, err := connect.ParseCert(intermediatePEM) - if err != nil { - return fmt.Errorf("error parsing intermediate cert: %v", err) + if err := setLeafSigningCert(newActiveRoot, intermediatePEM); err != nil { + return err } - // Append the new intermediate to our local active root entry. This is - // where the root representations start to diverge. - newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediatePEM) - newActiveRoot.SigningKeyID = connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId) - c.logger.Info("generated new intermediate certificate for primary datacenter") return nil } @@ -1093,20 +1089,28 @@ func (c *CAManager) secondaryRenewIntermediate(provider ca.Provider, newActiveRo return fmt.Errorf("Failed to set the intermediate certificate with the CA provider: %v", err) } - intermediateCert, err := connect.ParseCert(intermediatePEM) - if err != nil { - return fmt.Errorf("error parsing intermediate cert: %v", err) + if err := setLeafSigningCert(newActiveRoot, intermediatePEM); err != nil { + return err } - // Append the new intermediate to our local active root entry. This is - // where the root representations start to diverge. - newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediatePEM) - newActiveRoot.SigningKeyID = connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId) - c.logger.Info("received new intermediate certificate from primary datacenter") return nil } +// setLeafSigningCert updates the CARoot by appending the pem to the list of +// intermediate certificates, and setting the SigningKeyID to the encoded +// SubjectKeyId of the certificate. +func setLeafSigningCert(caRoot *structs.CARoot, pem string) error { + cert, err := connect.ParseCert(pem) + if err != nil { + return fmt.Errorf("error parsing leaf signing cert: %w", err) + } + + caRoot.IntermediateCerts = append(caRoot.IntermediateCerts, pem) + caRoot.SigningKeyID = connect.EncodeSigningKeyID(cert.SubjectKeyId) + return nil +} + // intermediateCertRenewalWatch periodically attempts to renew the intermediate cert. func (c *CAManager) intermediateCertRenewalWatch(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 176eb2f55..846144460 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -14,6 +14,8 @@ import ( "testing" "time" + "github.com/hashicorp/consul/testrpc" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -24,7 +26,6 @@ import ( "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" - "github.com/hashicorp/consul/testrpc" ) // TODO(kyhavlov): replace with t.Deadline() @@ -495,3 +496,58 @@ func TestCAManager_Initialize_Logging(t *testing.T) { require.Contains(t, buf.String(), "consul CA provider configured") } + +func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) { + ca.SkipIfVaultNotPresent(t) + vault := ca.NewTestVaultServer(t) + + _, s1 := testServerWithConfig(t, func(c *Config) { + c.PrimaryDatacenter = "dc1" + c.CAConfig = &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": vault.Addr, + "Token": vault.RootToken, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }, + } + }) + defer func() { + s1.Shutdown() + s1.leaderRoutineManager.Wait() + }() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + _, origRoot, err := s1.fsm.State().CARootActive(nil) + require.NoError(t, err) + require.Len(t, origRoot.IntermediateCerts, 1) + + cert, err := connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(origRoot)) + require.NoError(t, err) + require.Equal(t, connect.HexString(cert.SubjectKeyId), origRoot.SigningKeyID) + + err = s1.caManager.UpdateConfiguration(&structs.CARequest{ + Config: &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": vault.Addr, + "Token": vault.RootToken, + "RootPKIPath": "pki-root-2/", + "IntermediatePKIPath": "pki-intermediate-2/", + }, + }, + }) + require.NoError(t, err) + + _, newRoot, err := s1.fsm.State().CARootActive(nil) + require.NoError(t, err) + require.Len(t, newRoot.IntermediateCerts, 2, + "expected one cross-sign cert and one local leaf sign cert") + require.NotEqual(t, origRoot.ID, newRoot.ID) + + cert, err = connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(newRoot)) + require.NoError(t, err) + require.Equal(t, connect.HexString(cert.SubjectKeyId), newRoot.SigningKeyID) +}