sql,server: support SCRAM authentication for SQL sessions #74301
Conversation
|
This change is |
This comment has been minimized.
This comment has been minimized.
knz
force-pushed
the
20211228-scram
branch
4 times, most recently
from
be29d39 to
fac24f2
Compare
knz
changed the title
knz
force-pushed
the
20211228-scram
branch
2 times, most recently
from
68a256b to
c7c6b51
Compare
|
@bdarnell all the pieces are in place to implement your magic migration idea:
The way I'd see it, we would make this auto-migration enabled by default, but opt-out with a cluster setting. The code would also be gated behind a cluster version, so that it only runs after all nodes know how to use SCRAM stored hashes. Can you confirm my understanding is right? |
This comment has been minimized.
This comment has been minimized.
Correct. I'm not sure I'd even bother with an opt-out, but it does need the cluster version gate.(or the opt-out could be based on the Do we need the
So the admin must rotate 100% of passwords (or lock out non-compliant users) before they can enable scram authentication for anybody. That's unfortunate, and requires that we have a setting to prohibit storing new bcrypt-hashed passwords (including pre-hashed) to avoid races. It would be better to look at the user's stored password so we could fall back to plain password auth if the stored password doesn't support scram. I realize this conflicts with the performance improvement you just put in, but I think I'd rather have support for incremental movement to the new auth protocol than removing a little latency from the new connection process. |
Yes, correct.
To control the latency of the hash validation (and thus the connection latency); I'm thinking about this PR specifically: https://github.com/cockroachlabs/managed-service/pull/7408
Yes that's also why I'm not pushing to have the session var be writable at all.
I am aware of the opportunity to do this encoding-dependent protocol detection. But consider this: pg itself does do auto-selection of protocol between scram and md5; but not between scram and cleartext. (Interestingly: pg does support md5 authn, for backward compatibility, if the HBA config says "scram", and the other way around: it does scram if the HBA config says "md5". It's all about promising that once the HBA config is set, there's no cleartext on the wire anymore)
I don't see how that follows? Can you explain more? |
|
@bdarnell @aaron-crl are suggesting to extend this work to make the scram iteration count configurable. Will add a commit to do this. |
|
@ben and I discussed how to finalize the work here. There are two separate sub-projects:
As to next steps:
|
There was a problem hiding this comment.
reviewed r11-r20
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aaron-crl, @ajwerner, @bdarnell, and @knz)
pkg/security/password.go, line 143 at r14 (raw file):
) ( isPreHashed, supportedScheme bool, issueLink int,
nit: should be named issueNum
pkg/security/password.go, line 74 at r16 (raw file):
// PasswordHash represents the type of a password hash loaded from a credential store. type PasswordHash interface {
nit: add no-op declarations for all the implementing types
pkg/security/password.go, line 411 at r17 (raw file):
var envMaxHashComputeConcurrency = envutil.EnvOrDefaultInt("COCKROACH_MAX_PW_HASH_COMPUTE_CONCURRENCY", 0) // getExpensiveHashComputeSem retrieves the bcrypt semaphore.
nit: reword "bcrypt semaphore"
pkg/security/password.go, line 419 at r17 (raw file):
n = envMaxHashComputeConcurrency } else { // We divide by 8 so that the max CPU usage of bcrypt checks
nit: reword "bcrypt" here and 2 occurrences right below this
pkg/sql/pgwire/testdata/auth/scram, line 7 at r20 (raw file):
CREATE USER foo WITH PASSWORD 'abc'; SET CLUSTER SETTING server.user_login.password_encryption = 'scram-sha-256'; CREATE USER abc WITH PASSWORD 'abc'
nit: maybe amend the usernames here to indicate that one is using bcrypt, and one is using scram
also, can we keep a test for creating a user with a pre-hashed scram password? (i'm looking at the r19->r20 diff right now; maybe this test already exists somewhere else?)
|
perhaps we can do the github.com/xdg-go/scram dependency upgrade as a separate PR, to reduce the hassle of iterating on this PR. |
To understand this commit, it is perhaps useful to remember that password credentials, in Postgres's SQL dialect, have their own expiration timestamp, set via the VALID UNTIL role option. The timestamp is attached to the password, not the user account, such that a user can still log in using a separate authentication method even when their password is expired. Additionally, it is perhaps useful to understand that throughout the authentication stack, we distinguish the phase where we determine whether a principal *exists* and is allowed to log in (i.e. it has the LOGIN privilege); and the phase where we load its password credentials from storage (or from an in-RAM cache). In particular, we are keen to skip the loading of the password credentials in the case where the client is authenticating using another method than passwords (for example TLS client certs, which is still pretty common). In this context, this commit performs two changes. The first change is an API cleanup. Prior to this commit, we were loading the password expiration timestamp in the first phase (checking that the principal exists), rather than the second phase (loading the credentials). This was a misdesign because, as explained above, the expiry timestamp is a property of the password, and thus needs not be looked at when passwords are not used. We fix this in this commit by making the expiration timestamp part of the second phase, as it should have been from the start. The second change in this commit is a reduction of security risk. Prior to this commit, the password expiration timestamp was checked (compared to the current time) again and again in each of the call points for the password retrieval function. It was the responsibility of each caller to perform this check. This constituted a security risk because a new caller could forget to add the expiry check and bypass the expiration mechanism entirely. We fix this by performing the expiry validation in a central location, in the common function that retrieves the credentials from storage (or cache). If the password is expired, the function also hides the password hash by returning the new "password missing" pseudo-hash, so that the caller cannot any more mistakenly attempt to validate a password despite the expiration. Release note: None
Release note (security update): CockroachDB now supports the SCRAM-SHA-256 authentication method for SQL clients in a way compatible with PostgreSQL servers and most PostgreSQL-compatible client drivers. SCRAM is a standard authentication protocol defined by IETF RFCs [5802](https://datatracker.ietf.org/doc/html/rfc5802) and [7677](https://datatracker.ietf.org/doc/html/rfc7677). In contrast to the cleartext-based mechanism that CockroachDB was previously using, SCRAM offers: - computational burden moved to the client: the computational cost to compute the authentication hash is borne by the client, and thus prevents DoS attacks enabled by forcing the server to compute many hashes simultaneously by malicious clients. - non-replayability: a malicious intermediary cannot reuse a password observed in one session to re-gain access later. - protection against credential stuffing: a malicious intermediary cannot reuse a password to gain access to other services. - credential secrecy: the server never learns the cleartext password and cannot impersonate the client to other servers. As before, the SCRAM credentials are stored on the server in hashed form, and prevent a malicious attacker from gaining knowledge about passwords even if they gain access to a copy of the hashes. To use SCRAM, an operator must take care of the following: 1. the stored credentials for the SQL accounts that want to use SCRAM must use the SCRAM password hash format. To store SCRAM hashes in CockroachDB, at this time it is necessary to pre-compute the SCRAM hash in a SQL client and store it pre-hashed using a CREATE/ALTER USER/ROLE WITH PASSWORD statement. This was documented in a previous release note already. A mechanism to compute the SCRAM hash server-side from a cleartext password might be provided at a later date. Note however that such a mechanism is generally undesirable: one of the main benefits of SCRAM is to remove the need for the server to know the client's cleartext password at any time; a SCRAM hash generation server-side would defeat this benefit. A plan also exists to auto-migrate existing passwords to the new format (refer to a later release note). 2. the SCRAM authentication method must be enabled. This can be done e.g. explicitly to require SCRAM specifically via a HBA configuration via the cluster setting `server.host_based_authentication.configuration`. For this, two new authentication methods are available: `scram-sha-256` `certs-scram-sha-256`. The first one is akin to PostgreSQL and requires a SCRAM authentication flow with the client. The second one is CockroachDB-specific and allows SQL client to authenticate *either* using a TLS client certificate *or* a valid SCRAM authentication flow. For example, the configuration line `host all all all scram-sha-256` will require a SCRAM authentication flow for all clients besides the `root` user. A plan also exists to automatically opt existing clusters into SCRAM (refer to a later release note). Known limitations: - HTTP authentication (web UI, HTTP APIs) still uses cleartext passwords. Security there can be enhanced in two ways: - enable and use OIDC authentication for the web UI. - use separate user accounts for access to HTTP than those used for access to SQL. - the CockroachDB implementation of SCRAM differs from PostgreSQL in two ways: - the extended protocol SCRAM-SHA-256-PLUS is not yet supported. SCRAM-SHA-256-PLUS adds *channel binding* over TLS, a mechanism that offers MITM protection from malicious intermediaries even when these have access to well-signed TLS certificates. Without this extension, proper MITM protection requires the client to verify the server certificate against a known CA and server fingerprint. CockroachDB does not yet support SCRAM-SHA-256-PLUS because we have observed that support for channel binding is not yet common in SQL client drivers besides PostgreSQL's own `libpq` driver. - CockroachDB does not yet implement zero-knowledge authentication failures like PostgreSQL. In PostgreSQL, the implementation ensures that a SQL client cannot distinguish the causes of an authentication failure: whether a password is missing, expired, invalid, or the user account does not exist, the SQL client is forced to pay the price of the SCRAM handshake and does not learn the exact cause of the failure. This ensures that a malicious attacker cannot use the type of authentication failure as a mechanism to learn properties of a target SQL account. This mechanism may be implemented in CockroachDB at a later time.
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.
This just lifts the closures into named functions and gives them explicit argument lists. Release note: None
Release note: None
Release note (security update): When using the HBA authentication method `cert-password` (the default) for SQL client connections, and the SQL client does not present a TLS client certificate to the server, CockroachDB now automatically upgrades the password handshake protocol to use SCRAM-SHA-256 if the user's stored password uses the SCRAM encoding. The previous behavior of requesting a cleartext password is still used if the stored password is encoded using the CRDB-BCRYPT format. An operator can force to _always_ request SCRAM-SHA-256 when a TLS client cert is not provided (so as to guarantee the various security benefits of SCRAM) using the authentication methods `cert-scram-sha-256` (either TLS client cert _or_ SCRAM-SHA-256); and `scram-sha-256` (only SCRAM-SHA-256). (As previously, mandatory cleartext password authentication can be requested, e.g. for debugging purposes, by using the HBA method `password`.) This automatic protocol upgrade can be manually disabled using the new cluster setting `server.user_login.cert_password_method.auto_scram_promotion.enable` (by setting it to `false`), for example if certain client drivers are found to not support SCRAM-SHA-256 authentication properly. Release note (sql change): When possible, CockroachDB will now automatically require the PostgreSQL-compatible SCRAM-SHA-256 protocol when performing password validation when SQL client log in. (Reminder: this mechanism is not used when SQL clients use TLS client certs, which is the recommended approach.) This assumes support for SCRAM-SHA-256 in client drivers. As of 2020, we have found this to be prevalent in the pg driver ecosystem. However, users should be mindful of the following possible behavior changes: - an application that tries to detect whether password verification has failed by checking server error messages, might observe different error messages with SCRAM-SHA-256. Those checks, if present, need to be updated. - if a client driver simply does not support SCRAM-SHA-256 at all, the operator retains the option to set the cluster setting `server.user_login.cert_password_method.auto_scram_promotion.enable` to `false` to force the previous password verification method instead.
knz
dismissed
rafiss’s stale review
All the sub-PRs have been approved, with @rafiss approving most of them. Thanks!
|
All the sub-PRs have been approved, will proceed to merge this once CI is happy. |
|
bors r=rafiss,JeffSwenson,bdarnell,aaron-crl,kpatron-cockroachlabs |
|
merge skew bors r- |
|
Canceled. |
Release note (security update): In order to promote a transition to SCRAM-SHA-256 for password authentication, CockroachDB now automatically attempts to convert stored password hashes after a cleartext password authentication succeeds to SCRAM-SHA-256, if the target hash method configured via `server.user_login.password_encryption` is `scram-sha-256`. This auto-conversion can happen either during SQL logins or HTTP logins that use passwords, whichever happens first. When it happens, a structured event of type `password_hash_converted` is logged to the SESSIONS channel. The PKBDF2 iteration count on the hash is chosen in order to preserve the latency of client logins, to remain similar to the latency incurred from the starting bcrypt cost. (For example, the default configuration of bcrypt cost 10 is converted to a SCRAM iteration count of 119680.) This choice, however, lowers the cost of bruteforcing passwords for an attacker with access to the encoded password hashes, if they have access to ASICs or GPUs, by a factor of ~10. For example, if it would previously cost them $1M to bruteforce a `crdb-bcrypt` hash, it would now cost them "just" $100k to bruteforce the `scram-sha-256` hash that results from this conversion. If an operator wishes to compensate for this, three options are available: 1. they can set up their infrastructure such that only passwords with a high entropy can be used. This can be achieved by e.g. disabling the ability of end-users to select their own passwords and auto-generating passwords for them, or enforce some entropy checks during password selection. This way, the entropy of the password itself compensates for the lower hash complexity. 2. they can manually select a higher SCRAM iteration count. This can be done either by pre-computing SCRAM hashes client-side and providing the precomputed hash using ALTER USER WITH PASSWORD; or adjusting the cluster setting `server.user_login.password_hashes.default_cost.scram_sha_256` and asking CockroachDB to recompute the hash. 3. they can disable the auto-conversion of `crdb-bcrypt` hashes to `scram-sha-256` altogether, using the new cluster setting `server.user_login.upgrade_bcrypt_stored_passwords_to_scram.enabled`. This approach is discouraged however, as it removes the other security protections offered by SCRAM authentication. NB: the conversion also only happens if the target configured method via `server.user_login.password_encryption` is `scram-sha-256`, because the goal of the conversion is to move clusters towards that configuration. This acknowledges cases when the operator had a legitimate reason to select another value and future versions of of CockroachDB that may use a new authentication method. Release note (sql change): After a cluster upgrade, the first time a SQL client logs in using password authentication, the password will be converted to a new format (`scram-sha-256`) if it was encoded with `crdb-bcrypt` previously. This conversion will increase the latency of that initial login by a factor of ~2x, but it will be reduced again after the conversion completes. When login latency is a concern, operators are invited to perform the password conversion ahead of time, by computing new SCRAM hashes for the clients via ALTER USER/ROLE WITH PASSWORD. This conversion can also be disabled via the new cluster setting `server.user_login.upgrade_bcrypt_stored_passwords_to_scram.enabled`.
|
bors r=rafiss,JeffSwenson,bdarnell,aaron-crl,kpatron-cockroachlabs |
|
Build failed: |
|
unrelated flake bors r=rafiss,JeffSwenson,bdarnell,aaron-crl,kpatron-cockroachlabs |
|
Build succeeded: |
This comment was marked as spam.
This comment was marked as spam.
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 and 7677 and as supported by PostgreSQL. This offers multiple security benefits over CockroachDB's current use of cleartext password authentication. To use this feature,
server.host_based_authentication.configuration).How to review this work
The addition of the feature goes as follows:
adding HBA syntax and authentication method hooks for
scram-sha-256andcert-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: pgwire,sqlproxyccl: base support for a SCRAM authentication flow (scram 2/10) #74842
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: sql,security: support storing SCRAM hashes in ALTER/CREATE USER/ROLE (scram 3/10) #74843
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: sql,security: create a password hash abstraction (scram 4/10) #74844 and security: enable plaintext authentication with SCRAM credentials (scram 5/10) #74845
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: sql,pgwire,server: factor the password expiry code (scram 6/10) #74846 and pgwire: support the SCRAM authentication flow (scram 7/10) #74847
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 variablepassword_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: sql: make the stored hash method configurable (scram 8/10) #74848
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: pgwire: auto-upgrade password authentication with SCRAM hashes (scram 9/10) #74849
Auto-upgrade the stored credentials from bcrypt to SCRAM, when enabled and possible.
Split into separate PR: pgwire,security: auto-upgrade bcrypt hashes to SCRAM-SHA-256 (scram 10/10) #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.