diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index d234018b5715..80c3d3353ff2 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -68,6 +68,8 @@ 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.min_password_length integer 1 the minimum length accepted for passwords set in cleartext via SQL. Note that a value lower than 1 is ignored: passwords cannot be empty in any case. +server.user_login.password_hashes.default_cost.crdb_bcrypt integer 10 the hashing cost to use when storing passwords supplied as cleartext by SQL clients with the hashing method crdb-bcrypt (allowed range: 4-31) server.user_login.store_client_pre_hashed_passwords.enabled boolean true whether the server accepts to store passwords pre-hashed by clients 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/settings/settings.html b/docs/generated/settings/settings.html index 24bd4ec3fa4d..6d804c80b0e4 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -74,6 +74,8 @@ server.shutdown.lease_transfer_waitduration5sthe 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_waitduration10sthe 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_deadduration5m0sthe time after which if there is no new gossiped information about a store, it is considered dead +server.user_login.min_password_lengthinteger1the minimum length accepted for passwords set in cleartext via SQL. Note that a value lower than 1 is ignored: passwords cannot be empty in any case. +server.user_login.password_hashes.default_cost.crdb_bcryptinteger10the hashing cost to use when storing passwords supplied as cleartext by SQL clients with the hashing method crdb-bcrypt (allowed range: 4-31) server.user_login.store_client_pre_hashed_passwords.enabledbooleantruewhether the server accepts to store passwords pre-hashed by clients server.user_login.timeoutduration10stimeout after which client authentication times out if some system range is unavailable (0 = no timeout) server.web_session.auto_logout.timeoutduration168h0m0sthe duration that web sessions will survive before being periodically purged, since they were last used diff --git a/pkg/ccl/serverccl/role_authentication_test.go b/pkg/ccl/serverccl/role_authentication_test.go index 308ebedc76e9..8a155875fd1f 100644 --- a/pkg/ccl/serverccl/role_authentication_test.go +++ b/pkg/ccl/serverccl/role_authentication_test.go @@ -42,11 +42,12 @@ func TestVerifyPassword(t *testing.T) { s.ExecutorConfig().(sql.ExecutorConfig).Settings, ) + ts := s.(*server.TestServer) + if util.RaceEnabled { // The default bcrypt cost makes this test approximately 30s slower when the // race detector is on. - defer func(prev int) { security.BcryptCost = prev }(security.BcryptCost) - security.BcryptCost = bcrypt.MinCost + security.BcryptCost.Override(ctx, &ts.Cfg.Settings.SV, int64(bcrypt.MinCost)) } //location is used for timezone testing. diff --git a/pkg/security/password.go b/pkg/security/password.go index bfaca3ea469c..14a8fd666553 100644 --- a/pkg/security/password.go +++ b/pkg/security/password.go @@ -14,6 +14,7 @@ import ( "bytes" "context" "crypto/sha256" + "fmt" "regexp" "runtime" "sync" @@ -28,13 +29,34 @@ import ( "golang.org/x/crypto/bcrypt" ) -// BcryptCost is the cost to use when hashing passwords. It is exposed for -// testing. +// BcryptCost is the cost to use when hashing passwords. +// It is exposed for testing. // -// BcryptCost should increase along with computation power. -// For estimates, see: http://security.stackexchange.com/questions/17207/recommended-of-rounds-for-bcrypt -// For now, we use the library's default cost. -var BcryptCost = bcrypt.DefaultCost +// The default value of BcryptCost should increase along with +// computation power. +// +// For estimates, see: +// http://security.stackexchange.com/questions/17207/recommended-of-rounds-for-bcrypt +var BcryptCost = settings.RegisterIntSetting( + settings.TenantWritable, + BcryptCostSettingName, + fmt.Sprintf( + "the hashing cost to use when storing passwords supplied as cleartext by SQL clients "+ + "with the hashing method crdb-bcrypt (allowed range: %d-%d)", + bcrypt.MinCost, bcrypt.MaxCost), + // The default value 10 is equal to bcrypt.DefaultCost. + // It incurs a password check latency of ~60ms on AMD 3950X 3.7GHz. + // For reference, value 11 incurs ~110ms latency on the same hw, value 12 incurs ~390ms. + 10, + func(i int64) error { + if i < int64(bcrypt.MinCost) || i > int64(bcrypt.MaxCost) { + return bcrypt.InvalidCostError(int(i)) + } + return nil + }).WithPublic() + +// BcryptCostSettingName is the name of the cluster setting BcryptCost. +const BcryptCostSettingName = "server.user_login.password_hashes.default_cost.crdb_bcrypt" // ErrEmptyPassword indicates that an empty password was attempted to be set. var ErrEmptyPassword = errors.New("empty passwords are not permitted") @@ -74,14 +96,14 @@ func CompareHashAndPassword(ctx context.Context, hashedPassword []byte, password } // HashPassword takes a raw password and returns a bcrypt hashed password. -func HashPassword(ctx context.Context, password string) ([]byte, error) { +func HashPassword(ctx context.Context, sv *settings.Values, password string) ([]byte, error) { sem := getBcryptSem(ctx) alloc, err := sem.Acquire(ctx, 1) if err != nil { return nil, err } defer alloc.Release() - return bcrypt.GenerateFromPassword(appendEmptySha256(password), BcryptCost) + return bcrypt.GenerateFromPassword(appendEmptySha256(password), int(BcryptCost.Get(sv))) } // AutoDetectPasswordHashes is the cluster setting that configures whether @@ -168,7 +190,7 @@ var MinPasswordLength = settings.RegisterIntSetting( "Note that a value lower than 1 is ignored: passwords cannot be empty in any case.", 1, settings.NonNegativeInt, -) +).WithPublic() // bcryptSemOnce wraps a semaphore that limits the number of concurrent calls // to the bcrypt hash functions. This is needed to avoid the risk of a diff --git a/pkg/server/authentication_test.go b/pkg/server/authentication_test.go index decf366d005f..39099c77c5ab 100644 --- a/pkg/server/authentication_test.go +++ b/pkg/server/authentication_test.go @@ -201,8 +201,7 @@ func TestVerifyPassword(t *testing.T) { if util.RaceEnabled { // The default bcrypt cost makes this test approximately 30s slower when the // race detector is on. - defer func(prev int) { security.BcryptCost = prev }(security.BcryptCost) - security.BcryptCost = bcrypt.MinCost + security.BcryptCost.Override(ctx, &ts.Cfg.Settings.SV, int64(bcrypt.MinCost)) } //location is used for timezone testing. diff --git a/pkg/sql/create_role.go b/pkg/sql/create_role.go index 02b2bbcd7247..a84f97d93c3b 100644 --- a/pkg/sql/create_role.go +++ b/pkg/sql/create_role.go @@ -282,7 +282,7 @@ func (p *planner) checkPasswordAndGetHash( "Passwords must be %d characters or longer.", minLength) } - hashedPassword, err = security.HashPassword(ctx, password) + hashedPassword, err = security.HashPassword(ctx, &st.SV, password) if err != nil { return hashedPassword, err } diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role index 2dbac4103b59..414128261628 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/pkg/sql/logictest/testdata/logic_test/role @@ -1458,4 +1458,21 @@ hash6 $2a$10$ false # Reset cluster setting after test completion. statement ok -SET CLUSTER SETTING server.user_login.store_client_pre_hashed_passwords.enabled = true +RESET CLUSTER SETTING server.user_login.store_client_pre_hashed_passwords.enabled; + +subtest bcrypt_cost + +statement ok +SET CLUSTER SETTING server.user_login.password_hashes.default_cost.crdb_bcrypt = 20 + +statement ok +CREATE USER hash7 WITH PASSWORD 'hello' + +# Check that the configured cost was embedded in the setting. +query TT +SELECT username, substr("hashedPassword", 1, 7) FROM system.users WHERE username = 'hash7' +---- +hash7 $2a$20$ + +statement ok +RESET CLUSTER SETTING server.user_login.password_hashes.default_cost.crdb_bcrypt;