security: enable plaintext authentication with SCRAM credentials (scram 5/10)#74845
security: enable plaintext authentication with SCRAM credentials (scram 5/10)#74845craig[bot] merged 1 commit intomasterfrom
Conversation
rafiss
left a comment
There was a problem hiding this comment.
lgtm, but will let a security expert review
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @bdarnell and @kpatron-cockroachlabs)
kpatron-cockroachlabs
left a comment
There was a problem hiding this comment.
Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @bdarnell and @knz)
pkg/security/password.go, line 287 at r2 (raw file):
saltedPassword := pbkdf2.Key([]byte(prepared), []byte(s.decoded.Salt), s.decoded.Iters, sha256.Size, sha256.New) // As per xdg-go/scram and pg's scram_ServerKey(). serverKey := computeHMAC(scram.SHA256, saltedPassword, []byte("Server Key"))
Nit: Any chance we can drop a comment here about "Server Key"? It is correct but as someone reading the code without the details of SCRAM in their head it looks...odd/scary
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @bdarnell)
pkg/security/password.go, line 287 at r2 (raw file):
Previously, kpatron-cockroachlabs wrote…
Nit: Any chance we can drop a comment here about "Server Key"? It is correct but as someone reading the code without the details of SCRAM in their head it looks...odd/scary
Done, good call.
|
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). |
This commit introduces support for cleartext password authentication against SCRAM-SHA-256 stored credentials. This feature is necessary *in addition to* support for the full SCRAM handshake, because the web UI authn will continue using cleartext passwords for the foreseeable future, and we want to continue supporting the `password` and `cert-password` HBA methods for backward compatibility. When performing cleartext authn against SCRAM credentials, the hash must be computed server-side. Like bcrypt, this is meant to be computationally expensive. Therefore, for the same reason as bcrypt, we want to restrict CPU usage for this using a semaphore. This commit achieves this by using the same semaphore as was used for bcrypt computations. Because we're using the same semaphore, the commit also renames the semaphore and its configuration env var to denote the more general usage. Release note (backward-incompatible change): The environment variable that controls the max amount of CPU that can be taken by password hash computations during authentication was renamed from `COCKROACH_MAX_BCRYPT_CONCURRENCY` to `COCKROACH_MAX_PW_HASH_COMPUTE_CONCURRENCY`. Its semantics remain unchanged. Release note (security update): CockroachDB is now able to authenticate users via the web UI and through SQL sessions when the client provides a cleartext password and the stored credentials are encoded using the SCRAM-SHA-256 algorithm. (Note: support for a SCRAM authentication flow is a separate feature and is not the target of this release note.) In particular, for SQL client sessions it makes it possible to use the authentication methods `password` (cleartext passwords), and `cert-password` (TLS client cert or cleartext password) with either CRDB-BCRYPT or SCRAM-SHA-256 stored credentials. Previously, only CRDB-BCRYPT stored credentials were supported for cleartext password authentication.
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 <knz@thaumogen.net>
(PR peeled away from #74301; previous PR in series #74844; next PR in series: #74846)
Epic CRDB-5349
This commit introduces support for cleartext password authentication
against SCRAM-SHA-256 stored credentials.
This feature is necessary in addition to support for the full SCRAM
handshake, because the web UI authn will continue using cleartext
passwords for the foreseeable future, and we want to continue
supporting the
passwordandcert-passwordHBA methods for backwardcompatibility.
When performing cleartext authn against SCRAM credentials, the hash
must be computed server-side. Like bcrypt, this is meant to be
computationally expensive. Therefore, for the same reason as bcrypt,
we want to restrict CPU usage for this using a semaphore.
This commit achieves this by using the same semaphore as was used for
bcrypt computations. Because we're using the same semaphore, the
commit also renames the semaphore and its configuration env var to
denote the more general usage.
Release note (backward-incompatible change): The environment variable
that controls the max amount of CPU that can be taken by password
hash computations during authentication was renamed from
COCKROACH_MAX_BCRYPT_CONCURRENCYtoCOCKROACH_MAX_PW_HASH_COMPUTE_CONCURRENCY.Its semantics remain unchanged.
Release note (security update): CockroachDB is now able to
authenticate users via the web UI and through SQL sessions when the
client provides a cleartext password and the stored credentials are
encoded using the SCRAM-SHA-256 algorithm.
(Note: support for a SCRAM authentication flow is a separate
feature and is not the target of this release note.)
In particular, for SQL client sessions it makes it possible
to use the authentication methods
password(cleartext passwords),and
cert-password(TLS client cert or cleartext password) witheither CRDB-BCRYPT or SCRAM-SHA-256 stored credentials. Previously,
only CRDB-BCRYPT stored credentials were supported for cleartext
password authentication.