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.4.1 should be required for L1 #1773

Closed
appills opened this issue Oct 30, 2023 · 4 comments
Closed

2.4.1 should be required for L1 #1773

appills opened this issue Oct 30, 2023 · 4 comments
Labels
4a) Waiting for another This issue is waiting for another issue to be resolved requirement level Issue related to requirement levels V2 _5.0 - draft This should be discussed once a 5.0 draft has been prepared.

Comments

@appills
Copy link

appills commented Oct 30, 2023

2.4.1

Application Security Verification Levels:

ASVS Level 1 is for low assurance levels, and is completely penetration testable.

Level 1 is the only level that is completely penetration testable using humans. All others require access to documentation, source code, configuration, and the people involved in the development process.

Insecure password storage or insecure credential handling can be tested by a human in at least one method, such as detecting non-timing-safe string comparisons. Secure password hashing is surely testable as a human from a black box and can be inferred to some extent based on response timing.

Using ASVS for Level 1 states:

An application achieves ASVS Level 1 if it adequately defends against application security vulnerabilities that are easy to discover, and included in the OWASP Top 10 and other similar checklists.

And insecure password hashing, storage, and non-timing-safe string comparisons surely fall under A02 Cryptographic Failures

@elarlang
Copy link
Collaborator

elarlang commented Oct 30, 2023

So you copy-paste selective texts from around documents, but not the one which is written to the mentioned section? :)

https://github.com/OWASP/ASVS/blob/master/5.0/en/0x11-V2-Authentication.md#v24-credential-storage

This section cannot be penetration tested, so controls are not marked as L1. However, this section is of vital importance to the security of credentials if they are stolen, so if forking the ASVS for an architecture or coding guideline or source code review checklist, please place these controls back to L1 in your private version.

Requirement itself:

# Description L1 L2 L3 CWE NIST §
2.4.1 [MODIFIED] Verify that one of the following password hashing functions is used when storing the user's password for the application: argon2id, scrypt, bcrypt or PBKDF2. (C6) 916 5.1.1.2

In general I agree it should be level 1, but based on level description for ASVS v4.0.* it is defined as level 2 and the reason is also described in the section.

So at the moment I see it as duplicate of #1495

@elarlang elarlang added the V2 label Oct 30, 2023
@appills
Copy link
Author

appills commented Oct 30, 2023

So you copy-paste selective texts from around documents, but not the one which is written to the mentioned section? :)

Hehe good point, I should've copied the text from the section (& probably read that it states to "Verify", which of course there's probably someone out there doing 100k md5's and that might be timing-indistinguishable)

@Sjord brings up a valid point in that #1495 thread:

For me, the levels relate to the security requirements of your application. If you are building a nuclear missile launch system, you would comply to L3. If you are building a tic-tac-toe website, you would comply to L1.

I'll continue my separate thought over there, thank you!

@elarlang elarlang added the requirement level Issue related to requirement levels label Oct 30, 2023
@elarlang
Copy link
Collaborator

I don't close it at the moment, just marked it as "level question". If we can declare the levels on one nice day, we need to recheck those issues as well.

@elarlang elarlang added the 4a) Waiting for another This issue is waiting for another issue to be resolved label Oct 30, 2023
@tghosth tghosth added the _5.0 - prep This needs to be addressed to prepare 5.0 label Oct 31, 2023
@tghosth tghosth added _5.0 - draft This should be discussed once a 5.0 draft has been prepared. and removed _5.0 - prep This needs to be addressed to prepare 5.0 labels Jan 25, 2024
@jmanico
Copy link
Member

jmanico commented Nov 7, 2024

We made major changes to password storage in v6, please re-open this if you think I'm closing it prematurely,

@jmanico jmanico closed this as completed Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4a) Waiting for another This issue is waiting for another issue to be resolved requirement level Issue related to requirement levels V2 _5.0 - draft This should be discussed once a 5.0 draft has been prepared.
Projects
None yet
Development

No branches or pull requests

4 participants