diff --git a/agent/connect/ca/provider_consul_config.go b/agent/connect/ca/provider_consul_config.go index 48faa0717..ee0b551df 100644 --- a/agent/connect/ca/provider_consul_config.go +++ b/agent/connect/ca/provider_consul_config.go @@ -44,13 +44,13 @@ func ParseConsulCAConfig(raw map[string]interface{}) (*structs.ConsulCAProviderC func defaultConsulCAProviderConfig() structs.ConsulCAProviderConfig { return structs.ConsulCAProviderConfig{ CommonCAProviderConfig: defaultCommonConfig(), - IntermediateCertTTL: 24 * 365 * time.Hour, } } func defaultCommonConfig() structs.CommonCAProviderConfig { return structs.CommonCAProviderConfig{ - LeafCertTTL: 3 * 24 * time.Hour, - PrivateKeyType: connect.DefaultPrivateKeyType, - PrivateKeyBits: connect.DefaultPrivateKeyBits, + LeafCertTTL: 3 * 24 * time.Hour, + IntermediateCertTTL: 24 * 365 * time.Hour, + PrivateKeyType: connect.DefaultPrivateKeyType, + PrivateKeyBits: connect.DefaultPrivateKeyBits, } } diff --git a/agent/connect/ca/provider_test.go b/agent/connect/ca/provider_test.go index 9d0344b31..5b0459a57 100644 --- a/agent/connect/ca/provider_test.go +++ b/agent/connect/ca/provider_test.go @@ -26,12 +26,13 @@ func TestStructs_CAConfiguration_MsgpackEncodeDecode(t *testing.T) { "PrivateKeyBits": int64(4096), } expectCommonBase := &structs.CommonCAProviderConfig{ - LeafCertTTL: 30 * time.Hour, - SkipValidate: true, - CSRMaxPerSecond: 5.25, - CSRMaxConcurrent: 55, - PrivateKeyType: "rsa", - PrivateKeyBits: 4096, + LeafCertTTL: 30 * time.Hour, + IntermediateCertTTL: 90 * time.Hour, + SkipValidate: true, + CSRMaxPerSecond: 5.25, + CSRMaxConcurrent: 55, + PrivateKeyType: "rsa", + PrivateKeyBits: 4096, } cases := map[string]testcase{ @@ -60,7 +61,6 @@ func TestStructs_CAConfiguration_MsgpackEncodeDecode(t *testing.T) { PrivateKey: "key", RootCert: "cert", RotationPeriod: 5 * time.Minute, - IntermediateCertTTL: 90 * time.Hour, DisableCrossSigning: true, }, parseFunc: func(t *testing.T, raw map[string]interface{}) interface{} { @@ -86,6 +86,7 @@ func TestStructs_CAConfiguration_MsgpackEncodeDecode(t *testing.T) { "Token": "token", "RootPKIPath": "root-pki/", "IntermediatePKIPath": "im-pki/", + "IntermediateCertTTL": "90h", "CAFile": "ca-file", "CAPath": "ca-path", "CertFile": "cert-file", @@ -126,8 +127,9 @@ func TestStructs_CAConfiguration_MsgpackEncodeDecode(t *testing.T) { ModifyIndex: 99, }, Config: map[string]interface{}{ - "ExistingARN": "arn://foo", - "DeleteOnExit": true, + "ExistingARN": "arn://foo", + "DeleteOnExit": true, + "IntermediateCertTTL": "90h", }, }, expectConfig: &structs.AWSCAProviderConfig{ diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 8dbac7e22..1a307113d 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -153,7 +153,7 @@ func (v *VaultProvider) setupIntermediatePKIPath() error { Type: "pki", Description: "intermediate CA backend for Consul Connect", Config: vaultapi.MountConfigInput{ - MaxLeaseTTL: "2160h", + MaxLeaseTTL: v.config.IntermediateCertTTL.String(), }, }) diff --git a/agent/connect/generate_test.go b/agent/connect/generate_test.go index 6d79cd4af..0785d3c87 100644 --- a/agent/connect/generate_test.go +++ b/agent/connect/generate_test.go @@ -38,9 +38,10 @@ var badParams = []KeyConfig{ func makeConfig(kc KeyConfig) structs.CommonCAProviderConfig { return structs.CommonCAProviderConfig{ - LeafCertTTL: 3 * 24 * time.Hour, - PrivateKeyType: kc.keyType, - PrivateKeyBits: kc.keyBits, + LeafCertTTL: 3 * 24 * time.Hour, + IntermediateCertTTL: 365 * 24 * time.Hour, + PrivateKeyType: kc.keyType, + PrivateKeyBits: kc.keyBits, } } diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 3755929f1..6b57e9e39 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -314,7 +314,8 @@ func (c *CAConfiguration) GetCommonConfig() (*CommonCAProviderConfig, error) { } type CommonCAProviderConfig struct { - LeafCertTTL time.Duration + LeafCertTTL time.Duration + IntermediateCertTTL time.Duration SkipValidate bool @@ -360,6 +361,10 @@ type CommonCAProviderConfig struct { var MinLeafCertTTL = time.Hour var MaxLeafCertTTL = 365 * 24 * time.Hour +// intermediateCertRenewInterval is the interval at which the expiration +// of the intermediate cert is checked and renewed if necessary. +var IntermediateCertRenewInterval = time.Hour + func (c CommonCAProviderConfig) Validate() error { if c.SkipValidate { return nil @@ -373,6 +378,33 @@ func (c CommonCAProviderConfig) Validate() error { return fmt.Errorf("leaf cert TTL must be less than %s", MaxLeafCertTTL) } + if c.IntermediateCertTTL < (3 * IntermediateCertRenewInterval) { + // Intermediate Certificates are checked every + // hour(intermediateCertRenewInterval) if they are about to + // expire. Recreating an intermediate certs is started once + // more than half its lifetime has passed. + // If it would be 2h, worst case is that the check happens + // right before half time and when the check happens again, the + // certificate is very close to expiring, leaving only a small + // timeframe to renew. 3h leaves more than 30min to recreate. + // Right now the minimum LeafCertTTL is 1h, which means this + // check not strictly needed, because the same thing is covered + // in the next check too. But just in case minimum LeafCertTTL + // changes at some point, this validation must still be + // performed. + return fmt.Errorf("Intermediate Cert TTL must be greater or equal than %dh", 3*int(IntermediateCertRenewInterval.Hours())) + } + if c.IntermediateCertTTL < (3 * c.LeafCertTTL) { + // Intermediate Certificates are being sent to the proxy when + // the Leaf Certificate changes because they are bundled + // together. + // That means that the Intermediate Certificate TTL must be at + // a minimum of 3 * Leaf Certificate TTL to ensure that the new + // Intermediate is being set together with the Leaf Certificate + // before it expires. + return fmt.Errorf("Intermediate Cert TTL must be greater or equal than 3 * LeafCertTTL (>=%s).", 3*c.LeafCertTTL) + } + switch c.PrivateKeyType { case "ec": if c.PrivateKeyBits != 224 && c.PrivateKeyBits != 256 && c.PrivateKeyBits != 384 && c.PrivateKeyBits != 521 { @@ -392,10 +424,9 @@ func (c CommonCAProviderConfig) Validate() error { type ConsulCAProviderConfig struct { CommonCAProviderConfig `mapstructure:",squash"` - PrivateKey string - RootCert string - RotationPeriod time.Duration - IntermediateCertTTL time.Duration + PrivateKey string + RootCert string + RotationPeriod time.Duration // DisableCrossSigning is really only useful in test code to use the built in // provider while exercising logic that depends on the CA provider ability to @@ -404,37 +435,7 @@ type ConsulCAProviderConfig struct { DisableCrossSigning bool } -// intermediateCertRenewInterval is the interval at which the expiration -// of the intermediate cert is checked and renewed if necessary. -var IntermediateCertRenewInterval = time.Hour - func (c *ConsulCAProviderConfig) Validate() error { - if c.IntermediateCertTTL < (3 * IntermediateCertRenewInterval) { - // Intermediate Certificates are checked every - // hour(intermediateCertRenewInterval) if they are about to - // expire. Recreating an intermediate certs is started once - // more than half its lifetime has passed. - // If it would be 2h, worst case is that the check happens - // right before half time and when the check happens again, the - // certificate is very close to expiring, leaving only a small - // timeframe to renew. 3h leaves more than 30min to recreate. - // Right now the minimum LeafCertTTL is 1h, which means this - // check not strictly needed, because the same thing is covered - // in the next check too. But just in case minimum LeafCertTTL - // changes at some point, this validation must still be - // performed. - return fmt.Errorf("Intermediate Cert TTL must be greater or equal than %dh", 3*int(IntermediateCertRenewInterval.Hours())) - } - if c.IntermediateCertTTL < (3 * c.CommonCAProviderConfig.LeafCertTTL) { - // Intermediate Certificates are being sent to the proxy when - // the Leaf Certificate changes because they are bundled - // together. - // That means that the Intermediate Certificate TTL must be at - // a minimum of 3 * Leaf Certificate TTL to ensure that the new - // Intermediate is being set together with the Leaf Certificate - // before it expires. - return fmt.Errorf("Intermediate Cert TTL must be greater or equal than 3 * LeafCertTTL (>=%s).", 3*c.CommonCAProviderConfig.LeafCertTTL) - } return nil } diff --git a/agent/structs/connect_ca_test.go b/agent/structs/connect_ca_test.go index 4f79a91eb..a79b3d38e 100644 --- a/agent/structs/connect_ca_test.go +++ b/agent/structs/connect_ca_test.go @@ -18,14 +18,16 @@ func TestCAConfiguration_GetCommonConfig(t *testing.T) { name: "basic defaults", cfg: &CAConfiguration{ Config: map[string]interface{}{ - "RotationPeriod": "2160h", - "LeafCertTTL": "72h", - "CSRMaxPerSecond": "50", + "RotationPeriod": "2160h", + "LeafCertTTL": "72h", + "IntermediateCertTTL": "4320h", + "CSRMaxPerSecond": "50", }, }, want: &CommonCAProviderConfig{ - LeafCertTTL: 72 * time.Hour, - CSRMaxPerSecond: 50, + LeafCertTTL: 72 * time.Hour, + IntermediateCertTTL: 4320 * time.Hour, + CSRMaxPerSecond: 50, }, }, { @@ -38,13 +40,15 @@ func TestCAConfiguration_GetCommonConfig(t *testing.T) { name: "basic defaults after encoding fun", cfg: &CAConfiguration{ Config: map[string]interface{}{ - "RotationPeriod": []uint8("2160h"), - "LeafCertTTL": []uint8("72h"), + "RotationPeriod": []uint8("2160h"), + "LeafCertTTL": []uint8("72h"), + "IntermediateCertTTL": []uint8("4320h"), }, }, want: &CommonCAProviderConfig{ - LeafCertTTL: 72 * time.Hour, - CSRMaxPerSecond: 50, // The default value + LeafCertTTL: 72 * time.Hour, + IntermediateCertTTL: 4320 * time.Hour, + CSRMaxPerSecond: 50, // The default value }, }, } @@ -63,39 +67,60 @@ func TestCAConfiguration_GetCommonConfig(t *testing.T) { func TestCAProviderConfig_Validate(t *testing.T) { tests := []struct { name string - cfg *ConsulCAProviderConfig + cfg *CommonCAProviderConfig wantErr bool wantMsg string }{ { name: "defaults", - cfg: &ConsulCAProviderConfig{}, + cfg: &CommonCAProviderConfig{}, wantErr: true, - wantMsg: "Intermediate Cert TTL must be greater or equal than 3h", + wantMsg: "leaf cert TTL must be greater or equal than 1h0m0s", }, { name: "intermediate cert ttl too short", - cfg: &ConsulCAProviderConfig{ - CommonCAProviderConfig: CommonCAProviderConfig{LeafCertTTL: 2 * time.Hour}, - IntermediateCertTTL: 4 * time.Hour, + cfg: &CommonCAProviderConfig{ + LeafCertTTL: 2 * time.Hour, + IntermediateCertTTL: 4 * time.Hour, }, wantErr: true, wantMsg: "Intermediate Cert TTL must be greater or equal than 3 * LeafCertTTL (>=6h0m0s).", }, { name: "intermediate cert ttl too short", - cfg: &ConsulCAProviderConfig{ - CommonCAProviderConfig: CommonCAProviderConfig{LeafCertTTL: 5 * time.Hour}, - IntermediateCertTTL: 15*time.Hour - 1, + cfg: &CommonCAProviderConfig{ + LeafCertTTL: 5 * time.Hour, + IntermediateCertTTL: 15*time.Hour - 1, }, wantErr: true, wantMsg: "Intermediate Cert TTL must be greater or equal than 3 * LeafCertTTL (>=15h0m0s).", }, { - name: "good intermediate and leaf cert TTL", - cfg: &ConsulCAProviderConfig{ - CommonCAProviderConfig: CommonCAProviderConfig{LeafCertTTL: 1 * time.Hour}, - IntermediateCertTTL: 4 * time.Hour, + name: "good intermediate and leaf cert TTL, missing key type", + cfg: &CommonCAProviderConfig{ + LeafCertTTL: 1 * time.Hour, + IntermediateCertTTL: 4 * time.Hour, + }, + wantErr: true, + wantMsg: "private key type must be either 'ec' or 'rsa'", + }, + { + name: "good intermediate/leaf cert TTL/key type, missing bits", + cfg: &CommonCAProviderConfig{ + LeafCertTTL: 1 * time.Hour, + IntermediateCertTTL: 4 * time.Hour, + PrivateKeyType: "ec", + }, + wantErr: true, + wantMsg: "EC key length must be one of (224, 256, 384, 521) bits", + }, + { + name: "good intermediate/leaf cert TTL/key type/bits", + cfg: &CommonCAProviderConfig{ + LeafCertTTL: 1 * time.Hour, + IntermediateCertTTL: 4 * time.Hour, + PrivateKeyType: "ec", + PrivateKeyBits: 256, }, wantErr: false, },