From d6ca015a42c920722686ae98389c68f44b8bb685 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Mon, 16 Jul 2018 02:46:10 -0700 Subject: [PATCH] connect/ca: add configurable leaf cert TTL --- agent/config/builder.go | 3 ++ agent/config/runtime_test.go | 7 ++-- agent/connect/ca/provider_consul.go | 3 +- agent/connect/ca/provider_consul_config.go | 12 +++++-- agent/connect/ca/provider_consul_test.go | 38 ++++++++++++---------- agent/connect/ca/provider_vault.go | 7 ++-- agent/connect/ca/provider_vault_test.go | 22 ++++++++++--- agent/connect_ca_endpoint_test.go | 4 ++- agent/consul/config.go | 1 + agent/structs/connect_ca.go | 8 +++++ api/connect_ca.go | 7 ++++ api/connect_ca_test.go | 1 + 12 files changed, 81 insertions(+), 32 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index e7958056e1..ea42b09104 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -544,6 +544,9 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { "token": "Token", "root_pki_path": "RootPKIPath", "intermediate_pki_path": "IntermediatePKIPath", + + // Common CA config + "leaf_cert_ttl": "LeafCertTTL", }) } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 4df0bd5a09..27e9e0bbb8 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2545,7 +2545,8 @@ func TestFullConfig(t *testing.T) { "connect": { "ca_provider": "consul", "ca_config": { - "RotationPeriod": "90h" + "RotationPeriod": "90h", + "LeafCertTTL": "1h" }, "enabled": true, "proxy_defaults": { @@ -3006,7 +3007,8 @@ func TestFullConfig(t *testing.T) { connect { ca_provider = "consul" ca_config { - "RotationPeriod" = "90h" + rotation_period = "90h" + leaf_cert_ttl = "1h" } enabled = true proxy_defaults { @@ -3610,6 +3612,7 @@ func TestFullConfig(t *testing.T) { ConnectCAProvider: "consul", ConnectCAConfig: map[string]interface{}{ "RotationPeriod": "90h", + "LeafCertTTL": "1h", }, ConnectProxyAllowManagedRoot: false, ConnectProxyAllowManagedAPIRegistration: false, diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 3d2f4ceeb1..5cbf744ab8 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -214,8 +214,7 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) { x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth, }, - // todo(kyhavlov): add a way to set the cert lifetime here from the CA config - NotAfter: effectiveNow.Add(3 * 24 * time.Hour), + NotAfter: effectiveNow.Add(c.config.LeafCertTTL), NotBefore: effectiveNow, AuthorityKeyId: keyId, SubjectKeyId: keyId, diff --git a/agent/connect/ca/provider_consul_config.go b/agent/connect/ca/provider_consul_config.go index 9eae88610a..0ef5fe1408 100644 --- a/agent/connect/ca/provider_consul_config.go +++ b/agent/connect/ca/provider_consul_config.go @@ -10,10 +10,12 @@ import ( ) func ParseConsulCAConfig(raw map[string]interface{}) (*structs.ConsulCAProviderConfig, error) { - var config structs.ConsulCAProviderConfig + config := structs.ConsulCAProviderConfig{ + CommonCAProviderConfig: defaultCommonConfig(), + } + decodeConf := &mapstructure.DecoderConfig{ DecodeHook: ParseDurationFunc(), - ErrorUnused: true, Result: &config, WeaklyTypedInput: true, } @@ -75,3 +77,9 @@ func Uint8ToString(bs []uint8) string { } return string(b) } + +func defaultCommonConfig() structs.CommonCAProviderConfig { + return structs.CommonCAProviderConfig{ + LeafCertTTL: 3 * 24 * time.Hour, + } +} diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 7c510fff18..2a37f1b94f 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -117,12 +117,13 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) { func TestConsulCAProvider_SignLeaf(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) conf := testConsulCAConfig() + conf.Config["LeafCertTTL"] = "1h" delegate := newMockDelegate(t, conf) provider, err := NewConsulProvider(conf.Config, delegate) - assert.NoError(err) + require.NoError(err) spiffeService := &connect.SpiffeIDService{ Host: "node1", @@ -136,20 +137,21 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { raw, _ := connect.TestCSR(t, spiffeService) csr, err := connect.ParseCSR(raw) - assert.NoError(err) + require.NoError(err) cert, err := provider.Sign(csr) - assert.NoError(err) + require.NoError(err) parsed, err := connect.ParseCert(cert) - assert.NoError(err) - assert.Equal(parsed.URIs[0], spiffeService.URI()) - assert.Equal(parsed.Subject.CommonName, "foo") - assert.Equal(uint64(2), parsed.SerialNumber.Uint64()) + require.NoError(err) + require.Equal(parsed.URIs[0], spiffeService.URI()) + require.Equal(parsed.Subject.CommonName, "foo") + require.Equal(uint64(2), parsed.SerialNumber.Uint64()) // Ensure the cert is valid now and expires within the correct limit. - assert.True(parsed.NotAfter.Sub(time.Now()) < 3*24*time.Hour) - assert.True(parsed.NotBefore.Before(time.Now())) + now := time.Now() + require.True(parsed.NotAfter.Sub(now) < time.Hour) + require.True(parsed.NotBefore.Before(now)) } // Generate a new cert for another service and make sure @@ -159,20 +161,20 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { raw, _ := connect.TestCSR(t, spiffeService) csr, err := connect.ParseCSR(raw) - assert.NoError(err) + require.NoError(err) cert, err := provider.Sign(csr) - assert.NoError(err) + require.NoError(err) parsed, err := connect.ParseCert(cert) - assert.NoError(err) - assert.Equal(parsed.URIs[0], spiffeService.URI()) - assert.Equal(parsed.Subject.CommonName, "bar") - assert.Equal(parsed.SerialNumber.Uint64(), uint64(2)) + require.NoError(err) + require.Equal(parsed.URIs[0], spiffeService.URI()) + require.Equal(parsed.Subject.CommonName, "bar") + require.Equal(parsed.SerialNumber.Uint64(), uint64(2)) // Ensure the cert is valid now and expires within the correct limit. - assert.True(parsed.NotAfter.Sub(time.Now()) < 3*24*time.Hour) - assert.True(parsed.NotBefore.Before(time.Now())) + require.True(parsed.NotAfter.Sub(time.Now()) < 3*24*time.Hour) + require.True(parsed.NotBefore.Before(time.Now())) } } diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 8eaf945657..82a6d5c1f1 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -227,6 +227,7 @@ func (v *VaultProvider) Sign(csr *x509.CertificateRequest) (string, error) { // Use the leaf cert role to sign a new cert for this CSR. response, err := v.client.Logical().Write(v.config.IntermediatePKIPath+"sign/"+VaultCALeafCertRole, map[string]interface{}{ "csr": pemBuf.String(), + "ttl": fmt.Sprintf("%.0fh", v.config.LeafCertTTL.Hours()), }) if err != nil { return "", fmt.Errorf("error issuing cert: %v", err) @@ -283,10 +284,12 @@ func (v *VaultProvider) Cleanup() error { } func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderConfig, error) { - var config structs.VaultCAProviderConfig + config := structs.VaultCAProviderConfig{ + CommonCAProviderConfig: defaultCommonConfig(), + } decodeConf := &mapstructure.DecoderConfig{ - ErrorUnused: true, + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), Result: &config, WeaklyTypedInput: true, } diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 37d686549e..3769d79d16 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -16,6 +16,10 @@ import ( ) func testVaultCluster(t *testing.T) (*VaultProvider, *vault.Core, net.Listener) { + return testVaultClusterWithConfig(t, nil) +} + +func testVaultClusterWithConfig(t *testing.T, rawConf map[string]interface{}) (*VaultProvider, *vault.Core, net.Listener) { if err := vault.AddTestLogicalBackend("pki", pki.Factory); err != nil { t.Fatal(err) } @@ -23,12 +27,17 @@ func testVaultCluster(t *testing.T) (*VaultProvider, *vault.Core, net.Listener) ln, addr := vaulthttp.TestServer(t, core) - provider, err := NewVaultProvider(map[string]interface{}{ + conf := map[string]interface{}{ "Address": addr, "Token": token, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", - }, "asdf") + } + for k, v := range rawConf { + conf[k] = v + } + + provider, err := NewVaultProvider(conf, "asdf") if err != nil { t.Fatal(err) } @@ -87,7 +96,9 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { t.Parallel() require := require.New(t) - provider, core, listener := testVaultCluster(t) + provider, core, listener := testVaultClusterWithConfig(t, map[string]interface{}{ + "LeafCertTTL": "1h", + }) defer core.Shutdown() defer listener.Close() client, err := vaultapi.NewClient(&vaultapi.Config{ @@ -120,8 +131,9 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { firstSerial = parsed.SerialNumber.Uint64() // Ensure the cert is valid now and expires within the correct limit. - require.True(parsed.NotAfter.Sub(time.Now()) < 3*24*time.Hour) - require.True(parsed.NotBefore.Before(time.Now())) + now := time.Now() + require.True(parsed.NotAfter.Sub(now) < time.Hour) + require.True(parsed.NotBefore.Before(now)) } // Generate a new cert for another service and make sure diff --git a/agent/connect_ca_endpoint_test.go b/agent/connect_ca_endpoint_test.go index afaa5f049b..9d2b4eb50f 100644 --- a/agent/connect_ca_endpoint_test.go +++ b/agent/connect_ca_endpoint_test.go @@ -68,6 +68,7 @@ func TestConnectCAConfig(t *testing.T) { expected := &structs.ConsulCAProviderConfig{ RotationPeriod: 90 * 24 * time.Hour, } + expected.LeafCertTTL = 72 * time.Hour // Get the initial config. { @@ -89,7 +90,8 @@ func TestConnectCAConfig(t *testing.T) { { "Provider": "consul", "Config": { - "RotationPeriod": 3600000000000 + "LeafCertTTL": "72h", + "RotationPeriod": "1h" } }`)) req, _ := http.NewRequest("PUT", "/v1/connect/ca/configuration", body) diff --git a/agent/consul/config.go b/agent/consul/config.go index b878d9a99d..66ca27d6e5 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -438,6 +438,7 @@ func DefaultConfig() *Config { Provider: "consul", Config: map[string]interface{}{ "RotationPeriod": "2160h", + "LeafCertTTL": "72h", }, }, diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 375a7df325..353275d78a 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -192,7 +192,13 @@ type CAConfiguration struct { RaftIndex } +type CommonCAProviderConfig struct { + LeafCertTTL time.Duration +} + type ConsulCAProviderConfig struct { + CommonCAProviderConfig `mapstructure:",squash"` + PrivateKey string RootCert string RotationPeriod time.Duration @@ -208,6 +214,8 @@ type CAConsulProviderState struct { } type VaultCAProviderConfig struct { + CommonCAProviderConfig `mapstructure:",squash"` + Address string Token string RootPKIPath string diff --git a/api/connect_ca.go b/api/connect_ca.go index 947f709763..a863d21d4a 100644 --- a/api/connect_ca.go +++ b/api/connect_ca.go @@ -21,8 +21,15 @@ type CAConfig struct { ModifyIndex uint64 } +// CommonCAProviderConfig is the common options available to all CA providers. +type CommonCAProviderConfig struct { + LeafCertTTL time.Duration +} + // ConsulCAProviderConfig is the config for the built-in Consul CA provider. type ConsulCAProviderConfig struct { + CommonCAProviderConfig `mapstructure:",squash"` + PrivateKey string RootCert string RotationPeriod time.Duration diff --git a/api/connect_ca_test.go b/api/connect_ca_test.go index 77d047e953..26a8ec4071 100644 --- a/api/connect_ca_test.go +++ b/api/connect_ca_test.go @@ -64,6 +64,7 @@ func TestAPI_ConnectCAConfig_get_set(t *testing.T) { expected := &ConsulCAProviderConfig{ RotationPeriod: 90 * 24 * time.Hour, } + expected.LeafCertTTL = 72 * time.Hour // This fails occasionally if server doesn't have time to bootstrap CA so // retry