From 17234c30dcf7df79d270724cdc1dc304b49bd90c Mon Sep 17 00:00:00 2001
From: Raphael 'kena' Poss 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
diff --git a/docs/generated/sql/functions.md b/docs/generated/sql/functions.md
index 3953e55b6fd5..48a997dee3f0 100644
--- a/docs/generated/sql/functions.md
+++ b/docs/generated/sql/functions.md
@@ -2887,6 +2887,8 @@ may increase either contention or retry errors, or both.server.web_session.purge.max_deletions_per_cycle
integer 10
the maximum number of old sessions to delete for each purge
Example usage: SELECT * FROM crdb_internal.check_consistency(true, ‘\x02’, ‘\x04’)
+crdb_internal.check_password_hash(password: bytes) → string
This function checks whether a string is a precomputed password hash. Returns the hash algorithm.
+crdb_internal.cluster_id() → uuid
Returns the cluster ID.
crdb_internal.cluster_name() → string
Returns the cluster name.
diff --git a/pkg/security/password.go b/pkg/security/password.go index d25e1bfc481a..571e2e397efc 100644 --- a/pkg/security/password.go +++ b/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( diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go index bcbaf59f0e72..a3fc1525eb11 100644 --- a/pkg/sql/alter_role.go +++ b/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( diff --git a/pkg/sql/create_role.go b/pkg/sql/create_role.go index 95c8ef1d460e..02b2bbcd7247 100644 --- a/pkg/sql/create_role.go +++ b/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) } diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index dc55449970e4..62ab8797d912 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/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 + diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role index 906834626dc2..142cc7b4a219 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/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, "hashedPassword", 'BCRYPT'||"hashedPassword" = '$bcrypt_pw' FROM system.users WHERE username LIKE 'hash%' ORDER BY 1 +---- +hash1 $2a$10$vcmoIBvgeHjgScVHWRMWI.Z3v03WMixAw2bBS6qZihljSUuwi88Yq true +hash4 $2a$10$GiwDkV1A2SAMdsZJHjQlFOR.vJZ0EaSz8xBwT3.ISRlngTLnJ2.j2 false +hash5 $2a$10$bNbWDesjhnC9i3mOJE3dH.i/.y4HtUClo78kqY6nBPb0lk/nQjmV6 false +hash6 $2a$10$jKPPbkYZMhXoljyMZ7riXuk0QPfKtgrZpjQyGu52Rnlh.5aG7b0fm false + +# Reset cluster setting after test completion. +statement ok +SET CLUSTER SETTING server.user_login.detect_password_encoding.enabled = true diff --git a/pkg/sql/pgwire/testdata/auth/password_change b/pkg/sql/pgwire/testdata/auth/password_change index d235bde1b73a..d9a5945c8078 100644 --- a/pkg/sql/pgwire/testdata/auth/password_change +++ b/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. diff --git a/pkg/sql/sem/builtins/builtins.go b/pkg/sql/sem/builtins/builtins.go index f30a942abf61..b11a34b93af3 100644 --- a/pkg/sql/sem/builtins/builtins.go +++ b/pkg/sql/sem/builtins/builtins.go @@ -41,6 +41,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/catalog" @@ -6096,6 +6097,30 @@ table's zone configuration this will return NULL.`, }, ), + "crdb_internal.check_password_hash": makeBuiltin( + tree.FunctionProperties{ + Category: categorySystemInfo, + }, + tree.Overload{ + Types: tree.ArgTypes{{"password", types.Bytes}}, + ReturnType: tree.FixedReturnType(types.String), + Fn: func(evalCtx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { + arg := []byte(tree.MustBeDBytes(args[0])) + ctx := evalCtx.Ctx() + isHashed, _, schemeName, _, err := security.CheckPasswordHashValidity(ctx, arg) + if err != nil { + return tree.DNull, err + } + if !isHashed { + return tree.DNull, errors.New("hash format not recognized") + } + return tree.NewDString(schemeName), nil + }, + Info: "This function checks whether a string is a precomputed password hash. Returns the hash algorithm.", + Volatility: tree.VolatilityImmutable, + }, + ), + "crdb_internal.schedule_sql_stats_compaction": makeBuiltin( tree.FunctionProperties{ Category: categorySystemInfo,