From e184a18e4bcdb841dbe386c74f989de3236e4f78 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 6 Sep 2018 19:18:54 -0700 Subject: [PATCH 1/3] 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 } From 8fc2c77fdfec5b871ab8a0e954cb9022f9b1cac8 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 11 Sep 2018 16:43:04 -0700 Subject: [PATCH 2/3] connect/ca: some cleanup and reorganizing of the new methods --- agent/connect/ca/provider_consul.go | 47 +++++++++++------------- agent/connect/ca/provider_consul_test.go | 10 ++--- agent/connect/ca/provider_vault.go | 20 ++++------ agent/connect/ca/provider_vault_test.go | 2 +- agent/consul/leader.go | 4 +- agent/structs/connect_ca.go | 1 - 6 files changed, 38 insertions(+), 46 deletions(-) diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 946f558a0..7d7244fd9 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -21,9 +21,11 @@ import ( var ErrNotInitialized = errors.New("provider not initialized") type ConsulProvider struct { - config *structs.ConsulCAProviderConfig - id string - delegate ConsulProviderStateDelegate + Delegate ConsulProviderStateDelegate + + config *structs.ConsulCAProviderConfig + id string + isRoot bool sync.RWMutex } @@ -32,14 +34,7 @@ type ConsulProviderStateDelegate interface { ApplyCARequest(*structs.CARequest) error } -// NewConsulProvider returns a new instance of the Consul CA provider, -// bootstrapping its state in the state store necessary -func NewConsulProvider(delegate ConsulProviderStateDelegate) *ConsulProvider { - return &ConsulProvider{ - delegate: delegate, - } -} - +// Configure sets up the provider using the given configuration. 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) @@ -47,10 +42,11 @@ func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[ return err } c.config = config + c.isRoot = isRoot 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) + _, providerState, err := c.Delegate.State().CAProviderState(c.id) if err != nil { return err } @@ -61,24 +57,23 @@ func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[ // Write the provider state to the state store. newState := structs.CAConsulProviderState{ - ID: c.id, - IsRoot: isRoot, + ID: c.id, } args := &structs.CARequest{ Op: structs.CAOpSetProviderState, ProviderState: &newState, } - if err := c.delegate.ApplyCARequest(args); err != nil { + if err := c.Delegate.ApplyCARequest(args); err != nil { return err } return nil } -// Return the active root CA and generate a new one if needed +// ActiveRoot returns the active root CA certificate. func (c *ConsulProvider) ActiveRoot() (string, error) { - state := c.delegate.State() + state := c.Delegate.State() _, providerState, err := state.CAProviderState(c.id) if err != nil { return "", err @@ -87,8 +82,10 @@ func (c *ConsulProvider) ActiveRoot() (string, error) { return providerState.RootCert, nil } +// GenerateRoot initializes a new root certificate and private key +// if needed. func (c *ConsulProvider) GenerateRoot() error { - state := c.delegate.State() + state := c.Delegate.State() idx, providerState, err := state.CAProviderState(c.id) if err != nil { return err @@ -97,7 +94,7 @@ func (c *ConsulProvider) GenerateRoot() error { if providerState == nil { return ErrNotInitialized } - if !providerState.IsRoot { + if !c.isRoot { return fmt.Errorf("provider is not the root certificate authority") } if providerState.RootCert != "" { @@ -132,7 +129,7 @@ func (c *ConsulProvider) GenerateRoot() error { Op: structs.CAOpSetProviderState, ProviderState: &newState, } - if err := c.delegate.ApplyCARequest(args); err != nil { + if err := c.Delegate.ApplyCARequest(args); err != nil { return err } @@ -157,7 +154,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 } @@ -173,7 +170,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 @@ -265,7 +262,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 @@ -333,7 +330,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 } @@ -342,7 +339,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 diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 3be80d10e..3bce52a0b 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -75,7 +75,7 @@ func TestConsulCAProvider_Bootstrap(t *testing.T) { conf := testConsulCAConfig() delegate := newMockDelegate(t, conf) - provider := NewConsulProvider(delegate) + provider := &ConsulProvider{Delegate: delegate} require.NoError(provider.Configure(conf.ClusterID, true, conf.Config)) require.NoError(provider.GenerateRoot()) @@ -106,7 +106,7 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) { } delegate := newMockDelegate(t, conf) - provider := NewConsulProvider(delegate) + provider := &ConsulProvider{Delegate: delegate} require.NoError(provider.Configure(conf.ClusterID, true, conf.Config)) require.NoError(provider.GenerateRoot()) @@ -123,7 +123,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { conf.Config["LeafCertTTL"] = "1h" delegate := newMockDelegate(t, conf) - provider := NewConsulProvider(delegate) + provider := &ConsulProvider{Delegate: delegate} require.NoError(provider.Configure(conf.ClusterID, true, conf.Config)) require.NoError(provider.GenerateRoot()) @@ -186,14 +186,14 @@ func TestConsulCAProvider_CrossSignCA(t *testing.T) { conf1 := testConsulCAConfig() delegate1 := newMockDelegate(t, conf1) - provider1 := NewConsulProvider(delegate1) + provider1 := &ConsulProvider{Delegate: delegate1} require.NoError(provider1.Configure(conf1.ClusterID, true, conf1.Config)) require.NoError(provider1.GenerateRoot()) conf2 := testConsulCAConfig() conf2.CreateIndex = 10 delegate2 := newMockDelegate(t, conf2) - provider2 := NewConsulProvider(delegate2) + provider2 := &ConsulProvider{Delegate: delegate2} require.NoError(provider2.Configure(conf2.ClusterID, true, conf2.Config)) require.NoError(provider2.GenerateRoot()) diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index f8c6b2dee..4ad30ffb8 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -28,14 +28,7 @@ type VaultProvider struct { clusterId string } -// NewVaultProvider returns a vault provider with its root and intermediate PKI -// 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() *VaultProvider { - return &VaultProvider{} -} - +// Configure sets up the provider using the given configuration. func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error { config, err := ParseVaultCAConfig(rawConfig) if err != nil { @@ -59,6 +52,12 @@ func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[s return nil } +// 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") @@ -103,10 +102,7 @@ func (v *VaultProvider) GenerateRoot() error { return 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) } diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 07b179774..b116de62d 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -38,7 +38,7 @@ func testVaultClusterWithConfig(t *testing.T, rawConf map[string]interface{}) (* } require := require.New(t) - provider := NewVaultProvider() + provider := &VaultProvider{} require.NoError(provider.Configure("asdf", true, conf)) require.NoError(provider.GenerateRoot()) _, err := provider.GenerateIntermediate() diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 31cf5d063..31eff8369 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -526,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(&consulCADelegate{s}), nil + return &ca.ConsulProvider{Delegate: &consulCADelegate{s}}, nil case structs.VaultCAProvider: - return ca.NewVaultProvider(), nil + return &ca.VaultProvider{}, 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 588e6ec82..8dc83f877 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -253,7 +253,6 @@ type CAConsulProviderState struct { ID string PrivateKey string RootCert string - IsRoot bool IntermediateCert string RaftIndex From 70c43a27c3a6361c55b3aab5dddff535ef1ef46b Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Wed, 12 Sep 2018 13:44:15 -0700 Subject: [PATCH 3/3] connect/ca: hash the consul provider ID and include isRoot --- agent/connect/ca/provider_consul.go | 44 +++++++++++++++++++++--- agent/connect/ca/provider_consul_test.go | 29 ++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 7d7244fd9..803ce2ac1 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -3,6 +3,7 @@ package ca import ( "bytes" "crypto/rand" + "crypto/sha256" "crypto/x509" "crypto/x509/pkix" "encoding/pem" @@ -10,6 +11,7 @@ import ( "fmt" "math/big" "net/url" + "strings" "sync" "time" @@ -35,7 +37,7 @@ type ConsulProviderStateDelegate interface { } // Configure sets up the provider using the given configuration. -func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error { +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 { @@ -43,7 +45,8 @@ func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[ } c.config = config c.isRoot = isRoot - c.id = fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert) + hash := sha256.Sum256([]byte(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, isRoot))) + 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) @@ -55,6 +58,37 @@ func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[ return nil } + // Check if there's an entry with the old ID scheme. + oldID := fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert) + _, providerState, err = c.Delegate.State().CAProviderState(oldID) + if err != nil { + return err + } + + // Found an entry with the old ID, so update it to the new ID and + // delete the old entry. + if providerState != nil { + newState := *providerState + newState.ID = c.id + createReq := &structs.CARequest{ + Op: structs.CAOpSetProviderState, + ProviderState: &newState, + } + if err := c.Delegate.ApplyCARequest(createReq); err != nil { + return err + } + + deleteReq := &structs.CARequest{ + Op: structs.CAOpDeleteProviderState, + ProviderState: providerState, + } + if err := c.Delegate.ApplyCARequest(deleteReq); err != nil { + return err + } + + return nil + } + // Write the provider state to the state store. newState := structs.CAConsulProviderState{ ID: c.id, @@ -363,9 +397,9 @@ func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error serialNum := &big.Int{} serialNum.SetUint64(sn) template := x509.Certificate{ - SerialNumber: serialNum, - Subject: pkix.Name{CommonName: name}, - URIs: []*url.URL{id.URI()}, + SerialNumber: serialNum, + Subject: pkix.Name{CommonName: name}, + URIs: []*url.URL{id.URI()}, BasicConstraintsValid: true, KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign | diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 3bce52a0b..857823458 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -271,3 +271,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) +}