From 25e263394ebb42dd33edcc62623aa0801b5af23f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Fri, 5 Jun 2026 19:48:35 +0700 Subject: [PATCH] chore: update passwap v0.12.1 and align hash validation defaults/errors (#12179) # Which Problems Are Solved - Upgrading to `zitadel/passwap` v0.12.1 introduced new encoded-hash validation paths that still had review feedback open. - Secret hasher defaults were internally inconsistent (`Hasher.Cost: 4` vs `Limits.Bcrypt.MinCost: 10`), which could reject hashes created by the configured hasher. - New validation error IDs/messages and test coverage needed to be aligned with project conventions and expected behavior branches. # How the Problems Are Solved - Kept the dependency upgrade to `zitadel/passwap` v0.12.1 and completed the validation integration. - Updated `ValidateEncodedHash` error handling in `internal/crypto/passwap.go` to: - use unique random-style error IDs, - return `Errors.Hash.NotSupported` for no-verifier cases, - keep invalid-hash branches mapped to invalid argument errors. - Expanded `TestHasher_ValidateEncodedHash` in `internal/crypto/passwap_test.go` to cover and assert: - bounds error branch, - no-verifier branch, - generic invalid-hash branch, - expected ZITADEL error IDs/messages. - Restored lost inline verifier-context comments for argon2 and md5plain verifier entries. # Additional Changes - Added the missing explanatory `Limits` comment for `SecretHasher` in `cmd/defaults.yaml`. - Corrected `SecretHasher.Limits.Bcrypt.MinCost` from `10` to `4` to match the configured default bcrypt cost and avoid configuration footguns. # Additional Context - Follow-up for PR review feedback in https://github.com/zitadel/zitadel/pull/12179#pullrequestreview-4313121965 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Livio Spring <9405495+livio-a@users.noreply.github.com> --- cmd/defaults.yaml | 70 ++++- go.mod | 2 +- go.sum | 4 +- internal/api/grpc/gerrors/zitadel_errors.go | 1 - internal/command/crypto_test.go | 2 +- internal/command/main_test.go | 8 + internal/command/user_human.go | 12 +- internal/command/user_human_password.go | 5 + internal/command/user_human_test.go | 3 +- internal/command/user_v2_human.go | 4 +- internal/crypto/passwap.go | 285 +++++++++++++++----- internal/crypto/passwap_test.go | 100 ++++++- 12 files changed, 421 insertions(+), 75 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 02384119e..bae378979 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -859,7 +859,7 @@ SystemDefaults: # Supported algorithms: "argon2i", "argon2id", "bcrypt", "scrypt", "pbkdf2", "sha2" # Depending on the algorithm, different configuration options take effect. Algorithm: bcrypt # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_HASHER_ALGORITHM - # Cost takes effect for the algorithms bcrypt and scrypt + # Cost takes effect for bcrypt (work factor) and scrypt (LN, log2 of N) Cost: 14 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_HASHER_COST # Time takes effect for the algorithms argon2i and argon2id Time: 3 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_HASHER_TIME @@ -873,6 +873,39 @@ SystemDefaults: # Can be "sha1", "sha224", "sha256", "sha384" or "sha512" for pbkdf2 # Can be "sha256" or "sha512" for sha2 Hash: sha256 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_HASHER_HASH + # Limits bound cost parameters of imported or pre-encoded password hashes. + Limits: + Bcrypt: + MinCost: 10 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_BCRYPT_MINCOST + MaxCost: 16 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_BCRYPT_MAXCOST + Argon2: + MinTime: 1 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_ARGON2_MINTIME + MaxTime: 10 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_ARGON2_MAXTIME + MinMemory: 8192 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_ARGON2_MINMEMORY + MaxMemory: 524288 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_ARGON2_MAXMEMORY + MinThreads: 1 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_ARGON2_MINTHREADS + MaxThreads: 16 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_ARGON2_MAXTHREADS + Scrypt: + MinLN: 14 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SCRYPT_MINLN + MaxLN: 20 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SCRYPT_MAXLN + MinR: 8 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SCRYPT_MINR + MaxR: 32 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SCRYPT_MAXR + MinP: 1 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SCRYPT_MINP + MaxP: 16 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SCRYPT_MAXP + PBKDF2: + MinRounds: 1000 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_PBKDF2_MINROUNDS + MaxRounds: 10000000 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_PBKDF2_MAXROUNDS + Sha2: + MinSha256Rounds: 1000 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SHA2_MINSHA256ROUNDS + MaxSha256Rounds: 999999999 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SHA2_MAXSHA256ROUNDS + MinSha512Rounds: 1000 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SHA2_MINSHA512ROUNDS + MaxSha512Rounds: 999999999 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_SHA2_MAXSHA512ROUNDS + PHPass: + MinRounds: 7 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_PHPASS_MINROUNDS + MaxRounds: 31 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_PHPASS_MAXROUNDS + Drupal7: + MinIterations: 1000 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_DRUPAL7_MINITERATIONS + MaxIterations: 500000 # ZITADEL_SYSTEMDEFAULTS_PASSWORDHASHER_LIMITS_DRUPAL7_MAXITERATIONS # Verifiers enable the possibility of verifying # passwords that are previously hashed using another @@ -903,7 +936,7 @@ SystemDefaults: # Supported algorithms: "argon2i", "argon2id", "bcrypt", "scrypt", "pbkdf2" # Depending on the algorithm, different configuration options take effect. Algorithm: bcrypt # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_HASHER_ALGORITHM - # Cost takes effect for the algorithms bcrypt and scrypt + # Cost takes effect for bcrypt (work factor) and scrypt (LN, log2 of N) Cost: 4 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_HASHER_COST # Time takes effect for the algorithms argon2i and argon2id Time: 3 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_HASHER_TIME @@ -916,6 +949,39 @@ SystemDefaults: # Hash takes effect for the algorithm pbkdf2 # Can be "sha1", "sha224", "sha256", "sha384" or "sha512" Hash: sha256 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_HASHER_HASH + # Limits bound cost parameters of imported or pre-encoded secret hashes. + Limits: + Bcrypt: + MinCost: 4 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_BCRYPT_MINCOST + MaxCost: 16 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_BCRYPT_MAXCOST + Argon2: + MinTime: 1 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_ARGON2_MINTIME + MaxTime: 10 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_ARGON2_MAXTIME + MinMemory: 8192 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_ARGON2_MINMEMORY + MaxMemory: 524288 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_ARGON2_MAXMEMORY + MinThreads: 1 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_ARGON2_MINTHREADS + MaxThreads: 16 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_ARGON2_MAXTHREADS + Scrypt: + MinLN: 14 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SCRYPT_MINLN + MaxLN: 20 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SCRYPT_MAXLN + MinR: 8 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SCRYPT_MINR + MaxR: 32 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SCRYPT_MAXR + MinP: 1 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SCRYPT_MINP + MaxP: 16 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SCRYPT_MAXP + PBKDF2: + MinRounds: 1000 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_PBKDF2_MINROUNDS + MaxRounds: 10000000 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_PBKDF2_MAXROUNDS + Sha2: + MinSha256Rounds: 1000 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SHA2_MINSHA256ROUNDS + MaxSha256Rounds: 999999999 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SHA2_MAXSHA256ROUNDS + MinSha512Rounds: 1000 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SHA2_MINSHA512ROUNDS + MaxSha512Rounds: 999999999 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_SHA2_MAXSHA512ROUNDS + PHPass: + MinRounds: 7 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_PHPASS_MINROUNDS + MaxRounds: 31 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_PHPASS_MAXROUNDS + Drupal7: + MinIterations: 1000 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_DRUPAL7_MINITERATIONS + MaxIterations: 500000 # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_LIMITS_DRUPAL7_MAXITERATIONS Verifiers: # ZITADEL_SYSTEMDEFAULTS_SECRETHASHER_VERIFIERS Multifactors: OTP: diff --git a/go.mod b/go.mod index f2c5853ec..b66aaddfd 100644 --- a/go.mod +++ b/go.mod @@ -101,7 +101,7 @@ require ( github.com/zitadel/exifremove v0.1.0 github.com/zitadel/logging v0.7.0 github.com/zitadel/oidc/v3 v3.47.5 - github.com/zitadel/passwap v0.11.0 + github.com/zitadel/passwap v0.12.1 github.com/zitadel/saml v0.4.1 github.com/zitadel/schema v1.3.2 github.com/zitadel/sloggcp v0.6.0 diff --git a/go.sum b/go.sum index 4b2f191ae..baa58aa6a 100644 --- a/go.sum +++ b/go.sum @@ -841,8 +841,8 @@ github.com/zitadel/logging v0.7.0 h1:eugftwMM95Wgqwftsvj81isL0JK/hoScVqp/7iA2adQ github.com/zitadel/logging v0.7.0/go.mod h1:9A6h9feBF/3u0IhA4uffdzSDY7mBaf7RE78H5sFMINQ= github.com/zitadel/oidc/v3 v3.47.5 h1:cR2z0oqa5XZkwpXQiPCUGqKtndrjHgEXb81y3oXocK4= github.com/zitadel/oidc/v3 v3.47.5/go.mod h1:XxFh0666HRXycyrKmono+3gY0RACpYJLgy4r/+kliKY= -github.com/zitadel/passwap v0.11.0 h1:EL1A8dWz83xiJYru2326i+chkVKVR7YZq5Wep1GKQ3k= -github.com/zitadel/passwap v0.11.0/go.mod h1:Jn7sqawaxrwRL24byEJ1CVIAmznR6LsMsKcCfO/dIsg= +github.com/zitadel/passwap v0.12.1 h1:QAMccBfdQ2b0hRKXJ/qY4/kdddXkXqlDNXeVwEr2318= +github.com/zitadel/passwap v0.12.1/go.mod h1:atRozvPTTrwYUJdyxpxoxlIbI6Sr2O/ef/9URcn5EmY= github.com/zitadel/saml v0.4.1 h1:4PPOAvnUFcfwOFIAFZ3oGTMD+tKVeF96ySfGBf5j3IU= github.com/zitadel/saml v0.4.1/go.mod h1:ybs3e4tIWdYgSYBpuCsvf3T4FNDfbXYM+GPv5vIpHYk= github.com/zitadel/schema v1.3.2 h1:gfJvt7dOMfTmxzhscZ9KkapKo3Nei3B6cAxjav+lyjI= diff --git a/internal/api/grpc/gerrors/zitadel_errors.go b/internal/api/grpc/gerrors/zitadel_errors.go index 58a330fe9..5a8308606 100644 --- a/internal/api/grpc/gerrors/zitadel_errors.go +++ b/internal/api/grpc/gerrors/zitadel_errors.go @@ -155,7 +155,6 @@ func grpcStatusLevel(code codes.Code) slog.Level { } } - func getErrorInfo(ctx context.Context, id, key string, err error) protoadapt.MessageV1 { var wpe *commandErrors.WrongPasswordError var zerr *zerrors.ZitadelError diff --git a/internal/command/crypto_test.go b/internal/command/crypto_test.go index 33f9dc6c3..4b4e6ed75 100644 --- a/internal/command/crypto_test.go +++ b/internal/command/crypto_test.go @@ -262,7 +262,7 @@ func Test_newHashedSecretWithDefault(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { hasher := &crypto.Hasher{ - Swapper: passwap.NewSwapper(bcrypt.New(bcrypt.MinCost)), + Swapper: passwap.NewSwapper(bcrypt.New(bcrypt.DefaultMinCost, nil)), } defaultConfig := &crypto.GeneratorConfig{ Length: 32, diff --git a/internal/command/main_test.go b/internal/command/main_test.go index 784b20eee..d112160f9 100644 --- a/internal/command/main_test.go +++ b/internal/command/main_test.go @@ -293,6 +293,14 @@ func (h plainHasher) Hash(password string) (string, error) { return strings.Join([]string{"", "plain", h.x, password}, "$"), nil } +func (h plainHasher) Validate(encoded string) (verifier.Result, error) { + nodes := strings.Split(encoded, "$") + if len(nodes) != 4 || nodes[1] != "plain" { + return verifier.Skip, nil + } + return verifier.OK, nil +} + func (h plainHasher) Verify(encoded, password string) (verifier.Result, error) { nodes := strings.Split(encoded, "$") if len(nodes) != 4 || nodes[1] != "plain" { diff --git a/internal/command/user_human.go b/internal/command/user_human.go index 061cff7e9..6fc337496 100644 --- a/internal/command/user_human.go +++ b/internal/command/user_human.go @@ -119,8 +119,8 @@ func (h *AddHuman) Validate(hasher *crypto.Hasher) (err error) { } } if h.EncodedPasswordHash != "" { - if !hasher.EncodingSupported(h.EncodedPasswordHash) { - return zerrors.ThrowInvalidArgument(nil, "USER-JDk4t", "Errors.User.Password.NotSupported") + if err := hasher.ValidateEncodedHash(h.EncodedPasswordHash); err != nil { + return err } } return nil @@ -374,6 +374,9 @@ func addHumanCommandPassword(ctx context.Context, filter preparation.FilterToQue } if human.EncodedPasswordHash != "" { + if err := hasher.ValidateEncodedHash(human.EncodedPasswordHash); err != nil { + return err + } createCmd.AddPasswordData(human.EncodedPasswordHash, human.PasswordChangeRequired) } return nil @@ -569,6 +572,11 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain. return nil, nil, nil, err } } + if human.HashedPassword != "" { + if err := c.userPasswordHasher.ValidateEncodedHash(human.HashedPassword); err != nil { + return nil, nil, nil, err + } + } addedHuman = NewHumanWriteModel(human.AggregateID, orgID) diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index c4daafd3f..eee9ab981 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -228,6 +228,11 @@ func (c *Commands) setPasswordCommand(ctx context.Context, agg *eventstore.Aggre encodedPassword = newEncodedPassword } } + if encodedPassword != "" { + if err := c.userPasswordHasher.ValidateEncodedHash(encodedPassword); err != nil { + return nil, err + } + } // If password is provided, let's check if is compliant with the policy. // If only a encodedPassword is passed, we can skip this. if password != "" { diff --git a/internal/command/user_human_test.go b/internal/command/user_human_test.go index 75b91225b..dc889d810 100644 --- a/internal/command/user_human_test.go +++ b/internal/command/user_human_test.go @@ -9,6 +9,7 @@ import ( "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zitadel/passwap" "go.uber.org/mock/gomock" "golang.org/x/text/language" @@ -4007,7 +4008,7 @@ func TestAddHumanCommand(t *testing.T) { hasher: mockPasswordHasher("x"), }, want: Want{ - ValidationErr: zerrors.ThrowInvalidArgument(nil, "USER-JDk4t", "Errors.User.Password.NotSupported"), + ValidationErr: zerrors.ThrowInvalidArgument(passwap.ErrNoVerifier, "CRYPT-2xK7d", "Errors.Hash.NotSupported"), }, }, { diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index 6969bad9f..da69ebb52 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -80,8 +80,8 @@ func (h *ChangeHuman) Validate(hasher *crypto.Hasher) (err error) { func (p *Password) Validate(hasher *crypto.Hasher) error { if p.EncodedPasswordHash != "" { - if !hasher.EncodingSupported(p.EncodedPasswordHash) { - return zerrors.ThrowInvalidArgument(nil, "USER-oz74onzvqr", "Errors.User.Password.NotSupported") + if err := hasher.ValidateEncodedHash(p.EncodedPasswordHash); err != nil { + return err } } if p.Password == "" && p.EncodedPasswordHash == "" { diff --git a/internal/crypto/passwap.go b/internal/crypto/passwap.go index 66b5441f7..535059c9c 100644 --- a/internal/crypto/passwap.go +++ b/internal/crypto/passwap.go @@ -2,6 +2,7 @@ package crypto import ( "encoding/hex" + "errors" "fmt" "slices" "strings" @@ -44,6 +45,28 @@ func (h *Hasher) EncodingSupported(encodedHash string) bool { return false } +// ValidateEncodedHash checks that encoded is parseable by a configured verifier +// and that its cost parameters are within configured bounds. +// Use when accepting pre-encoded hashes from untrusted sources (import, create with hash). +// It intentionally returns password-scoped errors for compatibility with current callers. +func (h *Hasher) ValidateEncodedHash(encoded string) error { + if encoded == "" { + return nil + } + err := h.Validate(encoded) + if err == nil { + return nil + } + if errors.Is(err, passwap.ErrNoVerifier) { + return zerrors.ThrowInvalidArgument(err, "CRYPT-2xK7d", "Errors.Hash.NotSupported") + } + var bounds *verifier.BoundsError + if errors.As(err, &bounds) { + return zerrors.ThrowInvalidArgument(err, "CRYPT-5uV9n", "Errors.User.Password.Invalid") + } + return zerrors.ThrowInvalidArgument(err, "CRYPT-r8Qm2", "Errors.User.Password.Invalid") +} + type HashName string const ( @@ -76,6 +99,121 @@ const ( type HashConfig struct { Verifiers []HashName Hasher HasherConfig + Limits HashLimitsConfig +} + +type HashLimitsConfig struct { + Bcrypt BcryptLimitsConfig + Argon2 Argon2LimitsConfig + Scrypt ScryptLimitsConfig + PBKDF2 PBKDF2LimitsConfig + Sha2 Sha2LimitsConfig + PHPass PHPassLimitsConfig + Drupal7 Drupal7LimitsConfig +} + +type BcryptLimitsConfig struct { + MinCost int + MaxCost int +} + +func (l BcryptLimitsConfig) validationOpts() *bcrypt.ValidationOpts { + return &bcrypt.ValidationOpts{ + MinCost: l.MinCost, + MaxCost: l.MaxCost, + } +} + +type Argon2LimitsConfig struct { + MinTime uint32 + MaxTime uint32 + MinMemory uint32 + MaxMemory uint32 + MinThreads uint8 + MaxThreads uint8 +} + +func (l Argon2LimitsConfig) validationOpts() *argon2.ValidationOpts { + return &argon2.ValidationOpts{ + MinTime: l.MinTime, + MaxTime: l.MaxTime, + MinMemory: l.MinMemory, + MaxMemory: l.MaxMemory, + MinThreads: l.MinThreads, + MaxThreads: l.MaxThreads, + } +} + +type ScryptLimitsConfig struct { + MinLN int + MaxLN int + MinR int + MaxR int + MinP int + MaxP int +} + +func (l ScryptLimitsConfig) validationOpts() *scrypt.ValidationOpts { + return &scrypt.ValidationOpts{ + MinLN: l.MinLN, + MaxLN: l.MaxLN, + MinR: l.MinR, + MaxR: l.MaxR, + MinP: l.MinP, + MaxP: l.MaxP, + } +} + +type PBKDF2LimitsConfig struct { + MinRounds uint32 + MaxRounds uint32 +} + +func (l PBKDF2LimitsConfig) validationOpts() *pbkdf2.ValidationOpts { + return &pbkdf2.ValidationOpts{ + MinRounds: l.MinRounds, + MaxRounds: l.MaxRounds, + } +} + +type Sha2LimitsConfig struct { + MinSha256Rounds int + MaxSha256Rounds int + MinSha512Rounds int + MaxSha512Rounds int +} + +func (l Sha2LimitsConfig) validationOpts() *sha2.ValidationOpts { + return &sha2.ValidationOpts{ + MinSha256Rounds: l.MinSha256Rounds, + MaxSha256Rounds: l.MaxSha256Rounds, + MinSha512Rounds: l.MinSha512Rounds, + MaxSha512Rounds: l.MaxSha512Rounds, + } +} + +type PHPassLimitsConfig struct { + MinRounds int + MaxRounds int +} + +func (l PHPassLimitsConfig) validationOpts() *phpass.ValidationOpts { + return &phpass.ValidationOpts{ + MinRounds: l.MinRounds, + MaxRounds: l.MaxRounds, + } +} + +type Drupal7LimitsConfig struct { + MinIterations int + MaxIterations int +} + +func (l Drupal7LimitsConfig) validationOpts() *drupal7.ValidationOpts { + return &drupal7.ValidationOpts{ + MinIterations: l.MinIterations, + MaxIterations: l.MaxIterations, + } } func (c *HashConfig) NewHasher() (*Hasher, error) { @@ -83,7 +221,7 @@ func (c *HashConfig) NewHasher() (*Hasher, error) { if err != nil { return nil, zerrors.ThrowInvalidArgument(err, "CRYPT-sahW9", "password hash config invalid") } - hasher, hPrefixes, err := c.Hasher.buildHasher() + hasher, hPrefixes, err := c.Hasher.buildHasher(c.Limits) if err != nil { return nil, zerrors.ThrowInvalidArgument(err, "CRYPT-Que4r", "password hash config invalid") } @@ -99,49 +237,70 @@ type prefixVerifier struct { verifier verifier.Verifier } -// map HashNames to Verifier instances. -var knowVerifiers = map[HashName]prefixVerifier{ - HashNameArgon2: { - // only argon2i and argon2id are suppored. - // The Prefix constant also covers argon2d. - prefixes: []string{argon2.Prefix}, - verifier: argon2.Verifier, +type verifierFactory func(limits HashLimitsConfig) prefixVerifier + +var knowVerifiers = map[HashName]verifierFactory{ + HashNameArgon2: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + // verifier for both argon2i and argon2id. + prefixes: []string{argon2.Prefix}, + verifier: argon2.NewVerifier(l.Argon2.validationOpts()), + } }, - HashNameBcrypt: { - prefixes: []string{bcrypt.Prefix}, - verifier: bcrypt.Verifier, + HashNameBcrypt: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{bcrypt.Prefix}, + verifier: bcrypt.NewVerifier(l.Bcrypt.validationOpts()), + } }, - HashNameMd5: { - prefixes: []string{md5.Prefix}, - verifier: md5.Verifier, + HashNameMd5: func(_ HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{md5.Prefix}, + verifier: md5.NewVerifier(), + } }, - HashNameMd5Plain: { - prefixes: nil, // hex encoded without identifier or prefix - verifier: md5plain.Verifier, + HashNameMd5Plain: func(_ HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + // hex encoded without identifier or prefix. + prefixes: nil, + verifier: md5plain.NewVerifier(), + } }, - HashNameScrypt: { - prefixes: []string{scrypt.Prefix, scrypt.Prefix_Linux}, - verifier: scrypt.Verifier, + HashNameScrypt: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{scrypt.Prefix, scrypt.Prefix_Linux}, + verifier: scrypt.NewVerifier(l.Scrypt.validationOpts()), + } }, - HashNamePBKDF2: { - prefixes: []string{pbkdf2.Prefix}, - verifier: pbkdf2.Verifier, + HashNamePBKDF2: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{pbkdf2.Prefix}, + verifier: pbkdf2.NewVerifier(l.PBKDF2.validationOpts()), + } }, - HashNameMd5Salted: { - prefixes: []string{md5salted.Prefix}, - verifier: md5salted.Verifier, + HashNameMd5Salted: func(_ HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{md5salted.Prefix}, + verifier: md5salted.NewVerifier(), + } }, - HashNameSha2: { - prefixes: []string{sha2.Sha256Identifier, sha2.Sha512Identifier}, - verifier: sha2.Verifier, + HashNameSha2: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{sha2.Sha256Identifier, sha2.Sha512Identifier}, + verifier: sha2.NewVerifier(l.Sha2.validationOpts()), + } }, - HashNamePHPass: { - prefixes: []string{phpass.IdentifierP, phpass.IdentifierH}, - verifier: phpass.Verifier, + HashNamePHPass: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{phpass.IdentifierP, phpass.IdentifierH}, + verifier: phpass.NewVerifier(l.PHPass.validationOpts()), + } }, - HashNameDrupal7: { - prefixes: []string{drupal7.Identifier}, - verifier: drupal7.Verifier, + HashNameDrupal7: func(l HashLimitsConfig) prefixVerifier { + return prefixVerifier{ + prefixes: []string{drupal7.Identifier}, + verifier: drupal7.NewVerifier(l.Drupal7.validationOpts()), + } }, } @@ -149,10 +308,11 @@ func (c *HashConfig) buildVerifiers() (verifiers []verifier.Verifier, prefixes [ verifiers = make([]verifier.Verifier, len(c.Verifiers)) prefixes = make([]string, 0, len(c.Verifiers)+1) for i, name := range c.Verifiers { - v, ok := knowVerifiers[name] + factory, ok := knowVerifiers[name] if !ok { return nil, nil, fmt.Errorf("invalid verifier %q", name) } + v := factory(c.Limits) verifiers[i] = v.verifier prefixes = append(prefixes, v.prefixes...) } @@ -164,20 +324,20 @@ type HasherConfig struct { Params map[string]any `mapstructure:",remain"` } -func (c *HasherConfig) buildHasher() (hasher passwap.Hasher, prefixes []string, err error) { +func (c *HasherConfig) buildHasher(limits HashLimitsConfig) (hasher passwap.Hasher, prefixes []string, err error) { switch c.Algorithm { case HashNameArgon2i: - return c.argon2i() + return c.argon2i(limits) case HashNameArgon2id: - return c.argon2id() + return c.argon2id(limits) case HashNameBcrypt: - return c.bcrypt() + return c.bcrypt(limits) case HashNameScrypt: - return c.scrypt() + return c.scrypt(limits) case HashNamePBKDF2: - return c.pbkdf2() + return c.pbkdf2(limits) case HashNameSha2: - return c.sha2() + return c.sha2(limits) case "": return nil, nil, fmt.Errorf("missing hasher algorithm") case HashNameArgon2, HashNameMd5, HashNameMd5Plain, HashNameMd5Salted, HashNamePHPass, HashNameDrupal7: @@ -220,20 +380,20 @@ func (c *HasherConfig) argon2Params(p argon2.Params) (argon2.Params, error) { return p, nil } -func (c *HasherConfig) argon2i() (passwap.Hasher, []string, error) { +func (c *HasherConfig) argon2i(limits HashLimitsConfig) (passwap.Hasher, []string, error) { p, err := c.argon2Params(argon2.RecommendedIParams) if err != nil { return nil, nil, err } - return argon2.NewArgon2i(p), []string{argon2.Prefix}, nil + return argon2.NewArgon2i(p, limits.Argon2.validationOpts()), []string{argon2.Prefix}, nil } -func (c *HasherConfig) argon2id() (passwap.Hasher, []string, error) { +func (c *HasherConfig) argon2id(limits HashLimitsConfig) (passwap.Hasher, []string, error) { p, err := c.argon2Params(argon2.RecommendedIDParams) if err != nil { return nil, nil, err } - return argon2.NewArgon2id(p), []string{argon2.Prefix}, nil + return argon2.NewArgon2id(p, limits.Argon2.validationOpts()), []string{argon2.Prefix}, nil } func (c *HasherConfig) bcryptCost() (int, error) { @@ -246,12 +406,12 @@ func (c *HasherConfig) bcryptCost() (int, error) { return dst.Cost, nil } -func (c *HasherConfig) bcrypt() (passwap.Hasher, []string, error) { +func (c *HasherConfig) bcrypt(limits HashLimitsConfig) (passwap.Hasher, []string, error) { cost, err := c.bcryptCost() if err != nil { return nil, nil, err } - return bcrypt.New(cost), []string{bcrypt.Prefix}, nil + return bcrypt.New(cost, limits.Bcrypt.validationOpts()), []string{bcrypt.Prefix}, nil } func (c *HasherConfig) scryptParams() (scrypt.Params, error) { @@ -262,16 +422,16 @@ func (c *HasherConfig) scryptParams() (scrypt.Params, error) { return scrypt.Params{}, fmt.Errorf("decode scrypt params: %w", err) } p := scrypt.RecommendedParams // copy - p.N = 1 << dst.Cost + p.LN = dst.Cost return p, nil } -func (c *HasherConfig) scrypt() (passwap.Hasher, []string, error) { +func (c *HasherConfig) scrypt(limits HashLimitsConfig) (passwap.Hasher, []string, error) { p, err := c.scryptParams() if err != nil { return nil, nil, err } - return scrypt.New(p), []string{scrypt.Prefix, scrypt.Prefix_Linux}, nil + return scrypt.New(p, limits.Scrypt.validationOpts()), []string{scrypt.Prefix, scrypt.Prefix_Linux}, nil } func (c *HasherConfig) pbkdf2Params() (p pbkdf2.Params, _ HashMode, _ error) { @@ -298,23 +458,24 @@ func (c *HasherConfig) pbkdf2Params() (p pbkdf2.Params, _ HashMode, _ error) { return p, dst.Hash, nil } -func (c *HasherConfig) pbkdf2() (passwap.Hasher, []string, error) { +func (c *HasherConfig) pbkdf2(limits HashLimitsConfig) (passwap.Hasher, []string, error) { p, hash, err := c.pbkdf2Params() if err != nil { return nil, nil, err } + opts := limits.PBKDF2.validationOpts() prefix := []string{pbkdf2.Prefix} switch hash { case HashModeSHA1: - return pbkdf2.NewSHA1(p), prefix, nil + return pbkdf2.NewSHA1(p, opts), prefix, nil case HashModeSHA224: - return pbkdf2.NewSHA224(p), prefix, nil + return pbkdf2.NewSHA224(p, opts), prefix, nil case HashModeSHA256: - return pbkdf2.NewSHA256(p), prefix, nil + return pbkdf2.NewSHA256(p, opts), prefix, nil case HashModeSHA384: - return pbkdf2.NewSHA384(p), prefix, nil + return pbkdf2.NewSHA384(p, opts), prefix, nil case HashModeSHA512: - return pbkdf2.NewSHA512(p), prefix, nil + return pbkdf2.NewSHA512(p, opts), prefix, nil default: return nil, nil, fmt.Errorf("unsupported pbkdf2 hash mode: %s", hash) } @@ -346,14 +507,14 @@ func (c *HasherConfig) sha2Params() (use512 bool, rounds int, err error) { return use512, rounds, nil } -func (c *HasherConfig) sha2() (passwap.Hasher, []string, error) { +func (c *HasherConfig) sha2(limits HashLimitsConfig) (passwap.Hasher, []string, error) { use512, rounds, err := c.sha2Params() if err != nil { return nil, nil, err } + opts := limits.Sha2.validationOpts() if use512 { - return sha2.New512(rounds), []string{sha2.Sha256Identifier, sha2.Sha512Identifier}, nil - } else { - return sha2.New256(rounds), []string{sha2.Sha256Identifier, sha2.Sha512Identifier}, nil + return sha2.New512(rounds, opts), []string{sha2.Sha256Identifier, sha2.Sha512Identifier}, nil } + return sha2.New256(rounds, opts), []string{sha2.Sha256Identifier, sha2.Sha512Identifier}, nil } diff --git a/internal/crypto/passwap_test.go b/internal/crypto/passwap_test.go index 5fcc2df50..081518537 100644 --- a/internal/crypto/passwap_test.go +++ b/internal/crypto/passwap_test.go @@ -4,10 +4,12 @@ import ( "crypto/sha1" "crypto/sha256" "crypto/sha512" + "errors" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zitadel/passwap" "github.com/zitadel/passwap/argon2" "github.com/zitadel/passwap/bcrypt" "github.com/zitadel/passwap/drupal7" @@ -16,8 +18,25 @@ import ( "github.com/zitadel/passwap/pbkdf2" "github.com/zitadel/passwap/scrypt" "github.com/zitadel/passwap/sha2" + "github.com/zitadel/passwap/verifier" + + "github.com/zitadel/zitadel/internal/zerrors" ) +type skipHasher struct{} + +func (skipHasher) Hash(string) (string, error) { + return "", nil +} + +func (skipHasher) Validate(string) (verifier.Result, error) { + return verifier.Skip, nil +} + +func (skipHasher) Verify(string, string) (verifier.Result, error) { + return verifier.Skip, nil +} + func TestPasswordHasher_EncodingSupported(t *testing.T) { tests := []struct { name string @@ -460,6 +479,85 @@ func TestPasswordHashConfig_PasswordHasher(t *testing.T) { } } +func TestHasher_ValidateEncodedHash(t *testing.T) { + validateCfg := &HashConfig{ + Hasher: HasherConfig{ + Algorithm: HashNameBcrypt, + Params: map[string]any{ + "cost": 12, + }, + }, + Limits: HashLimitsConfig{ + Bcrypt: BcryptLimitsConfig{MinCost: 10, MaxCost: 16}, + }, + } + hasher, err := validateCfg.NewHasher() + require.NoError(t, err) + + valid, err := hasher.Hash("password") + require.NoError(t, err) + require.NoError(t, hasher.ValidateEncodedHash(valid)) + + lowCostCfg := &HashConfig{ + Hasher: HasherConfig{ + Algorithm: HashNameBcrypt, + Params: map[string]any{ + "cost": 4, + }, + }, + Limits: HashLimitsConfig{ + Bcrypt: BcryptLimitsConfig{MinCost: 4, MaxCost: 16}, + }, + } + lowCostHasher, err := lowCostCfg.NewHasher() + require.NoError(t, err) + outOfBounds, err := lowCostHasher.Hash("password") + require.NoError(t, err) + noVerifierHasher := &Hasher{Swapper: passwap.NewSwapper(skipHasher{})} + + tests := []struct { + name string + hasher *Hasher + encoded string + wantErr error + wantParent error + }{ + { + name: "bounds error", + hasher: hasher, + encoded: outOfBounds, + wantErr: zerrors.ThrowInvalidArgument(nil, "CRYPT-5uV9n", "Errors.User.Password.Invalid"), + wantParent: &verifier.BoundsError{}, + }, + { + name: "no verifier", + hasher: noVerifierHasher, + encoded: "$unknown$hash", + wantErr: zerrors.ThrowInvalidArgument(nil, "CRYPT-2xK7d", "Errors.Hash.NotSupported"), + wantParent: passwap.ErrNoVerifier, + }, + { + name: "invalid hash", + hasher: hasher, + encoded: "not-a-hash", + wantErr: zerrors.ThrowInvalidArgument(nil, "CRYPT-r8Qm2", "Errors.User.Password.Invalid"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.hasher.ValidateEncodedHash(tt.encoded) + require.Error(t, err) + assert.ErrorIs(t, err, tt.wantErr) + if tt.name == "bounds error" { + var bounds *verifier.BoundsError + assert.True(t, errors.As(err, &bounds)) + } else if tt.wantParent != nil { + assert.ErrorIs(t, err, tt.wantParent) + } + }) + } +} + func TestHasherConfig_decodeParams(t *testing.T) { type dst struct { A int @@ -666,7 +764,7 @@ func TestHasherConfig_scryptParams(t *testing.T) { }, }, want: scrypt.Params{ - N: 4, + LN: 2, R: 8, P: 1, KeyLen: 32,