release-21.2: sql: support client-provided password hashes

[jeffswenson]

Backport 3/3 commits from #72579.

/cc @cockroachdb/release

Release Justification: Serverless uses this feature to reduce the connection setup latency from ~120ms to ~40ms. The backport of the feature was approved by @abhinavg6.

There is a small functional change between the backport and the original change. In the backport the feature setting server.user_login.store_client_pre_hashed_passwords.enabled defaults to false.

---

Fixes #50757

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 auto-detection is subject to the new cluster setting
server.user_login.store_client_pre_hashed_passwords.enabled. This setting
defaults to false (i.e. feature disabled).

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.

Should a deployment require such checks to be performed database-side,
the operator would need to disable the mechanism via the cluster
setting named above. 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.store_client_pre_hashed_passwords.enabled', 'false', 'b');

(We do not recommend relying on the database to perform password
checks. Our recommended deployment best practice is to implement
credential definitions in a control plane / identity provider that is
separate from the database.)

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 'CRDB-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.store_client_pre_hashed_passwords.enabled to false.

Note: this design mimics the behavior of PostgreSQL, which recognizes
pre-computed password hashes when presented to the regular PASSWORD
option (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
CRDB-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_format().

Currently, CockroachDB only recognizes password hashes computed using
Bcrypt, as follows (we detail this algorithm so that orchestration
software can implement their own password hash computation, separate
from the database):

1. take the cleartext password string.

2. append the following byte array to the password:
   e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
   (these are 32 hex-encoded bytes)

   (What are these bytes? 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, as per

   https://en.wikipedia.org/wiki/Bcrypt

   or

   https://bcrypt.online

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

5. Encode the hash into the format recognized by CockroachDB: the
   string CRDB-BCRYPT, followed by the standard bcrypt hash
   encoding ($2a$...).

Summary:

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

[blathers-crl]

Thanks for opening a backport.

Please check the backport criteria before merging:

• Patches should only be created for serious issues or test-only changes.
• Patches should not break backwards-compatibility.
• Patches should change as little code as possible.
• Patches should not change on-disk formats or node communication protocols.
• Patches should not add new functionality.
• Patches must not add, edit, or otherwise modify cluster versions; or add version gates.

If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.

• There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
• The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
• New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
• The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

• What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
• Will this work in a cluster of mixed patch versions? Did we test that?
• If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

[cockroach-teamcity]

This change is 

[jeffswenson]

Things to watch out for in the review:

1. The default value of server.user_login.store_client_pre_hashed_passwords.enabled was changed to false. This required some minor changes to the setting, documentation, and data driven tests.
2. The logic tests were adjusted to account for the lack of sql/parser: use RoleSpec for ALTER ROLE #71498. The normalized form of the ALTER ROLE command is different in 21.2.
3. There was a trivial merge conflict in pkg/sql/create_role.go.

All of the non-trivial changes made to backport the CL are in the "sql: changes to backport client-provided hashes" commit.

[knz]

Reviewed 2 of 2 files at r1, 4 of 4 files at r2, 10 of 10 files at r3, 5 of 7 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jeffswenson and @knz)

---

-- commits, line 121 at r4:
Can you rephrase that commit message and explain what the change is and why it was necessary.

Also, this needs a release note that amends the release note in the previous commit, to explain the new default: the doc writer needs to know this for when they propagate the change to the doc branch.

---

pkg/sql/parser/testdata/alter_user, line 22 at r4 (raw file):

ALTER USER foo WITH ENCRYPTED PASSWORD bar
----
ALTER USER 'foo' WITH PASSWORD '*****' -- normalized!

Ouch what just happened here? This change can't be right?

---

pkg/sql/parser/testdata/create_user, line 110 at r4 (raw file):

CREATE USER foo ENCRYPTED PASSWORD bar
----
CREATE USER 'foo' WITH PASSWORD '*****' -- normalized!

ditto

[jeffswenson]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jeffswenson and @knz)

---

-- commits, line 121 at r4:

Previously, knz (kena) wrote…

Can you rephrase that commit message and explain what the change is and why it was necessary.

Also, this needs a release note that amends the release note in the previous commit, to explain the new default: the doc writer needs to know this for when they propagate the change to the doc branch.

Done.

---

pkg/sql/parser/testdata/alter_user, line 22 at r4 (raw file):

Previously, knz (kena) wrote…

Ouch what just happened here? This change can't be right?

This is related to #71498. Since the backport does not contain #71498, the tests need to treat the role name as a literal.

[knz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jeffswenson and @knz)

---

pkg/sql/parser/testdata/alter_user, line 22 at r4 (raw file):

Previously, JeffSwenson (Jeff Swenson) wrote…

This is related to #71498. Since the backport does not contain #71498, the tests need to treat the role name as a literal.

Then you need to place these test changes as an amend in the first commit of the PR: we try very hard to enforce that individual commits inside a PR pass all tests, and your change here is needed to ensure the 1st commit passes its test.

(If you find this complicated, alternatively you can squash the two commits together)

[jeffswenson]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jeffswenson and @knz)

---

pkg/sql/parser/testdata/alter_user, line 22 at r4 (raw file):

Previously, knz (kena) wrote…

Then you need to place these test changes as an amend in the first commit of the PR: we try very hard to enforce that individual commits inside a PR pass all tests, and your change here is needed to ensure the 1st commit passes its test.

(If you find this complicated, alternatively you can squash the two commits together)

Done! I figured out how to do it with rebase and git commit -a. I merged the test changes into the commit that introduced the encrypted keyword.

[knz]

Nice.

Reviewed 2 of 7 files at r4, 10 of 10 files at r6, 10 of 10 files at r7, 1 of 5 files at r8, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jeffswenson and @knz)