connect/ca: undo the interface changes and use sign-self-issued in Vault

agent/connect/ca/provider.go

@@ -32,12 +32,7 @@ type Provider interface {
 	// intemediate and any cross-signed intermediates managed by Consul.
 	Sign(*x509.CertificateRequest) (string, error)
 
-	// GetCrossSigningCSR returns a CSR that can be signed by another root
+	// CrossSignCA must accept a CA certificate from another CA provider
-	// to create an intermediate cert that forms a chain of trust back to
-	// that root CA.
-	GetCrossSigningCSR() (*x509.CertificateRequest, error)
-
-	// CrossSignCA must accept a CA CSR 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.
@@ -45,7 +40,7 @@ type Provider interface {
 	// 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.CertificateRequest) (string, error)
+	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/connect/ca/provider_consul.go

@@ -242,53 +242,8 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 	return buf.String(), nil
 }
 
-// GetCrossSigningCSR creates a CSR from our root CA certificate to be signed
+// CrossSignCA returns the given CA cert signed by the current active root.
-// by another CA provider.
+func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
-func (c *ConsulProvider) GetCrossSigningCSR() (*x509.CertificateRequest, error) {
-	c.RLock()
-	defer c.RUnlock()
-
-	// Get the provider state
-	state := c.delegate.State()
-	_, providerState, err := state.CAProviderState(c.id)
-	if err != nil {
-		return nil, err
-	}
-	privKey, err := connect.ParseSigner(providerState.PrivateKey)
-	if err != nil {
-		return nil, err
-	}
-	rootCA, err := connect.ParseCert(providerState.RootCert)
-	if err != nil {
-		return nil, err
-	}
-
-	// Create the CSR
-	template := &x509.CertificateRequest{
-		DNSNames:           rootCA.DNSNames,
-		EmailAddresses:     rootCA.EmailAddresses,
-		IPAddresses:        rootCA.IPAddresses,
-		URIs:               rootCA.URIs,
-		SignatureAlgorithm: rootCA.SignatureAlgorithm,
-		Subject:            rootCA.Subject,
-		Extensions:         rootCA.Extensions,
-		ExtraExtensions:    rootCA.ExtraExtensions,
-	}
-
-	bs, err := x509.CreateCertificateRequest(rand.Reader, template, privKey)
-	if err != nil {
-		return nil, err
-	}
-	csr, err := x509.ParseCertificateRequest(bs)
-	if err != nil {
-		return nil, err
-	}
-
-	return csr, nil
-}
-
-// CrossSignCA returns the given intermediate CA cert signed by the current active root.
-func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, error) {
 	c.Lock()
 	defer c.Unlock()
 
@@ -309,12 +264,7 @@ func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, erro
 		return "", err
 	}
 
-	authKeyId, err := connect.KeyId(privKey.Public())
+	keyId, err := connect.KeyId(privKey.Public())
-	if err != nil {
-		return "", err
-	}
-
-	subjKeyId, err := connect.KeyId(csr.PublicKey)
 	if err != nil {
 		return "", err
 	}
@@ -322,26 +272,10 @@ func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, erro
 	// Create the cross-signing template from the existing root CA
 	serialNum := &big.Int{}
 	serialNum.SetUint64(idx + 1)
-	template := &x509.Certificate{
+	template := *cert
-		SerialNumber:          serialNum,
+	template.SerialNumber = serialNum
-		SignatureAlgorithm:    rootCA.SignatureAlgorithm,
+	template.SignatureAlgorithm = rootCA.SignatureAlgorithm
-		DNSNames:              csr.DNSNames,
+	template.AuthorityKeyId = keyId
-		EmailAddresses:        csr.EmailAddresses,
-		IPAddresses:           csr.IPAddresses,
-		URIs:                  csr.URIs,
-		SubjectKeyId:          subjKeyId,
-		AuthorityKeyId:        authKeyId,
-		Subject:               csr.Subject,
-		Extensions:            csr.Extensions,
-		ExtraExtensions:       csr.ExtraExtensions,
-		BasicConstraintsValid: true,
-		KeyUsage: x509.KeyUsageCertSign |
-			x509.KeyUsageCRLSign |
-			x509.KeyUsageDigitalSignature,
-		IsCA:      true,
-		NotAfter:  time.Now().Add(7 * 24 * time.Hour),
-		NotBefore: time.Now(),
-	}
 
 	// Sign the certificate valid from 1 minute in the past, this helps it be
 	// accepted right away even when nodes are not in close time sync accross the
@@ -355,7 +289,7 @@ func (c *ConsulProvider) CrossSignCA(csr *x509.CertificateRequest) (string, erro
 	template.NotAfter = effectiveNow.Add(7 * 24 * time.Hour)
 
 	bs, err := x509.CreateCertificate(
-		rand.Reader, template, rootCA, csr.PublicKey, privKey)
+		rand.Reader, &template, rootCA, cert.PublicKey, privKey)
 	if err != nil {
 		return "", fmt.Errorf("error generating CA certificate: %s", err)
 	}

agent/connect/ca/provider_consul_test.go

@@ -179,46 +179,55 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 func TestConsulCAProvider_CrossSignCA(t *testing.T) {
 	t.Parallel()
 
-	require := require.New(t)
-
 	conf1 := testConsulCAConfig()
 	delegate1 := newMockDelegate(t, conf1)
 	provider1, err := NewConsulProvider(conf1.Config, delegate1)
+	require.NoError(t, err)
 
 	conf2 := testConsulCAConfig()
 	conf2.CreateIndex = 10
 	delegate2 := newMockDelegate(t, conf2)
 	provider2, err := NewConsulProvider(conf2.Config, delegate2)
+	require.NoError(t, err)
+
+	testCrossSignProviders(t, provider1, provider2)
+}
+
+func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
+	require := require.New(t)
+
+	// Get the root from the new provider to be cross-signed.
+	newRootPEM, err := provider2.ActiveRoot()
+	require.NoError(err)
+	newRoot, err := connect.ParseCert(newRootPEM)
+	require.NoError(err)
+	oldSubject := newRoot.Subject.CommonName
+
+	newInterPEM, err := provider2.ActiveIntermediate()
+	require.NoError(err)
+	newIntermediate, err := connect.ParseCert(newInterPEM)
 	require.NoError(err)
 
-	require.NoError(err)
+	// Have provider1 cross sign our new root cert.
-
+	xcPEM, err := provider1.CrossSignCA(newRoot)
-	// Have provider2 generate a cross-signing CSR
-	csr, err := provider2.GetCrossSigningCSR()
-	require.NoError(err)
-	oldSubject := csr.Subject.CommonName
-
-	// Have provider1 cross sign our new CA cert.
-	xcPEM, err := provider1.CrossSignCA(csr)
 	require.NoError(err)
 	xc, err := connect.ParseCert(xcPEM)
 	require.NoError(err)
 
-	rootPEM, err := provider1.ActiveRoot()
+	oldRootPEM, err := provider1.ActiveRoot()
 	require.NoError(err)
-	root, err := connect.ParseCert(rootPEM)
+	oldRoot, err := connect.ParseCert(oldRootPEM)
 	require.NoError(err)
 
-	// AuthorityKeyID should be the signing root's, SubjectKeyId should be different.
+	// AuthorityKeyID should now be the signing root's, SubjectKeyId should be kept.
-	require.Equal(root.AuthorityKeyId, xc.AuthorityKeyId)
+	require.Equal(oldRoot.AuthorityKeyId, xc.AuthorityKeyId)
-	require.NotEqual(root.SubjectKeyId, xc.SubjectKeyId)
+	require.Equal(newRoot.SubjectKeyId, xc.SubjectKeyId)
 
 	// Subject name should not have changed.
-	require.NotEqual(root.Subject.CommonName, xc.Subject.CommonName)
 	require.Equal(oldSubject, xc.Subject.CommonName)
 
 	// Issuer should be the signing root.
-	require.Equal(root.Issuer.CommonName, xc.Issuer.CommonName)
+	require.Equal(oldRoot.Issuer.CommonName, xc.Issuer.CommonName)
 
 	// Get a leaf cert so we can verify against the cross-signed cert.
 	spiffeService := &connect.SpiffeIDService{
@@ -238,18 +247,20 @@ func TestConsulCAProvider_CrossSignCA(t *testing.T) {
 	cert, err := connect.ParseCert(leafPEM)
 	require.NoError(err)
 
+	// Check that the leaf signed by the new cert can be verified by either root
+	// certificate by using the new intermediate + cross-signed cert.
 	intermediatePool := x509.NewCertPool()
+	intermediatePool.AddCert(newIntermediate)
 	intermediatePool.AddCert(xc)
 
-	rootPool := x509.NewCertPool()
+	for _, root := range []*x509.Certificate{oldRoot, newRoot} {
-	rootPool.AddCert(root)
+		rootPool := x509.NewCertPool()
+		rootPool.AddCert(root)
 
-	// Check that the leaf signed by the new cert can be verified by the
+		_, err = cert.Verify(x509.VerifyOptions{
-	// chain of cross-signed cert + old root (as would be the case on any
+			Intermediates: intermediatePool,
-	// proxies that haven't received the new root yet) for backwards compatibility.
+			Roots:         rootPool,
-	_, err = cert.Verify(x509.VerifyOptions{
+		})
-		Intermediates: intermediatePool,
+		require.NoError(err)
-		Roots:         rootPool,
+	}
-	})
-	require.NoError(err)
 }

agent/connect/ca/provider_vault.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/go-uuid"
 	vaultapi "github.com/hashicorp/vault/api"
 	"github.com/mitchellh/mapstructure"
 )
@@ -72,8 +73,13 @@ func NewVaultProvider(rawConfig map[string]interface{}, clusterId string) (*Vaul
 		fallthrough
 	case ErrBackendNotInitialized:
 		spiffeID := connect.SpiffeIDSigning{ClusterID: clusterId, Domain: "consul"}
-		_, err := client.Logical().Write(conf.RootPKIPath+"root/generate/internal", map[string]interface{}{
+		uuid, err := uuid.GenerateUUID()
-			"uri_sans": spiffeID.URI().String(),
+		if err != nil {
+			return nil, err
+		}
+		_, err = client.Logical().Write(conf.RootPKIPath+"root/generate/internal", map[string]interface{}{
+			"common_name": fmt.Sprintf("Vault CA Root Authority %s", uuid),
+			"uri_sans":    spiffeID.URI().String(),
 		})
 		if err != nil {
 			return nil, err
@@ -167,6 +173,7 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) {
 			"allowed_uri_sans": "spiffe://*",
 			"key_type":         "any",
 			"max_ttl":          "72h",
+			"require_cn":       false,
 		})
 		if err != nil {
 			return "", err
@@ -240,48 +247,18 @@ func (v *VaultProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 	return fmt.Sprintf("%s\n%s", cert, ca), nil
 }
 
-// GetCrossSigningCSR creates a CSR for an intermediate CA certificate to be signed
+// CrossSignCA takes a CA certificate and cross-signs it to form a trust chain
-// by another CA provider.
+// back to our active root.
-func (v *VaultProvider) GetCrossSigningCSR() (*x509.CertificateRequest, error) {
+func (v *VaultProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
-	// Generate a new intermediate CSR for the root to sign.
+	var pemBuf bytes.Buffer
-	spiffeID := connect.SpiffeIDSigning{ClusterID: v.clusterId, Domain: "consul"}
+	err := pem.Encode(&pemBuf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
-	csr, err := v.client.Logical().Write(v.config.IntermediatePKIPath+"intermediate/generate/internal", map[string]interface{}{
-		"common_name": "Vault CA Intermediate Authority",
-		"uri_sans":    spiffeID.URI().String(),
-	})
-	if err != nil {
-		return nil, err
-	}
-	if csr == nil || csr.Data["csr"] == "" {
-		return nil, fmt.Errorf("got empty value when generating intermediate CSR")
-	}
-
-	// Return the parsed CSR.
-	pem, ok := csr.Data["csr"].(string)
-	if !ok {
-		return nil, fmt.Errorf("CSR was not a string")
-	}
-	result, err := connect.ParseCSR(pem)
-	if err != nil {
-		return nil, err
-	}
-
-	return result, nil
-}
-
-// CrossSignCA creates a CSR from the given CA certificate and uses the
-// configured root PKI backend to issue a cert from the request.
-func (v *VaultProvider) CrossSignCA(cert *x509.CertificateRequest) (string, error) {
-	var csrBuf bytes.Buffer
-	err := pem.Encode(&csrBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: cert.Raw})
 	if err != nil {
 		return "", err
 	}
 
-	// Have the root PKI backend sign this CSR.
+	// Have the root PKI backend sign this cert.
-	response, err := v.client.Logical().Write(v.config.RootPKIPath+"root/sign-intermediate", map[string]interface{}{
+	response, err := v.client.Logical().Write(v.config.RootPKIPath+"root/sign-self-issued", map[string]interface{}{
-		"csr":            csrBuf.String(),
+		"certificate": pemBuf.String(),
-		"use_csr_values": true,
 	})
 	if err != nil {
 		return "", fmt.Errorf("error having Vault cross-sign cert: %v", err)

agent/connect/ca/provider_vault_test.go

@@ -1,7 +1,6 @@
 package ca
 
 import (
-	"crypto/x509"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -118,7 +117,6 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) {
 		parsed, err := connect.ParseCert(cert)
 		require.NoError(err)
 		require.Equal(parsed.URIs[0], spiffeService.URI())
-		require.Equal(parsed.Subject.CommonName, "foo")
 		firstSerial = parsed.SerialNumber.Uint64()
 
 		// Ensure the cert is valid now and expires within the correct limit.
@@ -141,7 +139,6 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) {
 		parsed, err := connect.ParseCert(cert)
 		require.NoError(err)
 		require.Equal(parsed.URIs[0], spiffeService.URI())
-		require.Equal(parsed.Subject.CommonName, "bar")
 		require.NotEqual(firstSerial, parsed.SerialNumber.Uint64())
 
 		// Ensure the cert is valid now and expires within the correct limit.
@@ -153,7 +150,6 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) {
 func TestVaultCAProvider_CrossSignCA(t *testing.T) {
 	t.Parallel()
 
-	require := require.New(t)
 	provider1, core1, listener1 := testVaultCluster(t)
 	defer core1.Shutdown()
 	defer listener1.Close()
@@ -162,65 +158,5 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) {
 	defer core2.Shutdown()
 	defer listener2.Close()
 
-	// Have provider2 generate a cross-signing CSR
+	testCrossSignProviders(t, provider1, provider2)
-	csr, err := provider2.GetCrossSigningCSR()
-	require.NoError(err)
-	oldSubject := csr.Subject.CommonName
-	_, err = provider2.GenerateIntermediate()
-	require.NoError(err)
-
-	// Have provider1 cross sign our new CA cert.
-	xcPEM, err := provider1.CrossSignCA(csr)
-	require.NoError(err)
-	xc, err := connect.ParseCert(xcPEM)
-	require.NoError(err)
-
-	rootPEM, err := provider1.ActiveRoot()
-	require.NoError(err)
-	root, err := connect.ParseCert(rootPEM)
-	require.NoError(err)
-
-	// AuthorityKeyID should be the signing root's, SubjectKeyId should be different.
-	require.Equal(root.AuthorityKeyId, xc.AuthorityKeyId)
-	require.NotEqual(root.SubjectKeyId, xc.SubjectKeyId)
-
-	// Subject name should not have changed.
-	require.NotEqual(root.Subject.CommonName, xc.Subject.CommonName)
-	require.Equal(oldSubject, xc.Subject.CommonName)
-
-	// Issuer should be the signing root.
-	require.Equal(root.Issuer.CommonName, xc.Issuer.CommonName)
-
-	// Get a leaf cert so we can verify against the cross-signed cert.
-	spiffeService := &connect.SpiffeIDService{
-		Host:       "node1",
-		Namespace:  "default",
-		Datacenter: "dc1",
-		Service:    "foo",
-	}
-	raw, _ := connect.TestCSR(t, spiffeService)
-
-	leafCsr, err := connect.ParseCSR(raw)
-	require.NoError(err)
-
-	leafPEM, err := provider2.Sign(leafCsr)
-	require.NoError(err)
-
-	cert, err := connect.ParseCert(leafPEM)
-	require.NoError(err)
-
-	intermediatePool := x509.NewCertPool()
-	intermediatePool.AddCert(xc)
-
-	rootPool := x509.NewCertPool()
-	rootPool.AddCert(root)
-
-	// Check that the leaf signed by the new cert can be verified by the
-	// chain of cross-signed cert + old root (as would be the case on any
-	// proxies that haven't received the new root yet) for backwards compatibility.
-	_, err = cert.Verify(x509.VerifyOptions{
-		Intermediates: intermediatePool,
-		Roots:         rootPool,
-	})
-	require.NoError(err)
 }

agent/connect/csr.go

@@ -5,24 +5,16 @@ import (
 	"crypto"
 	"crypto/rand"
 	"crypto/x509"
-	"crypto/x509/pkix"
 	"encoding/pem"
-	"fmt"
 	"net/url"
 )
 
 // CreateCSR returns a CSR to sign the given service along with the PEM-encoded
 // private key for this certificate.
 func CreateCSR(uri CertURI, privateKey crypto.Signer) (string, error) {
-	serviceId, ok := uri.(*SpiffeIDService)
-	if !ok {
-		return "", fmt.Errorf("SPIFFE ID in CSR must be a service ID")
-	}
-
 	template := &x509.CertificateRequest{
 		URIs:               []*url.URL{uri.URI()},
 		SignatureAlgorithm: x509.ECDSAWithSHA256,
-		Subject:            pkix.Name{CommonName: serviceId.Service},
 	}
 
 	// Create the CSR itself

agent/connect/testing_ca.go

@@ -201,15 +201,9 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) (string, string
 // TestCSR returns a CSR to sign the given service along with the PEM-encoded
 // private key for this certificate.
 func TestCSR(t testing.T, uri CertURI) (string, string) {
-	serviceId, ok := uri.(*SpiffeIDService)
-	if !ok {
-		t.Fatalf("SPIFFE ID in CSR must be a service ID")
-	}
-
 	template := &x509.CertificateRequest{
 		URIs:               []*url.URL{uri.URI()},
 		SignatureAlgorithm: x509.ECDSAWithSHA256,
-		Subject:            pkix.Name{CommonName: serviceId.Service},
 	}
 
 	// Create the private key we'll use

agent/consul/connect_ca_endpoint.go

@@ -135,25 +135,35 @@ func (s *ConnectCA) ConfigurationSet(
 	// to use a different root certificate.
 
 	// If it's a config change that would trigger a rotation (different provider/root):
-	// 1. Get the intermediate from the new provider
+	// 1. Get the root from the new provider.
-	// 2. Generate a CSR for the new intermediate, call SignCA on the old/current provider
+	// 2. Call CrossSignCA on the old provider to sign the new root with the old one to
-	// to get the cross-signed intermediate
+	// get a cross-signed certificate.
-	// 3. Get the active root for the new provider, append the intermediate from step 3
+	// 3. Take the active root for the new provider and append the intermediate from step 2
-	// to its list of intermediates
+	// to its list of intermediates.
-	csr, err := newProvider.GetCrossSigningCSR()
+	newRoot, err := connect.ParseCert(newRootPEM)
+	if err != nil {
+		return err
+	}
 
 	// Have the old provider cross-sign the new intermediate
 	oldProvider, _ := s.srv.getCAProvider()
 	if oldProvider == nil {
 		return fmt.Errorf("internal error: CA provider is nil")
 	}
-	xcCert, err := oldProvider.CrossSignCA(csr)
+	xcCert, err := oldProvider.CrossSignCA(newRoot)
 	if err != nil {
 		return err
 	}
 
-	// Add the cross signed cert to the new root's intermediates
+	// Add the cross signed cert to the new root's intermediates.
 	newActiveRoot.IntermediateCerts = []string{xcCert}
+	intermediate, err := newProvider.GenerateIntermediate()
+	if err != nil {
+		return err
+	}
+	if intermediate != newRootPEM {
+		newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediate)
+	}
 
 	// Update the roots and CA config in the state store at the same time
 	idx, roots, err := state.CARoots(nil)

agent/dns.go

@@ -790,8 +790,8 @@ func (d *DNSServer) trimTCPResponse(req, resp *dns.Msg) (trimmed bool) {
 	originalNumRecords := len(resp.Answer)
 
 	// It is not possible to return more than 4k records even with compression
-        // Since we are performing binary search it is not a big deal, but it
+	// Since we are performing binary search it is not a big deal, but it
-        // improves a bit performance, even with binary search
+	// improves a bit performance, even with binary search
 	truncateAt := 4096
 	if req.Question[0].Qtype == dns.TypeSRV {
 		// More than 1024 SRV records do not fit in 64k