Merge pull request #11661 from hashicorp/dnephin/ca-remove-one-call-to-active-root

ca: remove one call to Provider.ActiveRoot
This commit is contained in:
Daniel Nephin 2022-01-13 16:48:12 -05:00 committed by GitHub
commit f31e0b8b1a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 44 additions and 69 deletions

View File

@ -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

View File

@ -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) {

View File

@ -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)
},
}
}