pgwire: support the SCRAM authentication flow (scram 7/10)#74847
pgwire: support the SCRAM authentication flow (scram 7/10)#74847craig[bot] merged 1 commit intomasterfrom
Conversation
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @knz and @kpatron-cockroachlabs)
pkg/sql/pgwire/auth_methods.go, line 235 at r1 (raw file):
// code fallback, to hide the reason for the failure from the // client. return creds, errors.New("password is expired")
similar question as i had in the previous PR; why have a different error?
pkg/sql/pgwire/auth_methods.go, line 243 at r1 (raw file):
// The method check above ensures this cast is always valid. scramCreds := hashedPassword.(*security.ScramHash)
the ScramHash struct could remain unexported if StoredCredentials() was a method on the PasswordHash interface. that may be a better abstraction so we can avoid the chance someone sees this type assertion and copies it incorrectly.
kpatron-cockroachlabs
left a comment
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @knz, @kpatron-cockroachlabs, and @rafiss)
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @kpatron-cockroachlabs and @rafiss)
pkg/sql/pgwire/auth_methods.go, line 235 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
similar question as i had in the previous PR; why have a different error?
Similar answer as here: you're right and this can be improved.
Are you OK with me doing so in a followup PR?
pkg/sql/pgwire/auth_methods.go, line 243 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
the
ScramHashstruct could remain unexported ifStoredCredentials()was a method on thePasswordHashinterface. that may be a better abstraction so we can avoid the chance someone sees this type assertion and copies it incorrectly.
Yeah I've thought about this, but the return type of StoredCredentials is really scram-specific.
I changed this to a function call. Do you like it better?
rafiss
left a comment
There was a problem hiding this comment.
lgtm; optional suggestion
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @knz, @kpatron-cockroachlabs, and @rafiss)
pkg/sql/pgwire/auth_methods.go, line 235 at r1 (raw file):
Previously, knz (kena) wrote…
Similar answer as here: you're right and this can be improved.
Are you OK with me doing so in a followup PR?
yes that sounds great
pkg/sql/pgwire/auth_methods.go, line 243 at r1 (raw file):
Previously, knz (kena) wrote…
Yeah I've thought about this, but the return type of
StoredCredentialsis really scram-specific.
I changed this to a function call. Do you like it better?
yes this seems nice. if you'd like, i feel it the most safe of all options (to prevent the GetSCRAMStoredCredentials from being misused in the future and cause a panic) is something like
func GetSCRAMStoredCredentials(hash PasswordHash) (scram.StoredCredentials, bool) {
sh, ok := hash.(*scramHash)
return sh, ok
}
and the Method() check above could be elided
} else if scramCreds, ok := security.GetSCRAMStoredCredentials(hashedPassword); ok {
return scramCreds
}
const credentialsNotSCRAM = "user password hash not in SCRAM format"
c.LogAuthInfof(ctx, credentialsNotSCRAM)
return creds, errors.New(credentialsNotSCRAM)
i don't feel strongly about it
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @knz, @kpatron-cockroachlabs, and @rafiss)
pkg/sql/pgwire/auth_methods.go, line 243 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
yes this seems nice. if you'd like, i feel it the most safe of all options (to prevent the
GetSCRAMStoredCredentialsfrom being misused in the future and cause a panic) is something likefunc GetSCRAMStoredCredentials(hash PasswordHash) (scram.StoredCredentials, bool) { sh, ok := hash.(*scramHash) return sh, ok }and the
Method()check above could be elided} else if scramCreds, ok := security.GetSCRAMStoredCredentials(hashedPassword); ok { return scramCreds } const credentialsNotSCRAM = "user password hash not in SCRAM format" c.LogAuthInfof(ctx, credentialsNotSCRAM) return creds, errors.New(credentialsNotSCRAM)i don't feel strongly about it
Done.
|
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): 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.
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>
|
@knz: Thanks a lot for your work on it :) Note about HTTP, there is an RFC: https://tools.ietf.org/html/rfc7804. LDAP exists too: More info here: scram-sasl/info#1... |
(PR peeled away from #74301; previous PR in series #74846; next PR in series: #74848)
Fixes #42519.
Informs #65117.
Epic CRDB-5349
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 and
7677. In contrast to
the cleartext-based mechanism that CockroachDB was previously using,
SCRAM offers:
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.
a password observed in one session to re-gain access later.
intermediary cannot reuse a password to gain access to other services.
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:
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).
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-256certs-scram-sha-256. The first one is akin toPostgreSQL 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-256will require a SCRAM authentication flow for allclients besides the
rootuser.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:
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
libpqdriver.Tracking issue: security: implement SCRAM channel binding #74300
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.