V2.1.2 - No Password Upper Bound

ThunderSon commented 4 years ago

In V2.1.2, the password is mentioned to have a lower bound, but nothing related to the upper bounds. There should be a clear mention of an acceptable upper bound. In the Password Storage CS we discussed about this and provided 2 attacks that occurred because of the lack of a maximum length. There is no possible explanation to have passwords above 128 characters, where even security tokens do not even reach to that length, and those are used by bots and automation tools.

jmanico commented 4 years ago

NIST sp800-63 says 64 for minimum upper limit.

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On May 4, 2020, at 4:10 PM, ThunderSon notifications@github.com wrote:

In V2.1.2, the password is mentioned to have a lower bound, but nothing related to the upper bounds. There should be a clear mention of an acceptable upper bound. In the Password Storage CS we discussed about this and provided 2 attacks that occurred because of the lack of a maximum length. There is no possible explanation to have passwords above 128 characters, where even security tokens do not even reach to that length, and those are used by bots and automation tools.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ThunderSon commented 4 years ago

It doesn't mention though a maximum upper limit. Not setting a maximum upper limit risks setting the applications in trouble.

tghosth commented 4 years ago

I agree with @ThunderSon that this is something to consider

@jmanico do you see a problem with having a requirement for this even if NIST does not specifically mention it?

elarlang commented 4 years ago

For traceability - current V2.1.2:

V2.1.2 Verify that passwords 64 characters or longer are permitted.

With just setting upper limit we don't address the base problem why it's needed - upper limit must take away potential DoS attack vector.

Maybe the new requirement (or merged to current V2.1.2) should have an idea like (random pieces, no actual requirement): Verify that long passwords does not cause Denial-of-Service attacks / taking too much resources / have pre-hashing used, if needed.

jmanico commented 4 years ago

That’s a really good point. Older versions of Django used PBKDF2 to store passwords and they had no size limit. One malicious login with a large password and boom, server down.

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On May 19, 2020, at 3:33 AM, Elar Lang notifications@github.com wrote:

For traceability - current V2.1.2:

V2.1.2 Verify that passwords 64 characters or longer are permitted.

With just setting upper limit we don't address the base problem why it's needed - upper limit must take away potential DoS attack vector.

Maybe the new requirement (or merged to current V2.1.2) should have an idea like (random pieces, no actual requirement): Verify that long passwords does not cause Denial-of-Service attacks / taking too much resources / have pre-hashing used, if needed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

tghosth commented 4 years ago

@ThunderSon please can you draft a text for a new requirement along these lines and put it here. Is there a best practice you can refer to for this?

ThunderSon commented 4 years ago

Sure, I'll open a PR in a bit.

tghosth commented 4 years ago

I'd you do a PR, needs to be against master and 4.0.2 branch :)

ThunderSon commented 4 years ago

This requires 2 branches, as these are 2 separate bases for the commit. What do you think about this text?

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters).

I tried multiple variations to the text. Keeping it this simple came out to be the best option to me

It's not only DoS issues. There is the truncation issue as well. Who knows what comes out next. Requirements need to stay actionable and simple. As for guidance, my recommendation would be the password storage CS - maximum password lengths section. This CS is one of the best crafted guides for password storage.

elarlang commented 4 years ago

truncation

I would say, that truncation part is already covered by V2.1.3

V2.1.3 Verify that password truncation is not performed. However, consecutive multiple spaces may be replaced by a single space.

Upper bound - optional or required?

Does it makes sense to "force" upper bound or leave it as optional solution for situations when it's needed?

Let's take situation, when you don't use bcrypt, what is the reasoning to have upper-bound limit then?

In reality it's mostly theoretical problem - I don't think it's big chance that someone is using passwords longer than 128 symbols, but I want reasoning to be clear.

Basically, if I verify that some application is allowing users to use longer than 128 symbols passwords and long passwords does not cause any performance issues and does not have truncation, what is the risk they have? Why I should recommend them to use upper bound. Currently I can not see reasoning for "strong requirement" here.

ThunderSon commented 4 years ago

I am not worried about users. I am worried about attackers. That's exactly why this requirement is required, because no normal user will set a password that is in the 250 characters length. When an attacker sends a huge password in, are you telling me that no performance issue is going to arise? How about I run multiple threads with automatic IP and username update (that is doable, and not theoretical)?

elarlang commented 4 years ago

When an attacker sends a huge password in, are you telling me that no performance issue is going to arise?

I don't tell anything, I was asking reasoning. If there is security requirement, it must be clear what it solves and what attack vector it takes away.

And I'm reflecting it back with question - are you telling me IT IS causing performance issues and it does not depend what kind of hashing method is in use?

How about I run multiple threads with automatic IP and username update (that is doable, and not theoretical)?

This is already anti-automation topic and it is covered by other requirements. This is something what you need to have anyway, even with 20 symbol long passwords.

My question stays - if we take anti-automation away from your example (as it is separate requirement and it is already covered), what is the risk or problem to solve and is it enough to have strong requirement instead of optional "if needed" requirement?

ThunderSon commented 4 years ago

Hmmm I see your point of view. People forget usually that input restriction must be applied to the password field. Just like you'd limit a variable to allow for example a certain amount of characters, the password field needs to abide by that as well. It's really straight-forward and people just ignore it for some reason? All KDFs require processing. That's why they're used vs normal hashing. The stronger the KDF (fights off side-channel attacks, has more rounds, etc.), the more intensive it'll be (most of the times).

My attack vector is not solved by anti-automation. I am replicating a burst of users sending login requests. You can't block my attack. It needs to be handled on the code level. My attack changes the username, the IP, and can if need be update the user-agent string. How are you going to block me? You can't finger print the attack. If a CSRF token is added, I'll make it a 2 staged request and extract the received CSRF token and use it. As long as there is no strong identifier for the user (authenticated), an attacker will be able to bypass these mechanisms. Following this example, the web server unlocked multiple threads to handle the logins coming in, and each of them require for example 0.3 seconds. Let's say I increase the password size to 1MB, and KDFs require memory to run through the rounds, and will need more CPU resources, that'll slowly escalate to taking the server down with a dedicated attack team (Suddenly we need a WAF to block a DDoS attack happening, and might be late since the amount of processing happening is bigger than the normal threashold).

elarlang commented 4 years ago

dos part

So, it is pure DoS mitigation requirement then like I pointed out in my first comment.

I would say, your attack scenario is mostly anti-automation problem to solve - hashing resources is "helping" parameter in this attack. From attack(er) perspective, you can compensate "password upper-bound" with more resources, if you claim that it's not solvable by anti-automation. Do you solve this attack vector by setting max length 128 for password? I doubt. And it is still depending on what kind of hashing function you use.

You can not make your house safe against getting smashed by bulldozer by installing stronger lock for door.

upper-bound

To be clear - I'm not against upper-bound limit and I can not find any business need for more than 128 (for example) symbols long passwords. If it is "critical" application, authentication should not be done with asking (only) username-password anyway.

(edited) NIST actually mentions the max length also: https://pages.nist.gov/800-63-3/sp800-63b.html#a2-length

Extremely long passwords (perhaps megabytes in length) could conceivably require excessive processing time to hash, so it is reasonable to have some limit.

My question here was: Is "reasonable" with meaning "you need to have it" or "you may have it". I'm ok with both solutions. I got my info and answers to build my own risk and recommendations based on that.

tghosth commented 4 years ago

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters).

So I agree with the upper bounds limit and this seems like a nice clean requirement.

However, I also note the requirement from the cheatsheet:

In order to protect against both of these issues, a maximum password length should be enforced. This should be 64 characters for Bcrypt (due to limitations in the algorithm and implementations), and between 64 and 128 characters for other algorithms.

Do we need to take this Bcrypt issue into account in ASVS as well?

jmanico commented 4 years ago

64 is the NIST standard, I suggest we stick with it. 128 char passwords is not at all needed mathematically.

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On May 27, 2020, at 1:30 AM, Josh Grossman notifications@github.com wrote:

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters).

So I agree with the upper bounds limit and this seems like a nice clean requirement.

However, I also note the requirement from the cheatsheet:

In order to protect against both of these issues, a maximum password length should be enforced. This should be 64 characters for Bcrypt (due to limitations in the algorithm and implementations), and between 64 and 128 characters for other algorithms.

Do we need to take this Bcrypt issue into account in ASVS as well?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

elarlang commented 4 years ago

@jmanico, be careful with "64 symbols limit" and context here :) - "at least 64 symbols long" vs "maximum 64 symbols long"

5.1.1.2 Memorized Secret Verifiers URL: https://pages.nist.gov/800-63-3/sp800-63b.html#-5112-memorized-secret-verifiers

Verifiers SHALL require subscriber-chosen memorized secrets to be at least 8 characters in length. Verifiers SHOULD permit subscriber-chosen memorized secrets at least 64 characters in length.

A.2 Length URL: 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.

jmanico commented 4 years ago

I hear you. How subtle.

Mathematically passwords beyond 64 are just not needed at all and I don’t want to force developers to support it

So I suggest we state that passwords need to support 64 characters and not exceed that value. Suggesting “at least” 64 and a max of 128 is way overkill.

Grrrr I keep circling on this trying to avoid two separate requirements on this topic....

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On May 27, 2020, at 8:22 AM, Elar Lang notifications@github.com wrote:

@jmanico, be careful with "64 symbols limit" and context here :) - "at least 64 symbols long" vs "maximum 64 symbols long"

5.1.1.2 Memorized Secret Verifiers URL: https://pages.nist.gov/800-63-3/sp800-63b.html#-5112-memorized-secret-verifiers

Verifiers SHALL require subscriber-chosen memorized secrets to be at least 8 characters in length. Verifiers SHOULD permit subscriber-chosen memorized secrets at least 64 characters in length.

A.2 Length URL: 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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

tghosth commented 4 years ago

V2.1.2 Verify that passwords of up to 64 characters are allowed but no more than that to avoid resource overuse and other bugs.

Maybe we add a note in the 2.1 intro which explains why.

Any further comments?

elarlang commented 4 years ago

Is there any known other "other bug" than truncation?

tghosth commented 4 years ago

Maybe not, I guess we could say:

V2.1.2 Verify that passwords of up to 64 characters are allowed but no more than that to avoid resource overuse and the bcrypt truncation bug.

jmanico commented 4 years ago

64 is a low max now that I think of it.

Django's max is 4096 bytes....

Old school but a good read.

https://www.djangoproject.com/weblog/2013/sep/15/security/

On 6/2/20 3:10 PM, Josh Grossman wrote:

V2.1.2 Verify that passwords of up to 64 characters are allowed but no more than that to avoid resource overuse and other bugs.

Maybe we add a note in the 2.1 intro which explains why.

Any further comments?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/756#issuecomment-637750132, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMLDLJLYPTJH7R4XJLRUVFAXANCNFSM4MZBQ4JA.

-- Jim Manico Manicode Security https://www.manicode.com

tghosth commented 4 years ago

@jmanico so how to we handle the fact that bcrypt can only handle ~64 characters, advise pre-hashing with bcrypt?

jmanico commented 4 years ago

Bcrypt limit is 72 bytes. The advice is to pre-hash before using an adaptive hash. This is exactly how digital signatures solve performance problems of signing big files.

A good read: https://dropbox.tech/security/how-dropbox-securely-stores-your-passwords

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On Jun 4, 2020, at 2:19 AM, Josh Grossman notifications@github.com wrote:

@jmanico so how to we handle the fact that bcrypt can only handle ~64 characters, advise pre-hashing with bcrypt?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

tghosth commented 4 years ago

Well this has been a ride. We currently have v2.4.4 which mentions bcrypt. I think we should add the pre-hashing requirement to this requirement and leave the password length requirement simple. This would leave us with:

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters). V2.4.4 Verify that if bcrypt is used, the work factor SHOULD be as large as verification server performance will allow (typically at least 13) and passwords are pre-hashed to avoid truncation issues.

We can then add a note to 2.4.4 about the truncation issue.

Any further comments?

jmanico commented 4 years ago

Perhaps be more specific?

...and passwords are pre-hashed to avoid bcrypt's 72 byte limit.

On 6/10/20 1:13 PM, Josh Grossman wrote:

Well this has been a ride. We currently have v2.4.4 https://github.com/OWASP/ASVS/blob/master/4.0/en/0x11-V2-Authentication.md#v24-credential-storage-requirements which mentions bcrypt. I think we should add the pre-hashing requirement to this requirement and leave the password length requirement simple. This would leave us with:
V2.1.2 Verify that passwords allow for long passwords (e.g. 64
characters) *and have an upper bound limit (e.g. 128 characters)*.
V2.4.4 Verify that if bcrypt is used, the work factor SHOULD be as
large as verification server performance will allow (typically at
least 13) *and passwords are pre-hashed to avoid truncation issues*.
We can then add a note to 2.4.4 about the truncation issue.

Any further comments?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/756#issuecomment-642144272, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCNS7PGXIOYTST6T423RV65KHANCNFSM4MZBQ4JA.

-- Jim Manico Manicode Security https://www.manicode.com

elarlang commented 4 years ago

More fuel to the fire: do we need strong pre-hashing is must have or "if needed" requirement? ... "if longer than 64 symbols long passwords are accepted by the application"

tghosth commented 4 years ago

More fuel to the fire: do we need strong pre-hashing is must have or "if needed" requirement? ... "if longer than 64 symbols long passwords are accepted by the application"

I am a little worried it is getting too complicated.

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters). V2.4.4 Verify that if bcrypt is used, the work factor SHOULD be as large as verification server performance will allow (typically at least 13) and passwords are pre-hashed to avoid bcrypt's 72 byte limits.

@jmanico what do you think about this change now?

jmanico commented 4 years ago

Pre hashing is a must for performance of long long passwords hitting work factor based algorithms.

I like 2.1.2 and 2.4.4 below, they are specific and actionable even if a bit complicated.. but happy to discuss this more good sir.

Jim

On 6/12/20 8:49 AM, Josh Grossman wrote:

More fuel to the fire: do we need strong pre-hashing is must have
or "if needed" requirement?
... "if longer than 64 symbols long passwords are accepted by the
application"
I am a little worried it is getting too complicated.
V2.1.2 Verify that passwords allow for long passwords (e.g. 64
characters) and have an upper bound limit (e.g. 128 characters).
V2.4.4 Verify that if bcrypt is used, the work factor SHOULD be as
large as verification server performance will allow (typically at
least 13) and passwords are *pre-hashed to avoid bcrypt's 72 byte
limits*.
@jmanico https://github.com/jmanico what do you think about this change now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/756#issuecomment-643252728, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCO3YAYGDCD7EPCFA6TRWIP4TANCNFSM4MZBQ4JA.

tghosth commented 4 years ago

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters).

@jmanico This requirement requires an upper bound limit to be defined meaning that pre-hashing should not be necessary, no?

jmanico commented 4 years ago

Pre-hashing as needed above 72 bytes for bcrypts truncation and for long passwords in general....

-- Jim Manico @Manicode

On Jun 16, 2020, at 4:13 PM, Josh Grossman notifications@github.com wrote:

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters).

@jmanico This requirement requires an upper bound limit to be defined meaning that pre-hashing should not be necessary, no?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

tghosth commented 4 years ago

So @jmanico maybe forget the change to v2.4.4 and go for :

V2.1.2 Verify that passwords 50 characters or longer are permitted and that pre-hashing is used for passwords longer than 50 characters to avoid resource overuse and the bcrypt truncation bug.

Source of 50 characters is this: https://security.stackexchange.com/questions/39849/does-bcrypt-have-a-maximum-password-length

kingthorin commented 4 years ago

https://www.acunetix.com/vulnerabilities/web/long-password-denial-of-service/

By sending a very long password (1.000.000 characters) it's possible to cause a denial a service attack on the server. This may lead to the website becoming unavailable or unresponsive. Usually this problem is caused by a vulnerable password hashing implementation. When a long password is sent, the password hashing process will result in CPU and memory exhaustion.

Associated with CWE-400 and CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H

rbsec commented 4 years ago

Hi @tghosth,

I was one of the authors of the re-written password storage cheat sheet. We had a lot of discussion (and a certain amount of disagreement) about password length and pre-hashing, so I thought I'd weigh in with some of the reasoning behind it.

Bcrypt Maximum Lengths

The Stack Exchange thread you linked talks about the 50 character limit, but as per the second response, this "limit" seems to be largely based on a difference between the implementations and the original article about Bcrypt, and although it's commonly repeated, I wasn't able to find any real-world implementations that have this limit - they all seem to truncate at 71/72 bytes (the null at the end).

In order to give a bit of a safety margin (as characters don't necessarily equal bytes), and so that it's not necessarily obvious the site is using Bcrypt, we settled on 64 characters as a maximum length. This still gives about 420 bits of security (assuming 95 possible characters), which in practical terms is no different to the 473 bits that you'd get with the full 72 characters.

Pre-Hashing

Pre-hashing is an interesting solution, and one that I'm personally not a fan of for a number of reasons - mostly because it's easy to do wrong. There are a few different thing you need to consider, such as:

What algorithm do you use?
- If you use SHA-256 then you're actually reducing the security to 256 bits (compared to 420 with truncation)
- If you use SHA-512, the common hex output will be more than 72 chars, so you need to encode as base64 or something.
How do you encode the output? Null bytes could cause problems if they get passed, so you need to have some kind of encoding.
If an attacker has a leaked database that uses sha256($password) and you're using bcrypt(sha256($password)), they can use the leaked hashes to quickly identify people who are re-using passwords, and then focus on cracking the much weaker SHA-256 hashes instead of your strong Bcrypt ones.
Who actually benefits from pre-hashing rather than truncating at 64 characters?
- A random 64 character password is uncrackable, so there's no real security benefit going from 420 bits to 512 bits.
- A weak/predictable 128 character password (common phrase, repeated string, etc) isn't significantly stronger than a 64 character one.
- The vast, vast majority of users won't even use 20 characters, let alone 64.

All in all, pre-hashing adds complexity and creates far more opportunities for you to screw up the password hashing code compared to just truncating at 64 characters, without providing any tangible security befits. That's not to say that there are never cases where it might be appropriate, but for the majority of use-cases it's better just to truncate.

My thoughts on the key points are something like:

The maximum password length should be at least 64 characters
The maximum password length should not be more than 128 (256?) characters unless pre-hashing is used.
If pre-hashing is used, the initial algorithm should be at least 256 (512?) bits, and the initial hash must not be truncated
If Bcrypt is used, the maximum length should not be more than 71 characters (to account for the null byte).

These are simple, actionable points, and should be easy to both follow and check.

Hopefully this won't start too much of an argument - I know that @jmanico and I don't quite see eye-to-eye on this topic.

jmanico commented 4 years ago

I'm all for logical debates, keep going. :)

I dug into bcrypt pre-hashing, sha512 null bytes and more down the rabbit hole and can understand why pre-hashing is controversial.

At this point I want to suggest we no longer even recommend bcrypt and recommend scrypt, Argon2i and maybe even PBKDF2 and fully drop bcrypt as a suggested algorithm. It's not even the default in PHP anymore, Argon2i is. :)

Aloha, Jim

On 7/7/20 7:17 AM, rbsec wrote:

Hi @tghosth https://github.com/tghosth,

I was one of the authors of the re-written password storage cheat sheet. We had a lot of discussion (and a certain amount of disagreement) about password length and pre-hashing, so I thought I'd weigh in with some of the reasoning behind it.

Bcrypt Maximum Lengths

The Stack Exchange thread you linked talks about the 50 character limit, but as per the second response, this "limit" seems to be largely based on a difference between the implementations and the original article about Bcrypt, and although it's commonly repeated, I wasn't able to find any real-world implementations that have this limit - they all seem to truncate at 71/72 bytes (the null at the end).

In order to give a bit of a safety margin (as characters don't necessarily equal bytes), and so that it's not necessarily obvious the site is using Bcrypt, we settled on 64 characters as a maximum length. This still gives about 420 bits of security (assuming 95 possible characters), which in practical terms is no different to the 473 bits that you'd get with the full 72 characters.

Pre-Hashing

Pre-hashing is an interesting solution, and one that I'm personally not a fan of for a number of reasons - mostly because it's easy to do wrong. There are a few different thing you need to consider, such as:

What algorithm do you use? o If you use SHA-256 then you're actually reducing the security to 256 bits (compared to 420 with truncation) o If you use SHA-512, the common hex output will be more than 72 chars, so you need to encode as base64 or something.

How do you encode the output? Null bytes could cause problems if they get passed, so you need to have some kind of encoding.

If an attacker has a leaked database that uses |sha256($password)| and you're using |bcrypt(sha256($password))|, they can use the leaked hashes to quickly identify people who are re-using passwords, and then focus on cracking the much weaker SHA-256 hashes instead of your strong Bcrypt ones.

Who actually benefits from pre-hashing rather than truncating at 64 characters? o A random 64 character password is uncrackable, so there's no real security benefit going from 420 bits to 512 bits. o A weak/predictable 128 character password (common phrase, repeated string, etc) isn't significantly stronger than a 64 character one. o The vast, vast majority of users won't even use 20 characters, let alone 64.

All in all, pre-hashing adds complexity and creates far more opportunities for you to screw up the password hashing code compared to just truncating at 64 characters, without providing any tangible security befits. That's not to say that there are /never/ cases where it might be appropriate, but for the majority of use-cases it's better just to truncate.

My thoughts on the key points are something like:

The maximum password length should be at least 64 characters

The maximum password length should not be more than 128 (256?) characters /unless/ pre-hashing is used.

If pre-hashing is used, the initial algorithm should be at least 256 (512?) bits, and the initial hash must not be truncated

If Bcrypt is used, the maximum length should not be more than 71 characters (to account for the null byte).

These are simple, actionable points, and should be easy to both follow and check.

Hopefully this won't start too much of an argument - I know that @jmanico https://github.com/jmanico and I don't quite see eye-to-eye on this topic.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/756#issuecomment-655006612, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCKTOWJ7CXUFS3ZCNJ3R2NKDRANCNFSM4MZBQ4JA.

-- Jim Manico Manicode Security https://www.manicode.com

ThunderSon commented 4 years ago

Scrypt didn't live up properly (it failed frankly), and here is a tweet by the people that chose argon2 to be the next PHF and decided it based on it being the best KDF (doesn't mean it's the best PHF), and as such they don't see it to be the almighty best. https://twitter.com/TerahashCorp/status/1155129705034653698

I hope you understand that whenever we write a full answer, myself or @rbsec , we don't cut it short on any side. PBKDF2 is harder to configure than bcrypt, and isn't much better! Every offensive cryptographer we chat with tells us to just use bcrypt properly. My question as well, why argon2i and not argon2id? Argon2i has a vulnerability published and isn't GPU resistant attacks, unlike argon2d and argon2id. I am answering from my phone as well, so not the best quoted answer. Please, ASVS should recommend the firmest and most robust. Bcrypt does its job perfectly. Again, as Robin stated. Why do you need more than 64 characters? Add complexity for what?

jmanico commented 4 years ago

Whoa full stop.

Argon and scrypt are only less powerful than bcrypt if you set low work factors or low memory. Please read the tweet more carefully.

When suggesting ANY algorithm for password storage, you must recommend specific settings.

For scrypt, the memory use MUST be more than 32mb for it to be useful. For bcrypt, a work factor of 13 is minimum best practice per the hashcat team.

To fully discount scrypt and Argon2 is very dangerous and bad advice. PHP defaults to Argon2i.

bcrypt only consumes 16k or less ram for each run making it very parallelizable when confronting significant cracking rigs.

So again, cryptographers do NOT suggest only using bcrypt they suggest using bcrypt, scrypt, Argon2i or PBKDF2 - but only with the right configuration settings.

For example, bcrypt implementations often default to work factor 10 which is super weak today. bcrypt does NOT do the job perfectly if you do not configure it correctly - just like the other algorithms in this family. Again, if you "just use bcrypt" out of the box for most frameworks or languages, you will default to a weak work factor.

Jim

On 7/7/20 8:29 AM, ThunderSon wrote:

Scrypt didn't live up properly (it failed frankly), and here is a tweet by the people that chose argon2 to be the next PHF and decided it based on it being the best KDF (doesn't mean it's the best PHF), and as such they don't see it to be the almighty best. https://twitter.com/TerahashCorp/status/1155129705034653698

I hope you understand that whenever we write a full answer, myself or @rbsec https://github.com/rbsec , we don't cut it short on any side. PBKDF2 is harder to configure than bcrypt, and isn't much better! Every offensive cryptographer we chat with tells us to just use bcrypt properly. My question as well, why argon2i and not argon2id? Argon2i has a vulnerability published and isn't GPU resistant attacks, unlike argon2d and argon2id. I am answering from my phone as well, so not the best quoted answer. Please, ASVS should recommend the firmest and most robust. Bcrypt does its job perfectly. Again, as Robin stated. Why do you need more than 64 characters? Add complexity for what?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/ASVS/issues/756#issuecomment-655044638, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCPQJWXY3EEQ35FVQ6TR2NSSHANCNFSM4MZBQ4JA.

-- Jim Manico Manicode Security https://www.manicode.com

jmanico commented 3 years ago

The "only bcrypt advice" is flat out wrong, even if passed around by some legacy cryptographers.

Here are the strength categories we need to consider for password storage.

Class 1
    PBKDF2
Class 2
    BCRYPT
    ARGONX
Class 3
    SCRYPT
    ARGON2ID

Others might argue class-equivalence as it pertains to [b|s]crypt. The reason I don’t is more of a 15-20 page paper than an GitHub post. Scrypt’s configurable parameters address all known weaknesses in underlying primitives. They also resist all known HW-accelerated approaches to brute force and any partial result/lookup optimizations.

One -might- argue that scrypt is hard to wield (tune, configure) compared with bcrypt, and that scrypt is horror show for the defenders’ servers to execute compared to bcrypt – and those arguments I’ll accept. However, in terms of “attack resistance” of emitted cipher text, scrypt is at least on a par with bcrypt – and properly configured is significantly stronger now and in the future.

I believe it’s out of scope and highly dangerous for ASVS to split hairs on preferential implementation such as this. There’s a difference between verify the application:

(bad) Supplies only cryptographically-strong pseudo-random data for key generation; or
(much better) Stores passwords and other credentials by obfuscating them using a cryptographically analyzed and yet-unbroken adaptive one-way function, tuned to use a work factor that generates at least 300ms “proof of work” for normal users, and 1000ms PoW for administrative users.

And saying

Only developers who use BCRYPT are cool.

Think about it. Would ASVS say,

React is whack, use Angular only;
XML is so passe, use JSON; or

No, because why on earth. Preferences on HOW are opinion. Driving developers to a WHAT that’s inarguable industry best practice (like an adaptive one-way function), and providing objective SUCCESS CRITERIA (such as a PoW of 300ms) is the way to go as opposed to continuing this never-ending debate on bcrypt and prehashing and more.

jmanico commented 3 years ago

I am addressing the original poster's issues here https://github.com/OWASP/ASVS/commit/336c235d731d7a30bb4a482842a6dfb991889392 if you wish to address the many other issues in this mega-thread, please open a new issue

Sjord commented 3 years ago

@ThunderSon, I am researching the issue of long password DoS again, and I have a couple of questions perhaps you can help me with. In the initial comment you state:

provided 2 attacks that occurred because of the lack of a maximum length

What are those two attacks? Can you elaborate on that?

Furthermore, you said that you discussed this in the cheat-sheet team. The cheat sheet used to suggest a maximum of 1000 characters, but I can't find a maximum in the current version. Did I miss it? Did they remove the maximum? Do you have information on that?

Feel free to drop me an email if you want to take this outside of this GitHub issue.

jmanico commented 3 years ago

I am not sure where the 128 max came from and I’m happy to revisit it

The issue with max passwords I know of is from Django: https://www.djangoproject.com/weblog/2013/sep/15/security/

The change was made in ASVS but I’d also like to update the relevant CS to be in sync. If you care to create an issue in the CS series I’ll make that change quickly.

If this answer is not satisfactory you’re also welcome to email me via @.*** or text me directly or keep the conversation going here. I want to make sure your concerns are addressed.

On Jun 23, 2021, at 11:37 AM, Sjoerd Langkemper @.***> wrote:

@ThunderSon, I am researching the issue of long password DoS again, and I have a couple of questions perhaps you can help me with. In the initial comment you state:

provided 2 attacks that occurred because of the lack of a maximum length

What are those two attacks? Can you elaborate on that?

Furthermore, you said that you discussed this in the cheat-sheet team. The cheat sheet used to suggest a maximum of 1000 characters, but I can't find a maximum in the current version. Did I miss it? Did they remove the maximum? Do you have information on that?

Feel free to drop me an email if you want to take this outside of this GitHub issue.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

Sjord commented 3 years ago

The underlying problem with Django was that they had a bug in their PBKDF2 implementation, which hashes the password on every iteration instead of just once at the beginning. Once they solved that, they also removed the maximum length for passwords.

In a proper password hashing function, the password length is only important once, not on every iteration. Calculating a PBKDF2 hash of a 1 MiB password takes only a couple of milliseconds longer than a one character password on my machine.

Denial of service can still be possible, but I don't think it's as straightforward to say that long passwords reliably cause denial of service.

ThunderSon commented 3 years ago

It's correct as you say. If you extend the length by a certain amount it's still almost the same. It's important to profile CPU and memory as well, across several algorithms and their "parameters".

It was the call of "What is a sensible upper bound that we are looking to set?" After reviewing the current state, 128 was a strong recommendation. One might ask "Why not 256", then again, what is the improvement you're getting?

Feel free to change it if you can find a "sensible" value that makes sense and provides more security.

The whole concern on this came from:

Extremely large payloads -- Let's not forget that any input field should have a limit, passwords are not different.
Bcrypt limitation (we know it's not 128, it raised flags for us to think about)
Bad implementations, like the one Django had.

And as usual, closed issues should only be referenced, and not be a place for discussions. Please open a new ticket for this to give proper visibility to your concerns.

OWASP / ASVS