From 91c8f747f4909adfec3a57e9ce7e4a14fcf04dc6 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Tue, 21 Sep 2021 09:19:50 -0400 Subject: [PATCH] certauthority: tolerate larger clock skew between API server and pinniped This change updates our certificate code to use the same 5 minute backdate that is used by the Kubernetes controller manager. This helps to account for clock skews between the API servers and the kubelets that are running the pinniped pods. While this backdating reflects a large percentage of the lifetime of our short lived certificates (100% for the 5 minute client certificates), even a 10 minute irrevocable client certificate is within our limits. When we move to the CSR based short lived certificates, they will always have at least a 15 minute lifetime (5 minute backdating plus 10 minute minimum valid duration). Signed-off-by: Monis Khan --- internal/certauthority/certauthority.go | 10 ++++------ internal/certauthority/certauthority_test.go | 4 ++-- .../impersonatorconfig/impersonator_config_test.go | 4 ++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index 60aed89f..9fcfdcfb 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -24,12 +24,10 @@ import ( ) // certBackdate is the amount of time before time.Now() that will be used to set -// a certificate's NotBefore field. -// -// This could certainly be made configurable by an installer of pinniped, but we -// will see if we can save adding a configuration knob with a reasonable default -// here. -const certBackdate = 10 * time.Second +// a certificate's NotBefore field. We use the same hard coded and unconfigurable +// backdate value as used by the Kubernetes controller manager certificate signer: +// https://github.com/kubernetes/kubernetes/blob/68d646a101005e95379d84160adf01d146bdd149/pkg/controller/certificates/signer/signer.go#L199 +const certBackdate = 5 * time.Minute type env struct { // secure random number generators for various steps (usually crypto/rand.Reader, but broken out here for tests). diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 016268be..4d55f6ff 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -96,7 +96,7 @@ func TestNew(t *testing.T) { caCert, err := x509.ParseCertificate(ca.caCertBytes) require.NoError(t, err) require.Equal(t, "Test CA", caCert.Subject.CommonName) - require.WithinDuration(t, now.Add(-10*time.Second), caCert.NotBefore, 10*time.Second) + 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) require.NotNil(t, ca.privateKey) @@ -153,7 +153,7 @@ func TestNewInternal(t *testing.T) { }, wantCommonName: "Test CA", wantNotAfter: now.Add(time.Minute), - wantNotBefore: now.Add(-10 * time.Second), + wantNotBefore: now.Add(-5 * time.Minute), }, } for _, tt := range tests { diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 00aeadd1..96829867 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -1056,7 +1056,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { caCert, err := x509.ParseCertificate(block.Bytes) require.NoError(t, err) require.Equal(t, "Pinniped Impersonation Proxy CA", caCert.Subject.CommonName) - require.WithinDuration(t, time.Now().Add(-10*time.Second), caCert.NotBefore, 10*time.Second) + require.WithinDuration(t, time.Now().Add(-5*time.Minute), caCert.NotBefore, 10*time.Second) require.WithinDuration(t, time.Now().Add(100*time.Hour*24*365), caCert.NotAfter, 10*time.Second) return createdCertPEM } @@ -1077,7 +1077,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NotNil(createdCertPEM) validCert := testutil.ValidateServerCertificate(t, string(caCert), string(createdCertPEM)) validCert.RequireMatchesPrivateKey(string(createdKeyPEM)) - validCert.RequireLifetime(time.Now().Add(-10*time.Second), time.Now().Add(100*time.Hour*24*365), 10*time.Second) + validCert.RequireLifetime(time.Now().Add(-5*time.Minute), time.Now().Add(100*time.Hour*24*365), 10*time.Second) } var requireSigningCertProviderHasLoadedCerts = func(certPEM, keyPEM []byte) {