connect/ca: tighten up the intermediate signing verification

agent/connect/ca/provider.go

@@ -23,12 +23,13 @@ type Provider interface {
 	ActiveRoot() (string, error)
 
 	// GenerateIntermediateCSR generates a CSR for an intermediate CA
-	// certificate, to be signed by the root of another datacenter. If isRoot is
-	// true, calling this is an error.
+	// certificate, to be signed by the root of another datacenter. If isRoot was
+	// set to true with Configure(), calling this is an error.
 	GenerateIntermediateCSR() (string, error)
 
 	// SetIntermediate sets the provider to use the given intermediate certificate
-	// as well as the root it was signed by.
+	// as well as the root it was signed by. This completes the initialization for
+	// a provider where isRoot was set to false in Configure().
 	SetIntermediate(intermediatePEM, rootPEM string) error
 
 	// ActiveIntermediate returns the current signing cert used by this provider

agent/connect/ca/provider_consul.go

@@ -189,7 +189,7 @@ func (c *ConsulProvider) GenerateIntermediateCSR() (string, error) {
 		return "", err
 	}
 
-	csr, err := connect.CreateCSR(c.spiffeID, signer)
+	csr, err := connect.CreateCACSR(c.spiffeID, signer)
 	if err != nil {
 		return "", err
 	}
@@ -249,6 +249,9 @@ func (c *ConsulProvider) SetIntermediate(intermediatePEM, rootPEM string) error
 	if !intermediate.IsCA {
 		return fmt.Errorf("intermediate is not a CA certificate")
 	}
+	if uriCount := len(intermediate.URIs); uriCount != 1 {
+		return fmt.Errorf("incoming intermediate cert has unexpected number of URIs: %d", uriCount)
+	}
 	if got, want := intermediate.URIs[0].String(), c.spiffeID.URI().String(); got != want {
 		return fmt.Errorf("incoming cert URI %q does not match current URI: %q", got, want)
 	}
@@ -420,15 +423,15 @@ func (c *ConsulProvider) SignIntermediate(csr *x509.CertificateRequest) (string,
 		return "", err
 	}
 
-	if len(csr.URIs) < 1 {
-		return "", fmt.Errorf("intermediate CSR has no URIs")
+	if uriCount := len(csr.URIs); uriCount != 1 {
+		return "", fmt.Errorf("incoming CSR has unexpected number of URIs: %d", uriCount)
 	}
 	certURI, err := connect.ParseCertURI(csr.URIs[0])
 	if err != nil {
 		return "", err
 	}
 
-	// Verify that the trust domain is valid
+	// Verify that the trust domain is valid.
 	if !c.spiffeID.CanSign(certURI) {
 		return "", fmt.Errorf("incoming CSR domain %q is not valid for our domain %q",
 			certURI.URI().String(), c.spiffeID.URI().String())
@@ -439,9 +442,6 @@ func (c *ConsulProvider) SignIntermediate(csr *x509.CertificateRequest) (string,
 	if err != nil {
 		return "", err
 	}
-	if signer == nil {
-		return "", ErrNotInitialized
-	}
 	subjectKeyId, err := connect.KeyId(csr.PublicKey)
 	if err != nil {
 		return "", err

agent/connect/ca/provider_consul_test.go

@@ -330,8 +330,8 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
 	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.
+	// Check that the leaf signed by the new cert can be verified using the
+	// returned cert chain (signed intermediate + remote root).
 	intermediatePool := x509.NewCertPool()
 	intermediatePool.AppendCertsFromPEM([]byte(intermediatePEM))
 	rootPool := x509.NewCertPool()

agent/connect/ca/provider_vault.go

@@ -148,6 +148,7 @@ func (v *VaultProvider) generateIntermediateCSR() (string, error) {
 			"allowed_uri_sans": "spiffe://*",
 			"key_type":         "any",
 			"max_ttl":          v.config.LeafCertTTL.String(),
+			"no_store":         true,
 			"require_cn":       false,
 		})
 		if err != nil {
@@ -158,6 +159,8 @@ func (v *VaultProvider) generateIntermediateCSR() (string, error) {
 	// Generate a new intermediate CSR for the root to sign.
 	data, err := v.client.Logical().Write(v.config.IntermediatePKIPath+"intermediate/generate/internal", map[string]interface{}{
 		"common_name": "Vault CA Intermediate Authority",
+		"key_bits":    224,
+		"key_type":    "ec",
 		"uri_sans":    spiffeID.URI().String(),
 	})
 	if err != nil {

agent/connect/ca/provider_vault_test.go

@@ -196,14 +196,34 @@ func TestVaultProvider_SignIntermediate(t *testing.T) {
 func TestVaultProvider_SignIntermediateConsul(t *testing.T) {
 	t.Parallel()
 
-	provider1, core1, listener1 := testVaultCluster(t)
-	defer core1.Shutdown()
-	defer listener1.Close()
+	require := require.New(t)
 
-	conf := testConsulCAConfig()
-	delegate := newMockDelegate(t, conf)
-	provider2 := &ConsulProvider{Delegate: delegate}
-	require.NoError(t, provider2.Configure(conf.ClusterID, false, conf.Config))
+	// primary = Vault, secondary = Consul
+	{
+		provider1, core, listener := testVaultCluster(t)
+		defer core.Shutdown()
+		defer listener.Close()
 
-	testSignIntermediateCrossDC(t, provider1, provider2)
+		conf := testConsulCAConfig()
+		delegate := newMockDelegate(t, conf)
+		provider2 := &ConsulProvider{Delegate: delegate}
+		require.NoError(provider2.Configure(conf.ClusterID, false, conf.Config))
+
+		testSignIntermediateCrossDC(t, provider1, provider2)
+	}
+
+	// primary = Consul, secondary = Vault
+	{
+		conf := testConsulCAConfig()
+		delegate := newMockDelegate(t, conf)
+		provider1 := &ConsulProvider{Delegate: delegate}
+		require.NoError(provider1.Configure(conf.ClusterID, true, conf.Config))
+		require.NoError(provider1.GenerateRoot())
+
+		provider2, core, listener := testVaultClusterWithConfig(t, false, nil)
+		defer core.Shutdown()
+		defer listener.Close()
+
+		testSignIntermediateCrossDC(t, provider1, provider2)
+	}
 }

agent/connect/csr.go

@@ -5,16 +5,19 @@ import (
 	"crypto"
 	"crypto/rand"
 	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/asn1"
 	"encoding/pem"
 	"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) {
+func CreateCSR(uri CertURI, privateKey crypto.Signer, extensions ...pkix.Extension) (string, error) {
 	template := &x509.CertificateRequest{
 		URIs:               []*url.URL{uri.URI()},
 		SignatureAlgorithm: x509.ECDSAWithSHA256,
+		ExtraExtensions:    extensions,
 	}
 
 	// Create the CSR itself
@@ -31,3 +34,34 @@ func CreateCSR(uri CertURI, privateKey crypto.Signer) (string, error) {
 
 	return csrBuf.String(), nil
 }
+
+// CreateCSR returns a CA CSR to sign the given service along with the PEM-encoded
+// private key for this certificate.
+func CreateCACSR(uri CertURI, privateKey crypto.Signer) (string, error) {
+	ext, err := CreateCAExtension()
+	if err != nil {
+		return "", err
+	}
+
+	return CreateCSR(uri, privateKey, ext)
+}
+
+// CreateCAExtension creates a pkix.Extension for the x509 Basic Constraints
+// IsCA field ()
+func CreateCAExtension() (pkix.Extension, error) {
+	type basicConstraints struct {
+		IsCA       bool `asn1:"optional"`
+		MaxPathLen int  `asn1:"optional"`
+	}
+	basicCon := basicConstraints{IsCA: true, MaxPathLen: 0}
+	bitstr, err := asn1.Marshal(basicCon)
+	if err != nil {
+		return pkix.Extension{}, err
+	}
+
+	return pkix.Extension{
+		Id:       []int{2, 5, 29, 19}, // from x509 package
+		Critical: true,
+		Value:    bitstr,
+	}, nil
+}