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.7.5 is a security problem and weakness #1812

Closed
jmanico opened this issue Dec 18, 2023 · 10 comments · Fixed by #2289
Closed

2.7.5 is a security problem and weakness #1812

jmanico opened this issue Dec 18, 2023 · 10 comments · Fixed by #2289
Assignees
Labels
6) PR awaiting review V2 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@jmanico
Copy link
Member

jmanico commented Dec 18, 2023

This requirement is a security weakness.

2.7.5 Verify that the out of band verifier retains only a hashed version of the authentication code.

Even 8 numerical digits when hashed are discoverable via rainbow tables.

I suggest we change this to:

2.7.5 Verify that the out of band verifier retains only a hashed version of the authentication code using password storage algorithms such as Argon2id.
@elarlang elarlang added the V2 label Dec 18, 2023
@elarlang
Copy link
Collaborator

We have separate requirement for password storage:

# 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

Do we want to have hashing requirements separately for "usual password" and "out of band verifier" (and also for "lookup secrets")? My question (and a bit recommendation is) - can we move/merge hashing part from those requirements to 2.4.1?

# Description L1 L2 L3 CWE NIST §
2.6.2 [MODIFIED, SPLIT TO 2.6.4] Verify that lookup secrets stored at the back-end with less than 112 bits of entropy (19 random alphanumeric characters or 34 random digits) are hashed with an approved password storage hashing algorithm that incorporates a 32-bit random salt. A standard hash function can be used if the secret has 112 bits of entropy or more. 330 5.1.2.2
2.7.5 Verify that the out of band verifier retains only a hashed version of the authentication code. 256 5.1.3.2

@jmanico
Copy link
Member Author

jmanico commented Dec 19, 2023 via email

@elarlang
Copy link
Collaborator

elarlang commented Dec 20, 2023

Feels a bit wordy AI output for me.

What do you think about direction like:

Verify that argon2id, scrypt, bcrypt, or PBKDF2 password hashing function is used when storing the user's password for the application, including out-of-band verifier and lookup secrets.

@jmanico
Copy link
Member Author

jmanico commented Dec 20, 2023 via email

@elarlang
Copy link
Collaborator

Anything to keep from current 2.6.2 and 2.7.5?

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label Dec 27, 2023
@elarlang
Copy link
Collaborator

@jmanico - should we keep anything from the current 2.6.2 and 2.7.5?

@tghosth - what do you think about it?

@jmanico
Copy link
Member Author

jmanico commented Dec 31, 2023 via email

@tghosth
Copy link
Collaborator

tghosth commented Jan 23, 2024

This is one of those NIST requirements that hurts my head to read and to be honest I think that it has been misinterpreted.

The NIST requirement talks about an identifying key and an authentication secret. It talks about hashing the former and it talks about the latter having at least 20 bits of entropy.

I think this is one of those sections that is going to need some sort of simplification to be usable so I would suggest either deleting this requirement or leaving this issue open for when someone does the V2 rewrite.

@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 4b Major-rework These issues need to be part of a full chapter rework labels Jan 24, 2024
@jmanico
Copy link
Member Author

jmanico commented Jun 6, 2024

My only point here is that secrets that only need to be verified (and not reversed) should use a password hash, not a normal hash (message integrity code).

@tghosth
Copy link
Collaborator

tghosth commented Nov 5, 2024

I propose deleting this as insufficient impact, I don't think this temporary code is important enough or long lived enough for me to care how it is stored.

@tghosth tghosth added 4) proposal for review Issue contains clear proposal for add/change something and removed 2) Awaiting response Awaiting a response from the original poster 4b Major-rework These issues need to be part of a full chapter rework labels Nov 5, 2024
@tghosth tghosth linked a pull request Nov 6, 2024 that will close this issue
@tghosth tghosth added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something labels Nov 6, 2024
@tghosth tghosth closed this as completed in e0e69f9 Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6) PR awaiting review V2 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants