From c112a728804b662ac1df63a4b33d48ecaf0ff181 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 11 Sep 2018 16:43:04 -0700 Subject: [PATCH] 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 946f558a09..7d7244fd9c 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 3be80d10ec..3bce52a0b8 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 f8c6b2deee..4ad30ffb86 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 07b179774a..b116de62da 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 31cf5d0631..31eff83695 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 588e6ec826..8dc83f8776 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