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.

[polarathene]

Legitimate users could use any length, so long as the client-side could pre-hash the secret it provides to the backend:

• Short answer
• Long answer

As for bcrypt with the 72 bytes limit, you can still have what looks like a short password, 👩‍👩‍👧‍👦👩‍👩‍👧‍👦👩‍👩‍👧‍👦🕵🏼‍♀️ (92 bytes) and 👩‍👩‍👧‍👦👩‍👩‍👧‍👦👩‍👩‍👧‍👦❤️ (81 bytes) but will verify for the same bcrypt hash (since the repeated family emoji at the start contribute 25 bytes each in length).

1024 bytes is plenty as a limit in that sense 👍

Putting a service like Caddy in front to limit abuse (which the pre-hash approach wouldn't help with obviously) is also fairly simple to configure.

---

so someone who would upgrade without breaking users would need to collect these data for some time before upgrading (without collecting the passwords obviously).

They could just contact an admin or perform a password reset? At that length it's only likely to affect someone who misunderstands security in the first place.

I guess that brute-force attacks are not effective for passwords longer then 256, not to mention 1024.

It's possible to attack depending on context. Length does not equate to secure. Entropy calculated based on Kerckhoff's Principle does establish actual security strength though.

---

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 :)

With pre-hashing you can pretty much have a fixed length compressed to 64 chars or less, regardless of input length. That's still more bits than you'd get from the bcrypt hash.

Collisions for SHA-256 start to occur around 128-bit IIRC, which isn't all that feasible considering the amount of energy that'd require to.

---

Additional info (password length vs entropy)

A password like detailed snail summons slim lab coat can be fairly strong (when paired with a trustworthy backend, otherwise aim for more entropy).

Kerckhoffs's Principle is what should be considered for security strength, not length. Assume the attacker knows the minimum entropy of your password generation method. In the prior example that could be to repeat the 3 family emoji followed by a specific emoji for a distinct password.

If targeting specifically 4 random emoji from Unicode 13.1 (log2(3521^4) == ~47 bits) it is similar to a 62 value charset ([A-Z, a-z, 0-9]) at double the length (log2(62^8) == ~47 bits). That amount of entropy can be fairly effective after being processed via a KDF to augment the entropy.

detailed snail summons slim lab coat is a little stronger at 48 bits of entropy (via getapasshrase.com generator). Bcrypt with a work factor of 10 would take an Nvidia RTX 4080 over 1,000 years to attack that. Thus the cost of such an attack needs to be worthwhile.

The real entropy of the above pattern shown however is very low (as opposed to that passphrase), although it may not be as easily guessed via typical brute-force patterns (unlikely even tried for emoji... but this is not an endorsement, avoid using emoji as passwords, there are numerous caveats that you could run into with compatibility).

All this to say that length shouldn't be a big concern. Passphrases are great when done right, and those are very unlikely to reach such lengths. Anyone using excessive lengths isn't taking the right approach as the actual entropy is what matters, while the final hash output itself is where any additional entropy of a password is wasted.

For reference, roughly 114-bits of entropy IIRC is sufficient to require enough energy to boil all the oceans on earth. If an attacker wants your data that badly they'll find a more affordable / realistic alternative to gain access. Encourage adoption of password generators and users don't have to worry much at all :)

[aeneasr]

Does this affect only argon2? I checked the code and BCrypt does limit the password length to 72 characters:

kratos/hash/hasher_bcrypt.go

Line 60 in 7674f46

if len(password) > 72 {

We really don't recommend using argon2 any more because it is too complex to tune correctly.

[polarathene]

Does this affect only argon2?

What makes you think that? No one mentioned that here.

Some want to see a byte limit to prevent malicious payloads. 72 bytes would not fit the 4 emoji sequence I shared though.

---

We really don't recommend using argon2 any more because it is too complex to tune correctly.

It's not that complex IIRC? You have two tunables for memory and cpu resources instead of just cpu.

In both cases you adjust the those for interactive login time (eg 250ms). You want argon2id with a reasonably amount of memory difficulty to reduce parallelism for an attacker.

Presently GPUs last I knew still don't have enough local memory per core to properly accel bcrypt, so FPGA is more affordable hardware choice for attacking bcrypt. Memory is expensive, argon2id shines at that.

[aeneasr]

I don‘t understand to be honest. There is a hard limit in our BCrypt implementation that limits the maximum length to 72 characters. So why do we need another configuration that limits it to some number when the implementation in any case errors for anything larger than 72?

This limit does not exist with Argon2id iirc, hence my assumption.

[polarathene]

So why do we need another configuration that limits it to some number when the implementation in any case errors for anything larger than 72?

Ok I just checked to find Kratos only has support for bcrypt and argon2id as password options, but that may not always be the case... a new one could come along? (EDIT: I'm trying to reason here about a length limit in general, but this is more to do with request body size as described below)

This limit does not exist with Argon2id iirc, hence my assumption.

The concern that users above were raising about length AFAIK wasn't specific to an implementation like bcrypt with 72 bytes.

It was the request itself could be provided with an absurd payload, say the password is sent with a size 2x or more than the servers available memory.... does your bcrypt 72 bytes check run prior to the server receiving that payload?

When the server receives a request with an excessive input, it's buffering that all into memory until it finishes receiving the request body, only then would your length check be run right? However if there isn't enough memory in the first place, Kratos or something else is likely to OOM yeah?

I haven't tried to verify if this affects Kratos. Just clarifying the earlier concern expressed.

---

I'm not familiar with Go or Kratos, so I can't really review the PR.

At a glance I don't see anything that actually addresses the OOM / DDoS attack concern, other than attempts to restrict the input length before and after the request itself via frontend and backend, but I don't see a limit set on the request body size for the endpoint?

This doesn't bother me since a reverse proxy can manage the max body size, but you need to know to do that since Kratos is not actively preventing it? (you also advised handling this via a reverse proxy in 2022)

For reference axum is quite popular in the Rust world for building a web service with, and it defaults to 2MB body size limit as a security measure.