pgwire: add password policy observability metrics#170106
Conversation
|
😎 Merged successfully - details. |
|
This change is |
souravcrl
force-pushed
the
add-password-policy-metrics
branch
from
8b589e4 to
ef00d2b
Compare
|
Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link) |
souravcrl
force-pushed
the
add-password-policy-metrics
branch
2 times, most recently
from
b19ad7e to
32e480d
Compare
souravcrl
changed the title
sanchit-CRL
left a comment
sanchit-CRL
left a comment
There was a problem hiding this comment.
the metrics share file, struct, plumbing, and test pattern. Commit 1 changes the newTenantSpecificMetrics signature (4 call sites) but the metrics.yaml regen lives entirely in commit 2 (covers both metrics due to alphabetical ordering). This makes commit 1 not independently consistent
| @@ -479,6 +508,17 @@ func newTenantSpecificMetrics( | |||
| getHistogramOptionsForIOLatency(AuthLDAPConnLatencyInternal, histogramWindow)), | |||
| AuthCertSANConnTotal: metric.NewCounter(AuthCertSANConnTotal), | |||
| AuthCertSANConnSuccess: metric.NewCounter(AuthCertSANConnSuccess), | |||
| PasswordEncryptionIsSCRAM: metric.NewFunctionalGauge( | |||
| MetaPasswordEncryptionIsSCRAM, func() int64 { | |||
| if security.GetConfiguredPasswordHashMethod(sv) == password.HashSCRAMSHA256 { | |||
There was a problem hiding this comment.
Tighten the closure to explicitly check HashBCrypt, or add a regression test that pins behavior across all defined HashMethod values.
There was a problem hiding this comment.
We only want to check for SCRAM passwords as bcrypt is the older format and SCRAM is the newer mandated newer one. These are the only 2 supported hashes in pkg/security/password.go#L102-L105. If a newer hash type comes up, we want to track that as a metric separate from SCRAM, so as far this closure is concerned functionally, adding this won't make sense as this is meant to track SCRAM enablement.
For the test part, I am checking for brcypt in the test TestPasswordEncryptionIsSCRAMGauge. I can add a type which is not SCRAM or bcrypt but it just be an empty validation as this metric gauge won't be updated.
Add a functional gauge `auth.password_encryption.is_scram` that reports whether `server.user_login.password_encryption` is set to `scram-sha-256` (1) or `crdb-bcrypt` (0). This provides observability into password encryption policy adoption across clusters and orgs. The gauge reads the cluster setting on each metric scrape, so it always reflects the current configuration without requiring instrumentation at each auth callsite. Informs: cockroachdb#167911 Release note (security change): A new metric `auth.password_encryption.is_scram` reports whether password encryption is configured to use SCRAM-SHA-256 (1) or crdb-bcrypt (0). This enables operators to verify and alert on password hashing policy across the cluster, supporting security compliance requirements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a functional gauge `auth.min_password_length` that reports the current value of `server.user_login.min_password_length`. This provides immediate visibility into password length policy configuration and enables compliance alerting if the value drops below organizational requirements. Informs: cockroachdb#167911 Release note (security change): A new metric `auth.min_password_length` reports the configured minimum password length for the cluster. This enables operators to monitor and alert on password length policy, ensuring compliance with organizational security requirements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
souravcrl
force-pushed
the
add-password-policy-metrics
branch
from
32e480d to
bac7950
Compare
Both the commits get rolled before the merge into the master branch. So there is never a case for |
Metrics change detectedThis PR adds or updates one or more CRDB metrics. If you want these metrics to be exported by CRDB Cloud clusters to Internal CRL Datadog and/or included in the customer metric export integration (Essential metrics for standard deployment, and Essential metrics for advanced deployment), refer to this Installation and Usage guide of a CLI tool that syncs the metric mappings in managed-service. Run this CLI tool after your CRDB PR is merged.
Note: Your metric will appear in Internal CRL Datadog only after the managed-service PR merges and the new OTel configuration rolls out to at least one cluster running a CRDB build that includes this metric. Docs: cockroach-metric-sync Questions: reach out to @obs-india-prs |
|
TFTR! /trunk merge |
|
Detected infrastructure failure on trunk-merge branch (matched: self-hosted runner lost communication with the server). Automatically resubmitting to merge queue (attempt 1 of 2). (run link) |
|
/trunk merge |
|
roachtest is failing in the trunk merge pipeline. retrying as it seems like a flake. /trunk merge |
3 participants
Summary
auth.password_encryption.is_scram— reports whetherserver.user_login.password_encryptionis set toscram-sha-256(1) orcrdb-bcrypt(0)auth.min_password_length— reports the current value ofserver.user_login.min_password_lengthRelease note (security change): Two new metrics are added for password
policy observability.
auth.password_encryption.is_scramreports whetherpassword encryption is configured to use SCRAM-SHA-256 (1) or crdb-bcrypt
(0), enabling operators to verify password hashing policy across the
cluster.
auth.min_password_lengthreports the configured minimumpassword length, enabling alerts when the value drops below organizational
security requirements.
Informs: #167911
Epic: none
🤖 Generated with Claude Code