From 530498b1e22f974a40d26fea25a17282c5bceb96 Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Mon, 22 Mar 2021 15:59:38 -0700 Subject: [PATCH 01/12] feature:bcrypt-hasher --- driver/config/.schema/config.schema.json | 6 ++ driver/config/config.go | 28 ++++++++ driver/registry_default.go | 6 +- hash/hasher_bcrypt.go | 57 +++++++++++++++ hash/hasher_test.go | 91 +++++++++++++++++++++++- 5 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 hash/hasher_bcrypt.go diff --git a/driver/config/.schema/config.schema.json b/driver/config/.schema/config.schema.json index f822be328b90..850d78459d0f 100644 --- a/driver/config/.schema/config.schema.json +++ b/driver/config/.schema/config.schema.json @@ -1294,6 +1294,12 @@ "title": "Hashing Algorithm Configuration", "type": "object", "properties": { + "algorithm": { + "title": "Password hashing algorithm", + "description": "One of the values: argon2, bcrypt", + "type": "string", + "default": "argon2" + }, "argon2": { "title": "Configuration for the Argon2id hasher.", "type": "object", diff --git a/driver/config/config.go b/driver/config/config.go index ef78c2ca4a64..f38853dce6f3 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -36,6 +36,7 @@ const ( DefaultIdentityTraitsSchemaID = "default" DefaultBrowserReturnURL = "default_browser_return_url" DefaultSQLiteMemoryDSN = dbal.SQLiteInMemory + DefaultPasswordHashingAlgorithm = "argon2" UnknownVersion = "unknown version" ViperKeyDSN = "dsn" ViperKeyCourierSMTPURL = "courier.smtp.connection_uri" @@ -84,11 +85,13 @@ const ( ViperKeySelfServiceVerificationBrowserDefaultReturnTo = "selfservice.flows.verification.after." + DefaultBrowserReturnURL ViperKeyDefaultIdentitySchemaURL = "identity.default_schema_url" ViperKeyIdentitySchemas = "identity.schemas" + ViperKeyHasherAlgorithm = "hashers.algorithm" ViperKeyHasherArgon2ConfigMemory = "hashers.argon2.memory" ViperKeyHasherArgon2ConfigIterations = "hashers.argon2.iterations" ViperKeyHasherArgon2ConfigParallelism = "hashers.argon2.parallelism" ViperKeyHasherArgon2ConfigSaltLength = "hashers.argon2.salt_length" ViperKeyHasherArgon2ConfigKeyLength = "hashers.argon2.key_length" + ViperKeyHasherBcryptCost = "hashers.bcrypt.cost" ViperKeyPasswordMaxBreaches = "selfservice.methods.password.config.max_breaches" ViperKeyIgnoreNetworkErrors = "selfservice.methods.password.config.ignore_network_errors" ViperKeyVersion = "version" @@ -96,6 +99,7 @@ const ( Argon2DefaultIterations uint32 = 4 Argon2DefaultSaltLength uint32 = 16 Argon2DefaultKeyLength uint32 = 32 + BcryptDefaultCost uint32 = 12 ) // DefaultSessionCookieName returns the default cookie name for the kratos session. @@ -109,6 +113,9 @@ type ( SaltLength uint32 `json:"salt_length"` KeyLength uint32 `json:"key_length"` } + Bcrypt struct { + Cost uint32 `json:"cost"` + } SelfServiceHook struct { Name string `json:"hook"` Config json.RawMessage `json:"config"` @@ -235,6 +242,15 @@ func (p *Config) HasherArgon2() *Argon2 { } } + +func (p *Config) HasherBcrypt() *Bcrypt { + // warn about usage of default values and point to the docs + // warning will require https://github.com/ory/viper/issues/19 + return &Bcrypt{ + Cost: uint32(p.p.IntF(ViperKeyHasherBcryptCost, int(BcryptDefaultCost))), + } +} + func (p *Config) listenOn(key string) string { fb := 4433 if key == "admin" { @@ -732,3 +748,15 @@ func (p *Config) PasswordPolicyConfig() *PasswordPolicy { IgnoreNetworkErrors: p.p.BoolF(ViperKeyIgnoreNetworkErrors, true), } } + +func (p *Config) HasherPasswordHashingAlgorithm() string { + configValue := p.p.StringF(ViperKeyHasherAlgorithm, DefaultPasswordHashingAlgorithm) + switch configValue { + case "argon2": + return configValue + case "bcrypt": + return configValue + } + panic(fmt.Sprintf("Unexpected hashing algorithm value: %s", configValue)) +} + diff --git a/driver/registry_default.go b/driver/registry_default.go index 6c18ef5cf285..43869d0c2a9e 100644 --- a/driver/registry_default.go +++ b/driver/registry_default.go @@ -358,7 +358,11 @@ func (m *RegistryDefault) SessionHandler() *session.Handler { func (m *RegistryDefault) Hasher() hash.Hasher { if m.passwordHasher == nil { - m.passwordHasher = hash.NewHasherArgon2(m) + if m.c.HasherPasswordHashingAlgorithm() == "bcrypt" { + m.passwordHasher = hash.NewHasherBcrypt(m) + } else { + m.passwordHasher = hash.NewHasherArgon2(m) + } } return m.passwordHasher } diff --git a/hash/hasher_bcrypt.go b/hash/hasher_bcrypt.go new file mode 100644 index 000000000000..6bbbf994cc6a --- /dev/null +++ b/hash/hasher_bcrypt.go @@ -0,0 +1,57 @@ +package hash + +import ( + "context" + "github.com/ory/kratos/driver/config" + "github.com/pkg/errors" + "golang.org/x/crypto/bcrypt" +) + +type Bcrypt struct { + c BcryptConfiguration +} + +type BcryptConfiguration interface { + config.Provider +} + +func NewHasherBcrypt(c BcryptConfiguration) *Bcrypt { + return &Bcrypt{c: c} +} + +func (h *Bcrypt) Generate(ctx context.Context, password []byte) ([]byte, error) { + if err := validatePasswordLength(password); err != nil { + return nil, err + } + + hash, err := bcrypt.GenerateFromPassword(password, int(h.c.Config(ctx).HasherBcrypt().Cost)) + if err != nil { + return nil, err + } + + return hash, nil +} + +func (h *Bcrypt) Compare(_ context.Context, password []byte, hash []byte) error { + if err := validatePasswordLength(password); err != nil { + return err + } + + err := bcrypt.CompareHashAndPassword(hash, password) + if err != nil { + return err + } + + return nil +} + +func validatePasswordLength(password []byte) error { + // Bcrypt truncates the password to the first 72 bytes, following the OpenBSD implementation, + // so if password is longer than 72 bytes, function returns an error + // See https://en.wikipedia.org/wiki/Bcrypt#User_input + if len(password) > 72 { + return errors.New("password is too long") + } + return nil +} + diff --git a/hash/hasher_test.go b/hash/hasher_test.go index 45856f9ade87..53d563f136cb 100644 --- a/hash/hasher_test.go +++ b/hash/hasher_test.go @@ -20,7 +20,7 @@ func mkpw(t *testing.T, length int) []byte { return pw } -func TestHasher(t *testing.T) { +func TestArgonHasher(t *testing.T) { for k, pw := range [][]byte{ mkpw(t, 8), mkpw(t, 16), @@ -50,3 +50,92 @@ func TestHasher(t *testing.T) { }) } } + +func TestBcryptHasherGeneratesErrorWhenPasswordIsLong(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + hasher := hash.NewHasherBcrypt(reg) + + password := mkpw(t, 73) + res, err := hasher.Generate(context.Background(), password) + + assert.Error(t, err, "password is too long") + assert.Nil(t, res) +} + +func TestBcryptHasherGeneratesHash(t *testing.T) { + for k, pw := range [][]byte{ + mkpw(t, 8), + mkpw(t, 16), + mkpw(t, 32), + mkpw(t, 64), + mkpw(t, 72), + } { + t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + hasher := hash.NewHasherBcrypt(reg) + res, err := hasher.Generate(context.Background(), pw) + + assert.Nil(t, err) + + // Valid format: $2a$12$[22 character salt][31 character hash] + assert.Equal(t, 60, len(string(res)), "invalid bcrypt hash length") + assert.Equal(t, "$2a$12$", string(res)[:7], "invalid bcrypt identifier") + }) + } +} + + +func TestBcryptHasherComparesFailsWhenPasswordIsTooLong(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + hasher := hash.NewHasherBcrypt(reg) + + password := mkpw(t, 73) + err := hasher.Compare(context.Background(), password, []byte("hash")) + + assert.Error(t, err, "password is too long") +} + + +func TestBcryptHasherComparesHashSuccess(t *testing.T) { + for k, pw := range [][]byte{ + mkpw(t, 8), + mkpw(t, 16), + mkpw(t, 32), + mkpw(t, 64), + mkpw(t, 72), + } { + t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + hasher := hash.NewHasherBcrypt(reg) + + hs, err := hasher.Generate(context.Background(), pw) + + assert.Nil(t, err) + + err = hasher.Compare(context.Background(), pw, hs) + assert.Nil(t, err, "hash validation fails") + }) + } +} + +func TestBcryptHasherComparesHashFail(t *testing.T) { + for k, pw := range [][]byte{ + mkpw(t, 8), + mkpw(t, 16), + mkpw(t, 32), + mkpw(t, 64), + mkpw(t, 72), + } { + t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + hasher := hash.NewHasherBcrypt(reg) + + mod := make([]byte, len(pw)) + copy(mod, pw) + mod[len(pw)-1] = ^pw[len(pw)-1] + + err := hasher.Compare(context.Background(), pw, mod) + assert.Error(t, err) + }) + } +} From 69a0ffb67d1d8188cf8e88f6100bd5587e3c8fa6 Mon Sep 17 00:00:00 2001 From: seremenko-wish <60801091+seremenko-wish@users.noreply.github.com> Date: Tue, 23 Mar 2021 16:47:05 -0700 Subject: [PATCH 02/12] Update driver/config/config.go Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> --- driver/config/config.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/driver/config/config.go b/driver/config/config.go index f38853dce6f3..c935f427eeee 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -752,11 +752,10 @@ func (p *Config) PasswordPolicyConfig() *PasswordPolicy { func (p *Config) HasherPasswordHashingAlgorithm() string { configValue := p.p.StringF(ViperKeyHasherAlgorithm, DefaultPasswordHashingAlgorithm) switch configValue { - case "argon2": - return configValue case "bcrypt": return configValue + case "argon2": + default: + return configValue } - panic(fmt.Sprintf("Unexpected hashing algorithm value: %s", configValue)) } - From 3a3f12049406768b733ee2c3eb6f03f9c75f8686 Mon Sep 17 00:00:00 2001 From: seremenko-wish <60801091+seremenko-wish@users.noreply.github.com> Date: Tue, 23 Mar 2021 16:47:13 -0700 Subject: [PATCH 03/12] Update hash/hasher_bcrypt.go Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> --- hash/hasher_bcrypt.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hash/hasher_bcrypt.go b/hash/hasher_bcrypt.go index 6bbbf994cc6a..b9c314f425e3 100644 --- a/hash/hasher_bcrypt.go +++ b/hash/hasher_bcrypt.go @@ -50,8 +50,7 @@ func validatePasswordLength(password []byte) error { // so if password is longer than 72 bytes, function returns an error // See https://en.wikipedia.org/wiki/Bcrypt#User_input if len(password) > 72 { - return errors.New("password is too long") + return errors.New("passwords are limited to a maximum length of 72 characters") } return nil } - From c81ab8c903e0a2da61dd2f76d1d15c98cf8be4c3 Mon Sep 17 00:00:00 2001 From: seremenko-wish <60801091+seremenko-wish@users.noreply.github.com> Date: Wed, 24 Mar 2021 16:12:44 -0700 Subject: [PATCH 04/12] Update driver/config/config.go Co-authored-by: Patrik --- driver/config/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/driver/config/config.go b/driver/config/config.go index c935f427eeee..78d2b3d869d1 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -755,6 +755,7 @@ func (p *Config) HasherPasswordHashingAlgorithm() string { case "bcrypt": return configValue case "argon2": + fallthrough default: return configValue } From 5e99a99ae86b8957c3a8a6772ebcddad11abc615 Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Thu, 25 Mar 2021 15:23:31 -0700 Subject: [PATCH 05/12] Password hash comparator --- hash/hash_comparator.go | 107 +++++++++++++++++++++++++ hash/hasher.go | 3 - hash/hasher_argon2.go | 59 -------------- hash/hasher_bcrypt.go | 17 +--- hash/hasher_test.go | 42 ++++++---- selfservice/strategy/password/login.go | 3 +- 6 files changed, 137 insertions(+), 94 deletions(-) create mode 100644 hash/hash_comparator.go diff --git a/hash/hash_comparator.go b/hash/hash_comparator.go new file mode 100644 index 000000000000..3519876bef19 --- /dev/null +++ b/hash/hash_comparator.go @@ -0,0 +1,107 @@ +package hash + +import ( + "context" + "crypto/subtle" + "encoding/base64" + "fmt" + "github.com/ory/kratos/driver/config" + "github.com/pkg/errors" + "golang.org/x/crypto/argon2" + "golang.org/x/crypto/bcrypt" + "regexp" + "strings" +) + +var ErrUnknownHashAlgorithm = errors.New("unknown hash algorithm") + +func Compare(ctx context.Context, password []byte, hash []byte) error { + if isBcryptHash(hash) { + return CompareBcrypt(ctx, password, hash) + } else if isArgon2idHash(hash) { + return CompareArgon2id(ctx, password, hash) + } else { + return ErrUnknownHashAlgorithm + } +} + +func CompareBcrypt(_ context.Context, password []byte, hash []byte) error { + if err := validateBcryptPasswordLength(password); err != nil { + return err + } + + err := bcrypt.CompareHashAndPassword(hash, password) + if err != nil { + return err + } + + return nil +} + +func CompareArgon2id(_ context.Context, password []byte, hash []byte) error { + // Extract the parameters, salt and derived key from the encoded password + // hash. + p, salt, hash, err := decodeArgon2idHash(string(hash)) + if err != nil { + return err + } + + // Derive the key from the other password using the same parameters. + otherHash := argon2.IDKey([]byte(password), salt, p.Iterations, p.Memory, p.Parallelism, p.KeyLength) + + // Check that the contents of the hashed passwords are identical. Note + // that we are using the subtle.ConstantTimeCompare() function for this + // to help prevent timing attacks. + if subtle.ConstantTimeCompare(hash, otherHash) == 1 { + return nil + } + return ErrMismatchedHashAndPassword +} + +func isBcryptHash(hash []byte) bool { + res, _ := regexp.Match("^\\$2[abzy]?\\$", hash) + return res +} + +func isArgon2idHash(hash []byte) bool { + res, _ := regexp.Match("^\\$argon2id\\$", hash) + return res +} + +func decodeArgon2idHash(encodedHash string) (p *config.Argon2, salt, hash []byte, err error) { + parts := strings.Split(encodedHash, "$") + if len(parts) != 6 { + return nil, nil, nil, ErrInvalidHash + } + + var version int + _, err = fmt.Sscanf(parts[2], "v=%d", &version) + if err != nil { + return nil, nil, nil, err + } + if version != argon2.Version { + return nil, nil, nil, ErrIncompatibleVersion + } + + p = new(config.Argon2) + _, err = fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &p.Memory, &p.Iterations, &p.Parallelism) + if err != nil { + return nil, nil, nil, err + } + + salt, err = base64.RawStdEncoding.Strict().DecodeString(parts[4]) + if err != nil { + return nil, nil, nil, err + } + p.SaltLength = uint32(len(salt)) + + hash, err = base64.RawStdEncoding.Strict().DecodeString(parts[5]) + if err != nil { + return nil, nil, nil, err + } + p.KeyLength = uint32(len(hash)) + + return p, salt, hash, nil +} + + diff --git a/hash/hasher.go b/hash/hasher.go index 8641892e035f..28ca8553866e 100644 --- a/hash/hasher.go +++ b/hash/hasher.go @@ -4,9 +4,6 @@ import "context" // Hasher provides methods for generating and comparing password hashes. type Hasher interface { - // Compare a password to a hash and return nil if they match or an error otherwise. - Compare(ctx context.Context, password []byte, hash []byte) error - // Generate returns a hash derived from the password or an error if the hash method failed. Generate(ctx context.Context, password []byte) ([]byte, error) } diff --git a/hash/hasher_argon2.go b/hash/hasher_argon2.go index f3e2505ea7e7..d6f86810eed9 100644 --- a/hash/hasher_argon2.go +++ b/hash/hasher_argon2.go @@ -4,11 +4,8 @@ import ( "bytes" "context" "crypto/rand" - "crypto/subtle" "encoding/base64" "fmt" - "strings" - "github.com/pkg/errors" "golang.org/x/crypto/argon2" @@ -59,59 +56,3 @@ func (h *Argon2) Generate(ctx context.Context, password []byte) ([]byte, error) return b.Bytes(), nil } - -func (h *Argon2) Compare(ctx context.Context, password []byte, hash []byte) error { - // Extract the parameters, salt and derived key from the encoded password - // hash. - p, salt, hash, err := decodeHash(string(hash)) - if err != nil { - return err - } - - // Derive the key from the other password using the same parameters. - otherHash := argon2.IDKey([]byte(password), salt, p.Iterations, p.Memory, p.Parallelism, p.KeyLength) - - // Check that the contents of the hashed passwords are identical. Note - // that we are using the subtle.ConstantTimeCompare() function for this - // to help prevent timing attacks. - if subtle.ConstantTimeCompare(hash, otherHash) == 1 { - return nil - } - return ErrMismatchedHashAndPassword -} - -func decodeHash(encodedHash string) (p *config.Argon2, salt, hash []byte, err error) { - parts := strings.Split(encodedHash, "$") - if len(parts) != 6 { - return nil, nil, nil, ErrInvalidHash - } - - var version int - _, err = fmt.Sscanf(parts[2], "v=%d", &version) - if err != nil { - return nil, nil, nil, err - } - if version != argon2.Version { - return nil, nil, nil, ErrIncompatibleVersion - } - - p = new(config.Argon2) - _, err = fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &p.Memory, &p.Iterations, &p.Parallelism) - if err != nil { - return nil, nil, nil, err - } - - salt, err = base64.RawStdEncoding.DecodeString(parts[4]) - if err != nil { - return nil, nil, nil, err - } - p.SaltLength = uint32(len(salt)) - - hash, err = base64.RawStdEncoding.DecodeString(parts[5]) - if err != nil { - return nil, nil, nil, err - } - p.KeyLength = uint32(len(hash)) - - return p, salt, hash, nil -} diff --git a/hash/hasher_bcrypt.go b/hash/hasher_bcrypt.go index b9c314f425e3..758cecd4e6f0 100644 --- a/hash/hasher_bcrypt.go +++ b/hash/hasher_bcrypt.go @@ -20,7 +20,7 @@ func NewHasherBcrypt(c BcryptConfiguration) *Bcrypt { } func (h *Bcrypt) Generate(ctx context.Context, password []byte) ([]byte, error) { - if err := validatePasswordLength(password); err != nil { + if err := validateBcryptPasswordLength(password); err != nil { return nil, err } @@ -32,20 +32,7 @@ func (h *Bcrypt) Generate(ctx context.Context, password []byte) ([]byte, error) return hash, nil } -func (h *Bcrypt) Compare(_ context.Context, password []byte, hash []byte) error { - if err := validatePasswordLength(password); err != nil { - return err - } - - err := bcrypt.CompareHashAndPassword(hash, password) - if err != nil { - return err - } - - return nil -} - -func validatePasswordLength(password []byte) error { +func validateBcryptPasswordLength(password []byte) error { // Bcrypt truncates the password to the first 72 bytes, following the OpenBSD implementation, // so if password is longer than 72 bytes, function returns an error // See https://en.wikipedia.org/wiki/Bcrypt#User_input diff --git a/hash/hasher_test.go b/hash/hasher_test.go index 53d563f136cb..b74890cfb5cf 100644 --- a/hash/hasher_test.go +++ b/hash/hasher_test.go @@ -39,12 +39,12 @@ func TestArgonHasher(t *testing.T) { assert.NotEqual(t, pw, hs) t.Logf("hash: %s", hs) - require.NoError(t, h.Compare(context.Background(), pw, hs)) + require.NoError(t, hash.CompareArgon2id(context.Background(), pw, hs)) mod := make([]byte, len(pw)) copy(mod, pw) mod[len(pw)-1] = ^pw[len(pw)-1] - require.Error(t, h.Compare(context.Background(), mod, hs)) + require.Error(t, hash.CompareArgon2id(context.Background(), mod, hs)) }) } }) @@ -84,19 +84,14 @@ func TestBcryptHasherGeneratesHash(t *testing.T) { } } - -func TestBcryptHasherComparesFailsWhenPasswordIsTooLong(t *testing.T) { - _, reg := internal.NewFastRegistryWithMocks(t) - hasher := hash.NewHasherBcrypt(reg) - +func TestComparatorBcryptFailsWhenPasswordIsTooLong(t *testing.T) { password := mkpw(t, 73) - err := hasher.Compare(context.Background(), password, []byte("hash")) + err := hash.CompareBcrypt(context.Background(), password, []byte("hash")) assert.Error(t, err, "password is too long") } - -func TestBcryptHasherComparesHashSuccess(t *testing.T) { +func TestComparatorBcryptSuccess(t *testing.T) { for k, pw := range [][]byte{ mkpw(t, 8), mkpw(t, 16), @@ -112,13 +107,13 @@ func TestBcryptHasherComparesHashSuccess(t *testing.T) { assert.Nil(t, err) - err = hasher.Compare(context.Background(), pw, hs) + err = hash.CompareBcrypt(context.Background(), pw, hs) assert.Nil(t, err, "hash validation fails") }) } } -func TestBcryptHasherComparesHashFail(t *testing.T) { +func TestComparatorBcryptFail(t *testing.T) { for k, pw := range [][]byte{ mkpw(t, 8), mkpw(t, 16), @@ -127,15 +122,30 @@ func TestBcryptHasherComparesHashFail(t *testing.T) { mkpw(t, 72), } { t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { - _, reg := internal.NewFastRegistryWithMocks(t) - hasher := hash.NewHasherBcrypt(reg) - mod := make([]byte, len(pw)) copy(mod, pw) mod[len(pw)-1] = ^pw[len(pw)-1] - err := hasher.Compare(context.Background(), pw, mod) + err := hash.CompareBcrypt(context.Background(), pw, mod) assert.Error(t, err) }) } } + +func TestCompare(t *testing.T) { + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$2a$12$o6hx.Wog/wvFSkT/Bp/6DOxCtLRTDj7lm9on9suF/WaCGNVHbkfL6"))) + assert.Nil(t, hash.CompareBcrypt(context.Background(), []byte("test"), []byte("$2a$12$o6hx.Wog/wvFSkT/Bp/6DOxCtLRTDj7lm9on9suF/WaCGNVHbkfL6"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$2a$12$o6hx.Wog/wvFSkT/Bp/6DOxCtLRTDj7lm9on9suF/WaCGNVHbkfL7"))) + + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$2a$15$GRvRO2nrpYTEuPQX6AieaOlZ4.7nMGsXpt.QWMev1zrP86JNspZbO"))) + assert.Nil(t, hash.CompareBcrypt(context.Background(), []byte("test"), []byte("$2a$15$GRvRO2nrpYTEuPQX6AieaOlZ4.7nMGsXpt.QWMev1zrP86JNspZbO"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$2a$15$GRvRO2nrpYTEuPQX6AieaOlZ4.7nMGsXpt.QWMev1zrP86JNspZb1"))) + + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$argon2id$v=19$m=32,t=2,p=4$cm94YnRVOW5jZzFzcVE4bQ$MNzk5BtR2vUhrp6qQEjRNw"))) + assert.Nil(t, hash.CompareArgon2id(context.Background(), []byte("test"), []byte("$argon2id$v=19$m=32,t=2,p=4$cm94YnRVOW5jZzFzcVE4bQ$MNzk5BtR2vUhrp6qQEjRNw"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$argon2id$v=19$m=32,t=2,p=4$cm94YnRVOW5jZzFzcVE4bQ$MNzk5BtR2vUhrp6qQEjRN2"))) + + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$argon2id$v=19$m=32,t=5,p=4$cm94YnRVOW5jZzFzcVE4bQ$fBxypOL0nP/zdPE71JtAV71i487LbX3fJI5PoTN6Lp4"))) + assert.Nil(t, hash.CompareArgon2id(context.Background(), []byte("test"), []byte("$argon2id$v=19$m=32,t=5,p=4$cm94YnRVOW5jZzFzcVE4bQ$fBxypOL0nP/zdPE71JtAV71i487LbX3fJI5PoTN6Lp4"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$argon2id$v=19$m=32,t=5,p=4$cm94YnRVOW5jZzFzcVE4bQ$fBxypOL0nP/zdPE71JtAV71i487LbX3fJI5PoTN6Lp5"))) +} diff --git a/selfservice/strategy/password/login.go b/selfservice/strategy/password/login.go index bd3f2f034d75..99ad66cc6340 100644 --- a/selfservice/strategy/password/login.go +++ b/selfservice/strategy/password/login.go @@ -3,6 +3,7 @@ package password import ( "bytes" "encoding/json" + "github.com/ory/kratos/hash" "net/http" "github.com/julienschmidt/httprouter" @@ -146,7 +147,7 @@ func (s *Strategy) handleLogin(w http.ResponseWriter, r *http.Request, _ httprou return } - if err := s.d.Hasher().Compare(r.Context(), []byte(p.Password), []byte(o.HashedPassword)); err != nil { + if err := hash.Compare(r.Context(), []byte(p.Password), []byte(o.HashedPassword)); err != nil { s.handleLoginError(w, r, ar, &p, errors.WithStack(schema.NewInvalidCredentialsError())) return } From 359d6bb4e9d1e23c23a9f3fbcbd68f16247f001d Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Thu, 25 Mar 2021 15:52:40 -0700 Subject: [PATCH 06/12] updated doc --- .../credentials/username-email-password.mdx | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/docs/concepts/credentials/username-email-password.mdx b/docs/docs/concepts/credentials/username-email-password.mdx index b235912a4eba..659fa80aafd0 100644 --- a/docs/docs/concepts/credentials/username-email-password.mdx +++ b/docs/docs/concepts/credentials/username-email-password.mdx @@ -10,7 +10,7 @@ during registration and login. ORY Kratos hashes the password after registration, password reset, and password change using the [Argon2 Hashing Algorithm](../security.mdx#Argon2), the winner of the -[Password Hashing Competition (PHC)](https://github.com/P-H-C/phc-winner-argon2). +[Password Hashing Competition (PHC)](https://github.com/P-H-C/phc-winner-argon2), or Bcrypt. ## Configuration @@ -24,8 +24,9 @@ selfservice: enabled: true ``` -in your ORY Kratos configuration. You can configure the Argon2 hasher using the -following options: +in your ORY Kratos configuration. + +You can configure the Argon2 hasher using the following options: ```yaml title="path/to/my/kratos/config.yml" # $ kratos -c path/to/my/kratos/config.yml serve @@ -38,6 +39,23 @@ hashers: key_length: 32 ``` +By default, Kratos uses Argon2 algorithm for password hashing. Use the following option +to use Bcrypt algorithm: + +```yaml title="path/to/my/kratos/config.yml" +hashers: + algorithm: bcrypt +``` + +Bcrypt algorithm can be configured only by the following `cost` option (default value is 12): + +```yaml title="path/to/my/kratos/config.yml" +# $ kratos -c path/to/my/kratos/config.yml serve +hashers: + bcrypt: + cost: 12 +``` + To determine the ideal parameters, head over to the [setup guide](../../guides/setting-up-password-hashing-parameters). @@ -337,7 +355,7 @@ credentials: - john.doe@example.org - johndoe123 config: - hashed_password: ... # this would be `argon2(my-secret-password)` + hashed_password: ... # this would be a hash of `my-secret-password` string ``` Because credential identifiers need to be unique, no other identity can be From 7897c0c60a30e238404f352cfe80720747a64852 Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Thu, 25 Mar 2021 16:05:44 -0700 Subject: [PATCH 07/12] updated doc --- .../concepts/credentials/username-email-password.mdx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/docs/concepts/credentials/username-email-password.mdx b/docs/docs/concepts/credentials/username-email-password.mdx index 659fa80aafd0..876bba3e4ba2 100644 --- a/docs/docs/concepts/credentials/username-email-password.mdx +++ b/docs/docs/concepts/credentials/username-email-password.mdx @@ -10,7 +10,8 @@ during registration and login. ORY Kratos hashes the password after registration, password reset, and password change using the [Argon2 Hashing Algorithm](../security.mdx#Argon2), the winner of the -[Password Hashing Competition (PHC)](https://github.com/P-H-C/phc-winner-argon2), or Bcrypt. +[Password Hashing Competition (PHC)](https://github.com/P-H-C/phc-winner-argon2), +or Bcrypt. ## Configuration @@ -39,15 +40,16 @@ hashers: key_length: 32 ``` -By default, Kratos uses Argon2 algorithm for password hashing. Use the following option -to use Bcrypt algorithm: +By default, Kratos uses Argon2 algorithm for password hashing. Use the following +option to use Bcrypt algorithm: ```yaml title="path/to/my/kratos/config.yml" hashers: algorithm: bcrypt ``` -Bcrypt algorithm can be configured only by the following `cost` option (default value is 12): +Bcrypt algorithm can be configured only by the following `cost` option (default +value is 12): ```yaml title="path/to/my/kratos/config.yml" # $ kratos -c path/to/my/kratos/config.yml serve From f06b6c6e8a023f67615d627b44c9c1e9eae0ef66 Mon Sep 17 00:00:00 2001 From: seremenko-wish <60801091+seremenko-wish@users.noreply.github.com> Date: Fri, 26 Mar 2021 14:21:50 -0700 Subject: [PATCH 08/12] Update driver/config/.schema/config.schema.json Co-authored-by: Patrik --- driver/config/.schema/config.schema.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/driver/config/.schema/config.schema.json b/driver/config/.schema/config.schema.json index 850d78459d0f..b09d14cd1716 100644 --- a/driver/config/.schema/config.schema.json +++ b/driver/config/.schema/config.schema.json @@ -1298,7 +1298,8 @@ "title": "Password hashing algorithm", "description": "One of the values: argon2, bcrypt", "type": "string", - "default": "argon2" + "default": "argon2", + "enum": ["argon2", "bcrypt"] }, "argon2": { "title": "Configuration for the Argon2id hasher.", From 1355e019c7215aefb48c44ed5deac5b3f8c8e7d6 Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Fri, 26 Mar 2021 14:30:24 -0700 Subject: [PATCH 09/12] added bcrypt config into the schema --- driver/config/.schema/config.schema.json | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/driver/config/.schema/config.schema.json b/driver/config/.schema/config.schema.json index b09d14cd1716..679a203ddcf5 100644 --- a/driver/config/.schema/config.schema.json +++ b/driver/config/.schema/config.schema.json @@ -1327,6 +1327,20 @@ } }, "additionalProperties": false + }, + "bcrypt": { + "title": "Configuration for the Bcrypt hasher.", + "type": "object", + "additionalProperties": false, + "required": ["cost"], + "properties": { + "cost": { + "type": "integer", + "minimum": 12, + "maximum": 31, + "default": 12 + } + } } }, "additionalProperties": false From f2feb0f23c939875bc439345d156b53b4e1eaa15 Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Fri, 26 Mar 2021 14:33:44 -0700 Subject: [PATCH 10/12] format fix --- driver/config/config.go | 3 +-- hash/hash_comparator.go | 10 +++++----- hash/hasher_argon2.go | 1 + hash/hasher_bcrypt.go | 4 +++- selfservice/strategy/password/login.go | 3 ++- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/driver/config/config.go b/driver/config/config.go index 78d2b3d869d1..554f361f7968 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -114,7 +114,7 @@ type ( KeyLength uint32 `json:"key_length"` } Bcrypt struct { - Cost uint32 `json:"cost"` + Cost uint32 `json:"cost"` } SelfServiceHook struct { Name string `json:"hook"` @@ -242,7 +242,6 @@ func (p *Config) HasherArgon2() *Argon2 { } } - func (p *Config) HasherBcrypt() *Bcrypt { // warn about usage of default values and point to the docs // warning will require https://github.com/ory/viper/issues/19 diff --git a/hash/hash_comparator.go b/hash/hash_comparator.go index 3519876bef19..dfe129d1907a 100644 --- a/hash/hash_comparator.go +++ b/hash/hash_comparator.go @@ -5,12 +5,14 @@ import ( "crypto/subtle" "encoding/base64" "fmt" - "github.com/ory/kratos/driver/config" + "regexp" + "strings" + "github.com/pkg/errors" "golang.org/x/crypto/argon2" "golang.org/x/crypto/bcrypt" - "regexp" - "strings" + + "github.com/ory/kratos/driver/config" ) var ErrUnknownHashAlgorithm = errors.New("unknown hash algorithm") @@ -103,5 +105,3 @@ func decodeArgon2idHash(encodedHash string) (p *config.Argon2, salt, hash []byte return p, salt, hash, nil } - - diff --git a/hash/hasher_argon2.go b/hash/hasher_argon2.go index d6f86810eed9..88a40d9cd347 100644 --- a/hash/hasher_argon2.go +++ b/hash/hasher_argon2.go @@ -6,6 +6,7 @@ import ( "crypto/rand" "encoding/base64" "fmt" + "github.com/pkg/errors" "golang.org/x/crypto/argon2" diff --git a/hash/hasher_bcrypt.go b/hash/hasher_bcrypt.go index 758cecd4e6f0..39d025cc2fdd 100644 --- a/hash/hasher_bcrypt.go +++ b/hash/hasher_bcrypt.go @@ -2,9 +2,11 @@ package hash import ( "context" - "github.com/ory/kratos/driver/config" + "github.com/pkg/errors" "golang.org/x/crypto/bcrypt" + + "github.com/ory/kratos/driver/config" ) type Bcrypt struct { diff --git a/selfservice/strategy/password/login.go b/selfservice/strategy/password/login.go index 99ad66cc6340..fee57b5a4d2d 100644 --- a/selfservice/strategy/password/login.go +++ b/selfservice/strategy/password/login.go @@ -3,9 +3,10 @@ package password import ( "bytes" "encoding/json" - "github.com/ory/kratos/hash" "net/http" + "github.com/ory/kratos/hash" + "github.com/julienschmidt/httprouter" "github.com/pkg/errors" From 0a3d182c6f445adafb21f831424501b04d2b830a Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Tue, 30 Mar 2021 17:19:28 -0700 Subject: [PATCH 11/12] wrap error message --- hash/hasher_bcrypt.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hash/hasher_bcrypt.go b/hash/hasher_bcrypt.go index 39d025cc2fdd..d44e47a92753 100644 --- a/hash/hasher_bcrypt.go +++ b/hash/hasher_bcrypt.go @@ -2,8 +2,8 @@ package hash import ( "context" + "github.com/ory/kratos/schema" - "github.com/pkg/errors" "golang.org/x/crypto/bcrypt" "github.com/ory/kratos/driver/config" @@ -39,7 +39,10 @@ func validateBcryptPasswordLength(password []byte) error { // so if password is longer than 72 bytes, function returns an error // See https://en.wikipedia.org/wiki/Bcrypt#User_input if len(password) > 72 { - return errors.New("passwords are limited to a maximum length of 72 characters") + return schema.NewPasswordPolicyViolationError( + "#/password", + "passwords are limited to a maximum length of 72 characters", + ) } return nil } From ce92c7e5cb065b7b80d8ab4013a2c3ba18366447 Mon Sep 17 00:00:00 2001 From: Sergey Eremenko Date: Wed, 31 Mar 2021 10:52:35 -0700 Subject: [PATCH 12/12] make format --- hash/hasher_bcrypt.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hash/hasher_bcrypt.go b/hash/hasher_bcrypt.go index d44e47a92753..eb0625d2f70c 100644 --- a/hash/hasher_bcrypt.go +++ b/hash/hasher_bcrypt.go @@ -2,6 +2,7 @@ package hash import ( "context" + "github.com/ory/kratos/schema" "golang.org/x/crypto/bcrypt" @@ -42,7 +43,7 @@ func validateBcryptPasswordLength(password []byte) error { return schema.NewPasswordPolicyViolationError( "#/password", "passwords are limited to a maximum length of 72 characters", - ) + ) } return nil }