🤖 fix: date the serving CA's NotAfter from the injected clock

internal/aggregated/servingcert/servingcert.go

@@ -42,8 +42,10 @@ const (
 	// KeyKey is the Secret data key for the PEM serving private key.
 	KeyKey = corev1.TLSPrivateKeyKey
 
-	// ServingCertValidity is the lifetime of a serving certificate. The CA lives 10 years
-	// (client-go certutil.NewSelfSignedCACert).
+	// CAValidity is the lifetime of the CA, counted from the issuing clock (the same 10 years as
+	// client-go certutil.NewSelfSignedCACert).
+	CAValidity = 10 * 365 * 24 * time.Hour
+	// ServingCertValidity is the lifetime of a serving certificate.
 	ServingCertValidity = 365 * 24 * time.Hour
 	// clockSkewAllowance backdates serving certificates so small clock differences do not reject them.
 	clockSkewAllowance = time.Hour
@@ -113,16 +115,32 @@ func Generate(namespace string, now time.Time) (*Bundle, error) {
 	if err != nil {
 		return nil, fmt.Errorf("assertion failed: parse generated CA key: %w", err)
 	}
-	caCert, err := certutil.NewSelfSignedCACert(certutil.Config{
-		CommonName:   fmt.Sprintf("%s-ca@%d", ServiceName, now.Unix()),
-		Organization: []string{"coder-k8s"},
-		// certutil otherwise uses its own time.Now() with no backdating, so a clock that is behind
-		// the issuer's (another replica, kube-apiserver, or an injected now) sees a future CA.
-		NotBefore: now.Add(-clockSkewAllowance),
-	}, caKey)
+	serial, err := randomSerial()
+	if err != nil {
+		return nil, err
+	}
+	// The same certificate as client-go certutil.NewSelfSignedCACert, but dated from now: certutil
+	// takes NotAfter from its own time.Now(). NotBefore is backdated, so a clock that is behind the
+	// issuer's (another replica, kube-apiserver, or an injected now) does not see a future CA.
+	commonName := fmt.Sprintf("%s-ca@%d", ServiceName, now.Unix())
+	caTmpl := &x509.Certificate{
+		SerialNumber:          serial,
+		Subject:               pkix.Name{CommonName: commonName, Organization: []string{"coder-k8s"}},
+		DNSNames:              []string{commonName},
+		NotBefore:             now.Add(-clockSkewAllowance).UTC(),
+		NotAfter:              now.Add(CAValidity).UTC(),
+		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
+		IsCA:                  true,
+	}
+	caDER, err := x509.CreateCertificate(rand.Reader, caTmpl, caTmpl, caKey.Public(), caKey)
 	if err != nil {
 		return nil, fmt.Errorf("generate CA certificate: %w", err)
 	}
+	caCert, err := x509.ParseCertificate(caDER)
+	if err != nil {
+		return nil, fmt.Errorf("assertion failed: parse generated CA certificate: %w", err)
+	}
 	caCertPEM, err := certutil.EncodeCertificates(caCert)
 	if err != nil {
 		return nil, fmt.Errorf("encode CA certificate: %w", err)
@@ -143,13 +161,13 @@ func (b *Bundle) issueServingCert(namespace string, now time.Time) error {
 	if err != nil {
 		return fmt.Errorf("generate serving key: %w", err)
 	}
-	serial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64-1))
+	serial, err := randomSerial()
 	if err != nil {
-		return fmt.Errorf("generate serial: %w", err)
+		return err
 	}
 	names := DNSNames(namespace)
 	tmpl := &x509.Certificate{
-		SerialNumber: new(big.Int).Add(serial, big.NewInt(1)),
+		SerialNumber: serial,
 		Subject:      pkix.Name{CommonName: names[2]},
 		DNSNames:     names,
 		NotBefore:    now.Add(-clockSkewAllowance).UTC(),
@@ -266,6 +284,15 @@ func (b *Bundle) NeedsRenewal(namespace string, now time.Time) bool {
 	return false
 }
 
+// randomSerial returns a uniform random certificate serial number in [1, MaxInt64).
+func randomSerial() (*big.Int, error) {
+	serial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64-1))
+	if err != nil {
+		return nil, fmt.Errorf("generate serial: %w", err)
+	}
+	return serial.Add(serial, big.NewInt(1)), nil
+}
+
 func parseSingleCert(data []byte) (*x509.Certificate, error) {
 	certs, err := certutil.ParseCertsPEM(data)
 	if err != nil {

internal/aggregated/servingcert/servingcert_test.go

@@ -101,6 +101,39 @@ func TestGenerateDatesCAFromInjectedClock(t *testing.T) {
 	}
 }
 
+// Both ends of the CA's validity must come from the injected clock. A far-past now makes a wall
+// clock NotAfter visible: it would land decades after now instead of CAValidity after it.
+func TestGenerateDatesCAValidityWindowFromFarPastClock(t *testing.T) {
+	now := time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC)
+	b, err := Generate(testNS, now)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if want := now.Add(-clockSkewAllowance); !b.CACert.NotBefore.Equal(want) {
+		t.Fatalf("CA NotBefore = %s, want %s", b.CACert.NotBefore, want)
+	}
+	if want := now.Add(CAValidity); !b.CACert.NotAfter.Equal(want) {
+		t.Fatalf("CA NotAfter = %s, want %s", b.CACert.NotAfter, want)
+	}
+	if want := now.Add(ServingCertValidity); !b.Cert.NotAfter.Equal(want) {
+		t.Fatalf("serving NotAfter = %s, want %s", b.Cert.NotAfter, want)
+	}
+	if !b.CACert.IsCA || !b.CACert.BasicConstraintsValid || b.CACert.KeyUsage&x509.KeyUsageCertSign == 0 {
+		t.Fatal("CA certificate must be a CA with valid basic constraints and KeyUsageCertSign")
+	}
+	if _, err := Parse(secretFor(b), testNS, now); err != nil {
+		t.Fatalf("generated bundle does not parse at the injected time: %v", err)
+	}
+	if b.NeedsRenewal(testNS, now) {
+		t.Fatal("a fresh serving certificate must not need renewal at the injected time")
+	}
+	// The same bundle is long expired by the wall clock.
+	var corruptErr *CorruptSecretError
+	if _, err := Parse(secretFor(b), testNS, time.Now()); !errors.As(err, &corruptErr) || !strings.Contains(err.Error(), "expired") {
+		t.Fatalf("Parse at the wall clock = %v, want an expired CA", err)
+	}
+}
+
 func TestEnsureCreatesMarkedSecretAndServesIt(t *testing.T) {
 	client := fake.NewClientset()
 	m := newTestManager(t, client, time.Now())
@@ -631,7 +664,7 @@ func assertNoWrites(t *testing.T, client *fake.Clientset) {
 	}
 }
 
-// selfSignedCA builds a CA with an explicit validity window (certutil fixes it at 10 years).
+// selfSignedCA builds a CA with an explicit validity window (Generate fixes it at CAValidity).
 func selfSignedCA(t *testing.T, notBefore, notAfter time.Time) ([]byte, []byte) {
 	t.Helper()
 	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)