Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion authbridge/authlib/tlsbridge/decision.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
33 changes: 29 additions & 4 deletions authbridge/authlib/tlsbridge/minter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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()
Expand All @@ -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},
}
Expand All @@ -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
}
55 changes: 55 additions & 0 deletions authbridge/authlib/tlsbridge/minter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Loading