Skip to content

Commit e51de55

Browse files
security: cleanup clientcert expiration cache functionality
The work done for #142686 in #143081 was truncated to make the changeset more backportable. Some of the modifications held are ported instead to this PR, so that extra functions are removed, and the `clientcert/cache.go` is simpler in terms of its responsibility. Fixes: #144163 Epic: none Release note (ops change): cluster setting `server.client_cert_expiration_cache.capacity` has been deprecated. The client certificate cache now evicts client certificates based on time.
1 parent 0868f32 commit e51de55

File tree

8 files changed

+220
-344
lines changed

8 files changed

+220
-344
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ server.auth_log.sql_sessions.enabled boolean false if set, log verbose SQL sessi
115115
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
116116
server.child_metrics.enabled boolean false enables the exporting of child metrics, additional prometheus time series with extra labels application
117117
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
118-
server.client_cert_expiration_cache.capacity integer 1000 the maximum number of client cert expirations stored application
119118
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
120119
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
121120
server.eventlog.enabled boolean true if set, logged notable events are also stored in the table system.eventlog application

docs/generated/settings/settings.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@
146146
<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>
147147
<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>
148148
<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>
149-
<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>
150149
<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>
151150
<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>
152151
<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/certificate_manager.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/cockroachdb/cockroach/pkg/security/certnames"
1414
"github.com/cockroachdb/cockroach/pkg/security/clientcert"
1515
"github.com/cockroachdb/cockroach/pkg/security/username"
16-
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1716
"github.com/cockroachdb/cockroach/pkg/util/log"
1817
"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
1918
"github.com/cockroachdb/cockroach/pkg/util/log/severity"
@@ -57,7 +56,7 @@ type CertificateManager struct {
5756
certMetrics *Metrics
5857

5958
// Client cert expiration cache.
60-
clientCertExpirationCache *clientcert.ClientCertExpirationCache
59+
clientCertExpirationCache *clientcert.Cache
6160

6261
// mu protects all remaining fields.
6362
mu syncutil.RWMutex
@@ -201,12 +200,16 @@ func (cm *CertificateManager) RegisterSignalHandler(
201200
// It is called during server startup.
202201
func (cm *CertificateManager) RegisterExpirationCache(
203202
ctx context.Context,
204-
st *cluster.Settings,
205203
stopper *stop.Stopper,
206204
timeSrc timeutil.TimeSource,
207205
parentMon *mon.BytesMonitor,
208-
) {
209-
cm.clientCertExpirationCache = clientcert.NewClientCertExpirationCache(ctx, st, stopper, timeSrc, parentMon, cm.certMetrics.ClientExpiration, cm.certMetrics.ClientTTL)
206+
) error {
207+
m := mon.NewMonitorInheritWithLimit(mon.MakeName("client-expiration-caches"), 0 /* limit */, parentMon, true /* longLiving */)
208+
acc := m.MakeConcurrentBoundAccount()
209+
m.StartNoReserved(ctx, parentMon)
210+
211+
cm.clientCertExpirationCache = clientcert.NewCache(timeSrc, acc, cm.certMetrics.ClientExpiration, cm.certMetrics.ClientTTL)
212+
return cm.clientCertExpirationCache.StartPurgeJob(ctx, stopper)
210213
}
211214

212215
// MaybeUpsertClientExpiration updates or inserts the expiration time for the
@@ -216,7 +219,7 @@ func (cm *CertificateManager) MaybeUpsertClientExpiration(
216219
ctx context.Context, identity string, serial string, expiration int64,
217220
) {
218221
if cache := cm.clientCertExpirationCache; cache != nil {
219-
cache.MaybeUpsert(ctx,
222+
cache.Upsert(ctx,
220223
identity,
221224
serial,
222225
expiration,

pkg/security/clientcert/BUILD.bazel

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@ go_library(
66
importpath = "github.com/cockroachdb/cockroach/pkg/security/clientcert",
77
visibility = ["//visibility:public"],
88
deps = [
9-
"//pkg/settings",
10-
"//pkg/settings/cluster",
119
"//pkg/util/log",
1210
"//pkg/util/metric/aggmetric",
13-
"//pkg/util/mon",
1411
"//pkg/util/stop",
1512
"//pkg/util/syncutil",
1613
"//pkg/util/timeutil",
@@ -22,7 +19,6 @@ go_test(
2219
srcs = ["cert_expiry_cache_test.go"],
2320
deps = [
2421
":clientcert",
25-
"//pkg/settings/cluster",
2622
"//pkg/util/leaktest",
2723
"//pkg/util/metric",
2824
"//pkg/util/metric/aggmetric",

0 commit comments

Comments
 (0)