security: change client cert expiration caching eviction policy

Previously, we cached and emitted metrics for client certificates
indefinitely or until they expired. However, this strategy ignores
the case where a team rotates their certificates, discontinuing to
use soon-to-expire ones, in which crdb would continue to report on
their expiration.

This change modifies the caching behavior so that eviction from the
cache is based on last usage. It does a few additional things here:
 - Supports the removal of child metrics by key.
 - Removes the `server.client_cert_expiration_cache.capacity` cluster
	 setting.
 - Adds a shared utility for adding jitter.
 - Moves caching and metrics reporting from a single cache to two
   caches, one for expiration, one for ttl.

Fixes: #142686
Epic: CRDB-40209

Release note(ops change): Cluster setting
`server.client_cert_expiration_cache.capacity` removed.
Metrics `security.certificate.expiration.client` and
`security.certificate.ttl.client` now report lowest value seen for
user in last 24h.

Author: angles-n-daemons

docs/generated/settings/settings-for-tenants.txt

@@ -112,7 +112,6 @@ server.auth_log.sql_sessions.enabled	boolean	false	if set, log verbose SQL sessi
 server.authentication_cache.enabled	boolean	true	enables a cache used during authentication to avoid lookups to system tables when retrieving per-user authentication-related information	application
 server.child_metrics.enabled	boolean	false	enables the exporting of child metrics, additional prometheus time series with extra labels	application
 server.child_metrics.include_aggregate.enabled	boolean	true	include the reporting of the aggregate time series when child metrics are enabled. This cluster setting has no effect if child metrics are disabled.	application
-server.client_cert_expiration_cache.capacity	integer	1000	the maximum number of client cert expirations stored	application
 server.clock.forward_jump_check.enabled (alias: server.clock.forward_jump_check_enabled)	boolean	false	if enabled, forward clock jumps > max_offset/2 will cause a panic	application
 server.clock.persist_upper_bound_interval	duration	0s	the interval between persisting the wall time upper bound of the clock. The clock does not generate a wall time greater than the persisted timestamp and will panic if it sees a wall time greater than this value. When cockroach starts, it waits for the wall time to catch-up till this persisted timestamp. This guarantees monotonic wall time across server restarts. Not setting this or setting a value of 0 disables this feature.	application
 server.eventlog.enabled	boolean	true	if set, logged notable events are also stored in the table system.eventlog	application

docs/generated/settings/settings.html

@@ -143,7 +143,6 @@
 <tr><td><div id="setting-server-authentication-cache-enabled" class="anchored"><code>server.authentication_cache.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>enables a cache used during authentication to avoid lookups to system tables when retrieving per-user authentication-related information</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-server-child-metrics-enabled" class="anchored"><code>server.child_metrics.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>enables the exporting of child metrics, additional prometheus time series with extra labels</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-server-child-metrics-include-aggregate-enabled" class="anchored"><code>server.child_metrics.include_aggregate.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>include the reporting of the aggregate time series when child metrics are enabled. This cluster setting has no effect if child metrics are disabled.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-server-client-cert-expiration-cache-capacity" class="anchored"><code>server.client_cert_expiration_cache.capacity</code></div></td><td>integer</td><td><code>1000</code></td><td>the maximum number of client cert expirations stored</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-server-clock-forward-jump-check-enabled" class="anchored"><code>server.clock.forward_jump_check.enabled<br />(alias: server.clock.forward_jump_check_enabled)</code></div></td><td>boolean</td><td><code>false</code></td><td>if enabled, forward clock jumps &gt; max_offset/2 will cause a panic</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-server-clock-persist-upper-bound-interval" class="anchored"><code>server.clock.persist_upper_bound_interval</code></div></td><td>duration</td><td><code>0s</code></td><td>the interval between persisting the wall time upper bound of the clock. The clock does not generate a wall time greater than the persisted timestamp and will panic if it sees a wall time greater than this value. When cockroach starts, it waits for the wall time to catch-up till this persisted timestamp. This guarantees monotonic wall time across server restarts. Not setting this or setting a value of 0 disables this feature.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-server-consistency-check-max-rate" class="anchored"><code>server.consistency_check.max_rate</code></div></td><td>byte size</td><td><code>8.0 MiB</code></td><td>the rate limit (bytes/sec) to use for consistency checks; used in conjunction with server.consistency_check.interval to control the frequency of consistency checks. Note that setting this too high can negatively impact performance.</td><td>Dedicated/Self-Hosted</td></tr>

pkg/security/BUILD.bazel

@@ -40,6 +40,7 @@ go_library(
         "//pkg/util/log/severity",
         "//pkg/util/metric",
         "//pkg/util/metric/aggmetric",
+        "//pkg/util/mon",
         "//pkg/util/quotapool",
         "//pkg/util/randutil",
         "//pkg/util/stop",

pkg/security/auth.go

@@ -353,11 +353,15 @@ func UserAuthCertHook(
 
 		if ValidateUserScope(certUserScope, systemIdentity, tenantID, tenantName, roleSubject, certSubject) {
 			if certManager != nil {
-				certManager.MaybeUpsertClientExpiration(
+				err := certManager.MaybeUpsertClientExpiration(
 					ctx,
 					systemIdentity,
+					peerCert.SerialNumber.String(),
 					peerCert.NotAfter.Unix(),
 				)
+				if err != nil {
+					return err
+				}
 			}
 			return nil
 		}

pkg/security/certificate_manager.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
+	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/sysutil"
@@ -55,7 +56,10 @@ type CertificateManager struct {
 	certMetrics *Metrics
 
 	// Client cert expiration cache.
-	clientCertExpirationCache *clientcert.ClientCertExpirationCache
+	clientCertExpirationCache *clientcert.Cache
+
+	// Client cert ttl cache.
+	clientCertTTLCache *clientcert.Cache
 
 	// mu protects all remaining fields.
 	mu syncutil.RWMutex
@@ -182,7 +186,10 @@ func (cm *CertificateManager) RegisterSignalHandler(
 			case sig := <-ch:
 				log.Ops.Infof(ctx, "received signal %q, triggering certificate reload", sig)
 				if cache := cm.clientCertExpirationCache; cache != nil {
-					cache.Clear()
+					cache.Clear(ctx)
+				}
+				if cache := cm.clientCertTTLCache; cache != nil {
+					cache.Clear(ctx)
 				}
 				if err := cm.LoadCertificates(); err != nil {
 					log.Ops.Warningf(ctx, "could not reload certificates: %v", err)
@@ -195,26 +202,45 @@ func (cm *CertificateManager) RegisterSignalHandler(
 	})
 }
 
-// RegisterExpirationCache registers a cache for client certificate expiration.
-// It is called during server startup.
-func (cm *CertificateManager) RegisterExpirationCache(cache *clientcert.ClientCertExpirationCache) {
-	cm.clientCertExpirationCache = cache
+// RegisterExpirationCache sets up and attaches caches for client certificate
+// expiration times and TTLs.
+func (cm *CertificateManager) RegisterExpirationCaches(
+	ctx context.Context, stopper *stop.Stopper, parentMon *mon.BytesMonitor,
+) error {
+	m := mon.NewMonitorInheritWithLimit(
+		"client-expiration-caches", 0 /* limit */, parentMon, true, /* longLiving */
+	)
+	acc := m.MakeConcurrentBoundAccount()
+	m.StartNoReserved(ctx, parentMon)
+
+	cm.clientCertExpirationCache = clientcert.NewCache(acc, stopper, cm.certMetrics.ClientExpiration, &timeutil.DefaultTimeSource{}, false)
+	cm.clientCertTTLCache = clientcert.NewCache(acc, stopper, cm.certMetrics.ClientTTL, &timeutil.DefaultTimeSource{}, true)
+	err := cm.clientCertExpirationCache.StartPurgeLoop(ctx)
+	if err != nil {
+		return err
+	}
+	err = cm.clientCertTTLCache.StartPurgeLoop(ctx)
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 // MaybeUpsertClientExpiration updates or inserts the expiration time for the
 // given client certificate. An update is contingent on whether the old
 // expiration is after the new expiration.
 func (cm *CertificateManager) MaybeUpsertClientExpiration(
-	ctx context.Context, identity string, expiration int64,
-) {
-	if cache := cm.clientCertExpirationCache; cache != nil {
-		cache.MaybeUpsert(ctx,
-			identity,
-			expiration,
-			cm.certMetrics.ClientExpiration,
-			cm.certMetrics.ClientTTL,
-		)
+	ctx context.Context, identity, serial string, expiration int64,
+) error {
+	for _, cache := range []*clientcert.Cache{cm.clientCertExpirationCache, cm.clientCertTTLCache} {
+		if cache != nil {
+			err := cache.Upsert(ctx, identity, serial, expiration)
+			if err != nil {
+				return err
+			}
+		}
 	}
+	return nil
 }
 
 // CACert returns the CA cert. May be nil.

pkg/security/clientcert/BUILD.bazel

@@ -6,12 +6,7 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/security/clientcert",
     visibility = ["//visibility:public"],
     deps = [
-        "//pkg/settings",
-        "//pkg/settings/cluster",
-        "//pkg/util/cache",
-        "//pkg/util/log",
         "//pkg/util/metric/aggmetric",
-        "//pkg/util/mon",
         "//pkg/util/stop",
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",
@@ -23,14 +18,12 @@ go_test(
     srcs = ["cert_expiry_cache_test.go"],
     deps = [
         ":clientcert",
-        "//pkg/settings/cluster",
         "//pkg/util/leaktest",
         "//pkg/util/metric",
         "//pkg/util/metric/aggmetric",
         "//pkg/util/mon",
         "//pkg/util/stop",
         "//pkg/util/timeutil",
-        "@com_github_prometheus_client_model//go",
         "@com_github_stretchr_testify//require",
     ],
 )