From 8074383600660eaa91ea0abab258761cd40c9bb3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 2 Dec 2021 16:24:54 -0500 Subject: [PATCH] Merge pull request #11672 from hashicorp/dnephin/ca-fix-signing-key-id-post-update ca: set the correct SigningKeyID after config update with Vault provider --- .changelog/11672.txt | 3 ++ agent/consul/leader_connect_ca.go | 38 ++++++++++-------- agent/consul/leader_connect_ca_test.go | 54 ++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 17 deletions(-) create mode 100644 .changelog/11672.txt diff --git a/.changelog/11672.txt b/.changelog/11672.txt new file mode 100644 index 000000000000..d14f74b16198 --- /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 3c3b86adcde3..5eb6e9f6658e 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -951,7 +951,9 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error) 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 @@ -1010,16 +1012,10 @@ func (c *CAManager) getIntermediateCAPrimary(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 } @@ -1043,20 +1039,28 @@ func (c *CAManager) getIntermediateCASigned(provider ca.Provider, newActiveRoot 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 53da980f88c4..a841da50d08b 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -321,3 +321,57 @@ 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() + }() + + 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) +}