Skip to content

Commit 1ba9960

Browse files
authored
🤖 fix: date the serving CA from the injected clock and backdate it (#223)
Refs #221. Stacked on #222: merge #222 first, then this PR retargets to `main`. ## Summary `servingcert.Generate(namespace, now)` dated the serving CA from the wall clock, not from `now`, and did not backdate it. `Parse(..., now)` therefore rejected a fresh CA as "not valid until" whenever the caller's `now` was even one second behind the clock that issued the CA. This PR passes `NotBefore: now - clockSkewAllowance` (1 h, the same margin the serving certificate already uses) to `certutil.NewSelfSignedCACert`. ## Background The merge queue rejected #222 (run 38277477505) on this flake in `TestGenerateProducesVerifiableServingCert`: ``` servingcert_test.go:77: generated bundle does not parse: ... the CA in data["ca.crt"] is not valid until 2026-10-12T16:28:48Z ``` Root cause: - `internal/aggregated/servingcert/servingcert.go:116` called `certutil.NewSelfSignedCACert` without `NotBefore`. The vendored helper then uses its own `time.Now()` (`vendor/k8s.io/client-go/util/cert/cert.go:61,68`). x509 encodes the time to whole seconds. - `servingcert.go:215` rejects the CA when `now.Before(CACert.NotBefore)`. - The test takes `now := time.Now()` before `Generate`. When a second boundary falls between that call and certutil's `time.Now()`, the truncated CA `NotBefore` lands after `now`, and `Parse` fails. This predates #222. `vendor/k8s.io` did not change in #222, and the original test also fails on `main` (go1.26.8, old deps). Production impact (same cause): the serving certificate is backdated by 1 h for clock skew, but the CA was not. A replica, or a kube-apiserver, whose clock is behind the issuing pod's saw a CA from the future. A replica that adopts the Secret then got a `CorruptSecretError` ("delete it so coder-k8s generates a new CA"). I did not reproduce the kube-apiserver case on a cluster. ## Implementation - `Generate` sets `certutil.Config.NotBefore = now.Add(-clockSkewAllowance)`. The CA `NotAfter` is unchanged (certutil: wall clock + 10 years), and the serving certificate is still clamped to it. - New test `TestGenerateDatesCAFromInjectedClock` generates with `now` 30 min in the past. It asserts that the CA `NotBefore` is `now - 1h` (to the second), that the chain verifies at `now`, and that `Parse(..., now)` accepts the bundle. ## Validation Repro counts for the original test, `go test -count=N -run '^TestGenerateProducesVerifiableServingCert$' ./internal/aggregated/servingcert/`: | Tree | Idle, 500 | Idle, 20000 | `-race`, 500 | 32 CPU burners, 20000 | Burners + `-race`, 2000 | | --- | --- | --- | --- | --- | --- | | `main` 55e7f34 | 1 FAIL | 2 FAIL | 0 | 1 FAIL | not run | | #222 dda8efa | 0 | 0 | 0 | 1 FAIL | 1 FAIL | | this PR | 0 | 0 | 0 | 0 | 0 | The new test fails 100/100 without the fix (`CA NotBefore = <wall clock>, want <now - 1h>`) and passes 100/100 with it. ## Risks Low. Only newly generated CAs change: they are now valid from 1 h before issue. Existing Secrets are parsed as before. --- _Generated with `mux` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent f20b552 commit 1ba9960

2 files changed

Lines changed: 26 additions & 0 deletions

File tree

‎internal/aggregated/servingcert/servingcert.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ func Generate(namespace string, now time.Time) (*Bundle, error) {
116116
caCert, err := certutil.NewSelfSignedCACert(certutil.Config{
117117
CommonName: fmt.Sprintf("%s-ca@%d", ServiceName, now.Unix()),
118118
Organization: []string{"coder-k8s"},
119+
// certutil otherwise uses its own time.Now() with no backdating, so a clock that is behind
120+
// the issuer's (another replica, kube-apiserver, or an injected now) sees a future CA.
121+
NotBefore: now.Add(-clockSkewAllowance),
119122
}, caKey)
120123
if err != nil {
121124
return nil, fmt.Errorf("generate CA certificate: %w", err)

‎internal/aggregated/servingcert/servingcert_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,29 @@ func TestGenerateProducesVerifiableServingCert(t *testing.T) {
7878
}
7979
}
8080

81+
// The CA must take its validity from the injected clock and be backdated like the serving
82+
// certificate. Otherwise a replica or kube-apiserver whose clock is behind the issuer's rejects a
83+
// fresh CA as "not valid until", and Parse(Generate(now), now) fails across a second boundary.
84+
func TestGenerateDatesCAFromInjectedClock(t *testing.T) {
85+
now := time.Now().Add(-30 * time.Minute)
86+
b, err := Generate(testNS, now)
87+
if err != nil {
88+
t.Fatal(err)
89+
}
90+
// x509 encodes validity with one-second precision.
91+
if want := now.Add(-clockSkewAllowance).Truncate(time.Second); !b.CACert.NotBefore.Equal(want) {
92+
t.Fatalf("CA NotBefore = %s, want %s", b.CACert.NotBefore, want)
93+
}
94+
roots := x509.NewCertPool()
95+
roots.AddCert(b.CACert)
96+
if _, err := b.Cert.Verify(x509.VerifyOptions{DNSName: DNSNames(testNS)[2], Roots: roots, CurrentTime: now}); err != nil {
97+
t.Fatalf("serving certificate does not verify at the injected time: %v", err)
98+
}
99+
if _, err := Parse(secretFor(b), testNS, now); err != nil {
100+
t.Fatalf("generated bundle does not parse at the injected time: %v", err)
101+
}
102+
}
103+
81104
func TestEnsureCreatesMarkedSecretAndServesIt(t *testing.T) {
82105
client := fake.NewClientset()
83106
m := newTestManager(t, client, time.Now())

0 commit comments

Comments
 (0)