Merge pull request #4644 from hashicorp/ca-refactor

connect/ca: rework initialization/root generation in providers

agent/connect/ca/provider.go

@@ -8,7 +8,16 @@ import (
 // an external CA that provides leaf certificate signing for
 // given SpiffeIDServices.
 type Provider interface {
-	// Active root returns the currently active root CA for this
+	// Configure initializes the provider based on the given cluster ID, root status
+	// and configuration values.
+	Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error
+
+	// GenerateRoot causes the creation of a new root certificate for this provider.
+	// This can also be a no-op if a root certificate already exists for the given
+	// config. If isRoot is false, calling this method is an error.
+	GenerateRoot() error
+
+	// ActiveRoot returns the currently active root CA for this
 	// provider. This should be a parent of the certificate returned by
 	// ActiveIntermediate()
 	ActiveRoot() (string, error)

agent/connect/ca/provider_consul.go

@@ -3,12 +3,15 @@ package ca
 import (
 	"bytes"
 	"crypto/rand"
+	"crypto/sha256"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"math/big"
 	"net/url"
+	"strings"
 	"sync"
 	"time"
 
@@ -17,10 +20,14 @@ import (
 	"github.com/hashicorp/consul/agent/structs"
 )
 
+var ErrNotInitialized = errors.New("provider not initialized")
+
 type ConsulProvider struct {
-	config   *structs.ConsulCAProviderConfig
+	Delegate ConsulProviderStateDelegate
-	id       string
+
-	delegate ConsulProviderStateDelegate
+	config *structs.ConsulCAProviderConfig
+	id     string
+	isRoot bool
 	sync.RWMutex
 }
 
@@ -29,73 +36,126 @@ type ConsulProviderStateDelegate interface {
 	ApplyCARequest(*structs.CARequest) error
 }
 
-// NewConsulProvider returns a new instance of the Consul CA provider,
+// Configure sets up the provider using the given configuration.
-// bootstrapping its state in the state store necessary
+func (c *ConsulProvider) Configure(clusterID string, isRoot bool, rawConfig map[string]interface{}) error {
-func NewConsulProvider(rawConfig map[string]interface{}, delegate ConsulProviderStateDelegate) (*ConsulProvider, error) {
+	// Parse the raw config and update our ID.
-	conf, err := ParseConsulCAConfig(rawConfig)
+	config, err := ParseConsulCAConfig(rawConfig)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	provider := &ConsulProvider{
+	c.config = config
-		config:   conf,
+	c.isRoot = isRoot
-		delegate: delegate,
+	hash := sha256.Sum256([]byte(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, isRoot)))
-		id:       fmt.Sprintf("%s,%s", conf.PrivateKey, conf.RootCert),
+	c.id = strings.Replace(fmt.Sprintf("% x", hash), " ", ":", -1)
+
+	// Exit early if the state store has an entry for this provider's config.
+	_, providerState, err := c.Delegate.State().CAProviderState(c.id)
+	if err != nil {
+		return err
 	}
 
-	// Check if this configuration of the provider has already been
-	// initialized in the state store.
-	state := delegate.State()
-	_, providerState, err := state.CAProviderState(provider.id)
-	if err != nil {
-		return nil, err
-	}
-
-	// Exit early if the state store has already been populated for this config.
 	if providerState != nil {
-		return provider, nil
+		return nil
 	}
 
-	newState := structs.CAConsulProviderState{
+	// Check if there's an entry with the old ID scheme.
-		ID: provider.id,
+	oldID := fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert)
+	_, providerState, err = c.Delegate.State().CAProviderState(oldID)
+	if err != nil {
+		return err
 	}
 
-	// Write the initial provider state to get the index to use for the
+	// Found an entry with the old ID, so update it to the new ID and
-	// CA serial number.
+	// delete the old entry.
-	{
+	if providerState != nil {
-		args := &structs.CARequest{
+		newState := *providerState
+		newState.ID = c.id
+		createReq := &structs.CARequest{
 			Op:            structs.CAOpSetProviderState,
 			ProviderState: &newState,
 		}
-		if err := delegate.ApplyCARequest(args); err != nil {
+		if err := c.Delegate.ApplyCARequest(createReq); err != nil {
-			return nil, err
+			return err
 		}
+
+		deleteReq := &structs.CARequest{
+			Op:            structs.CAOpDeleteProviderState,
+			ProviderState: providerState,
+		}
+		if err := c.Delegate.ApplyCARequest(deleteReq); err != nil {
+			return err
+		}
+
+		return nil
 	}
 
-	idx, _, err := state.CAProviderState(provider.id)
+	// Write the provider state to the state store.
+	newState := structs.CAConsulProviderState{
+		ID: c.id,
+	}
+
+	args := &structs.CARequest{
+		Op:            structs.CAOpSetProviderState,
+		ProviderState: &newState,
+	}
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// ActiveRoot returns the active root CA certificate.
+func (c *ConsulProvider) ActiveRoot() (string, error) {
+	state := c.Delegate.State()
+	_, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
-		return nil, err
+		return "", err
+	}
+
+	return providerState.RootCert, nil
+}
+
+// GenerateRoot initializes a new root certificate and private key
+// if needed.
+func (c *ConsulProvider) GenerateRoot() error {
+	state := c.Delegate.State()
+	idx, providerState, err := state.CAProviderState(c.id)
+	if err != nil {
+		return err
+	}
+
+	if providerState == nil {
+		return ErrNotInitialized
+	}
+	if !c.isRoot {
+		return fmt.Errorf("provider is not the root certificate authority")
+	}
+	if providerState.RootCert != "" {
+		return nil
 	}
 
 	// Generate a private key if needed
-	if conf.PrivateKey == "" {
+	newState := *providerState
+	if c.config.PrivateKey == "" {
 		_, pk, err := connect.GeneratePrivateKey()
 		if err != nil {
-			return nil, err
+			return err
 		}
 		newState.PrivateKey = pk
 	} else {
-		newState.PrivateKey = conf.PrivateKey
+		newState.PrivateKey = c.config.PrivateKey
 	}
 
 	// Generate the root CA if necessary
-	if conf.RootCert == "" {
+	if c.config.RootCert == "" {
-		ca, err := provider.generateCA(newState.PrivateKey, idx+1)
+		ca, err := c.generateCA(newState.PrivateKey, idx+1)
 		if err != nil {
-			return nil, fmt.Errorf("error generating CA: %v", err)
+			return fmt.Errorf("error generating CA: %v", err)
 		}
 		newState.RootCert = ca
 	} else {
-		newState.RootCert = conf.RootCert
+		newState.RootCert = c.config.RootCert
 	}
 
 	// Write the provider state
@@ -103,22 +163,11 @@ func NewConsulProvider(rawConfig map[string]interface{}, delegate ConsulProvider
 		Op:            structs.CAOpSetProviderState,
 		ProviderState: &newState,
 	}
-	if err := delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
-		return nil, err
+		return err
 	}
 
-	return provider, nil
+	return nil
-}
-
-// Return the active root CA and generate a new one if needed
-func (c *ConsulProvider) ActiveRoot() (string, error) {
-	state := c.delegate.State()
-	_, providerState, err := state.CAProviderState(c.id)
-	if err != nil {
-		return "", err
-	}
-
-	return providerState.RootCert, nil
 }
 
 // We aren't maintaining separate root/intermediate CAs for the builtin
@@ -139,7 +188,7 @@ func (c *ConsulProvider) Cleanup() error {
 		Op:            structs.CAOpDeleteProviderState,
 		ProviderState: &structs.CAConsulProviderState{ID: c.id},
 	}
-	if err := c.delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
 		return err
 	}
 
@@ -155,7 +204,7 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 	defer c.Unlock()
 
 	// Get the provider state
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	idx, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
 		return "", err
@@ -247,7 +296,7 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
 	defer c.Unlock()
 
 	// Get the provider state
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	idx, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
 		return "", err
@@ -315,7 +364,7 @@ func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulP
 		Op:            structs.CAOpSetProviderState,
 		ProviderState: &newState,
 	}
-	if err := c.delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
 		return err
 	}
 
@@ -324,7 +373,7 @@ func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulP
 
 // generateCA makes a new root CA using the current private key
 func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error) {
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	_, config, err := state.CAConfig()
 	if err != nil {
 		return "", err
@@ -348,9 +397,9 @@ func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error
 	serialNum := &big.Int{}
 	serialNum.SetUint64(sn)
 	template := x509.Certificate{
-		SerialNumber: serialNum,
+		SerialNumber:          serialNum,
-		Subject:      pkix.Name{CommonName: name},
+		Subject:               pkix.Name{CommonName: name},
-		URIs:         []*url.URL{id.URI()},
+		URIs:                  []*url.URL{id.URI()},
 		BasicConstraintsValid: true,
 		KeyUsage: x509.KeyUsageCertSign |
 			x509.KeyUsageCRLSign |

agent/connect/ca/provider_consul_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -75,32 +74,33 @@ func testConsulCAConfig() *structs.CAConfiguration {
 func TestConsulCAProvider_Bootstrap(t *testing.T) {
 	t.Parallel()
 
-	assert := assert.New(t)
+	require := require.New(t)
 	conf := testConsulCAConfig()
 	delegate := newMockDelegate(t, conf)
 
-	provider, err := NewConsulProvider(conf.Config, delegate)
+	provider := &ConsulProvider{Delegate: delegate}
-	assert.NoError(err)
+	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
+	require.NoError(provider.GenerateRoot())
 
 	root, err := provider.ActiveRoot()
-	assert.NoError(err)
+	require.NoError(err)
 
 	// Intermediate should be the same cert.
 	inter, err := provider.ActiveIntermediate()
-	assert.NoError(err)
+	require.NoError(err)
-	assert.Equal(root, inter)
+	require.Equal(root, inter)
 
 	// Should be a valid cert
 	parsed, err := connect.ParseCert(root)
-	assert.NoError(err)
+	require.NoError(err)
-	assert.Equal(parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", conf.ClusterID))
+	require.Equal(parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", conf.ClusterID))
 }
 
 func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) {
 	t.Parallel()
 
 	// Make sure setting a custom private key/root cert works.
-	assert := assert.New(t)
+	require := require.New(t)
 	rootCA := connect.TestCA(t, nil)
 	conf := testConsulCAConfig()
 	conf.Config = map[string]interface{}{
@@ -109,12 +109,13 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) {
 	}
 	delegate := newMockDelegate(t, conf)
 
-	provider, err := NewConsulProvider(conf.Config, delegate)
+	provider := &ConsulProvider{Delegate: delegate}
-	assert.NoError(err)
+	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
+	require.NoError(provider.GenerateRoot())
 
 	root, err := provider.ActiveRoot()
-	assert.NoError(err)
+	require.NoError(err)
-	assert.Equal(root, rootCA.RootCert)
+	require.Equal(root, rootCA.RootCert)
 }
 
 func TestConsulCAProvider_SignLeaf(t *testing.T) {
@@ -125,8 +126,9 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 	conf.Config["LeafCertTTL"] = "1h"
 	delegate := newMockDelegate(t, conf)
 
-	provider, err := NewConsulProvider(conf.Config, delegate)
+	provider := &ConsulProvider{Delegate: delegate}
-	require.NoError(err)
+	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
+	require.NoError(provider.GenerateRoot())
 
 	spiffeService := &connect.SpiffeIDService{
 		Host:       "node1",
@@ -183,17 +185,20 @@ 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)
+	provider1 := &ConsulProvider{Delegate: delegate1}
-	require.NoError(t, err)
+	require.NoError(provider1.Configure(conf1.ClusterID, true, conf1.Config))
+	require.NoError(provider1.GenerateRoot())
 
 	conf2 := testConsulCAConfig()
 	conf2.CreateIndex = 10
 	delegate2 := newMockDelegate(t, conf2)
-	provider2, err := NewConsulProvider(conf2.Config, delegate2)
+	provider2 := &ConsulProvider{Delegate: delegate2}
-	require.NoError(t, err)
+	require.NoError(provider2.Configure(conf2.ClusterID, true, conf2.Config))
+	require.NoError(provider2.GenerateRoot())
 
 	testCrossSignProviders(t, provider1, provider2)
 }
@@ -269,3 +274,32 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
 		require.NoError(err)
 	}
 }
+
+func TestConsulCAProvider_MigrateOldID(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	conf := testConsulCAConfig()
+	delegate := newMockDelegate(t, conf)
+
+	// Create an entry with an old-style ID.
+	err := delegate.ApplyCARequest(&structs.CARequest{
+		Op: structs.CAOpSetProviderState,
+		ProviderState: &structs.CAConsulProviderState{
+			ID: ",",
+		},
+	})
+	require.NoError(err)
+	_, providerState, err := delegate.state.CAProviderState(",")
+	require.NoError(err)
+	require.NotNil(providerState)
+
+	provider := &ConsulProvider{Delegate: delegate}
+	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
+	require.NoError(provider.GenerateRoot())
+
+	// After running Configure, the old ID entry should be gone.
+	_, providerState, err = delegate.state.CAProviderState(",")
+	require.NoError(err)
+	require.Nil(providerState)
+}

agent/connect/ca/provider_vault.go

@@ -24,41 +24,50 @@ var ErrBackendNotInitialized = fmt.Errorf("backend not initialized")
 type VaultProvider struct {
 	config    *structs.VaultCAProviderConfig
 	client    *vaultapi.Client
+	isRoot    bool
 	clusterId string
 }
 
-// NewVaultProvider returns a vault provider with its root and intermediate PKI
+// Configure sets up the provider using the given configuration.
-// backends mounted and initialized. If the root backend is not set up already,
+func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error {
-// it will be mounted/generated as needed, but any existing state will not be
+	config, err := ParseVaultCAConfig(rawConfig)
-// overwritten.
-func NewVaultProvider(rawConfig map[string]interface{}, clusterId string) (*VaultProvider, error) {
-	conf, err := ParseVaultCAConfig(rawConfig)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	// todo(kyhavlov): figure out the right way to pass the TLS config
 	clientConf := &vaultapi.Config{
-		Address: conf.Address,
+		Address: config.Address,
 	}
 	client, err := vaultapi.NewClient(clientConf)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	client.SetToken(conf.Token)
+	client.SetToken(config.Token)
+	v.config = config
+	v.client = client
+	v.isRoot = isRoot
+	v.clusterId = clusterId
 
-	provider := &VaultProvider{
+	return nil
-		config:    conf,
+}
-		client:    client,
+
-		clusterId: clusterId,
+// ActiveRoot returns the active root CA certificate.
+func (v *VaultProvider) ActiveRoot() (string, error) {
+	return v.getCA(v.config.RootPKIPath)
+}
+
+// GenerateRoot mounts and initializes a new root PKI backend if needed.
+func (v *VaultProvider) GenerateRoot() error {
+	if !v.isRoot {
+		return fmt.Errorf("provider is not the root certificate authority")
 	}
 
 	// Set up the root PKI backend if necessary.
-	_, err = provider.ActiveRoot()
+	_, err := v.ActiveRoot()
 	switch err {
 	case ErrBackendNotMounted:
-		err := client.Sys().Mount(conf.RootPKIPath, &vaultapi.MountInput{
+		err := v.client.Sys().Mount(v.config.RootPKIPath, &vaultapi.MountInput{
 			Type:        "pki",
 			Description: "root CA backend for Consul Connect",
 			Config: vaultapi.MountConfigInput{
@@ -67,41 +76,33 @@ func NewVaultProvider(rawConfig map[string]interface{}, clusterId string) (*Vaul
 		})
 
 		if err != nil {
-			return nil, err
+			return err
 		}
 
 		fallthrough
 	case ErrBackendNotInitialized:
-		spiffeID := connect.SpiffeIDSigning{ClusterID: clusterId, Domain: "consul"}
+		spiffeID := connect.SpiffeIDSigning{ClusterID: v.clusterId, Domain: "consul"}
 		uuid, err := uuid.GenerateUUID()
 		if err != nil {
-			return nil, err
+			return err
 		}
-		_, err = client.Logical().Write(conf.RootPKIPath+"root/generate/internal", map[string]interface{}{
+		_, err = v.client.Logical().Write(v.config.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
+			return err
 		}
 	default:
 		if err != nil {
-			return nil, err
+			return err
 		}
 	}
 
-	// Set up the intermediate backend.
+	return nil
-	if _, err := provider.GenerateIntermediate(); err != nil {
-		return nil, err
-	}
-
-	return provider, nil
-}
-
-func (v *VaultProvider) ActiveRoot() (string, error) {
-	return v.getCA(v.config.RootPKIPath)
 }
 
+// ActiveIntermediate returns the current intermediate certificate.
 func (v *VaultProvider) ActiveIntermediate() (string, error) {
 	return v.getCA(v.config.IntermediatePKIPath)
 }

agent/connect/ca/provider_vault_test.go

@@ -39,10 +39,12 @@ func testVaultClusterWithConfig(t *testing.T, rawConf map[string]interface{}) (*
 		conf[k] = v
 	}
 
-	provider, err := NewVaultProvider(conf, "asdf")
+	require := require.New(t)
-	if err != nil {
+	provider := &VaultProvider{}
-		t.Fatal(err)
+	require.NoError(provider.Configure("asdf", true, conf))
-	}
+	require.NoError(provider.GenerateRoot())
+	_, err := provider.GenerateIntermediate()
+	require.NoError(err)
 
 	return provider, core, ln
 }

agent/consul/connect_ca_endpoint.go

@@ -95,6 +95,12 @@ func (s *ConnectCA) ConfigurationSet(
 	if err != nil {
 		return fmt.Errorf("could not initialize provider: %v", err)
 	}
+	if err := newProvider.Configure(args.Config.ClusterID, true, args.Config.Config); err != nil {
+		return fmt.Errorf("error configuring provider: %v", err)
+	}
+	if err := newProvider.GenerateRoot(); err != nil {
+		return fmt.Errorf("error generating CA root certificate: %v", err)
+	}
 
 	newRootPEM, err := newProvider.ActiveRoot()
 	if err != nil {

agent/consul/leader.go

@@ -427,11 +427,17 @@ func (s *Server) initializeCA() error {
 		return err
 	}
 
-	// Initialize the right provider based on the config
+	// Initialize the provider based on the current config.
 	provider, err := s.createCAProvider(conf)
 	if err != nil {
 		return err
 	}
+	if err := provider.Configure(conf.ClusterID, true, conf.Config); err != nil {
+		return fmt.Errorf("error configuring provider: %v", err)
+	}
+	if err := provider.GenerateRoot(); err != nil {
+		return fmt.Errorf("error generating CA root certificate: %v", err)
+	}
 
 	// Get the active root cert from the CA
 	rootPEM, err := provider.ActiveRoot()
@@ -520,9 +526,9 @@ func parseCARoot(pemValue, provider string) (*structs.CARoot, error) {
 func (s *Server) createCAProvider(conf *structs.CAConfiguration) (ca.Provider, error) {
 	switch conf.Provider {
 	case structs.ConsulCAProvider:
-		return ca.NewConsulProvider(conf.Config, &consulCADelegate{s})
+		return &ca.ConsulProvider{Delegate: &consulCADelegate{s}}, nil
 	case structs.VaultCAProvider:
-		return ca.NewVaultProvider(conf.Config, conf.ClusterID)
+		return &ca.VaultProvider{}, nil
 	default:
 		return nil, fmt.Errorf("unknown CA provider %q", conf.Provider)
 	}

agent/structs/connect_ca.go

@@ -252,9 +252,10 @@ type ConsulCAProviderConfig struct {
 
 // CAConsulProviderState is used to track the built-in Consul CA provider's state.
 type CAConsulProviderState struct {
-	ID         string
+	ID               string
-	PrivateKey string
+	PrivateKey       string
-	RootCert   string
+	RootCert         string
+	IntermediateCert string
 
 	RaftIndex
 }