Merge pull request #8646 from hashicorp/common-intermediate-ttl

Move IntermediateCertTTL to common CA config

.changelog/8646.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect: fix Vault provider not respecting IntermediateCertTTL
+```

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,
+		LeafCertTTL:         3 * 24 * time.Hour,
-		PrivateKeyType: connect.DefaultPrivateKeyType,
+		IntermediateCertTTL: 24 * 365 * time.Hour,
-		PrivateKeyBits: connect.DefaultPrivateKeyBits,
+		PrivateKeyType:      connect.DefaultPrivateKeyType,
+		PrivateKeyBits:      connect.DefaultPrivateKeyBits,
 	}
 }

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,
+		LeafCertTTL:         30 * time.Hour,
-		SkipValidate:     true,
+		IntermediateCertTTL: 90 * time.Hour,
-		CSRMaxPerSecond:  5.25,
+		SkipValidate:        true,
-		CSRMaxConcurrent: 55,
+		CSRMaxPerSecond:     5.25,
-		PrivateKeyType:   "rsa",
+		CSRMaxConcurrent:    55,
-		PrivateKeyBits:   4096,
+		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",
+					"ExistingARN":         "arn://foo",
-					"DeleteOnExit": true,
+					"DeleteOnExit":        true,
+					"IntermediateCertTTL": "90h",
 				},
 			},
 			expectConfig: &structs.AWSCAProviderConfig{

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(),
 			},
 		})
 

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,
+		LeafCertTTL:         3 * 24 * time.Hour,
-		PrivateKeyType: kc.keyType,
+		IntermediateCertTTL: 365 * 24 * time.Hour,
-		PrivateKeyBits: kc.keyBits,
+		PrivateKeyType:      kc.keyType,
+		PrivateKeyBits:      kc.keyBits,
 	}
 }
 

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
+	PrivateKey     string
-	RootCert            string
+	RootCert       string
-	RotationPeriod      time.Duration
+	RotationPeriod time.Duration
-	IntermediateCertTTL 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
 }
 

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",
+					"RotationPeriod":      "2160h",
-					"LeafCertTTL":     "72h",
+					"LeafCertTTL":         "72h",
-					"CSRMaxPerSecond": "50",
+					"IntermediateCertTTL": "4320h",
+					"CSRMaxPerSecond":     "50",
 				},
 			},
 			want: &CommonCAProviderConfig{
-				LeafCertTTL:     72 * time.Hour,
+				LeafCertTTL:         72 * time.Hour,
-				CSRMaxPerSecond: 50,
+				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"),
+					"RotationPeriod":      []uint8("2160h"),
-					"LeafCertTTL":    []uint8("72h"),
+					"LeafCertTTL":         []uint8("72h"),
+					"IntermediateCertTTL": []uint8("4320h"),
 				},
 			},
 			want: &CommonCAProviderConfig{
-				LeafCertTTL:     72 * time.Hour,
+				LeafCertTTL:         72 * time.Hour,
-				CSRMaxPerSecond: 50, // The default value
+				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{
+			cfg: &CommonCAProviderConfig{
-				CommonCAProviderConfig: CommonCAProviderConfig{LeafCertTTL: 2 * time.Hour},
+				LeafCertTTL:         2 * time.Hour,
-				IntermediateCertTTL:    4 * 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{
+			cfg: &CommonCAProviderConfig{
-				CommonCAProviderConfig: CommonCAProviderConfig{LeafCertTTL: 5 * time.Hour},
+				LeafCertTTL:         5 * time.Hour,
-				IntermediateCertTTL:    15*time.Hour - 1,
+				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",
+			name: "good intermediate and leaf cert TTL, missing key type",
-			cfg: &ConsulCAProviderConfig{
+			cfg: &CommonCAProviderConfig{
-				CommonCAProviderConfig: CommonCAProviderConfig{LeafCertTTL: 1 * time.Hour},
+				LeafCertTTL:         1 * time.Hour,
-				IntermediateCertTTL:    4 * 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,
 		},