diff --git a/authbridge/authlib/tlsbridge/decision.go b/authbridge/authlib/tlsbridge/decision.go index a8e1be497..a7c114b19 100644 --- a/authbridge/authlib/tlsbridge/decision.go +++ b/authbridge/authlib/tlsbridge/decision.go @@ -114,7 +114,11 @@ func (s *SkipSet) Add(host string) { delete(s.m, oldestK) } } - s.m[host] = now.Add(s.ttl) + // .Round(0) strips the monotonic reading so the expiry is a pure wall-clock + // time. Contains compares it against time.Now() via the wall clock, so an + // entry expires after skipTTL of real time even across a suspend (where the + // monotonic clock freezes and would otherwise keep the host skipped longer). + s.m[host] = now.Add(s.ttl).Round(0) } func (s *SkipSet) Contains(host string) bool { diff --git a/authbridge/authlib/tlsbridge/minter.go b/authbridge/authlib/tlsbridge/minter.go index 37d3aa6d6..b26510a9f 100644 --- a/authbridge/authlib/tlsbridge/minter.go +++ b/authbridge/authlib/tlsbridge/minter.go @@ -38,6 +38,12 @@ type cacheEntry struct { expires time.Time } +// renewBefore is the gap between the cache deadline (now+ttl, when Get +// re-mints) and the leaf's NotAfter (now+ttl+renewBefore). It gives a +// connection that grabbed the leaf just before re-mint ample remaining +// validity, and a window for Get's NotAfter backstop to act. +const renewBefore = time.Hour + func NewMinter(src CASource, o MinterOpts) *Minter { if o.CacheMax <= 0 { o.CacheMax = 1024 @@ -71,7 +77,15 @@ func (m *Minter) GetCertificateForHost(host string) (*tls.Certificate, error) { defer m.mu.Unlock() if el, ok := m.items[host]; ok { e := el.Value.(*cacheEntry) - if time.Now().Before(e.expires) { + // Gate freshness on WALL-clock deadlines, not the process monotonic + // clock. Across a host suspend / VM pause the monotonic clock freezes + // while wall time — and the leaf's x509 validity — keeps advancing, so + // a monotonic deadline would keep serving a leaf the client already + // rejects as expired. e.expires is monotonic-stripped (.Round(0) below); + // the Leaf.NotAfter check is the backstop tied to the cert's real + // validity, the only value the client actually verifies. + now := time.Now() + if now.Before(e.expires) && e.cert.Leaf != nil && now.Before(e.cert.Leaf.NotAfter) { m.ll.MoveToFront(el) return e.cert, nil } @@ -82,7 +96,10 @@ func (m *Minter) GetCertificateForHost(host string) (*tls.Certificate, error) { if err != nil { return nil, err } - el := m.ll.PushFront(&cacheEntry{host: host, cert: cert, expires: time.Now().Add(m.ttl)}) + // .Round(0) strips the monotonic reading so the deadline is a pure wall-clock + // time; comparisons against time.Now() then fall back to the wall clock and + // survive suspend (see the cache-hit gate above). + el := m.ll.PushFront(&cacheEntry{host: host, cert: cert, expires: time.Now().Add(m.ttl).Round(0)}) m.items[host] = el for m.ll.Len() > m.max { back := m.ll.Back() @@ -102,8 +119,9 @@ func (m *Minter) mint(host string) (*tls.Certificate, error) { SerialNumber: serial, Subject: pkix.Name{CommonName: host}, NotBefore: time.Now().Add(-time.Minute), - // Leaf validity must exceed the cache TTL so a cached leaf never serves past expiry. - NotAfter: time.Now().Add(m.ttl + time.Hour), + // Leaf validity outlasts the cache deadline (now+ttl) by renewBefore so + // a cached leaf is always re-minted before it can serve past expiry. + NotAfter: time.Now().Add(m.ttl + renewBefore), KeyUsage: x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, } @@ -116,8 +134,15 @@ func (m *Minter) mint(host string) (*tls.Certificate, error) { if err != nil { return nil, fmt.Errorf("tlsbridge: mint leaf for %s: %w", host, err) } + leaf, err := x509.ParseCertificate(der) + if err != nil { + return nil, fmt.Errorf("tlsbridge: parse minted leaf for %s: %w", host, err) + } return &tls.Certificate{ Certificate: [][]byte{der, caCert.Raw}, PrivateKey: m.leafKey, + // Populate Leaf so Get can gate on the cert's real wall-clock NotAfter + // (and the TLS stack avoids re-parsing on each handshake). + Leaf: leaf, }, nil } diff --git a/authbridge/authlib/tlsbridge/minter_test.go b/authbridge/authlib/tlsbridge/minter_test.go index 2a8df75a5..90f01f63f 100644 --- a/authbridge/authlib/tlsbridge/minter_test.go +++ b/authbridge/authlib/tlsbridge/minter_test.go @@ -123,3 +123,58 @@ func TestMinter_LRUEvictsOldest(t *testing.T) { t.Errorf("expected most-recent host \"c\" to still be cached") } } + +// TestMinter_ReMintsWallClockExpiredLeaf exercises the NotAfter backstop: if a +// cached leaf is ever wall-clock-expired while the cache deadline still reads +// fresh, Get must gate on the leaf's real NotAfter and re-mint rather than +// serve a cert the client rejects. (The primary suspend fix — the wall-clock +// cache deadline — is guarded by TestMinter_CacheDeadlineIsWallClock.) +func TestMinter_ReMintsWallClockExpiredLeaf(t *testing.T) { + m, _ := newTestMinter(t) // LeafTTL=time.Hour, so the cache deadline stays "fresh" + c1, err := m.GetCertificateForHost("h.example.com") + if err != nil { + t.Fatalf("first mint: %v", err) + } + // mint() must populate Leaf so the cache can gate on real validity. + m.mu.Lock() + e := m.items["h.example.com"].Value.(*cacheEntry) + if e.cert.Leaf == nil { + m.mu.Unlock() + t.Fatal("minted cert has no Leaf populated") + } + // Simulate the leaf having aged past its NotAfter while the monotonic cache + // deadline did not advance (host was suspended). + e.cert.Leaf.NotAfter = time.Now().Add(-time.Minute) + m.mu.Unlock() + + c2, err := m.GetCertificateForHost("h.example.com") + if err != nil { + t.Fatalf("second mint: %v", err) + } + if &c1.Certificate[0][0] == &c2.Certificate[0][0] { + t.Fatal("served a wall-clock-expired cached leaf; expected a re-mint") + } + if c2.Leaf == nil || !time.Now().Before(c2.Leaf.NotAfter) { + t.Fatal("re-minted leaf is not valid") + } +} + +// TestMinter_CacheDeadlineIsWallClock guards the actual suspend fix: the cache +// freshness deadline must carry NO monotonic clock reading, so the freshness +// check falls back to the wall clock and expires correctly across a host +// suspend (where the monotonic clock freezes but wall time advances). A +// monotonic-carrying deadline is exactly what made the cache keep serving a +// wall-clock-expired leaf. time.Time's == compares the monotonic reading too, +// so a deadline that still carried one would not equal its .Round(0) form. +func TestMinter_CacheDeadlineIsWallClock(t *testing.T) { + m, _ := newTestMinter(t) + if _, err := m.GetCertificateForHost("h.example.com"); err != nil { + t.Fatalf("mint: %v", err) + } + m.mu.Lock() + exp := m.items["h.example.com"].Value.(*cacheEntry).expires + m.mu.Unlock() + if exp != exp.Round(0) { + t.Errorf("cache deadline carries a monotonic clock reading; must be wall-clock (.Round(0)) to survive suspend") + } +}