Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move IntermediateCertTTL to common CA config #8646

Merged
merged 2 commits into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions agent/connect/ca/provider_consul_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
20 changes: 11 additions & 9 deletions agent/connect/ca/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{} {
Expand All @@ -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",
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
})

Expand Down
7 changes: 4 additions & 3 deletions agent/connect/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
71 changes: 36 additions & 35 deletions agent/structs/connect_ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ func (c *CAConfiguration) GetCommonConfig() (*CommonCAProviderConfig, error) {
}

type CommonCAProviderConfig struct {
LeafCertTTL time.Duration
LeafCertTTL time.Duration
IntermediateCertTTL time.Duration

SkipValidate bool

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
}

Expand Down
69 changes: 47 additions & 22 deletions agent/structs/connect_ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
{
Expand All @@ -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
},
},
}
Expand All @@ -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,
},
Expand Down