sql: support password hash autodetection like PostgreSQL

We wish to use this in the CC control plane, when provisioning SQL
accounts in new clusters, or when users manipulate their user list in
the CC management console.

Release note (security update): It is now possible to pre-compute the
hash of the password credentials of a SQL user client-side, and set
the SQL user's password using the hash, so that the CockroachDB never
sees the password string in clear in the SQL session.

This feature is meant for use in automation/orchestration, when the
control plane constructs passwords for users outside of CockroachDB,
and there is an architectural desire to ensure that cleartext
passwords are not transmitted/stored in-clear.

Note: when the client provides the password hash, CockroachDB
cannot carry any checks on the internal structure of the password,
such as minimum length, special characters, etc.

Release note (security update): CockroachDB now interprets
certain strings passed to 'CREATE/ALTER USER/ROLE WITH PASSWORD'
specially (see previous release note), subject
to the new cluster setting
`server.user_login.detect_password_encoding.enabled`.
This setting defaults to `true` (i.e. feature enabled).

When upgrading a cluster from a previous version, to ensure
that the feature remains disabled throughout the upgrade,
use the following statement prior to the upgrade:
```sql
INSERT INTO system.settings(name, value, "valueType")
VALUES('server.user_login.detect_password_encoding.enabled', 'false', 'b');
```

Release note (sql change):  The `CREATE ROLE` and `ALTER ROLE`
statements now
accept password hashes computed by the client app. For example:
`CREATE USER foo WITH PASSWORD 'BCRYPT$2a$10$.....'`.

Note: this feature is not meant for use by human users / in
interactive sessions; it is meant for use in programs, using the
computation algorithm described below.

This auto-detection can be disabled by changing the cluster setting
`server.user_login.detect_password_encoding.enabled` to `false`.

Note: this design mimics the behavior of PostgreSQL, which recognizes
pre-computed password hashes when presented to the regular PASSWORD
option (https://www.postgresql.org/docs/14/sql-createrole.html).

The password hashes are auto-detected based on their lexical
structure. For example, any password that starts with the prefix
`BCRYPT`, followed by a valid encoding of a bcrypt hash (as detailed
below), is considered a candidate password hash.

To ascertain whether a password hash will be recognized as such,
orchestration code can use the new built-in function
`crdb_internal.check_password_hash()`.

Currently, CockroachDB only recognizes password hashes computed using
Bcrypt, as follows:

1. take the cleartext password string
2. append the following string to the password:
`"\xe3\xb0\xc4B\x98\xfc\x1c\x14\x9a\xfb\xf4șo\xb9$'\xaeA\xe4d\x9b\x93L\xa4\x95\x99\x1bxR\xb8U"`

   (What is this string? it's the SHA-256 hash of an empty string. Why
   is it appended? This is a historical oddity in the CockroachDB with
   no particular reason. It adds no security.)

3. choose a Bcrypt cost. (CockroachDB servers use cost 10 by default.)
4. generate a bcrypt hash of the string generated at step 2 with the
   cost chosen at step 3. (This entails generating a 16 bytes random salt,
   base-64 encoding the salt, applying the bcrypt function.)
5. Encode the hash into the format recognized by CockroachDB: the
   string `BCRYPT$2a$` (Bcrypt major/minor versions), followed by the Bcrypt
   cost as two decimal digits, followed by `$`, followed by the base-64
   encoding of the salt, followed by the base-64 encoding of the bcrypt
   hash.

Note: the base64 encoding should use the following alphabet:
`./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789`

Note: at this point, CockroachDB only supports hashes computed using
Bcrypt version 2 or earlier.

Summary:

| Hash method     | Recognized by `crdb_internal.check_password_hash` | ALTER/CREATE USER WITH PASSWORD           |
|-----------------|---------------------------------------------------|-------------------------------------------|
| `bcrypt`        | yes (`BCRYPT$2a$...`)                             | recognized if enabled via cluster setting |
| `scram-sha-256` | yes (`SCRAM-SHA-256$4096:...`)                    | not implemented yet (issue cockroachdb#42519)        |
| `md5`           | yes (`md5...`)                                    | obsolete, will not be implemented         |

docs/generated/settings/settings-for-tenants.txt

@@ -69,6 +69,7 @@ server.shutdown.drain_wait	duration	0s	the amount of time a server waits in an u
 server.shutdown.lease_transfer_wait	duration	5s	the amount of time a server waits to transfer range leases before proceeding with the rest of the shutdown process (note that the --drain-wait parameter for cockroach node drain may need adjustment after changing this setting)
 server.shutdown.query_wait	duration	10s	the server will wait for at least this amount of time for active queries to finish (note that the --drain-wait parameter for cockroach node drain may need adjustment after changing this setting)
 server.time_until_store_dead	duration	5m0s	the time after which if there is no new gossiped information about a store, it is considered dead
+server.user_login.detect_password_encoding.enabled	boolean	true	whether the server recognizes pre-hashed user passwords
 server.user_login.timeout	duration	10s	timeout after which client authentication times out if some system range is unavailable (0 = no timeout)
 server.web_session.auto_logout.timeout	duration	168h0m0s	the duration that web sessions will survive before being periodically purged, since they were last used
 server.web_session.purge.max_deletions_per_cycle	integer	10	the maximum number of old sessions to delete for each purge

docs/generated/settings/settings.html

@@ -74,6 +74,7 @@
 <tr><td><code>server.shutdown.lease_transfer_wait</code></td><td>duration</td><td><code>5s</code></td><td>the amount of time a server waits to transfer range leases before proceeding with the rest of the shutdown process (note that the --drain-wait parameter for cockroach node drain may need adjustment after changing this setting)</td></tr>
 <tr><td><code>server.shutdown.query_wait</code></td><td>duration</td><td><code>10s</code></td><td>the server will wait for at least this amount of time for active queries to finish (note that the --drain-wait parameter for cockroach node drain may need adjustment after changing this setting)</td></tr>
 <tr><td><code>server.time_until_store_dead</code></td><td>duration</td><td><code>5m0s</code></td><td>the time after which if there is no new gossiped information about a store, it is considered dead</td></tr>
+<tr><td><code>server.user_login.detect_password_encoding.enabled</code></td><td>boolean</td><td><code>true</code></td><td>whether the server recognizes pre-hashed user passwords</td></tr>
 <tr><td><code>server.user_login.timeout</code></td><td>duration</td><td><code>10s</code></td><td>timeout after which client authentication times out if some system range is unavailable (0 = no timeout)</td></tr>
 <tr><td><code>server.web_session.auto_logout.timeout</code></td><td>duration</td><td><code>168h0m0s</code></td><td>the duration that web sessions will survive before being periodically purged, since they were last used</td></tr>
 <tr><td><code>server.web_session.purge.max_deletions_per_cycle</code></td><td>integer</td><td><code>10</code></td><td>the maximum number of old sessions to delete for each purge</td></tr>

docs/generated/sql/functions.md

@@ -2887,6 +2887,8 @@ may increase either contention or retry errors, or both.</p>
 <p>Example usage:
 SELECT * FROM crdb_internal.check_consistency(true, ‘\x02’, ‘\x04’)</p>
 </span></td></tr>
+<tr><td><a name="crdb_internal.check_password_hash"></a><code>crdb_internal.check_password_hash(password: <a href="bytes.html">bytes</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>This function checks whether a string is a precomputed password hash. Returns the hash algorithm.</p>
+</span></td></tr>
 <tr><td><a name="crdb_internal.cluster_id"></a><code>crdb_internal.cluster_id() &rarr; <a href="uuid.html">uuid</a></code></td><td><span class="funcdesc"><p>Returns the cluster ID.</p>
 </span></td></tr>
 <tr><td><a name="crdb_internal.cluster_name"></a><code>crdb_internal.cluster_name() &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the cluster name.</p>

pkg/security/password.go

@@ -11,8 +11,10 @@
 package security
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
+	"regexp"
 	"runtime"
 	"sync"
 
@@ -80,6 +82,75 @@ func HashPassword(ctx context.Context, password string) ([]byte, error) {
 	return bcrypt.GenerateFromPassword(appendEmptySha256(password), BcryptCost)
 }
 
+// AutoDetectPasswordHashes is the cluster setting that configures whether
+// the server recognizes pre-hashed passwords.
+var AutoDetectPasswordHashes = settings.RegisterBoolSetting(
+	"server.user_login.detect_password_encoding.enabled",
+	"whether the server recognizes pre-hashed user passwords",
+	true,
+).WithPublic()
+
+// bcryptHashRe matches the lexical structure of the bcrypt hash
+// format supported by CockroachDB. The base64 encoding of the hash
+// uses the alphabet used by the bcrypt package:
+// "./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+var bcryptHashRe = regexp.MustCompile(`^BCRYPT\$\d[a-z]?\$\d\d\$[0-9A-Za-z\./]{22}[0-9A-Za-z\./]+$`)
+
+func isBcryptHash(hashedPassword []byte) bool {
+	return bcryptHashRe.Match(hashedPassword)
+}
+
+// scramHashRe matches the lexical structure of PostgreSQL's
+// pre-computed SCRAM hashes.
+//
+// This structure is inspired from PosgreSQL's parse_scram_secret() function.
+// The base64 encoding uses the alphabet used by pg_b64_encode():
+// "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"
+// The salt must have size >0; the server key pair is two times 32 bytes,
+// which always encode to 44 base64 characters.
+var scramHashRe = regexp.MustCompile(`^SCRAM-SHA-256\$\d+:[A-Za-z0-9+/]+=*\$[A-Za-z0-9+/]{43}=:[A-Za-z0-9+/]{43}=$`)
+
+func isSCRAMHash(hashedPassword []byte) bool {
+	return scramHashRe.Match(hashedPassword)
+}
+
+func isMD5Hash(hashedPassword []byte) bool {
+	// This logic is inspired from PostgreSQL's get_password_type() function.
+	return bytes.HasPrefix(hashedPassword, []byte("md5")) &&
+		len(hashedPassword) == 35 &&
+		len(bytes.Trim(hashedPassword[3:], "0123456789abcdef")) == 0
+}
+
+// CheckPasswordHashValidity determines whether a (user-provided)
+// password is already hashed, and if already hashed, verifies whether
+// the hash is recognized as a valid hash.
+// Return values:
+// - isPreHashed indicates whether the password is already hashed.
+// - supportedScheme indicates whether the scheme is currently supported
+//   for authentication.
+// - hashedPassword is a translated version from the input,
+//   suitable for storage in the password database.
+func CheckPasswordHashValidity(
+	ctx context.Context, inputPassword []byte,
+) (isPreHashed, supportedScheme bool, schemeName string, hashedPassword []byte, err error) {
+	if isBcryptHash(inputPassword) {
+		// Trim the "BCRYPT" prefix. We trim this because previous version
+		// CockroachDB nodes do not understand the prefix when stored.
+		hashedPassword = inputPassword[6:]
+		// The bcrypt.Cost() function parses the hash and checks its syntax.
+		_, err = bcrypt.Cost(hashedPassword)
+		return true, true, "bcrypt", hashedPassword, err
+	}
+	if isSCRAMHash(inputPassword) {
+		return true, false /* unsupported yet */, "scram-sha-256", inputPassword, nil
+	}
+	if isMD5Hash(inputPassword) {
+		return true, false /* definitely not supported */, "md5", inputPassword, nil
+	}
+
+	return false, false, "", inputPassword, nil
+}
+
 // MinPasswordLength is the cluster setting that configures the
 // minimum SQL password length.
 var MinPasswordLength = settings.RegisterIntSetting(

pkg/sql/alter_role.go

@@ -185,30 +185,11 @@ func (n *alterRoleNode) startExec(params runParams) error {
 		}
 	}
 
-	if n.roleOptions.Contains(roleoption.PASSWORD) {
-		isNull, password, err := n.roleOptions.GetPassword()
-		if err != nil {
-			return err
-		}
-		if !isNull && params.extendedEvalCtx.ExecCfg.RPCContext.Config.Insecure {
-			// We disallow setting a non-empty password in insecure mode
-			// because insecure means an observer may have MITM'ed the change
-			// and learned the password.
-			//
-			// It's valid to clear the password (WITH PASSWORD NULL) however
-			// since that forces cert auth when moving back to secure mode,
-			// and certs can't be MITM'ed over the insecure SQL connection.
-			return pgerror.New(pgcode.InvalidPassword,
-				"setting or updating a password is not supported in insecure mode")
-		}
-
-		var hashedPassword []byte
-		if !isNull {
-			if hashedPassword, err = params.p.checkPasswordAndGetHash(params.ctx, password); err != nil {
-				return err
-			}
-		}
-
+	hasPasswordOpt, hashedPassword, err := retrievePasswordFromRoleOptions(params, n.roleOptions)
+	if err != nil {
+		return err
+	}
+	if hasPasswordOpt {
 		// Updating PASSWORD is a special case since PASSWORD lives in system.users
 		// while the rest of the role options lives in system.role_options.
 		_, err = params.extendedEvalCtx.ExecCfg.InternalExecutor.Exec(

pkg/sql/create_role.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 	"github.com/cockroachdb/errors"
 )
@@ -117,29 +118,9 @@ func (n *CreateRoleNode) startExec(params runParams) error {
 		opName = "create-user"
 	}
 
-	var hashedPassword []byte
-	if n.roleOptions.Contains(roleoption.PASSWORD) {
-		isNull, password, err := n.roleOptions.GetPassword()
-		if err != nil {
-			return err
-		}
-		if !isNull && params.extendedEvalCtx.ExecCfg.RPCContext.Config.Insecure {
-			// We disallow setting a non-empty password in insecure mode
-			// because insecure means an observer may have MITM'ed the change
-			// and learned the password.
-			//
-			// It's valid to clear the password (WITH PASSWORD NULL) however
-			// since that forces cert auth when moving back to secure mode,
-			// and certs can't be MITM'ed over the insecure SQL connection.
-			return pgerror.New(pgcode.InvalidPassword,
-				"setting or updating a password is not supported in insecure mode")
-		}
-
-		if !isNull {
-			if hashedPassword, err = params.p.checkPasswordAndGetHash(params.ctx, password); err != nil {
-				return err
-			}
-		}
+	_, hashedPassword, err := retrievePasswordFromRoleOptions(params, n.roleOptions)
+	if err != nil {
+		return err
 	}
 
 	// Check if the user/role exists.
@@ -242,6 +223,37 @@ func (*CreateRoleNode) Values() tree.Datums { return tree.Datums{} }
 // Close implements the planNode interface.
 func (*CreateRoleNode) Close(context.Context) {}
 
+func retrievePasswordFromRoleOptions(
+	params runParams, roleOptions roleoption.List,
+) (hasPasswordOpt bool, hashedPassword []byte, err error) {
+	if !roleOptions.Contains(roleoption.PASSWORD) {
+		return false, nil, nil
+	}
+	isNull, password, err := roleOptions.GetPassword()
+	if err != nil {
+		return true, nil, err
+	}
+	if !isNull && params.extendedEvalCtx.ExecCfg.RPCContext.Config.Insecure {
+		// We disallow setting a non-empty password in insecure mode
+		// because insecure means an observer may have MITM'ed the change
+		// and learned the password.
+		//
+		// It's valid to clear the password (WITH PASSWORD NULL) however
+		// since that forces cert auth when moving back to secure mode,
+		// and certs can't be MITM'ed over the insecure SQL connection.
+		return true, nil, pgerror.New(pgcode.InvalidPassword,
+			"setting or updating a password is not supported in insecure mode")
+	}
+
+	if !isNull {
+		if hashedPassword, err = params.p.checkPasswordAndGetHash(params.ctx, password); err != nil {
+			return true, nil, err
+		}
+	}
+
+	return true, hashedPassword, nil
+}
+
 func (p *planner) checkPasswordAndGetHash(
 	ctx context.Context, password string,
 ) (hashedPassword []byte, err error) {
@@ -250,8 +262,23 @@ func (p *planner) checkPasswordAndGetHash(
 	}
 
 	st := p.ExecCfg().Settings
+	if security.AutoDetectPasswordHashes.Get(&st.SV) {
+		var isPreHashed, schemeSupported bool
+		var schemeName string
+		isPreHashed, schemeSupported, schemeName, hashedPassword, err = security.CheckPasswordHashValidity(ctx, []byte(password))
+		if err != nil {
+			return hashedPassword, pgerror.WithCandidateCode(err, pgcode.Syntax)
+		}
+		if isPreHashed {
+			if !schemeSupported {
+				return hashedPassword, unimplemented.NewWithIssueDetailf(42519, schemeName, "the password hash scheme %q is not supported", schemeName)
+			}
+			return hashedPassword, nil
+		}
+	}
+
 	if minLength := security.MinPasswordLength.Get(&st.SV); minLength >= 1 && int64(len(password)) < minLength {
-		return hashedPassword, errors.WithHintf(security.ErrPasswordTooShort,
+		return nil, errors.WithHintf(security.ErrPasswordTooShort,
 			"Passwords must be %d characters or longer.", minLength)
 	}
 

pkg/sql/logictest/testdata/logic_test/builtin_function

@@ -3056,3 +3056,34 @@ query B
 SELECT 'ACTIVE'::text ~ similar_to_escape(NULL::text)
 ----
 NULL
+
+subtest password_hash
+
+# Need to escapes the dollar signs since the testlogic language gives them special meaning.
+
+query error pgcode 42601 bcrypt algorithm version .3. requested is newer than current version .2.
+SELECT crdb_internal.check_password_hash(('BCRYPT$'||'3a$'||'10$'||'vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq')::bytes)
+
+query error pgcode 42601 too short to be a bcrypted password
+SELECT crdb_internal.check_password_hash(('BCRYPT$'||'2a$'||'10$'||'vcmoIBvgeHjgScVHWRMWI.Z3v0')::bytes)
+
+query error pgcode 42601 cost 1 is outside allowed range \(4,31\)
+SELECT crdb_internal.check_password_hash(('BCRYPT$'||'2a$'||'01$'||'vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq')::bytes)
+
+query T
+SELECT crdb_internal.check_password_hash(('BCRYPT$'||'2a$'||'10$'||'vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq')::bytes)
+----
+bcrypt
+
+# Legacy format, only recognized for parsing, not for storing passwords.
+query T
+SELECT crdb_internal.check_password_hash('md5aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')
+----
+md5
+
+# Not yet supported for storing passwords, but recognized on input.
+query T
+SELECT crdb_internal.check_password_hash(('SCRAM-SHA-256$'||'4096:B5VaTCvCLDzZxSYL829oVA==$'||'3Ako3mNxNtdsaSOJl0Av3i6vyV2OiSP9Ly7famdFSbw=:d7BfSmrtjwbF74mSoOhQiDSpoIvlakXKdpBNb3Meunc=')::bytes)
+----
+scram-sha-256
+

pkg/sql/logictest/testdata/logic_test/role

@@ -1406,3 +1406,52 @@ user testuser
 # because of a missing ADMIN power, not because of a missing CREATEROLE option.
 statement error only users with the admin role are allowed to ALTER ROLE admin
 ALTER ROLE other_admin NOCREATEDB
+
+subtest pw_hashes
+
+user root
+
+let $bcrypt_pw
+SELECT 'BCRYPT$'||'2a$'||'10$'||'vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq'
+
+statement ok
+CREATE USER hash1 WITH PASSWORD '$bcrypt_pw'
+
+# We don't plan to support md5 ever.
+statement error pgcode 0A000 hash scheme "md5" is not supported
+CREATE USER hash2 WITH PASSWORD 'md5aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+
+let $scram_pw
+SELECT 'SCRAM-SHA-256$'||'4096:B5VaTCvCLDzZxSYL829oVA==$'||'3Ako3mNxNtdsaSOJl0Av3i6vyV2OiSP9Ly7famdFSbw=:d7BfSmrtjwbF74mSoOhQiDSpoIvlakXKdpBNb3Meunc='
+
+# Refers to https://github.com/cockroachdb/cockroach/issues/42519
+statement error pgcode 0A000 hash scheme "scram-sha-256" is not supported
+CREATE USER hash3 WITH PASSWORD '$scram_pw'
+
+
+statement ok
+SET CLUSTER SETTING server.user_login.detect_password_encoding.enabled = false
+
+# Password hash not recognized: hash considered as password input.
+statement ok
+CREATE USER hash4 WITH PASSWORD '$bcrypt_pw'
+
+# Password hash not recognized: hash considered as password input.
+statement ok
+CREATE USER hash5 WITH PASSWORD 'md5aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+
+# Password hash not recognized: hash considered as password input.
+statement ok
+CREATE USER hash6 WITH PASSWORD '$scram_pw'
+
+query TTB
+SELECT username, substr("hashedPassword", 1, 7), 'BCRYPT'||"hashedPassword" = '$bcrypt_pw' FROM system.users WHERE username LIKE 'hash%' ORDER BY 1
+----
+hash1  $2a$10$  true
+hash4  $2a$10$  false
+hash5  $2a$10$  false
+hash6  $2a$10$  false
+
+# Reset cluster setting after test completion.
+statement ok
+SET CLUSTER SETTING server.user_login.detect_password_encoding.enabled = true

pkg/sql/pgwire/testdata/auth/password_change

@@ -55,6 +55,43 @@ ERROR: password authentication failed for user userpw (SQLSTATE 28000)
 
 subtest end
 
+subtest precomputed_hash
+
+sql
+CREATE USER userhpw WITH PASSWORD 'BCRYPT$3a$10$vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq'
+----
+ERROR: crypto/bcrypt: bcrypt algorithm version '3' requested is newer than current version '2' (SQLSTATE 42601)
+
+sql
+CREATE USER userhpw WITH PASSWORD 'BCRYPT$2a$10$vcmoIBvgeHjgScVHWRMWI.Z3v0'
+----
+ERROR: crypto/bcrypt: hashedSecret too short to be a bcrypted password (SQLSTATE 42601)
+
+sql
+CREATE USER userhpw WITH PASSWORD 'BCRYPT$2a$01$vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq'
+----
+ERROR: crypto/bcrypt: cost 1 is outside allowed range (4,31) (SQLSTATE 42601)
+
+sql
+CREATE USER userhpw WITH PASSWORD 'BCRYPT$2a$10$vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq'
+----
+ok
+
+connect user=userhpw password=demo37559
+----
+ok defaultdb
+
+sql
+ALTER USER userhpw WITH PASSWORD 'BCRYPT$2a$10$jeDfxx9fI7dDp3p0I3BTGOX2uKjnErlmgf74U0bp9KusDpAVypc1.'
+----
+ok
+
+connect user=userhpw password=abc
+----
+ok defaultdb
+
+subtest end
+
 subtest root_pw
 
 # By default root cannot log in with a password.