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