From 92a6b7f4a4b3a86bf1a7514be71bce60079a44e6 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 27 Aug 2020 15:59:47 -0400 Subject: [PATCH] Use same lifetime for serving cert and CA cert So that operators won't look at the lifetime of the CA cert and be like, "wtf, why does the serving cert have the lifetime that I specified, but its CA cert is valid for 100 years". Signed-off-by: Andrew Keesler --- internal/certauthority/certauthority.go | 12 +++++++----- internal/certauthority/certauthority_test.go | 11 ++++++++--- internal/controller/apicerts/certs_manager.go | 6 +++--- internal/controller/apicerts/certs_manager_test.go | 4 ++++ pkg/config/api/types.go | 3 ++- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index 516ad52a..5c2c0bb3 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -85,11 +85,13 @@ func Load(certPEM string, keyPEM string) (*CA, error) { }, nil } -// New generates a fresh certificate authority with the given subject. -func New(subject pkix.Name) (*CA, error) { return newInternal(subject, secureEnv()) } +// New generates a fresh certificate authority with the given subject and ttl. +func New(subject pkix.Name, ttl time.Duration) (*CA, error) { + return newInternal(subject, ttl, secureEnv()) +} // newInternal is the internal guts of New, broken out for easier testing. -func newInternal(subject pkix.Name, env env) (*CA, error) { +func newInternal(subject pkix.Name, ttl time.Duration, env env) (*CA, error) { ca := CA{env: env} // Generate a random serial for the CA serialNumber, err := randomSerial(env.serialRNG) @@ -104,10 +106,10 @@ func newInternal(subject pkix.Name, env env) (*CA, error) { } ca.signer = privateKey - // Make a CA certificate valid for 100 years and backdated by some amount. + // Make a CA certificate valid for some ttl and backdated by some amount. now := env.clock() notBefore := now.Add(-certBackdate) - notAfter := now.Add(24 * time.Hour * 365 * 100) + notAfter := now.Add(ttl) // Create CA cert template caTemplate := x509.Certificate{ diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 5563074d..8b9b24bc 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -86,7 +86,8 @@ func TestLoad(t *testing.T) { } func TestNew(t *testing.T) { - got, err := New(pkix.Name{CommonName: "Test CA"}) + now := time.Now() + got, err := New(pkix.Name{CommonName: "Test CA"}, time.Minute) require.NoError(t, err) require.NotNil(t, got) @@ -94,6 +95,8 @@ func TestNew(t *testing.T) { caCert, err := x509.ParseCertificate(got.caCertBytes) require.NoError(t, err) require.Equal(t, "Test CA", caCert.Subject.CommonName) + require.WithinDuration(t, now.Add(-5*time.Minute), caCert.NotBefore, 10*time.Second) + require.WithinDuration(t, now.Add(time.Minute), caCert.NotAfter, 10*time.Second) } func TestNewInternal(t *testing.T) { @@ -101,6 +104,7 @@ func TestNewInternal(t *testing.T) { tests := []struct { name string + ttl time.Duration env env wantErr string wantCommonName string @@ -137,6 +141,7 @@ func TestNewInternal(t *testing.T) { }, { name: "success", + ttl: time.Minute, env: env{ serialRNG: strings.NewReader(strings.Repeat("x", 64)), keygenRNG: strings.NewReader(strings.Repeat("y", 64)), @@ -144,14 +149,14 @@ func TestNewInternal(t *testing.T) { clock: func() time.Time { return now }, }, wantCommonName: "Test CA", - wantNotAfter: now.Add(100 * 365 * 24 * time.Hour), + wantNotAfter: now.Add(time.Minute), wantNotBefore: now.Add(-5 * time.Minute), }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := newInternal(pkix.Name{CommonName: "Test CA"}, tt.env) + got, err := newInternal(pkix.Name{CommonName: "Test CA"}, tt.ttl, tt.env) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) require.Nil(t, got) diff --git a/internal/controller/apicerts/certs_manager.go b/internal/controller/apicerts/certs_manager.go index 030e0289..be09e0f2 100644 --- a/internal/controller/apicerts/certs_manager.go +++ b/internal/controller/apicerts/certs_manager.go @@ -37,8 +37,8 @@ type certsManagerController struct { aggregatorClient aggregatorclient.Interface secretInformer corev1informers.SecretInformer - // certDuration is the lifetime of the serving certificate that this - // controller will use when issuing the serving certificate. + // certDuration is the lifetime of both the serving certificate and its CA + // certificate that this controller will use when issuing the certificates. certDuration time.Duration } @@ -88,7 +88,7 @@ func (c *certsManagerController) Sync(ctx controller.Context) error { } // Create a CA. - aggregatedAPIServerCA, err := certauthority.New(pkix.Name{CommonName: "Pinniped CA"}) + aggregatedAPIServerCA, err := certauthority.New(pkix.Name{CommonName: "Pinniped CA"}, c.certDuration) if err != nil { return fmt.Errorf("could not initialize CA: %w", err) } diff --git a/internal/controller/apicerts/certs_manager_test.go b/internal/controller/apicerts/certs_manager_test.go index b78bfe08..9d089ac4 100644 --- a/internal/controller/apicerts/certs_manager_test.go +++ b/internal/controller/apicerts/certs_manager_test.go @@ -221,6 +221,10 @@ func TestManagerControllerSync(t *testing.T) { r.NotEmpty(actualPrivateKey) r.NotEmpty(actualCertChain) + // Validate the created CA's lifetime. + validCACert := testutil.ValidateCertificate(t, actualCACert, actualCACert) + validCACert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute) + // Validate the created cert using the CA, and also validate the cert's hostname validCert := testutil.ValidateCertificate(t, actualCACert, actualCertChain) validCert.RequireDNSName("pinniped-api." + installedInNamespace + ".svc") diff --git a/pkg/config/api/types.go b/pkg/config/api/types.go index 2114c0fa..81a8b1e1 100644 --- a/pkg/config/api/types.go +++ b/pkg/config/api/types.go @@ -45,7 +45,8 @@ type APIConfigSpec struct { type ServingCertificateConfigSpec struct { // DurationSeconds is the validity period, in seconds, of the API serving // certificate. By default, the serving certificate is issued for 31536000 - // seconds (1 year). + // seconds (1 year). This value is also used for the serving certificate's + // CA certificate. DurationSeconds *int64 `json:"durationSeconds,omitempty"` // RenewBeforeSeconds is the period of time, in seconds, that pinniped will