feat(cert): cert rotation state management

Enable the certificate handler to correctly issue certificates
from the correct issuer based on the set state. We don't actually
populate any additional states yet.
Signed-off-by: Sean Teeling <[email protected]>

Author: steeling

pkg/certificate/certificate.go

@@ -20,6 +20,19 @@ const (
 	noiseSeconds = 5
 )
 
+// mergeRoot will merge in the provided root CA for future calls to GetIssuingCA. It guarantees to not mutate
+// the underlying IssuingCA field. By doing so, we ensure that we don't need locks.
+// NOTE: this does not return a full copy, mutations to the other byte slices could cause data races.
+func (c *Certificate) newMergedWithRoot(root pem.RootCertificate) *Certificate {
+	cert := *c
+
+	buf := make([]byte, 0, len(root)+len(c.IssuingCA))
+	buf = append(buf, c.IssuingCA...)
+	buf = append(buf, root...)
+	cert.IssuingCA = buf
+	return &cert
+}
+
 // GetCommonName returns the Common Name of the certificate
 func (c *Certificate) GetCommonName() CommonName {
 	return c.CommonName

pkg/certificate/fake_manager.go

@@ -7,6 +7,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 
 	"github.com/openservicemesh/osm/pkg/apis/config/v1alpha2"
+	"github.com/openservicemesh/osm/pkg/certificate/pem"
 )
 
 var (
@@ -19,8 +20,8 @@ var (
 
 type fakeMRCClient struct{}
 
-func (c *fakeMRCClient) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (Issuer, error) {
-	return &fakeIssuer{}, nil
+func (c *fakeMRCClient) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (Issuer, string, error) {
+	return &fakeIssuer{}, "fake-issuer-1", nil
 }
 
 // List returns the single, pre-generated MRC. It is intended to implement the certificate.MRCClient interface.
@@ -33,13 +34,22 @@ func (c *fakeMRCClient) List() ([]*v1alpha2.MeshRootCertificate, error) {
 // method to implement the certificate.MRCClient interface.
 func (c *fakeMRCClient) AddEventHandler(cache.ResourceEventHandler) {}
 
-type fakeIssuer struct{}
+type fakeIssuer struct {
+	err bool
+	id  string
+}
 
 // IssueCertificate is a testing helper to satisfy the certificate client interface
 func (i *fakeIssuer) IssueCertificate(cn CommonName, validityPeriod time.Duration) (*Certificate, error) {
+	if i.err {
+		return nil, fmt.Errorf("%s failed", i.id)
+	}
 	return &Certificate{
 		CommonName: cn,
 		Expiration: time.Now().Add(validityPeriod),
+		// simply used to distinguish the private/public key from other issuers
+		IssuingCA:  pem.RootCertificate(i.id),
+		PrivateKey: pem.PrivateKey(i.id),
 	}, nil
 }
 

pkg/certificate/manager.go

@@ -1,6 +1,7 @@
 package certificate
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/pkg/errors"
@@ -22,14 +23,17 @@ func NewManager(mrcClient MRCClient, serviceCertValidityDuration time.Duration,
 		return nil, err
 	}
 
-	client, err := mrcClient.GetCertIssuerForMRC(mrcs[0])
+	client, clientID, err := mrcClient.GetCertIssuerForMRC(mrcs[0])
 	if err != nil {
 		return nil, err
 	}
 
+	c := &issuer{Issuer: client, ID: clientID}
+
 	m := &Manager{
 		// The root certificate signing all newly issued certificates
-		clients:                     []Issuer{client},
+		keyIssuer:                   c,
+		pubIssuer:                   c,
 		serviceCertValidityDuration: serviceCertValidityDuration,
 		msgBroker:                   msgBroker,
 	}
@@ -61,12 +65,7 @@ func (m *Manager) Start(checkInterval time.Duration, certRotation <-chan struct{
 }
 
 func (m *Manager) checkAndRotate() {
-	certs, err := m.ListCertificates()
-	if err != nil {
-		log.Error().Err(err).Msgf("Error listing all certificates")
-	}
-
-	for _, cert := range certs {
+	for _, cert := range m.ListIssuedCertificates() {
 		shouldRotate := cert.ShouldRotate()
 
 		word := map[bool]string{true: "will", false: "will not"}[shouldRotate]
@@ -106,16 +105,34 @@ func (m *Manager) getFromCache(cn CommonName) *Certificate {
 
 // IssueCertificate implements Manager and returns a newly issued certificate from the given client.
 func (m *Manager) IssueCertificate(cn CommonName, validityPeriod time.Duration) (*Certificate, error) {
+	// var additionalRoot pem.RootCertificate
+	var err error
+	cert := m.getFromCache(cn) // Don't call this while holding the lock
+
+	m.mu.RLock()
+	pubIssuer := m.pubIssuer
+	keyIssuer := m.keyIssuer
+	m.mu.RUnlock()
+
 	start := time.Now()
+	if cert == nil || cert.keyIssuer != keyIssuer.ID || cert.pubIssuer != pubIssuer.ID {
+		fmt.Println("issuing!")
+		cert, err = keyIssuer.IssueCertificate(cn, validityPeriod)
+		if err != nil {
+			return nil, err
+		}
+		if pubIssuer.ID != keyIssuer.ID {
+			pubCert, err := pubIssuer.IssueCertificate(cn, validityPeriod)
+			if err != nil {
+				return nil, err
+			}
 
-	if cert := m.getFromCache(cn); cert != nil {
-		return cert, nil
-	}
+			cert = cert.newMergedWithRoot(pubCert.GetIssuingCA())
+		}
 
-	// TODO(#4502): determine client(s) to use based on the rotation stage(s) of the client(s)
-	cert, err := m.clients[0].IssueCertificate(cn, validityPeriod)
-	if err != nil {
-		return cert, err
+		cert.keyIssuer = keyIssuer.ID
+		cert.pubIssuer = m.pubIssuer.ID
+		fmt.Println("issued!", cert.keyIssuer, keyIssuer.ID)
 	}
 
 	m.cache.Store(cn, cert)
@@ -131,14 +148,6 @@ func (m *Manager) ReleaseCertificate(cn CommonName) {
 	m.cache.Delete(cn)
 }
 
-// GetCertificate returns a certificate given its Common Name (CN)
-func (m *Manager) GetCertificate(cn CommonName) (*Certificate, error) {
-	if cert := m.getFromCache(cn); cert != nil {
-		return cert, nil
-	}
-	return nil, errCertNotFound
-}
-
 // RotateCertificate implements Manager and rotates an existing
 func (m *Manager) RotateCertificate(cn CommonName) (*Certificate, error) {
 	start := time.Now()
@@ -171,28 +180,36 @@ func (m *Manager) RotateCertificate(cn CommonName) (*Certificate, error) {
 	return newCert, nil
 }
 
-// ListCertificates lists all certificates issued
-func (m *Manager) ListCertificates() ([]*Certificate, error) {
+// ListIssuedCertificates implements CertificateDebugger interface and returns the list of issued certificates.
+func (m *Manager) ListIssuedCertificates() []*Certificate {
 	var certs []*Certificate
-	m.cache.Range(func(_ interface{}, certInterface interface{}) bool {
+	m.cache.Range(func(cnInterface interface{}, certInterface interface{}) bool {
 		certs = append(certs, certInterface.(*Certificate))
 		return true // continue the iteration
 	})
-	return certs, nil
-}
-
-// GetRootCertificate returns the root
-func (m *Manager) GetRootCertificate() *Certificate {
-	// TODO(#4502): determine client(s) to use based on the rotation stage(s) of the client(s)
-	return m.clients[0].GetRootCertificate()
+	return certs
 }
 
 // ListIssuedCertificates implements CertificateDebugger interface and returns the list of issued certificates.
-func (m *Manager) ListIssuedCertificates() []*Certificate {
+func (m *Manager) ListCertificates() ([]*Certificate, error) {
 	var certs []*Certificate
 	m.cache.Range(func(cnInterface interface{}, certInterface interface{}) bool {
 		certs = append(certs, certInterface.(*Certificate))
 		return true // continue the iteration
 	})
-	return certs
+	return certs, nil
+}
+
+// GetCertificate returns a certificate given its Common Name (CN)
+func (m *Manager) GetCertificate(cn CommonName) (*Certificate, error) {
+	if cert := m.getFromCache(cn); cert != nil {
+		return cert, nil
+	}
+	return nil, errCertNotFound
+}
+
+// GetRootCertificate returns the root
+func (m *Manager) GetRootCertificate() *Certificate {
+	// TODO(#4502): determine client(s) to use based on the rotation stage(s) of the client(s)
+	return m.keyIssuer.GetRootCertificate()
 }

pkg/certificate/manager_test.go

@@ -1,7 +1,6 @@
 package certificate
 
 import (
-	"fmt"
 	"testing"
 	time "time"
 
@@ -10,6 +9,7 @@ import (
 	tassert "github.com/stretchr/testify/assert"
 
 	"github.com/openservicemesh/osm/pkg/announcements"
+	"github.com/openservicemesh/osm/pkg/certificate/pem"
 	"github.com/openservicemesh/osm/pkg/messaging"
 )
 
@@ -70,11 +70,8 @@ func TestRotor(t *testing.T) {
 	assert.NoError(err)
 	certRotateChan := msgBroker.GetCertPubSub().Sub(announcements.CertificateRotated.String())
 
-	start := time.Now()
-	// Wait for one certificate rotation to be announced and terminate
+	// Wait for two certificate rotations to be announced and terminate
 	<-certRotateChan
-
-	fmt.Printf("It took %+v to rotate certificate %s\n", time.Since(start), cn)
 	newCert, err := certManager.IssueCertificate(cn, validityPeriod)
 	assert.NoError(err)
 	assert.NotEqual(certA.GetExpiration(), newCert.GetExpiration())
@@ -216,9 +213,127 @@ func TestListCertificate(t *testing.T) {
 func TestGetRootCertificate(t *testing.T) {
 	assert := tassert.New(t)
 
-	manager := &Manager{clients: []Issuer{&fakeIssuer{}}}
+	manager := &Manager{
+		keyIssuer: &issuer{ID: "fake-1", Issuer: &fakeIssuer{}},
+		pubIssuer: &issuer{ID: "fake-1", Issuer: &fakeIssuer{}},
+	}
 
 	got := manager.GetRootCertificate()
 
 	assert.Equal(caCert, got)
 }
+
+func TestIssueCertificate(t *testing.T) {
+	cn := CommonName("fake-cert-cn")
+	assert := tassert.New(t)
+
+	t.Run("single key issuer", func(t *testing.T) {
+		cm := &Manager{
+			// The root certificate signing all newly issued certificates
+			keyIssuer: &issuer{ID: "id1", Issuer: &fakeIssuer{id: "id1"}},
+			pubIssuer: &issuer{ID: "id1", Issuer: &fakeIssuer{id: "id1"}},
+		}
+		// single keyIssuer, not cached
+		cert1, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.NotNil(cert1)
+		assert.Equal(cert1.keyIssuer, "id1")
+		assert.Equal(cert1.pubIssuer, "id1")
+		assert.Equal(cert1.GetIssuingCA(), pem.RootCertificate("id1"))
+
+		// single keyIssuer cached
+		cert2, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.Equal(cert1, cert2)
+
+		// single key issuer, old version cached
+		// TODO: could use informer logic to test mrc updates instead of just manually making changes.
+		cm.keyIssuer = &issuer{ID: "id2", Issuer: &fakeIssuer{id: "id2"}}
+		cm.pubIssuer = &issuer{ID: "id2", Issuer: &fakeIssuer{id: "id2"}}
+
+		cert3, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.NotNil(cert3)
+		assert.Equal(cert3.keyIssuer, "id2")
+		assert.Equal(cert3.pubIssuer, "id2")
+		assert.NotEqual(cert2, cert3)
+		assert.Equal(cert3.GetIssuingCA(), pem.RootCertificate("id2"))
+	})
+
+	t.Run("2 issuers", func(t *testing.T) {
+		cm := &Manager{
+			// The root certificate signing all newly issued certificates
+			keyIssuer: &issuer{ID: "id1", Issuer: &fakeIssuer{id: "id1"}},
+			pubIssuer: &issuer{ID: "id2", Issuer: &fakeIssuer{id: "id2"}},
+		}
+
+		// Not cached
+		cert1, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.NotNil(cert1)
+		assert.Equal(cert1.keyIssuer, "id1")
+		assert.Equal(cert1.pubIssuer, "id2")
+		assert.Equal(cert1.GetIssuingCA(), pem.RootCertificate("idid2"))
+
+		// cached
+		cert2, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.Equal(cert1, cert2)
+
+		// cached, but pubIssuer is removed
+		cm.pubIssuer = cm.keyIssuer
+		cert3, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.NotEqual(cert1, cert3)
+		assert.Equal(cert3.keyIssuer, "id1")
+		assert.Equal(cert3.pubIssuer, "id1")
+		assert.Equal(cert3.GetIssuingCA(), pem.RootCertificate("id1"))
+
+		// cached, but keyIssuer is old
+		cm.keyIssuer = &issuer{ID: "id2", Issuer: &fakeIssuer{id: "id2"}}
+		cert4, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.NotEqual(cert3, cert4)
+		assert.Equal(cert4.keyIssuer, "id2")
+		assert.Equal(cert4.pubIssuer, "id1")
+		assert.Equal(cert4.GetIssuingCA(), pem.RootCertificate("idid1"))
+
+		// cached, but pubIssuer is old
+		cm.pubIssuer = &issuer{ID: "id3", Issuer: &fakeIssuer{id: "id3"}}
+		cert5, err := cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+		assert.NotEqual(cert4, cert5)
+		assert.Equal(cert5.keyIssuer, "id2")
+		assert.Equal(cert5.pubIssuer, "id3")
+		assert.Equal(cert5.GetIssuingCA(), pem.RootCertificate("idid3"))
+	})
+
+	t.Run("bad issuers", func(t *testing.T) {
+		cm := &Manager{
+			// The root certificate signing all newly issued certificates
+			keyIssuer: &issuer{ID: "id1", Issuer: &fakeIssuer{id: "id1", err: true}},
+			pubIssuer: &issuer{ID: "id2", Issuer: &fakeIssuer{id: "id2", err: true}},
+		}
+
+		// bad private key
+		cert, err := cm.IssueCertificate(cn, time.Minute)
+		assert.Nil(cert)
+		assert.EqualError(err, "id1 failed")
+
+		// bad public key
+		cm.keyIssuer = &issuer{ID: "id3", Issuer: &fakeIssuer{id: "id3"}}
+		cert, err = cm.IssueCertificate(cn, time.Minute)
+		assert.Nil(cert)
+		assert.EqualError(err, "id2 failed")
+
+		// insert a cached cert
+		cm.pubIssuer = cm.keyIssuer
+		cert, err = cm.IssueCertificate(cn, time.Minute)
+		assert.NoError(err)
+
+		// bad public key on an existing cached cert, because the pubIssuer is new
+		cm.pubIssuer = &issuer{ID: "id1", Issuer: &fakeIssuer{id: "id1", err: true}}
+		cert, err = cm.IssueCertificate(cn, time.Minute)
+		assert.EqualError(err, "id1 failed")
+	})
+}