diff --git a/internal/aggregated/servingcert/servingcert.go b/internal/aggregated/servingcert/servingcert.go index a7f733c82..fa13d591b 100644 --- a/internal/aggregated/servingcert/servingcert.go +++ b/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 { diff --git a/internal/aggregated/servingcert/servingcert_test.go b/internal/aggregated/servingcert/servingcert_test.go index bba28568d..00fe8e704 100644 --- a/internal/aggregated/servingcert/servingcert_test.go +++ b/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)