sql: make the stored hash method configurable (scram 8/10) #74848
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
This was referenced Jan 14, 2022
knz
force-pushed
the
20211228-scram8
branch
2 times, most recently
from
41b29ed to
cc70947
Compare
knz
changed the title
knz
force-pushed
the
20211228-scram8
branch
2 times, most recently
from
c7f918c to
606d0a7
Compare
rafiss
approved these changes
Jan 17, 2022
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @knz and @rafiss)
pkg/security/password.go, line 81 at r1 (raw file):
// The minimum value 4096 incurs a password check latency of ~2ms on AMD 3950X 3.7GHz. // // The default value 119680 incurs ~60ms latency on the sane hw.
nit: sane -> same
pkg/security/password.go, line 399 at r1 (raw file):
} // HashPassword takes a raw password and returns a bcrypt hashed password.
nit: update bcrypt in comment
knz
commented
Jan 19, 2022
There was a problem hiding this comment.
Reviewable status:
pkg/security/password.go, line 81 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
nit: sane -> same
Fixed.
pkg/security/password.go, line 399 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
nit: update bcrypt in comment
Good call. Fixed.
|
NB: I'm holding off merging this PR until all the PRs in the sequence are ready (otherwise, the entire chain will be auto-closed by github). |
Release note (security update): The hash method used to encode cleartext passwords before storing them is now configurable, via the new cluster setting `server.user_login.password_encryption`. Its supported values are `crdb-bcrypt` and `scram-sha-256`. The cluster setting only becomes effective and its default value is `scram-sha-256` after all cluster nodes have been upgraded. Prior to completion of the upgrade, the cluster behaves as if the cluster setting is fixed to `crdb-bcrypt` (for backward compatibility) Note that the preferred way to populate password credentials for SQL user accounts is to pre-compute the hash client-side, and pass the precomputed hash via CREATE/ALTER USER/ROLE WITH PASSWORD. This ensures that the server never sees the cleartext password. Release note (security update): The cost of the hashing function for `scram-sha-256` is now configurable via the new cluster setting `server.user_login.password_hashes.default_cost.scram_sha_256`. Its default value is 119680, which corresponds to an approximate password check latency of 50-100ms on modern hardware. This value should be increased over time to reflect improvements to CPU performance: the latency should not become so small that it becomes feasible to bruteforce passwords via repeated login attempts. Future versions of CockroachDB will likely update the default accordingly. Release note (sql change): The session variable `password_encryption` is now exposed to SQL clients. Note that SQL clients cannot modify its value directly; it is configurable via a cluster setting.
craig bot
pushed a commit
that referenced
this pull request
Jan 21, 2022
74301: sql,server: support SCRAM authentication for SQL sessions r=rafiss,JeffSwenson,bdarnell,aaron-crl,kpatron-cockroachlabs a=knz Fixes #42519. Fixes #74511. Informs #65117. Epic CRDB-5349 **tldr:** this PR adds support for the SCRAM-SHA-256 authentication method, as defined by IETF RFCs [5802](https://datatracker.ietf.org/doc/html/rfc5802) and [7677](https://datatracker.ietf.org/doc/html/rfc7677) and as supported by PostgreSQL. This offers [multiple security benefits](#scram-benefits) over CockroachDB's current use of cleartext password authentication. To use this feature, 1) the stored password hashes must use the SCRAM encoding (this requires a migration away from crdb's bcrypt-based hashes); and 2) one of the SCRAM authentication methods must be enabled explictly via the HBA configuration (cluster setting `server.host_based_authentication.configuration`). ### How to review this work The addition of the feature goes as follows: 1. adding HBA syntax and authentication method hooks for `scram-sha-256` and `cert-scram-sha-256`. These are gated behind a new cluster version, so that previous-version nodes do not get confused by the new syntax. Split into separate PR: #74842 2. extending ALTER/CREATE USER/ROLE WITH PASSWORD to recognize SCRAM hashes. This allows operators to use these SQL statements to populate SCRAM hashes. (This should also be seen as the more desirable way to configure SQL user accounts: it ensures that the CockroachDB node never ever sees the cleartext password of an account. Even if we later support computing the SCRAM hash server-side, pre-computing the hash client-side should be seen and documented as the superior approach.) Split into separate PR: #74843 3. extending the existing cleartext methods inside CockroachDB (used for the HTTP interface and the `password`-based methods in SQL) to compare a cleartext password to a pre-computed SCRAM hash. This ensures that the existing password mechanisms that are cleartext-based continue to work even after the stored credentials start using the SCRAM encoding. Split into separate PRs: #74844 and #74845 4. extending the code created at step 1 to actually use the stored SCRAM credentials for client authentication. This achieves the main goal. Split into separate PRs: #74846 and #74847 5. making the hash method used to encode cleartext passwords passed to ALTER/CREATE USER/ROLE WITH PASSWORD configurable, using a new cluster setting `server.user_login.password_encryption`, and exposing its value via the pg-compatible session variable `password_encryption`. This is a convenience feature - properly secured deployments should not need this, as they should use pre-hashing client-side instead (see note at point 2 above). Split into separate PR: #74848 6. making crdb auto-select SCRAM authn if the stored password uses SCRAM. This is one step to ensuring that previous-version clusters are automatically opted into SCRAM. Split into separate PR: #74849 7. Auto-upgrade the stored credentials from bcrypt to SCRAM, when enabled and possible. Split into separate PR: #74850 The reviewer is invited to consider each PR in turn, read its commit message(s) to understand the goal of that PR, then review the PR to ascertain that the implementation (and relevant tests) match the goals announced in the commit message. Co-authored-by: Raphael 'kena' Poss <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(PR peeled away from #74301; previous PR in series #74847; next PR in series: #74849)
Fixes #74511.
Epic CRDB-5349
Release note (security update): The hash method used to encode
cleartext passwords before storing them is now configurable, via the
new cluster setting
server.user_login.password_encryption. Itssupported values are
crdb-bcryptandscram-sha-256.The cluster setting only becomes effective and its default value
is
scram-sha-256after all cluster nodes have been upgraded. Priorto completion of the upgrade, the cluster behaves as if the cluster
setting is fixed to
crdb-bcrypt(for backward compatibility)Note that the preferred way to populate password credentials
for SQL user accounts is to pre-compute the hash client-side, and
pass the precomputed hash via CREATE/ALTER USER/ROLE WITH PASSWORD.
This ensures that the server never sees the cleartext password.
Release note (security update): The cost of the hashing function for
scram-sha-256is now configurable via the new cluster settingserver.user_login.password_hashes.default_cost.scram_sha_256. Itsdefault value is 119680, which corresponds to an approximate password
check latency of 50-100ms on modern hardware.
This value should be increased over time to reflect improvements to
CPU performance: the latency should not become so small that it
becomes feasible to bruteforce passwords via repeated login attempts.
Future versions of CockroachDB will likely update the default accordingly.
Release note (sql change): The session variable
password_encryptionis now exposed to SQL clients. Note that SQL clients cannot modify
its value directly; it is configurable via a cluster setting.