connect/ca: fix vault provider URI SANs and test

agent/connect/ca/provider_vault.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	vaultapi "github.com/hashicorp/vault/api"
 	"github.com/mitchellh/mapstructure"
@@ -29,24 +30,22 @@ type VaultProvider struct {
 // backends mounted and initialized. If the root backend is not set up already,
 // it will be mounted/generated as needed, but any existing state will not be
 // overwritten.
-func NewVaultProvider(rawConfig map[string]interface{}, clusterId string, client *vaultapi.Client) (*VaultProvider, error) {
+func NewVaultProvider(rawConfig map[string]interface{}, clusterId string) (*VaultProvider, error) {
 	conf, err := ParseVaultCAConfig(rawConfig)
 	if err != nil {
 		return nil, err
 	}
 
 	// todo(kyhavlov): figure out the right way to pass the TLS config
-	if client == nil {
 	clientConf := &vaultapi.Config{
 		Address: conf.Address,
 	}
-		client, err = vaultapi.NewClient(clientConf)
+	client, err := vaultapi.NewClient(clientConf)
 	if err != nil {
 		return nil, err
 	}
 
 	client.SetToken(conf.Token)
-	}
 
 	provider := &VaultProvider{
 		config:    conf,
@@ -72,8 +71,9 @@ func NewVaultProvider(rawConfig map[string]interface{}, clusterId string, client
 
 		fallthrough
 	case ErrBackendNotInitialized:
+		spiffeID := connect.SpiffeIDSigning{ClusterID: clusterId, Domain: "consul"}
 		_, err := client.Logical().Write(conf.RootPKIPath+"root/generate/internal", map[string]interface{}{
-			"alt_names": fmt.Sprintf("URI:spiffe://%s.consul", clusterId),
+			"uri_sans": spiffeID.URI().String(),
 		})
 		if err != nil {
 			return nil, err
@@ -160,9 +160,11 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) {
 	if err != nil {
 		return "", err
 	}
+	spiffeID := connect.SpiffeIDSigning{ClusterID: v.clusterId, Domain: "consul"}
 	if role == nil {
 		_, err := v.client.Logical().Write(rolePath, map[string]interface{}{
-			"allowed_domains": fmt.Sprintf("%s.consul", v.clusterId),
+			"allowed_domains":  spiffeID.Host(),
+			"allowed_uri_sans": "spiffe://*",
 			"max_ttl":          "72h",
 		})
 		if err != nil {
@@ -173,7 +175,7 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) {
 	// Generate a new intermediate CSR for the root to sign.
 	csr, err := v.client.Logical().Write(v.config.IntermediatePKIPath+"intermediate/generate/internal", map[string]interface{}{
 		"common_name": "Vault CA Intermediate Authority",
-		"alt_names":   fmt.Sprintf("URI:spiffe://%s.consul", v.clusterId),
+		"uri_sans":    spiffeID.URI().String(),
 	})
 	if err != nil {
 		return "", err

agent/connect/ca/provider_vault_test.go

@@ -1,52 +1,51 @@
 package ca
 
 import (
-	"fmt"
 	"io/ioutil"
+	"net"
 	"testing"
 
 	"github.com/hashicorp/consul/agent/connect"
+	vaultapi "github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/builtin/logical/pki"
 	vaulthttp "github.com/hashicorp/vault/http"
-	"github.com/hashicorp/vault/logical"
 	"github.com/hashicorp/vault/vault"
 	"github.com/stretchr/testify/require"
 )
 
-func testVaultCluster(t *testing.T) (*VaultProvider, *vault.TestCluster) {
-	coreConfig := &vault.CoreConfig{
-		LogicalBackends: map[string]logical.Factory{
-			"pki": pki.Factory,
-		},
+func testVaultCluster(t *testing.T) (*VaultProvider, *vault.Core, net.Listener) {
+	if err := vault.AddTestLogicalBackend("pki", pki.Factory); err != nil {
+		t.Fatal(err)
 	}
-	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
-		HandlerFunc: vaulthttp.Handler,
-		NumCores:    1,
-	})
-	cluster.Start()
+	core, _, token := vault.TestCoreUnsealedRaw(t)
 
-	client := cluster.Cores[0].Client
+	ln, addr := vaulthttp.TestServer(t, core)
 
 	provider, err := NewVaultProvider(map[string]interface{}{
-		"Address":             client.Address(),
-		"Token":               cluster.RootToken,
+		"Address":             addr,
+		"Token":               token,
 		"RootPKIPath":         "pki-root/",
 		"IntermediatePKIPath": "pki-intermediate/",
-	}, "asdf", client)
+	}, "asdf")
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	return provider, cluster
+	return provider, core, ln
 }
 
 func TestVaultCAProvider_Bootstrap(t *testing.T) {
 	t.Parallel()
 
 	require := require.New(t)
-	provider, vaultCluster := testVaultCluster(t)
-	defer vaultCluster.Cleanup()
-	client := vaultCluster.Cores[0].Client
+	provider, core, listener := testVaultCluster(t)
+	defer core.Shutdown()
+	defer listener.Close()
+	client, err := vaultapi.NewClient(&vaultapi.Config{
+		Address: "http://" + listener.Addr().String(),
+	})
+	require.NoError(err)
+	client.SetToken(provider.config.Token)
 
 	cases := []struct {
 		certFunc    func() (string, error)
@@ -66,16 +65,16 @@ func TestVaultCAProvider_Bootstrap(t *testing.T) {
 	for _, tc := range cases {
 		cert, err := tc.certFunc()
 		require.NoError(err)
-		req := client.NewRequest("GET", "v1/"+tc.backendPath+"ca/pem")
+		req := client.NewRequest("GET", "/v1/"+tc.backendPath+"ca/pem")
 		resp, err := client.RawRequest(req)
 		require.NoError(err)
 		bytes, err := ioutil.ReadAll(resp.Body)
 		require.NoError(err)
 		require.Equal(cert, string(bytes))
 
-		// Should be a valid cert
+		// Should be a valid CA cert
 		parsed, err := connect.ParseCert(cert)
 		require.NoError(err)
-		require.Equal(parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", provider.clusterId))
+		require.True(parsed.IsCA)
 	}
 }

agent/connect_ca_endpoint.go

@@ -83,11 +83,15 @@ func fixupConfig(conf *structs.CAConfiguration) {
 		if raw, ok := v.([]uint8); ok {
 			conf.Config[k] = ca.Uint8ToString(raw)
 		}
-		// todo(kyhavlov): should we be hiding this and the vault token?
-		if conf.Provider == structs.ConsulCAProvider {
+		switch conf.Provider {
+		case structs.ConsulCAProvider:
 			if v, ok := conf.Config["PrivateKey"]; ok && v != "" {
 				conf.Config["PrivateKey"] = "hidden"
 			}
+		case structs.VaultCAProvider:
+			if v, ok := conf.Config["Token"]; ok && v != "" {
+				conf.Config["Token"] = "hidden"
+			}
 		}
 	}
 }

agent/consul/leader.go

@@ -512,7 +512,7 @@ func (s *Server) createCAProvider(conf *structs.CAConfiguration) (ca.Provider, e
 	case structs.ConsulCAProvider:
 		return ca.NewConsulProvider(conf.Config, &consulCADelegate{s})
 	case structs.VaultCAProvider:
-		return ca.NewVaultProvider(conf.Config, conf.ClusterID, nil)
+		return ca.NewVaultProvider(conf.Config, conf.ClusterID)
 	default:
 		return nil, fmt.Errorf("unknown CA provider %q", conf.Provider)
 	}