feat: limit maximum password length

[mmeller-wikia]

Allow to specify max password length. See #2449

BREAKING CHANGES:
Passwords longer then `max_password_length` will not be recognizable. Currently default is set to 1024 characters
Workaround: users could use password recovery to set new password.

Related issue(s)

This change resolve issue mentioned in #2449

Go one step closer to ideal solution mentioned in #2396

add minlength and maxlength attributes to password inputs (when setting a new password)

Checklist

• I have read the contributing guidelines.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  security@ory.sh) from the maintainers to push
  the changes.
• I have added or changed the documentation.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have referenced an issue containing the design document if my change
  introduces a new feature.

Further Comments

[mmeller-wikia]

TODO: discuss default values
Should it be limited to max 72 characters to account for bcrypt hasher limitations?

kratos/hash/hasher_bcrypt.go

Lines 57 to 65 in 9710549

	// 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 schema.NewPasswordPolicyViolationError(
	"#/password",
	text.NewErrorValidationPasswordMaxLength(72, len(password)),
	)
	}

[mmeller-wikia]

TODO: update Breaking changes section with new value

[mmeller-wikia]

Hi @aeneasr 👋
I need some help to stabilise tests
They are failing with: (https://github.com/ory/kratos/actions/runs/8063188848/job/22024569688?pr=3781)

    --- FAIL: TestHandler/case=should_list_all_identities_with_credentials (0.01s)
        handler_test.go:1352: 
            	Error Trace:	/home/runner/work/kratos/kratos/identity/handler_test.go:1352
            	Error:      	Should be true
            	Test:       	TestHandler/case=should_list_all_identities_with_credentials
            	Messages:   	
FAIL

I believe my changes are not related to case=should list all identities with credentials test (at least to /identities API)
Do you have any ideas why this test is failing?

[zepatrik]

From NIST https://pages.nist.gov/800-63-3/sp800-63b.html#a2-length

Users should be encouraged to make their passwords as lengthy as they want, within reason. Since the size of a hashed password is independent of its length, there is no reason not to permit the use of lengthy passwords (or pass phrases) if the user wishes. Extremely long passwords (perhaps megabytes in length) could conceivably require excessive processing time to hash, so it is reasonable to have some limit.

I think the default should be at least 1024, probably even more. The bcrypt hasher already enforces the correct length, so I don't think there is any reason to account for that.

Setting the default to e.g. 2MB also makes it very unlikely that anyone uses a password of this lenght, and basically avoids the breaking change.

[aeneasr]

Sorry if this sounds dumb but what exactly is the use case for limiting password length? BCrypt will cut off passwords longer than 72 chars (so it does not matter what comes after the 72nd character).

[mmeller-wikia]

I think the default should be at least 1024, probably even more. The bcrypt hasher already enforces the correct length, so I don't think there is any reason to account for that.

Setting the default to e.g. 2MB also makes it very unlikely that anyone uses a password of this lenght, and basically avoids the breaking change.

I totally agree, that we should allow long passwords by default 👍
But I guess that brute-force attacks are not effective for passwords longer then 256, not to mention 1024.
I think at some point it is more relevant to account for hash collisions instead of guessing exact password phrase - in the end hashes are limited and collisions had to appear eventually :)

[mmeller-wikia]

Sorry if this sounds dumb but what exactly is the use case for limiting password length? BCrypt will cut off passwords longer than 72 chars (so it does not matter what comes after the 72nd character).

It is a fix for issue discussed in #2449
Currently one had to use nginx (or any other proxy) to avoid DDoS attacks.
Current option couples max password length with payload size (traits, cookies, headers etc.) - the longer payload is the shorted password is allowed, because of that it is impossible to communicate clear password policy

[mmeller-wikia]

@zepatrik @aeneasr
Don't want to bother you, but is there any update on this? :)

[zepatrik]

Looks good to me now 👍
The default of 1024 is a breaking change, but I'd guess no-one is really affected by this in the end. It is currently not possible to say how long passwords are, so someone who would upgrade without breaking users would need to collect these data for some time before upgrading (without collecting the passwords obviously).

Another idea to completely avoid a breaking change would be to ignore the limit if it is not set, but as explained I think a default is fine.