lemmy-ui icon indicating copy to clipboard operation
lemmy-ui copied to clipboard

Throw error if password is too long

Open Tealk opened this issue 1 year ago • 8 comments

Hello all,

I've read here that the password is limited to 60 show and the UI shortens the password if a longer one is entered, I think this is an absurdity. So the user does not even know his correct password, besides there is no reason to limit passwords.

Tealk avatar Jun 08 '23 07:06 Tealk

Passwords need to have a maximum length, otherwise an attacker can send passwords with 10k chars and overload the server with hashing. However too long passwords should probably throw an error instead of quietly truncating. Code is here: https://github.com/LemmyNet/lemmy/blob/main/crates/api_common/src/utils.rs#L150

Nutomic avatar Jun 08 '23 08:06 Nutomic

I became aware of the issue through this post: https://feddit.de/comment/130590

Tealk avatar Jun 08 '23 11:06 Tealk

Passwords need to have a maximum length, otherwise an attacker can send passwords with 10k chars and overload the server with hashing

Please reconsider this decision. A single thread on my old Ryzen 3 CPU can already process 3 Gbit/s of data when creating a md5 hash. On a server with a bandwidth of 1 Gbit/s the hashing will never be the bottleneck. An attacker would not be able to send more data then the server CPU can process.

Shortening passwords is a security thread and leads to obscure errors as we can see in this issue thread.

The easiest and most secure way to fix this bug, is to get rid of the password length constraint. Just remove the "60" from line 308.

Thank you in advance and thanks to all the Lemmy maintainers. Keep up the good work :heart:

JanUlrich avatar Jun 08 '23 14:06 JanUlrich

Shortening passwords is a security thread and leads to obscure errors as we can see in this issue thread.

This is not true. A randomly generated password of 20 characters or more, utilizing capital and lower case letters, numbers, and symbols, will take more time to crack than the current age of the universe. (it's actually better than that - I think something like 14 characters is sufficient, but I always advise people to just round up to 20)

novakeith avatar Jun 08 '23 15:06 novakeith

We already do throw an error. https://github.com/LemmyNet/lemmy/blob/15c84e2f7b5c82342d429547b060f848ba3962f2/crates/api_common/src/utils.rs#L307

Maybe this should be a lemmy-ui issue, to allow really long passwords in the input field rather than truncate them, so that the back end can correctly throw an error?

dessalines avatar Jun 08 '23 15:06 dessalines

The comments above about even 20 characters being more than enough, are correct, so there's no reason for us to increase this any more.

https://bitwarden.com/password-strength/

https://www.security.org/how-secure-is-my-password/

dessalines avatar Jun 08 '23 16:06 dessalines

I have faced this problem myself, however: what @dessalines said earlier about throwing an error does not happen when you configure an admin password via config.hjson file (to my knowledge, at least). I automated the setup via Ansible with Traefik as reverse proxy, upon trying to log in my password was 'wrong'.

When I went into dev tools I noticed that the password sent via the web socket was only 60 characters. I did not notice any error during setup, the password was just being cut off and I didn't know about that.

In my opinion 60 characters is too few, especially nowadays when a lot of people use password managers. I'm not saying there should not be a limit, but all my passwords in my password manager are >60 characters...

Jappie3 avatar Jun 14 '23 11:06 Jappie3

There is absolutely no security benefit to having passwords longer than 60 chars. Still there should be proper error handling.

Nutomic avatar Jun 14 '23 14:06 Nutomic

no security benefit to having passwords longer than 60 chars

~~I hope i can't read from this you aren't even hashing?~~ Edit: Reading the whole thread helps.

Sorry but why be this opinionated? As far as i can see there is no reason (and there never should be) to set such a small limit. (Even regarding hash functions, 60 vs something like 200 chars is not going to matter on a big scale)

Most password managers allow creating 128+ character passwords by default. I don't see an issue storing even 200+ characters just for the sake of it.

karpfediem avatar Jun 21 '23 21:06 karpfediem

If you have a lowercase alphanumeric password with 60 characters, there are 36 possible characters and 36^60 total possible passwords. TAssuming every single variant takes an attacker 0.001 seconds to try, then it will take more than 10^82 years to try every possible password. Whats the point in using a longer one? Just because its possible doesnt mean it makes any sense.

Nutomic avatar Jun 22 '23 08:06 Nutomic

Technology is constantly evolving, I believe it won't be long before 60 digit passwords will be an ease for any attack.

Therefore, I would simply not limit it and leave the choice to each user.

Tealk avatar Jun 22 '23 08:06 Tealk

Passwords need to have a maximum length, otherwise an attacker can send passwords with 10k chars and overload the server with hashing. However too long passwords should probably throw an error instead of quietly truncating. Code is here: https://github.com/LemmyNet/lemmy/blob/main/crates/api_common/src/utils.rs#L150

This isn't as much of an issue as you think, password hashes (or at least those that are correctly implemented, there was an issue like this for PBKDF2 in old versions of OpenSSL) only run a fast hash on the password once, then do subsequent iterations on the result of the previous iteration. In almost all cases an attacker will saturate your bandwidth before they saturate the blowfish hashing throughput.

The comments above about even 20 characters being more than enough, are correct, so there's no reason for us to increase this any more.

https://bitwarden.com/password-strength/

https://www.security.org/how-secure-is-my-password/

20 characters is ok but not ideal. For random passwords with dense entropy it's fine (20 characters of fully random base64 has 120 bits of entropy), but for diceware passwords it could be only ~51 bits of entropy (assuming words with 5 characters each, 4 words with log2(7776) bits of entropy each). That's still sufficient for most cases given that bcrypt is used, but it's certainly not crazy to have a limit above 20.

Technology is constantly evolving, I believe it won't be long before 60 digit passwords will be an ease for any attack.

Therefore, I would simply not limit it and leave the choice to each user.

This only makes sense if you don't understand how passwords are stored. Lemmy currently uses bcrypt, which outputs a 184 bit result. Any password containing more than 184 bits of entropy will necessarily be limited to 184 bits of entropy due to the hash output, so your 60 random characters of base64 with 360 bits of entropy don't help you beyond what 31 random characters of base64 would accomplish (and regardless, encryption keys commonly use 128 bits of entropy and aren't going to be cracked any time soon).


Now the important part: 60 is fine, but if the limit is increased it should only be increased to 72 due to bcrypt's 72 character limit. Silent truncation to 72 characters is significantly worse for security than throwing an error for too-long passwords (and to be fair to the maintainer of Rust's bcrypt crate, they understand the issue but decided it's more important to maintain compatibility with other implementations). If you want to allow passwords longer than 72 characters, you could either use Argon2 or prehash the password before sending it to bcrypt, the common way to do this is to SHA256 hash it and then base64 encode it before bcrypt hashing the result (otherwise null bytes in the SHA256 output will truncate the password, which is super bad because many passwords will end up using only a few bytes of entropy).

AndrolGenhald avatar Jun 22 '23 14:06 AndrolGenhald

60 is fine, but if the limit is increased it should only be increased to 72 due to bcrypt's 72 character limit.

It's not quite that simple, since bcrypt's limit is 72 bytes and a character can span up to 4 bytes.

alexcormier avatar Sep 30 '23 16:09 alexcormier