From e184a18e4bcdb841dbe386c74f989de3236e4f78 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 6 Sep 2018 19:18:54 -0700 Subject: [PATCH] connect/ca: add Configure/GenerateRoot to provider interface --- agent/connect/ca/provider.go | 11 +- agent/connect/ca/provider_consul.go | 136 +++++++++++++---------- agent/connect/ca/provider_consul_test.go | 45 ++++---- agent/connect/ca/provider_vault.go | 55 ++++----- agent/connect/ca/provider_vault_test.go | 10 +- agent/consul/connect_ca_endpoint.go | 6 + agent/consul/leader.go | 12 +- agent/structs/connect_ca.go | 8 +- 8 files changed, 168 insertions(+), 115 deletions(-) diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index f7daa5fc7..a812d6411 100644 --- a/agent/connect/ca/provider.go +++ b/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) diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 5cbf744ab..946f558a0 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -6,6 +6,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "errors" "fmt" "math/big" "net/url" @@ -17,6 +18,8 @@ import ( "github.com/hashicorp/consul/agent/structs" ) +var ErrNotInitialized = errors.New("provider not initialized") + type ConsulProvider struct { config *structs.ConsulCAProviderConfig id string @@ -31,83 +34,46 @@ type ConsulProviderStateDelegate interface { // NewConsulProvider returns a new instance of the Consul CA provider, // bootstrapping its state in the state store necessary -func NewConsulProvider(rawConfig map[string]interface{}, delegate ConsulProviderStateDelegate) (*ConsulProvider, error) { - conf, err := ParseConsulCAConfig(rawConfig) - if err != nil { - return nil, err - } - provider := &ConsulProvider{ - config: conf, +func NewConsulProvider(delegate ConsulProviderStateDelegate) *ConsulProvider { + return &ConsulProvider{ delegate: delegate, - id: fmt.Sprintf("%s,%s", conf.PrivateKey, conf.RootCert), } +} - // Check if this configuration of the provider has already been - // initialized in the state store. - state := delegate.State() - _, providerState, err := state.CAProviderState(provider.id) +func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error { + // Parse the raw config and update our ID. + config, err := ParseConsulCAConfig(rawConfig) if err != nil { - return nil, err + return err + } + c.config = config + c.id = fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert) + + // 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 } - // Exit early if the state store has already been populated for this config. if providerState != nil { - return provider, nil + return nil } + // Write the provider state to the state store. newState := structs.CAConsulProviderState{ - ID: provider.id, + ID: c.id, + IsRoot: isRoot, } - // Write the initial provider state to get the index to use for the - // CA serial number. - { - args := &structs.CARequest{ - Op: structs.CAOpSetProviderState, - ProviderState: &newState, - } - if err := delegate.ApplyCARequest(args); err != nil { - return nil, err - } - } - - idx, _, err := state.CAProviderState(provider.id) - if err != nil { - return nil, err - } - - // Generate a private key if needed - if conf.PrivateKey == "" { - _, pk, err := connect.GeneratePrivateKey() - if err != nil { - return nil, err - } - newState.PrivateKey = pk - } else { - newState.PrivateKey = conf.PrivateKey - } - - // 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 - } - - // Write the provider state args := &structs.CARequest{ Op: structs.CAOpSetProviderState, ProviderState: &newState, } - if err := delegate.ApplyCARequest(args); err != nil { - return nil, err + if err := c.delegate.ApplyCARequest(args); err != nil { + return err } - return provider, nil + return nil } // Return the active root CA and generate a new one if needed @@ -121,6 +87,58 @@ func (c *ConsulProvider) ActiveRoot() (string, error) { return providerState.RootCert, nil } +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 !providerState.IsRoot { + return fmt.Errorf("provider is not the root certificate authority") + } + if providerState.RootCert != "" { + return nil + } + + // Generate a private key if needed + newState := *providerState + if c.config.PrivateKey == "" { + _, pk, err := connect.GeneratePrivateKey() + if err != nil { + return err + } + newState.PrivateKey = pk + } else { + newState.PrivateKey = c.config.PrivateKey + } + + // Generate the root CA if necessary + if c.config.RootCert == "" { + ca, err := c.generateCA(newState.PrivateKey, idx+1) + if err != nil { + return fmt.Errorf("error generating CA: %v", err) + } + newState.RootCert = ca + } else { + newState.RootCert = c.config.RootCert + } + + // Write the provider state + args := &structs.CARequest{ + Op: structs.CAOpSetProviderState, + ProviderState: &newState, + } + if err := c.delegate.ApplyCARequest(args); err != nil { + return err + } + + return nil +} + // We aren't maintaining separate root/intermediate CAs for the builtin // provider, so just return the root. func (c *ConsulProvider) ActiveIntermediate() (string, error) { diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 2a37f1b94..3be80d10e 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/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" ) @@ -72,32 +71,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) - assert.NoError(err) + provider := NewConsulProvider(delegate) + 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) - assert.Equal(root, inter) + require.NoError(err) + require.Equal(root, inter) // Should be a valid cert parsed, err := connect.ParseCert(root) - assert.NoError(err) - assert.Equal(parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", conf.ClusterID)) + require.NoError(err) + 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{}{ @@ -106,12 +106,13 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) { } delegate := newMockDelegate(t, conf) - provider, err := NewConsulProvider(conf.Config, delegate) - assert.NoError(err) + provider := NewConsulProvider(delegate) + require.NoError(provider.Configure(conf.ClusterID, true, conf.Config)) + require.NoError(provider.GenerateRoot()) root, err := provider.ActiveRoot() - assert.NoError(err) - assert.Equal(root, rootCA.RootCert) + require.NoError(err) + require.Equal(root, rootCA.RootCert) } func TestConsulCAProvider_SignLeaf(t *testing.T) { @@ -122,8 +123,9 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { conf.Config["LeafCertTTL"] = "1h" delegate := newMockDelegate(t, conf) - provider, err := NewConsulProvider(conf.Config, delegate) - require.NoError(err) + provider := NewConsulProvider(delegate) + require.NoError(provider.Configure(conf.ClusterID, true, conf.Config)) + require.NoError(provider.GenerateRoot()) spiffeService := &connect.SpiffeIDService{ Host: "node1", @@ -180,17 +182,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) - require.NoError(t, err) + provider1 := NewConsulProvider(delegate1) + 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) - require.NoError(t, err) + provider2 := NewConsulProvider(delegate2) + require.NoError(provider2.Configure(conf2.ClusterID, true, conf2.Config)) + require.NoError(provider2.GenerateRoot()) testCrossSignProviders(t, provider1, provider2) } diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 743ea8957..f8c6b2dee 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -24,6 +24,7 @@ var ErrBackendNotInitialized = fmt.Errorf("backend not initialized") type VaultProvider struct { config *structs.VaultCAProviderConfig client *vaultapi.Client + isRoot bool clusterId string } @@ -31,34 +32,43 @@ 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) (*VaultProvider, error) { - conf, err := ParseVaultCAConfig(rawConfig) +func NewVaultProvider() *VaultProvider { + return &VaultProvider{} +} + +func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error { + config, 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{ - config: conf, - client: client, - clusterId: clusterId, + return nil +} + +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,35 +77,30 @@ 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. - if _, err := provider.GenerateIntermediate(); err != nil { - return nil, err - } - - return provider, nil + return nil } func (v *VaultProvider) ActiveRoot() (string, error) { diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 5c248e8dc..07b179774 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -37,10 +37,12 @@ func testVaultClusterWithConfig(t *testing.T, rawConf map[string]interface{}) (* conf[k] = v } - provider, err := NewVaultProvider(conf, "asdf") - if err != nil { - t.Fatal(err) - } + require := require.New(t) + provider := NewVaultProvider() + require.NoError(provider.Configure("asdf", true, conf)) + require.NoError(provider.GenerateRoot()) + _, err := provider.GenerateIntermediate() + require.NoError(err) return provider, core, ln } diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 9a02d6763..e61f2b80c 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/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 { diff --git a/agent/consul/leader.go b/agent/consul/leader.go index e959e365e..31cf5d063 100644 --- a/agent/consul/leader.go +++ b/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.NewConsulProvider(&consulCADelegate{s}), nil case structs.VaultCAProvider: - return ca.NewVaultProvider(conf.Config, conf.ClusterID) + return ca.NewVaultProvider(), nil default: return nil, fmt.Errorf("unknown CA provider %q", conf.Provider) } diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 1e869fd45..588e6ec82 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -250,9 +250,11 @@ type ConsulCAProviderConfig struct { // CAConsulProviderState is used to track the built-in Consul CA provider's state. type CAConsulProviderState struct { - ID string - PrivateKey string - RootCert string + ID string + PrivateKey string + RootCert string + IsRoot bool + IntermediateCert string RaftIndex }