From c6e1b72ccbf159864cc821d54b9bcad3b60bb3b0 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 24 Apr 2018 16:16:37 -0700 Subject: [PATCH] Simplify the CA provider interface by moving some logic out --- agent/connect/ca.go | 31 +++- agent/connect/ca_provider.go | 23 ++- agent/consul/connect_ca_endpoint.go | 35 +++-- agent/consul/connect_ca_provider.go | 217 +++++++++++----------------- agent/consul/leader.go | 29 +++- agent/consul/state/connect_ca.go | 2 +- agent/structs/connect_ca.go | 2 +- 7 files changed, 169 insertions(+), 170 deletions(-) diff --git a/agent/connect/ca.go b/agent/connect/ca.go index 818af9f9f4..87b01994ef 100644 --- a/agent/connect/ca.go +++ b/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) - if !ok { + switch raw.(type) { + 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 } diff --git a/agent/connect/ca_provider.go b/agent/connect/ca_provider.go index cb72196696..1eb5bde11f 100644 --- a/agent/connect/ca_provider.go +++ b/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 - // cross-signing CSR for it and sets it to the active intermediate. - GenerateIntermediate() (*structs.CARoot, *x509.CertificateRequest, error) + // GenerateIntermediate returns a new intermediate signing cert and + // sets it to the active intermediate. + 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. - SignCA(*x509.CertificateRequest) (string, error) + // CrossSignCA must accept a CA certificate signed by another CA's key + // 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 diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 5e3c2f6c7e..b0041423ed 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/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 } diff --git a/agent/consul/connect_ca_provider.go b/agent/consul/connect_ca_provider.go index 8a3c81b2b2..2f32a3d67a 100644 --- a/agent/consul/connect_ca_provider.go +++ b/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 - ca, err := provider.generateCA(newState.PrivateKey, conf.RootCert, idx+1) - if err != nil { - return nil, fmt.Errorf("error generating CA: %v", err) + // Generate the root CA if necessary + if conf.RootCert == "" { + ca, err := provider.generateCA(newState.PrivateKey, idx+1) + 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() - _, providerState, err := state.CAProviderState(c.id) - if err != nil { - return nil, nil, err - } - _, config, err := state.CAConfig() - if err != nil { - return nil, nil, err - } + // todo(kyhavlov): make a new intermediate here - id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"} - 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 + return ca, 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. -func (c *ConsulCAProvider) SignCA(csr *x509.CertificateRequest) (string, error) { +// CrossSignCA returns the given intermediate CA cert signed by the current active root. +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) - - // The URI (SPIFFE compatible) for the cert - _, config, err := state.CAConfig() + rootCA, err := connect.ParseCert(providerState.RootCert) 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{ - SerialNumber: serialNum, - Subject: pkix.Name{CommonName: name}, - URIs: csr.URIs, - Signature: csr.Signature, - PublicKeyAlgorithm: csr.PublicKeyAlgorithm, - 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, - } + template := *cert + template.SerialNumber = serialNum + template.Subject = rootCA.Subject + template.SignatureAlgorithm = rootCA.SignatureAlgorithm + template.SubjectKeyId = keyId + template.AuthorityKeyId = 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 - - if pemContents == "" { - // 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() + // 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 + return "", err } - return &structs.CARoot{ - ID: rootId, - Name: name, - RootCert: pemContents, - Active: true, - }, nil + // 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 "", 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 } diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 282393cd3e..1670add29e 100644 --- a/agent/consul/leader.go +++ b/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 root.ID != trustedCA.ID { - s.logger.Printf("[WARN] connect: CA root %q is not the active root (%q)", trustedCA.ID, root.ID) + if activeRoot != nil { + if activeRoot.ID != rootCA.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) diff --git a/agent/consul/state/connect_ca.go b/agent/consul/state/connect_ca.go index 99986c891c..7c4cea2945 100644 --- a/agent/consul/state/connect_ca.go +++ b/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", }, }, diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 39c46f0c44..4562fc1ea8 100644 --- a/agent/structs/connect_ca.go +++ b/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