diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 82956bed2c..0a6b0b47a4 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 @@ -1498,11 +1492,6 @@ func (c *CAManager) SignCertificate(csr *x509.CertificateRequest, spiffeID conne pem = pem + ca.EnsureTrailingNewline(p) } - // Append our local CA's intermediate if there is one. - if inter != root { - pem = pem + ca.EnsureTrailingNewline(inter) - } - modIdx, err := c.delegate.ApplyCALeafRequest() if err != nil { return nil, err diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index fc2150eea8..5e5e1de59d 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, @@ -400,7 +404,7 @@ func TestCAManager_UpdateConfigWhileRenewIntermediate(t *testing.T) { require.EqualValues(t, caStateInitialized, manager.state) } -func TestCAManager_SignLeafWithExpiredCert(t *testing.T) { +func TestCAManager_SignCertificate_WithExpiredCert(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") } @@ -422,8 +426,10 @@ func TestCAManager_SignLeafWithExpiredCert(t *testing.T) { {"root in the future", time.Now().AddDate(0, 0, 1), time.Now().AddDate(0, 0, 2), time.Now().AddDate(0, 0, -1), time.Now().AddDate(0, 0, 2), false, ""}, } - for _, arg := range args { + caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) + require.NoError(t, err, "failed to generate key") + for _, arg := range args { t.Run(arg.testName, func(t *testing.T) { // No parallel execution because we change globals // Set the interval and drift buffer low for renewing the cert. @@ -440,13 +446,15 @@ func TestCAManager_SignLeafWithExpiredCert(t *testing.T) { conf.ConnectEnabled = true conf.PrimaryDatacenter = "dc1" conf.Datacenter = "dc2" + + rootPEM := generateCertPEM(t, caPrivKey, arg.notBeforeRoot, arg.notAfterRoot) + 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) - err, rootPEM := generatePem(arg.notBeforeRoot, arg.notAfterRoot) - require.NoError(t, err) - err, intermediatePEM := generatePem(arg.notBeforeIntermediate, arg.notAfterIntermediate) - require.NoError(t, err) manager.providerShim = &mockCAProvider{ callbackCh: delegate.callbackCh, rootPEM: rootPEM, @@ -456,14 +464,13 @@ func TestCAManager_SignLeafWithExpiredCert(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{}) - + _, err := manager.SignCertificate(&x509.CertificateRequest{}, &connect.SpiffeIDAgent{}) if arg.isError { require.Error(t, err) require.Contains(t, err.Error(), arg.errorMsg) @@ -474,7 +481,8 @@ func TestCAManager_SignLeafWithExpiredCert(t *testing.T) { } } -func generatePem(notBefore time.Time, notAfter time.Time) (error, string) { +func generateCertPEM(t *testing.T, caPrivKey *rsa.PrivateKey, notBefore time.Time, notAfter time.Time) string { + t.Helper() ca := &x509.Certificate{ SerialNumber: big.NewInt(2019), Subject: pkix.Name{ @@ -491,27 +499,19 @@ func generatePem(notBefore time.Time, notAfter time.Time) (error, string) { ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, BasicConstraintsValid: true, + URIs: []*url.URL{connect.SpiffeIDAgent{Host: "foo"}.URI()}, } - caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - return err, "" - } + caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey) - if err != nil { - return err, "" - } + require.NoError(t, err, "failed to create cert") + caPEM := new(bytes.Buffer) - pem.Encode(caPEM, &pem.Block{ + err = pem.Encode(caPEM, &pem.Block{ Type: "CERTIFICATE", Bytes: caBytes, }) - - caPrivKeyPEM := new(bytes.Buffer) - pem.Encode(caPrivKeyPEM, &pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(caPrivKey), - }) - return err, caPEM.String() + require.NoError(t, err, "failed to encode") + return caPEM.String() } func TestCADelegateWithState_GenerateCASignRequest(t *testing.T) { diff --git a/agent/consul/leader_metrics.go b/agent/consul/leader_metrics.go index 7151adb74d..244d3fd5d0 100644 --- a/agent/consul/leader_metrics.go +++ b/agent/consul/leader_metrics.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/consul/agent/connect" - "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/logging" ) @@ -55,27 +54,14 @@ func getRootCAExpiry(s *Server) (time.Duration, error) { } func signingCAExpiryMonitor(s *Server) CertExpirationMonitor { - isPrimary := s.config.Datacenter == s.config.PrimaryDatacenter - if isPrimary { - return CertExpirationMonitor{ - Key: metricsKeyMeshActiveSigningCAExpiry, - Logger: s.logger.Named(logging.Connect), - Query: func() (time.Duration, error) { - provider, _ := s.caManager.getCAProvider() - - if _, ok := provider.(ca.PrimaryUsesIntermediate); ok { - return getActiveIntermediateExpiry(s) - } - return getRootCAExpiry(s) - }, - } - } - return CertExpirationMonitor{ Key: metricsKeyMeshActiveSigningCAExpiry, Logger: s.logger.Named(logging.Connect), Query: func() (time.Duration, error) { - return getActiveIntermediateExpiry(s) + if s.caManager.isIntermediateUsedToSignLeaf() { + return getActiveIntermediateExpiry(s) + } + return getRootCAExpiry(s) }, } }