Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.1.2 Verify that passwords of at least 64 characters are permitted #1772

Closed
appills opened this issue Oct 26, 2023 · 18 comments
Closed

2.1.2 Verify that passwords of at least 64 characters are permitted #1772

appills opened this issue Oct 26, 2023 · 18 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 2) Awaiting response Awaiting a response from the original poster V2 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@appills
Copy link

appills commented Oct 26, 2023

2.1.2

Needs to be clarified to consider character encoding - 64 characters has loose interpretation. For example, this symbol 𐍈 is 4 bytes in UTF-8 and can bypass certain string length functions that aren't multi-byte encoding aware (e.g. php's strlen() vs mb_strlen()). This is important because cryptography acts on bytes & is not character encoding aware. This means that 64 𐍈 symbols would be operating on more bytes than 128 "A"s. because 256 > 128. Note that this is different from the concerns in [756] (#756) because it does not reference the character encoding.

I think the verbiage should be something along the lines of:
Verify that passwords of at least 64 bytes are permitted after appropriate server-side character encoding canonicalization.

So that it does not contradict the "printable Unicode character, including emojis" requirement in 2.1.4

@elarlang elarlang added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V2 labels Oct 26, 2023
@elarlang
Copy link
Collaborator

elarlang commented Oct 26, 2023

Current requirement:

# Description L1 L2 L3 CWE NIST §
2.1.2 Verify that passwords of at least 64 characters are permitted, and that passwords of more than 128 characters are denied. (C6) 521 5.1.1.2

For me the section V2.1 is talking about password, or "Memorized Secrets" like NIST calling it, and this is the piece of text what user is using as a secret and entering to the form. Users are entering characters (from user point of view), not bytes. We are not talking about crypto in this chapter. From this point of view, correct way to measure password length is mb_strlen - "logical characters" for human, not bytes for computer.

The issue here describes storing the password, and those requirements are in chapter https://github.com/OWASP/ASVS/blob/master/5.0/en/0x11-V2-Authentication.md#v24-credential-storage

@elarlang
Copy link
Collaborator

Thinking more about it - the actual need for requirement V2.1.2 comes from password storage and the opened issue makes sense from this point of view.

If one want to store 4-byte characters as password, then bcrypt can handle only 18 characters (72 bytes).

@elarlang
Copy link
Collaborator

ping @jmanico @tghosth

@tghosth
Copy link
Collaborator

tghosth commented Oct 31, 2023

yeah I would particularly like @jmanico's thoughts on this...

@tghosth tghosth added the _5.0 - prep This needs to be addressed to prepare 5.0 label Oct 31, 2023
@jmanico
Copy link
Member

jmanico commented Oct 31, 2023

NIST language (and the typical classes that developers use with passwords) are all character-based (ie: the minimal length for passwords per NIST is 8 characters.). I know this is something devs may run into, but I suggest we drop it. Most of the abstractions to these password storage algorithms will handle this under the hood.

@jmanico
Copy link
Member

jmanico commented Oct 31, 2023

Here is what ChatGPT is saying on the topic. Summary, give both a character AND byte limit?

When it comes to password storage, Argon2id is a state-of-the-art key derivation function that is designed to be secure against a wide range of attacks, including timing-based side-channel attacks and offline brute-force attacks. However, the strength of the stored password hash is only as good as the underlying password itself.

When recommending password length, it's important to consider both the computational aspects (which Argon2id takes care of very well) and the human factors (ease of memorization and input). Generally, recommendations are given in terms of character length mainly for the latter reason, as it's easier for end-users to understand. The character length indirectly relates to the byte size, especially if you're using a character encoding where each character corresponds to a fixed number of bytes, such as UTF-8 for ASCII characters.

That being said, it's essential to understand that not all characters are equal in byte size when you move into multi-byte characters such as those found in UTF-8 encoding for non-ASCII characters. If your system is internationalized and allows for a wide range of Unicode characters in passwords, then focusing purely on character count could be misleading. In that context, byte size could be a more accurate measure of what you're actually storing and processing. However, moving to byte size might make the policy harder for end-users to understand, as they usually think in terms of characters, not bytes.

Technically speaking, Argon2id takes an arbitrary byte array as input, so it can handle both ASCII and Unicode without any issues. However, the processing time for Argon2id is more dependent on the computational parameters you set (time cost, memory cost, parallelism) than on the byte size of the input. Thus, from the algorithm's perspective, whether you use character length or byte size as a minimum requirement is not particularly critical.

To harmonize these factors, one approach could be to set a minimum character length (say, 8 characters, in line with NIST 800-63B) and then also have a byte-size check, especially if you're allowing a wide array of Unicode characters. This approach ensures a certain level of complexity from a human perspective (the character length) while also ensuring that you are accounting for the byte size, which could be variable due to multi-byte characters.

In summary, while character size is easier for humans to understand and is often used as a metric, byte size can be a more accurate representation of the "weight" of a password in an internationalized system. However, given that Argon2id's security parameters are far more dependent on its computational settings rather than the input length, the primary concern should be making a policy that encourages strong but memorable passwords from a user standpoint.

@elarlang
Copy link
Collaborator

The requirement points to NIST 5.1.1.2, but I can not see with quick review anything there to support the requirement.

https://pages.nist.gov/800-63-3/sp800-63b.html#-5112-memorized-secret-verifiers

@jmanico
Copy link
Member

jmanico commented Oct 31, 2023 via email

@elarlang
Copy link
Collaborator

The main sentence here at NIST is: /For purposes of the above length requirements, each Unicode code point SHALL be counted as a single character./

This is related to the opened issue, but not to the requirement.

Verify that passwords of at least 64 characters are permitted, and that passwords of more than 128 characters are denied.

@jmanico
Copy link
Member

jmanico commented Oct 31, 2023 via email

@appills
Copy link
Author

appills commented Oct 31, 2023

Maybe the OWASP requirement can be updated to:

Ensure that passwords of at least 64 Unicode codepoints can be submitted and INSERT REASONABLE UPPERBOUND HERE while being mindful of the 72-byte footgun in bcrypt

@elarlang
Copy link
Collaborator

elarlang commented Oct 31, 2023

bcrypt part is covered by 2.4.4:

# Description L1 L2 L3 CWE NIST §
2.4.4 [MODIFIED] Verify that if bcrypt is used, the work factor is a minimum of 10 and password size is limited to 72-bytes due to bcrypt's input limit. (C6) 916 5.1.1.2

I think the goal for the requirement 2.1.2 is to disallow short max-length limits for passwords a'la "your password can not be longer than 16 symbols".

@appills
Copy link
Author

appills commented Oct 31, 2023

I think the goal for the requirement 2.1.2 is to disallow short max-length limits for passwords a'la "your password can not be longer than 16 symbols".

This makes sense & I completely agree it's spirit. If I understand correctly then, the upper-limit of 128 in 2.1.2 is added because of potential DoS vectors? But otherwise no more than 128 Unicode symbols may be submitted as a password, so between 128 to 512 bytes in UTF-8.

# Description L1 L2 L3 CWE NIST §
2.1.2 Verify that passwords of at least 64 characters are permitted, and that passwords of more than 128 characters are denied. (C6) 521 5.1.1.2

I think what I'll do is gather any requirement regarding passwords and compare them for any inconsistency & possible contradictions, then share them all at once so all context is available. I won't close this, but feel free to do so if you think we've milked this for all that it's worth. I don't have a reasonable solution for this, but I would like to continue the discussion elsewhere - is there an ASVS channel/forum somewhere?

@elarlang
Copy link
Collaborator

As many of us got confused, it makes sense to rethink this requirement.

For me it seems, that lower limit is for users (that application need to allow at least 64 characters) and upper limit is for application/storage do not cause DoS attack from too long input.

@elarlang
Copy link
Collaborator

As many of us got confused, it makes sense to rethink this requirement.

For me it seems, that lower limit is for users (that application need to allow at least 64 characters) and upper limit is for application/storage do not cause DoS attack from too long input.

ping @tghosth @jmanico (and anyone else) - do you have any ideas for improvements? If not, we can just close this.

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label Nov 12, 2023
@jmanico
Copy link
Member

jmanico commented Nov 12, 2023

I'm ok to close this.

@tghosth
Copy link
Collaborator

tghosth commented Nov 14, 2023

Let's close for now

@elarlang
Copy link
Collaborator

Note: the same topic continues in another issue: #1923

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 2) Awaiting response Awaiting a response from the original poster V2 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

4 participants