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 <akeesler@vmware.com>
This commit is contained in:
Andrew Keesler 2020-08-27 15:59:47 -04:00
parent e39a38ecf2
commit 92a6b7f4a4
No known key found for this signature in database
GPG Key ID: 27CE0444346F9413
5 changed files with 24 additions and 12 deletions

View File

@ -85,11 +85,13 @@ func Load(certPEM string, keyPEM string) (*CA, error) {
}, nil }, nil
} }
// New generates a fresh certificate authority with the given subject. // New generates a fresh certificate authority with the given subject and ttl.
func New(subject pkix.Name) (*CA, error) { return newInternal(subject, secureEnv()) } 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. // 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} ca := CA{env: env}
// Generate a random serial for the CA // Generate a random serial for the CA
serialNumber, err := randomSerial(env.serialRNG) serialNumber, err := randomSerial(env.serialRNG)
@ -104,10 +106,10 @@ func newInternal(subject pkix.Name, env env) (*CA, error) {
} }
ca.signer = privateKey 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() now := env.clock()
notBefore := now.Add(-certBackdate) notBefore := now.Add(-certBackdate)
notAfter := now.Add(24 * time.Hour * 365 * 100) notAfter := now.Add(ttl)
// Create CA cert template // Create CA cert template
caTemplate := x509.Certificate{ caTemplate := x509.Certificate{

View File

@ -86,7 +86,8 @@ func TestLoad(t *testing.T) {
} }
func TestNew(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.NoError(t, err)
require.NotNil(t, got) require.NotNil(t, got)
@ -94,6 +95,8 @@ func TestNew(t *testing.T) {
caCert, err := x509.ParseCertificate(got.caCertBytes) caCert, err := x509.ParseCertificate(got.caCertBytes)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, "Test CA", caCert.Subject.CommonName) 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) { func TestNewInternal(t *testing.T) {
@ -101,6 +104,7 @@ func TestNewInternal(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
ttl time.Duration
env env env env
wantErr string wantErr string
wantCommonName string wantCommonName string
@ -137,6 +141,7 @@ func TestNewInternal(t *testing.T) {
}, },
{ {
name: "success", name: "success",
ttl: time.Minute,
env: env{ env: env{
serialRNG: strings.NewReader(strings.Repeat("x", 64)), serialRNG: strings.NewReader(strings.Repeat("x", 64)),
keygenRNG: strings.NewReader(strings.Repeat("y", 64)), keygenRNG: strings.NewReader(strings.Repeat("y", 64)),
@ -144,14 +149,14 @@ func TestNewInternal(t *testing.T) {
clock: func() time.Time { return now }, clock: func() time.Time { return now },
}, },
wantCommonName: "Test CA", wantCommonName: "Test CA",
wantNotAfter: now.Add(100 * 365 * 24 * time.Hour), wantNotAfter: now.Add(time.Minute),
wantNotBefore: now.Add(-5 * time.Minute), wantNotBefore: now.Add(-5 * time.Minute),
}, },
} }
for _, tt := range tests { for _, tt := range tests {
tt := tt tt := tt
t.Run(tt.name, func(t *testing.T) { 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 != "" { if tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr) require.EqualError(t, err, tt.wantErr)
require.Nil(t, got) require.Nil(t, got)

View File

@ -37,8 +37,8 @@ type certsManagerController struct {
aggregatorClient aggregatorclient.Interface aggregatorClient aggregatorclient.Interface
secretInformer corev1informers.SecretInformer secretInformer corev1informers.SecretInformer
// certDuration is the lifetime of the serving certificate that this // certDuration is the lifetime of both the serving certificate and its CA
// controller will use when issuing the serving certificate. // certificate that this controller will use when issuing the certificates.
certDuration time.Duration certDuration time.Duration
} }
@ -88,7 +88,7 @@ func (c *certsManagerController) Sync(ctx controller.Context) error {
} }
// Create a CA. // 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 { if err != nil {
return fmt.Errorf("could not initialize CA: %w", err) return fmt.Errorf("could not initialize CA: %w", err)
} }

View File

@ -221,6 +221,10 @@ func TestManagerControllerSync(t *testing.T) {
r.NotEmpty(actualPrivateKey) r.NotEmpty(actualPrivateKey)
r.NotEmpty(actualCertChain) 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 // Validate the created cert using the CA, and also validate the cert's hostname
validCert := testutil.ValidateCertificate(t, actualCACert, actualCertChain) validCert := testutil.ValidateCertificate(t, actualCACert, actualCertChain)
validCert.RequireDNSName("pinniped-api." + installedInNamespace + ".svc") validCert.RequireDNSName("pinniped-api." + installedInNamespace + ".svc")

View File

@ -45,7 +45,8 @@ type APIConfigSpec struct {
type ServingCertificateConfigSpec struct { type ServingCertificateConfigSpec struct {
// DurationSeconds is the validity period, in seconds, of the API serving // DurationSeconds is the validity period, in seconds, of the API serving
// certificate. By default, the serving certificate is issued for 31536000 // 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"` DurationSeconds *int64 `json:"durationSeconds,omitempty"`
// RenewBeforeSeconds is the period of time, in seconds, that pinniped will // RenewBeforeSeconds is the period of time, in seconds, that pinniped will