Simplify the CA provider interface by moving some logic out

agent/connect/ca.go

@@ -1,8 +1,11 @@
 package connect
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/ecdsa"
+	"crypto/rsa"
+	"crypto/sha1"
 	"crypto/sha256"
 	"crypto/x509"
 	"encoding/pem"
@@ -25,6 +28,23 @@ func ParseCert(pemValue string) (*x509.Certificate, error) {
 	return x509.ParseCertificate(block.Bytes)
 }
 
+// ParseCertFingerprint parses the x509 certificate from a PEM-encoded value
+// and returns the SHA-1 fingerprint.
+func ParseCertFingerprint(pemValue string) (string, error) {
+	// The _ result below is not an error but the remaining PEM bytes.
+	block, _ := pem.Decode([]byte(pemValue))
+	if block == nil {
+		return "", fmt.Errorf("no PEM-encoded data found")
+	}
+
+	hash := sha1.Sum(block.Bytes)
+	hexified := make([][]byte, len(hash))
+	for i, data := range hash {
+		hexified[i] = []byte(fmt.Sprintf("%02X", data))
+	}
+	return string(bytes.Join(hexified, []byte(":"))), nil
+}
+
 // ParseSigner parses a crypto.Signer from a PEM-encoded key. The private key
 // is expected to be the first block in the PEM value.
 func ParseSigner(pemValue string) (crypto.Signer, error) {
@@ -38,6 +58,9 @@ func ParseSigner(pemValue string) (crypto.Signer, error) {
 	case "EC PRIVATE KEY":
 		return x509.ParseECPrivateKey(block.Bytes)
 
+	case "RSA PRIVATE KEY":
+		return x509.ParsePKCS1PrivateKey(block.Bytes)
+
 	case "PRIVATE KEY":
 		signer, err := x509.ParsePKCS8PrivateKey(block.Bytes)
 		if err != nil {
@@ -74,15 +97,17 @@ func ParseCSR(pemValue string) (*x509.CertificateRequest, error) {
 // KeyId returns a x509 KeyId from the given signing key. The key must be
 // an *ecdsa.PublicKey currently, but may support more types in the future.
 func KeyId(raw interface{}) ([]byte, error) {
-	pub, ok := raw.(*ecdsa.PublicKey)
+	switch raw.(type) {
-	if !ok {
+	case *ecdsa.PublicKey:
+	case *rsa.PublicKey:
+	default:
 		return nil, fmt.Errorf("invalid key type: %T", raw)
 	}
 
 	// This is not standard; RFC allows any unique identifier as long as they
 	// match in subject/authority chains but suggests specific hashing of DER
 	// bytes of public key including DER tags.
-	bs, err := x509.MarshalPKIXPublicKey(pub)
+	bs, err := x509.MarshalPKIXPublicKey(raw)
 	if err != nil {
 		return nil, err
 	}

agent/connect/ca_provider.go

@@ -13,21 +13,28 @@ type CAProvider interface {
 	// Active root returns the currently active root CA for this
 	// provider. This should be a parent of the certificate returned by
 	// ActiveIntermediate()
-	ActiveRoot() (*structs.CARoot, error)
+	ActiveRoot() (string, error)
 
 	// ActiveIntermediate returns the current signing cert used by this
 	// provider for generating SPIFFE leaf certs.
-	ActiveIntermediate() (*structs.CARoot, error)
+	ActiveIntermediate() (string, error)
 
-	// GenerateIntermediate returns a new intermediate signing cert, a
+	// GenerateIntermediate returns a new intermediate signing cert and
-	// cross-signing CSR for it and sets it to the active intermediate.
+	// sets it to the active intermediate.
-	GenerateIntermediate() (*structs.CARoot, *x509.CertificateRequest, error)
+	GenerateIntermediate() (string, error)
 
 	// Sign signs a leaf certificate used by Connect proxies from a CSR.
-	Sign(*SpiffeIDService, *x509.CertificateRequest) (*structs.IssuedCert, error)
+	Sign(*x509.CertificateRequest) (*structs.IssuedCert, error)
 
-	// SignCA signs a CA CSR and returns the resulting cross-signed cert.
+	// CrossSignCA must accept a CA certificate signed by another CA's key
-	SignCA(*x509.CertificateRequest) (string, error)
+	// and cross sign it exactly as it is such that it forms a chain back the the
+	// CAProvider's current root. Specifically, the Distinguished Name, Subject
+	// Alternative Name, SubjectKeyID and other relevant extensions must be kept.
+	// The resulting certificate must have a distinct Serial Number and the
+	// AuthorityKeyID set to the CAProvider's current signing key as well as the
+	// Issuer related fields changed as necessary. The resulting certificate is
+	// returned as a PEM formatted string.
+	CrossSignCA(*x509.Certificate) (string, error)
 
 	// Cleanup performs any necessary cleanup that should happen when the provider
 	// is shut down permanently, such as removing a temporary PKI backend in Vault

agent/consul/connect_ca_endpoint.go

@@ -80,11 +80,22 @@ func (s *ConnectCA) ConfigurationSet(
 		return fmt.Errorf("could not initialize provider: %v", err)
 	}
 
-	newActiveRoot, err := newProvider.ActiveRoot()
+	newRootPEM, err := newProvider.ActiveRoot()
 	if err != nil {
 		return err
 	}
 
+	id, err := connect.ParseCertFingerprint(newRootPEM)
+	if err != nil {
+		return fmt.Errorf("error parsing root fingerprint: %v", err)
+	}
+	newActiveRoot := &structs.CARoot{
+		ID:       id,
+		Name:     fmt.Sprintf("%s CA Root Cert", config.Provider),
+		RootCert: newRootPEM,
+		Active:   true,
+	}
+
 	// Compare the new provider's root CA ID to the current one. If they
 	// match, just update the existing provider with the new config.
 	// If they don't match, begin the root rotation process.
@@ -121,13 +132,19 @@ func (s *ConnectCA) ConfigurationSet(
 	// to get the cross-signed intermediate
 	// 3. Get the active root for the new provider, append the intermediate from step 3
 	// to its list of intermediates
-	_, csr, err := newProvider.GenerateIntermediate()
+	intermediatePEM, err := newProvider.GenerateIntermediate()
 	if err != nil {
 		return err
 	}
 
+	intermediateCA, err := connect.ParseCert(intermediatePEM)
+	if err != nil {
+		return err
+	}
+
+	// Have the old provider cross-sign the new intermediate
 	oldProvider := s.srv.getCAProvider()
-	xcCert, err := oldProvider.SignCA(csr)
+	xcCert, err := oldProvider.CrossSignCA(intermediateCA)
 	if err != nil {
 		return err
 	}
@@ -237,21 +254,11 @@ func (s *ConnectCA) Sign(
 		return err
 	}
 
-	// Parse the SPIFFE ID
-	spiffeId, err := connect.ParseCertURI(csr.URIs[0])
-	if err != nil {
-		return err
-	}
-	serviceId, ok := spiffeId.(*connect.SpiffeIDService)
-	if !ok {
-		return fmt.Errorf("SPIFFE ID in CSR must be a service ID")
-	}
-
 	// todo(kyhavlov): more validation on the CSR before signing
 
 	provider := s.srv.getCAProvider()
 
-	cert, err := provider.Sign(serviceId, csr)
+	cert, err := provider.Sign(csr)
 	if err != nil {
 		return err
 	}

agent/consul/connect_ca_provider.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
-	uuid "github.com/hashicorp/go-uuid"
 	"github.com/mitchellh/mapstructure"
 )
 
@@ -96,12 +95,16 @@ func NewConsulCAProvider(rawConfig map[string]interface{}, srv *Server) (*Consul
 		newState.PrivateKey = conf.PrivateKey
 	}
 
-	// Generate the root CA
+	// Generate the root CA if necessary
-	ca, err := provider.generateCA(newState.PrivateKey, conf.RootCert, idx+1)
+	if conf.RootCert == "" {
-	if err != nil {
+		ca, err := provider.generateCA(newState.PrivateKey, idx+1)
-		return nil, fmt.Errorf("error generating CA: %v", err)
+		if err != nil {
+			return nil, fmt.Errorf("error generating CA: %v", err)
+		}
+		newState.RootCert = ca
+	} else {
+		newState.RootCert = conf.RootCert
 	}
-	newState.CARoot = ca
 
 	// Write the provider state
 	args := &structs.CARequest{
@@ -133,68 +136,33 @@ func decodeConfig(raw map[string]interface{}) (*ConsulCAProviderConfig, error) {
 }
 
 // Return the active root CA and generate a new one if needed
-func (c *ConsulCAProvider) ActiveRoot() (*structs.CARoot, error) {
+func (c *ConsulCAProvider) ActiveRoot() (string, error) {
 	state := c.srv.fsm.State()
 	_, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
-	return providerState.CARoot, nil
+	return providerState.RootCert, nil
 }
 
 // We aren't maintaining separate root/intermediate CAs for the builtin
 // provider, so just return the root.
-func (c *ConsulCAProvider) ActiveIntermediate() (*structs.CARoot, error) {
+func (c *ConsulCAProvider) ActiveIntermediate() (string, error) {
 	return c.ActiveRoot()
 }
 
 // We aren't maintaining separate root/intermediate CAs for the builtin
 // provider, so just generate a CSR for the active root.
-func (c *ConsulCAProvider) GenerateIntermediate() (*structs.CARoot, *x509.CertificateRequest, error) {
+func (c *ConsulCAProvider) GenerateIntermediate() (string, error) {
 	ca, err := c.ActiveIntermediate()
 	if err != nil {
-		return nil, nil, err
+		return "", err
 	}
 
-	state := c.srv.fsm.State()
+	// todo(kyhavlov): make a new intermediate here
-	_, providerState, err := state.CAProviderState(c.id)
-	if err != nil {
-		return nil, nil, err
-	}
-	_, config, err := state.CAConfig()
-	if err != nil {
-		return nil, nil, err
-	}
 
-	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
+	return ca, err
-	template := &x509.CertificateRequest{
-		URIs: []*url.URL{id.URI()},
-	}
-
-	signer, err := connect.ParseSigner(providerState.PrivateKey)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	// Create the CSR itself
-	var csrBuf bytes.Buffer
-	bs, err := x509.CreateCertificateRequest(rand.Reader, template, signer)
-	if err != nil {
-		return nil, nil, fmt.Errorf("error creating CSR: %s", err)
-	}
-
-	err = pem.Encode(&csrBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: bs})
-	if err != nil {
-		return nil, nil, fmt.Errorf("error encoding CSR: %s", err)
-	}
-
-	csr, err := connect.ParseCSR(csrBuf.String())
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return ca, csr, err
 }
 
 // Remove the state store entry for this provider instance.
@@ -216,7 +184,7 @@ func (c *ConsulCAProvider) Cleanup() error {
 
 // Sign returns a new certificate valid for the given SpiffeIDService
 // using the current CA.
-func (c *ConsulCAProvider) Sign(serviceId *connect.SpiffeIDService, csr *x509.CertificateRequest) (*structs.IssuedCert, error) {
+func (c *ConsulCAProvider) Sign(csr *x509.CertificateRequest) (*structs.IssuedCert, error) {
 	// Lock during the signing so we don't use the same index twice
 	// for different cert serial numbers.
 	c.Lock()
@@ -242,8 +210,18 @@ func (c *ConsulCAProvider) Sign(serviceId *connect.SpiffeIDService, csr *x509.Ce
 		return nil, err
 	}
 
+	// Parse the SPIFFE ID
+	spiffeId, err := connect.ParseCertURI(csr.URIs[0])
+	if err != nil {
+		return nil, err
+	}
+	serviceId, ok := spiffeId.(*connect.SpiffeIDService)
+	if !ok {
+		return nil, fmt.Errorf("SPIFFE ID in CSR must be a service ID")
+	}
+
 	// Parse the CA cert
-	caCert, err := connect.ParseCert(providerState.CARoot.RootCert)
+	caCert, err := connect.ParseCert(providerState.RootCert)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing CA cert: %s", err)
 	}
@@ -312,8 +290,8 @@ func (c *ConsulCAProvider) Sign(serviceId *connect.SpiffeIDService, csr *x509.Ce
 	}, nil
 }
 
-// SignCA returns an intermediate CA cert signed by the current active root.
+// CrossSignCA returns the given intermediate CA cert signed by the current active root.
-func (c *ConsulCAProvider) SignCA(csr *x509.CertificateRequest) (string, error) {
+func (c *ConsulCAProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
 	c.Lock()
 	defer c.Unlock()
 
@@ -329,44 +307,28 @@ func (c *ConsulCAProvider) SignCA(csr *x509.CertificateRequest) (string, error)
 		return "", fmt.Errorf("error parsing private key %q: %v", providerState.PrivateKey, err)
 	}
 
-	name := fmt.Sprintf("Consul cross-signed CA %d", providerState.LeafIndex+1)
+	rootCA, err := connect.ParseCert(providerState.RootCert)
-
-	// The URI (SPIFFE compatible) for the cert
-	_, config, err := state.CAConfig()
 	if err != nil {
 		return "", err
 	}
-	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
+
 	keyId, err := connect.KeyId(privKey.Public())
 	if err != nil {
 		return "", err
 	}
 
-	// Create the CA cert
+	// Create the cross-signing template from the existing root CA
 	serialNum := &big.Int{}
 	serialNum.SetUint64(providerState.LeafIndex + 1)
-	template := x509.Certificate{
+	template := *cert
-		SerialNumber:                serialNum,
+	template.SerialNumber = serialNum
-		Subject:                     pkix.Name{CommonName: name},
+	template.Subject = rootCA.Subject
-		URIs:                        csr.URIs,
+	template.SignatureAlgorithm = rootCA.SignatureAlgorithm
-		Signature:                   csr.Signature,
+	template.SubjectKeyId = keyId
-		PublicKeyAlgorithm:          csr.PublicKeyAlgorithm,
+	template.AuthorityKeyId = keyId
-		PublicKey:                   csr.PublicKey,
-		PermittedDNSDomainsCritical: true,
-		PermittedDNSDomains:         []string{id.URI().Hostname()},
-		BasicConstraintsValid:       true,
-		KeyUsage: x509.KeyUsageCertSign |
-			x509.KeyUsageCRLSign |
-			x509.KeyUsageDigitalSignature,
-		IsCA:           true,
-		NotAfter:       time.Now().Add(10 * 365 * 24 * time.Hour),
-		NotBefore:      time.Now(),
-		AuthorityKeyId: keyId,
-		SubjectKeyId:   keyId,
-	}
 
 	bs, err := x509.CreateCertificate(
-		rand.Reader, &template, &template, privKey.Public(), privKey)
+		rand.Reader, &template, rootCA, cert.PublicKey, privKey)
 	if err != nil {
 		return "", fmt.Errorf("error generating CA certificate: %s", err)
 	}
@@ -405,75 +367,58 @@ func generatePrivateKey() (string, error) {
 }
 
 // generateCA makes a new root CA using the current private key
-func (c *ConsulCAProvider) generateCA(privateKey, contents string, sn uint64) (*structs.CARoot, error) {
+func (c *ConsulCAProvider) generateCA(privateKey string, sn uint64) (string, error) {
 	state := c.srv.fsm.State()
 	_, config, err := state.CAConfig()
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
 	privKey, err := connect.ParseSigner(privateKey)
 	if err != nil {
-		return nil, fmt.Errorf("error parsing private key %q: %v", privateKey, err)
+		return "", fmt.Errorf("error parsing private key %q: %v", privateKey, err)
 	}
 
 	name := fmt.Sprintf("Consul CA %d", sn)
 
-	pemContents := contents
+	// The URI (SPIFFE compatible) for the cert
-
+	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
-	if pemContents == "" {
+	keyId, err := connect.KeyId(privKey.Public())
-		// The URI (SPIFFE compatible) for the cert
-		id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
-		keyId, err := connect.KeyId(privKey.Public())
-		if err != nil {
-			return nil, err
-		}
-
-		// Create the CA cert
-		serialNum := &big.Int{}
-		serialNum.SetUint64(sn)
-		template := x509.Certificate{
-			SerialNumber: serialNum,
-			Subject:      pkix.Name{CommonName: name},
-			URIs:         []*url.URL{id.URI()},
-			PermittedDNSDomainsCritical: true,
-			PermittedDNSDomains:         []string{id.URI().Hostname()},
-			BasicConstraintsValid:       true,
-			KeyUsage: x509.KeyUsageCertSign |
-				x509.KeyUsageCRLSign |
-				x509.KeyUsageDigitalSignature,
-			IsCA:           true,
-			NotAfter:       time.Now().Add(10 * 365 * 24 * time.Hour),
-			NotBefore:      time.Now(),
-			AuthorityKeyId: keyId,
-			SubjectKeyId:   keyId,
-		}
-
-		bs, err := x509.CreateCertificate(
-			rand.Reader, &template, &template, privKey.Public(), privKey)
-		if err != nil {
-			return nil, fmt.Errorf("error generating CA certificate: %s", err)
-		}
-
-		var buf bytes.Buffer
-		err = pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: bs})
-		if err != nil {
-			return nil, fmt.Errorf("error encoding private key: %s", err)
-		}
-
-		pemContents = buf.String()
-	}
-
-	// Generate an ID for the new CA cert
-	rootId, err := uuid.GenerateUUID()
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
-	return &structs.CARoot{
+	// Create the CA cert
-		ID:       rootId,
+	serialNum := &big.Int{}
-		Name:     name,
+	serialNum.SetUint64(sn)
-		RootCert: pemContents,
+	template := x509.Certificate{
-		Active:   true,
+		SerialNumber: serialNum,
-	}, nil
+		Subject:      pkix.Name{CommonName: name},
+		URIs:         []*url.URL{id.URI()},
+		PermittedDNSDomainsCritical: true,
+		PermittedDNSDomains:         []string{id.URI().Hostname()},
+		BasicConstraintsValid:       true,
+		KeyUsage: x509.KeyUsageCertSign |
+			x509.KeyUsageCRLSign |
+			x509.KeyUsageDigitalSignature,
+		IsCA:           true,
+		NotAfter:       time.Now().Add(10 * 365 * 24 * time.Hour),
+		NotBefore:      time.Now(),
+		AuthorityKeyId: keyId,
+		SubjectKeyId:   keyId,
+	}
+
+	bs, err := x509.CreateCertificate(
+		rand.Reader, &template, &template, privKey.Public(), privKey)
+	if err != nil {
+		return "", fmt.Errorf("error generating CA certificate: %s", err)
+	}
+
+	var buf bytes.Buffer
+	err = pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: bs})
+	if err != nil {
+		return "", fmt.Errorf("error encoding private key: %s", err)
+	}
+
+	return buf.String(), nil
 }

agent/consul/leader.go

@@ -214,7 +214,9 @@ func (s *Server) establishLeadership() error {
 	s.autopilot.Start()
 
 	// todo(kyhavlov): start a goroutine here for handling periodic CA rotation
-	s.initializeCA()
+	if err := s.initializeCA(); err != nil {
+		return err
+	}
 
 	s.setConsistentReadReady()
 	return nil
@@ -232,6 +234,8 @@ func (s *Server) revokeLeadership() error {
 		return err
 	}
 
+	s.setCAProvider(nil)
+
 	s.resetConsistentReadReady()
 	s.autopilot.Stop()
 	return nil
@@ -412,22 +416,33 @@ func (s *Server) initializeCA() error {
 	s.setCAProvider(provider)
 
 	// Get the active root cert from the CA
-	trustedCA, err := provider.ActiveRoot()
+	rootPEM, err := provider.ActiveRoot()
 	if err != nil {
 		return fmt.Errorf("error getting root cert: %v", err)
 	}
 
+	id, err := connect.ParseCertFingerprint(rootPEM)
+	if err != nil {
+		return fmt.Errorf("error parsing root fingerprint: %v", err)
+	}
+	rootCA := &structs.CARoot{
+		ID:       id,
+		Name:     fmt.Sprintf("%s CA Root Cert", conf.Provider),
+		RootCert: rootPEM,
+		Active:   true,
+	}
+
 	// Check if the CA root is already initialized and exit if it is.
 	// Every change to the CA after this initial bootstrapping should
 	// be done through the rotation process.
 	state := s.fsm.State()
-	_, root, err := state.CARootActive(nil)
+	_, activeRoot, err := state.CARootActive(nil)
 	if err != nil {
 		return err
 	}
-	if root != nil {
+	if activeRoot != nil {
-		if root.ID != trustedCA.ID {
+		if activeRoot.ID != rootCA.ID {
-			s.logger.Printf("[WARN] connect: CA root %q is not the active root (%q)", trustedCA.ID, root.ID)
+			s.logger.Printf("[WARN] connect: CA root %q is not the active root (%q)", rootCA.ID, activeRoot.ID)
 		}
 		return nil
 	}
@@ -442,7 +457,7 @@ func (s *Server) initializeCA() error {
 	resp, err := s.raftApply(structs.ConnectCARequestType, &structs.CARequest{
 		Op:    structs.CAOpSetRoots,
 		Index: idx,
-		Roots: []*structs.CARoot{trustedCA},
+		Roots: []*structs.CARoot{rootCA},
 	})
 	if err != nil {
 		s.logger.Printf("[ERR] connect: Apply failed %v", err)

agent/consul/state/connect_ca.go

@@ -62,7 +62,7 @@ func caRootTableSchema() *memdb.TableSchema {
 				Name:         "id",
 				AllowMissing: false,
 				Unique:       true,
-				Indexer: &memdb.UUIDFieldIndex{
+				Indexer: &memdb.StringFieldIndex{
 					Field: "ID",
 				},
 			},

agent/structs/connect_ca.go

@@ -164,7 +164,7 @@ type CAConfiguration struct {
 type CAConsulProviderState struct {
 	ID         string
 	PrivateKey string
-	CARoot     *CARoot
+	RootCert   string
 	LeafIndex  uint64
 
 	RaftIndex