diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 3087fb84a..0a6b0b47a 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -1463,28 +1463,22 @@ func (c *CAManager) SignCertificate(csr *x509.CertificateRequest, spiffeID conne connect.HackSANExtensionForCSR(csr) - root, err := provider.ActiveRoot() - if err != nil { - return nil, err - } // Check if the root expired before using it to sign. - err = c.checkExpired(root) + // TODO: we store NotBefore and NotAfter on this struct, so we could avoid + // parsing the cert here. + err = c.checkExpired(caRoot.RootCert) if err != nil { return nil, fmt.Errorf("root expired: %w", err) } - inter, err := provider.ActiveIntermediate() - if err != nil { - return nil, err - } - // Check if the intermediate expired before using it to sign. - err = c.checkExpired(inter) - if err != nil { - return nil, fmt.Errorf("intermediate expired: %w", err) + if c.isIntermediateUsedToSignLeaf() && len(caRoot.IntermediateCerts) > 0 { + inter := caRoot.IntermediateCerts[len(caRoot.IntermediateCerts)-1] + if err := c.checkExpired(inter); err != nil { + return nil, fmt.Errorf("intermediate expired: %w", err) + } } // All seems to be in order, actually sign it. - pem, err := provider.Sign(csr) if err == ca.ErrRateLimited { return nil, ErrRateLimited diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index 869459cb7..5e5e1de59 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -12,6 +12,7 @@ import ( "fmt" "math/big" "net/rpc" + "net/url" "testing" "time" @@ -131,11 +132,12 @@ func verifyLeafCert(t *testing.T, root *structs.CARoot, leafCertPEM string) { } type mockCAServerDelegate struct { - t *testing.T - config *Config - store *state.Store - primaryRoot *structs.CARoot - callbackCh chan string + t *testing.T + config *Config + store *state.Store + primaryRoot *structs.CARoot + secondaryIntermediate string + callbackCh chan string } func NewMockCAServerDelegate(t *testing.T, config *Config) *mockCAServerDelegate { @@ -198,7 +200,7 @@ func (m *mockCAServerDelegate) forwardDC(method, dc string, args interface{}, re roots.ActiveRootID = m.primaryRoot.ID case "ConnectCA.SignIntermediate": r := reply.(*string) - *r = m.primaryRoot.RootCert + *r = m.secondaryIntermediate default: return fmt.Errorf("received call to unsupported method %q", method) } @@ -305,13 +307,14 @@ func initTestManager(t *testing.T, manager *CAManager, delegate *mockCAServerDel } func TestCAManager_Initialize(t *testing.T) { - conf := DefaultConfig() conf.ConnectEnabled = true conf.PrimaryDatacenter = "dc1" conf.Datacenter = "dc2" delegate := NewMockCAServerDelegate(t, conf) + delegate.secondaryIntermediate = delegate.primaryRoot.RootCert manager := NewCAManager(delegate, nil, testutil.Logger(t), conf) + manager.providerShim = &mockCAProvider{ callbackCh: delegate.callbackCh, rootPEM: delegate.primaryRoot.RootCert, @@ -356,6 +359,7 @@ func TestCAManager_UpdateConfigWhileRenewIntermediate(t *testing.T) { conf.PrimaryDatacenter = "dc1" conf.Datacenter = "dc2" delegate := NewMockCAServerDelegate(t, conf) + delegate.secondaryIntermediate = delegate.primaryRoot.RootCert manager := NewCAManager(delegate, nil, testutil.Logger(t), conf) manager.providerShim = &mockCAProvider{ callbackCh: delegate.callbackCh, @@ -447,6 +451,8 @@ func TestCAManager_SignCertificate_WithExpiredCert(t *testing.T) { intermediatePEM := generateCertPEM(t, caPrivKey, arg.notBeforeIntermediate, arg.notAfterIntermediate) delegate := NewMockCAServerDelegate(t, conf) + delegate.primaryRoot.RootCert = rootPEM + delegate.secondaryIntermediate = intermediatePEM manager := NewCAManager(delegate, nil, testutil.Logger(t), conf) manager.providerShim = &mockCAProvider{ @@ -458,14 +464,13 @@ func TestCAManager_SignCertificate_WithExpiredCert(t *testing.T) { // Simulate Wait half the TTL for the cert to need renewing. manager.timeNow = func() time.Time { - return time.Now().Add(500 * time.Millisecond) + return time.Now().UTC().Add(500 * time.Millisecond) } // Call RenewIntermediate and then confirm the RPCs and provider calls // happen in the expected order. _, err := manager.SignCertificate(&x509.CertificateRequest{}, &connect.SpiffeIDAgent{}) - if arg.isError { require.Error(t, err) require.Contains(t, err.Error(), arg.errorMsg) @@ -494,6 +499,7 @@ func generateCertPEM(t *testing.T, caPrivKey *rsa.PrivateKey, notBefore time.Tim ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, BasicConstraintsValid: true, + URIs: []*url.URL{connect.SpiffeIDAgent{Host: "foo"}.URI()}, } caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey)