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

password security #1540

Closed
drwetter opened this issue Feb 13, 2023 · 33 comments · Fixed by #1732
Closed

password security #1540

drwetter opened this issue Feb 13, 2023 · 33 comments · Fixed by #1732
Assignees
Labels
6) PR awaiting review josh/elar V2.1 passwords Passwords, password storage related issues _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@drwetter
Copy link

drwetter commented Feb 13, 2023

I was just testing for a customer their password security and actually I also wanted to link hereto and not only to the NIST password standard (specifically: https://pages.nist.gov/800-63-3/sp800-63b.html#-5112-memorized-secret-verifiers)

But as it turned out, some points in the NIST standard are not in ASVS (https://github.com/OWASP/ASVS/blob/9b80241af66c09f4b8cc75a2d222ca62733e4f5f/5.0/en/0x11-V2-Authentication.md#v21-password-security) if I see that correctly:

  • Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’).
  • Context-specific words, such as the name of the service, the username, and derivatives thereof.

Let me know if you like a PR and if so against which branch.

@elarlang
Copy link
Collaborator

elarlang commented Feb 14, 2023

Repetitive or sequential characters (e.g. 'aaaaaa', '1234abcd').

I think those examples are covered in practice with current requirement as most likely those are leaked.

# Description L1 L2 L3 CWE NIST §
2.1.7 [MODIFIED, SPLIT TO 2.1.14] Verify that passwords submitted during account registration or password change are checked against an available set of, at least, the top 3000 passwords. (C6) 521 5.1.1.2
2.1.14 [ADDED, SPLIT FROM 2.1.7, LEVEL L1 > L3] Verify that passwords submitted during account registration or password changes are checked against a set of breached username/password pairs. (C6) 5.1.1.2

Context-specific words, such as the name of the service, the username, and derivatives thereof.

This I have rised myself in #683 (comment)

Recommendation no 5: add requirement, that password should not contain expected and context-specific words, like user data or site data and "1337 speaking version" of them (example: owaspasvs > 0wa5pasvs) (part of passwords blacklist).

From pen-testing practice, this is one of the most important requirement to have in password topic - people have learned to use "unique" passwords per site, but they too often use site data with some other pattern for that.

We ask for PR if we have agreement here in the issue, what kind of change we want/need - otherwise it's discussion over PR's and it's not nice to follow or understand later, why some changes were made.

@drwetter
Copy link
Author

drwetter commented Feb 15, 2023

I think those examples are covered in practice with current requirement as most likely those are leaked.

not all. In a case I had they checked via haveibeenpwend. They were some sequences in there (API) like the examples NIST gave but if you look at the wording of NIST it's much stronger.. So in a retest it was only possible to add two unique chars in a row.

Context-specific words, such as the name of the service, the username, and derivatives thereof

I stumbled over password = username or password = mail address, password = first or last name , password = name of the site, password = service description of the site ....

@drwetter
Copy link
Author

And to add: I wouldn't rely on leaked PWs only. So assuming aaaa.... (dots for chars to follow) is in a leaked password list maybe right. But what about bbbb... , ääää...., åååå... or ÇÇÇÇ.... ? Especially for offline attacks that maybe a low handing fruit

@elarlang
Copy link
Collaborator

elarlang commented Feb 16, 2023

I don't agree that allowing mentioned (bbbb... , ääää...., åååå... or ÇÇÇÇ....) passwords make authentication more weak. If we talk about online attacks, amount of attempts must be limited

# Description L1 L2 L3 CWE NIST §
2.2.1 [MODIFIED] Verify that anti-automation controls are effective at mitigating breached credential testing, brute force, and account lockout attacks. Such controls include blocking the most common breached passwords, soft lockouts, rate limiting, CAPTCHA, ever increasing delays between attempts, IP address restrictions, or risk-based restrictions such as location, first login on a device, recent attempts to unlock the account, or similar. More than 5 failed authentication attempts per hour for a single account should trigger some sort of reaction or alert. 307 5.2.2 / 5.1.1.2 / 5.1.4.2 / 5.1.5.2

Those passwords/patterns should not be my first 1000 list. And more popular ones exists already in leakage files or APIs. My point with those - in my opinion the benefit from this (proposed new) requirement is with questionable impact or if there is, it is too small to be worth separate requirement. If you think about risk rating - what is the risk for this finding in the report? How much more secure application is with not allowing those passwords? Or the actual security comes from somewhere else.

context specific passwords - and with context specific password I have clear opposite opinion - both based on my own experience as a pentester. People are using too often context specific words as passwords and those are effective as online attacks even when attempts are limited per user and/or per IP.

@drwetter
Copy link
Author

passwords make authentication more weak. If we talk about online attacks, amount of attempts must be limited

I am in general talking about offline and online attacks. With online attacks only I could be way more relaxed concerning the rules if appropriate measures are in place. That's why bbbb... , ääää...., åååå... or ÇÇÇÇ are important to avoid. Don't know whether you know how offline attacks with hashcat and friends work? Also it's a NIST requirement. and I believe we should not weaken that.

Why do you believe OWASP should not follow that?

This is also true for context specific passwords. This is just crazy. Why should we not say no to "[email protected] as both the username and The password.

Or ~"(| |.)" as a password, especially when the login is "[email protected].

This is the first thing I try as a pentester for targetted attacks as it's the lowest handing fruit. And probably smart black hats too.

@elarlang
Copy link
Collaborator

Against offline attack - first, how did you get the hashes, or were there hashes at all?

If they were correctly hashed

  • password must be long enough against brute force
  • do not contain only dictionary words
  • do not predictable phrases by context

Personally I watch this Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’). as composition rule and we have requirement "Verify that there are no password composition rules limiting the type of characters permitted. There should be no requirement for upper or lower case or numbers or special characters."

My question stays:

If you think about risk rating - what is the risk for this finding in the report? How much more secure application is with not allowing those passwords? Or the actual security comes from somewhere else?

@drwetter
Copy link
Author

Sorry, I am done with discussing this. If the project thinks, you need to be less strict than NIST, fine.

I believe you shouldn't

@jmanico
Copy link
Member

jmanico commented Feb 17, 2023

I was just testing for a customer their password security and actually I also wanted to link hereto and not only to the NIST password standard (specifically: https://pages.nist.gov/800-63-3/sp800-63b.html#-5112-memorized-secret-verifiers)

But as it turned out, some points in the NIST standard are not in ASVS (https://github.com/OWASP/ASVS/blob/9b80241af66c09f4b8cc75a2d222ca62733e4f5f/5.0/en/0x11-V2-Authentication.md#v21-password-security) if I see that correctly:

  • Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’).
  • Context-specific words, such as the name of the service, the username, and derivatives thereof.

Let me know if you like a PR and if so against which branch.

I would like to see a PR for this. This is all now very standard password advice right from NIST 800-63b. Good stuff!

@jimfenton
Copy link

You seem to have missed a little context here. The bulleted list to which you refer is introduced by, "For example, the list MAY include, but is not limited to:" In other words, the bulleted list is there merely to provide some ideas for things to include in the blocklist, not as a requirement that any particular thing be included.

The intent of the blocklist is primarily to protect against online attacks, and the throttling requirements limit that to 100 guesses per account. Offline attacks should be protected against using good salted hashing, as memory- and compute-hard as practical. Also strongly consider using a secret-keyed hash or encryption, perhaps as a final step, using a very well protected key (perhaps doing this step in a separate processor, TEE/TPM, or the like).

It's a very slippery slope what to include in the blocklist, and if you get carried away with a huge one it's easy to end up with very frustrated users. By the way, the thought on "context-specific words" was that perhaps an IRS taxpayer site might have a bunch of people using "Ihatetaxes" (or something ruder) as their password, which is why the blocklist needs to be context specific.

Feel free to tag me if you have any more questions on this.

Disclaimer: Although I am a co-author of SP 800-63B, I am not a NIST employee and the above comments represent my personal opinion, not an official statement from NIST.

Hint, hint: The draft SP 800-63 revision 4 (including SP 800-63B-4) is currently out for public comment; NIST would welcome public comments through March 24, 2023. https://pages.nist.gov/800-63-4/

@elarlang
Copy link
Collaborator

elarlang commented Feb 18, 2023

Sorry, I am done with discussing this. If the project thinks, you need to be less strict than NIST, fine.

No hard feelings. All requirements should have clear argumentation, why it exists. Just the fact it exists somewhere else (in this case as optional consideration), is not enough. The reason is simple - if we have questionable requirements we will have in the future issues opened with question - what is the point of the requirements and what risk it mitigates. We just removed 2 requirements which also came as optional from NIST (ASVS v4.0.3 requirements 2.1.8, 2.1.12), so I have already went through this "lifecycle".

The question I asked here - to have arguments, why some requirements should exists and I provided my arguments.

I have proposed requirements and then got arguments against my proposals which I did not have in my mind yet - this is the way how to learn and how to get things clear. This is normal process. Just it's important to read everything pragmatically. In short - @drwetter please keep opening issues with your ideas and concerns, with this issue I think there is material for one new requirement and I'll focus on that.

@jmanico - reading through @jimfenton response which says the same points like I did, so you still stand for "PR"? Pushing some thumbs down is not really ... just arguments please.

We should have easy answers for those questions:

If you think about risk rating - what is the risk for this finding in the report? How much more secure application is with not allowing those passwords? Or the actual security comes from somewhere else?

To be clear, just in case I repeat:

Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’).

I don't think this requirement is with sufficient impact and it's complicated to develop (compared to potential/theoretical benefit) to be worth requirement in ASVS.

Context-specific words, such as the name of the service, the username, and derivatives thereof.

I have used this in pen-tests and I can say (and I have said previously) this is missing requirement from ASVS.

@drwetter
Copy link
Author

Will take care of a PR by the end of the week.

PS: That were no hard feelings I had. It's just that my desk is full with other things. And If my arguments weren't convincing I'd rather move on. The thing with pw cracking I'd still need to explain. I have a different stance then Jim F.

@elarlang
Copy link
Collaborator

I point out my comment from #1540 (comment)

We ask for PR if we have agreement here in the issue, what kind of change we want/need - otherwise it's discussion over PR's and it's not nice to follow or understand later, why some changes were made.

@elarlang elarlang added the V2.1 passwords Passwords, password storage related issues label Mar 7, 2023
@tghosth
Copy link
Collaborator

tghosth commented Mar 14, 2023

Ok, there is a lot going on here but I think we need to clarify a few things:

  • As @jimfenton pointed out, the specifics here are in a "MAY" section which is probably why they are not in the ASVS to begin with. I think I would interpret @jimfenton's comment as advising general caution in requiring this section (bearing in mind that he was co-author of the NIST document). Remember that we do not have a "MAY" concept in ASVS and therefore I don't think this makes us less strict than NIST.
  • NIST says in 5.1.1.1 "For example, the list MAY include, but is not limited to:... Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’)." but then a few lines later says "Verifiers SHALL NOT impose other composition rules (e.g., ...prohibiting consecutively repeated characters) for memorized secrets" To me this is contradictory and I have raised this question on the call for feedback for revision 4.
  • Contradictory or not, blocking repetitive or sequential characters feels like composition rules which both us and NIST have been arguing against for ages now :) So for this reason and the contradiction reason in the previous point, I agree with @elarlang on this particular point that I don't think we should be including it.
  • I think there is slightly more merit to the "context-specific words" requirement. I wonder how much improvement impact it would really have compared to effort it would take? However, it seems like both @elarlang and @drwetter and probably @jmanico support including this so I would be prepared to support as well.

Anything I have not covered?

@elarlang
Copy link
Collaborator

I think there is slightly more merit to the "context-specific words" requirement. I wonder how much improvement impact it would really have compared to effort it would take? However, it seems like both @elarlang and @drwetter and probably @jmanico support including this so I would be prepared to support as well.

In reality, this is one of the lowest hanging fruit for web apps where authentication is done with username-password. Go-ahead from my side. Rest is said in: #1540 (comment)

@jmanico
Copy link
Member

jmanico commented Mar 28, 2023

I'm with Elar, go ahead from my side.

@elarlang elarlang added 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos josh/elar and removed 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet labels Apr 5, 2023
@tghosth
Copy link
Collaborator

tghosth commented May 23, 2023

So @elarlang @drwetter I am open to a PR related to context specific words but I would like any requirement to make it clear what is actually required.

@tghosth tghosth added 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels May 23, 2023
@elarlang
Copy link
Collaborator

but I would like any requirement to make it clear what is actually required.

Can you explain that?

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label May 30, 2023
@tghosth
Copy link
Collaborator

tghosth commented May 30, 2023

but I would like any requirement to make it clear what is actually required.

Can you explain that?

As I understand it, you want to open a PR with a new requirement focused on "context-specific words". I am saying that I am comfortable with that but the text of the requirement needs to make it very clear what the the implementer actually needs to do or what the tester needs to verify as it seems like it could be very complicated. E.g. are we just expecting them to chose some basic words like the company name and user name or are we expecting them to run a tool like https://github.com/digininja/CeWL and disallow anything it returns? I can see this being a difficult to interpret requirement otherwise.

@elarlang
Copy link
Collaborator

elarlang commented Jul 3, 2023

From NIST: https://pages.nist.gov/800-63-4/sp800-63b.html#memsecretver

When processing requests to establish and change memorized secrets, verifiers SHALL compare the prospective secrets against a blocklist that contains values known to be commonly used, expected, or compromised. For example, the list MAY include, but is not limited to:

  • Context-specific words, such as the name of the service, the username, and derivatives thereof.

Starter for the proposal:
Verify that passwords can not contain context-specific words, such as information of the service (name, host, address) and information of the user (username, names, email, phone, etc), and derivatives thereof.

Problematic parts to solve:

  • "and derivates there of" I just copied from NIST at the moment but I think it's not clear enough for practical test. It should just cover all classical letter-to-number 1337-5p3ak1nG changes.
  • "can not contain" can cause misfire when company, service, user or whatever other name is just 2-3 letters and then password can not contain that.

@tghosth
Copy link
Collaborator

tghosth commented Jul 9, 2023

I think that both the problems you highlight are highly significant and I am not sure we will successfully come up with a requirement that can overcome them...

@jimfenton
Copy link

The key words here are, "For example,". This is just a list of ideas for things that might be included in the blocklist, and is not intended to be normative at all. That said, some of the suggestions do seem oddly specific, and more general examples might be more appropriate.

@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 and removed 2) Awaiting response Awaiting a response from the original poster labels Jul 10, 2023
@jmanico
Copy link
Member

jmanico commented Aug 30, 2023

Maybe one big ASVS requirement that lists possibilities just like NIST? This is not ideal but would at least address this gap.

@elarlang
Copy link
Collaborator

In practice I feel it's so important problem to solve that it is worth separate requirement. And in general I like more one thing per requirement solutions.

@jmanico
Copy link
Member

jmanico commented Aug 31, 2023

In practice I feel it's so important problem to solve that it is worth a separate requirement. And in general, I like more one thing per requirement solutions.

I can get behind that. That seems very reasonable, @elarlang

The only problem is - some of these requirements are just "for example" and not "you must do this" but we still should address it in some way. Any suggestions on how to make that happen?

@elarlang
Copy link
Collaborator

Any suggestions on how to make that happen?

Yes, if it makes sense, we make new requirement and if it does not, we don't make :) No need to overthink.

@jmanico
Copy link
Member

jmanico commented Aug 31, 2023 via email

@elarlang
Copy link
Collaborator

I said with kept that in mind and it is still valid (for me).

@tghosth
Copy link
Collaborator

tghosth commented Sep 7, 2023

The only problem is - some of these requirements are just "for example" and not "you must do this" but we still should address it in some way. Any suggestions on how to make that happen?

To me, the key challenge is that if we are providing examples rather than explicit rules, how does someone know when they are complying and how does someone validate that an organization is complying?

The only way that I can think of handling this is with two requirements along the lines of:

Verify that a list of context specific words are documented in order to prevent their use in passwords.

Verify that the documented list of context specific words is used to prevent easy to guess passwords being created.

(this second requirement could also be rolled into either 2.1.7 or 2.1.14.

@jmanico
Copy link
Member

jmanico commented Sep 7, 2023

I agree this is a problem. But there are no hard and fast specific rules for password policy, implementors have a wide variety of good choices to choose from.

So I suggest we make a specific ASVS rule like "Implementors should choose 3 or more password rules from the following options: a,b,c,d,e,f,g." to turn the choice into a specific rule.

@tghosth
Copy link
Collaborator

tghosth commented Sep 21, 2023

I agree this is a problem. But there are no hard and fast specific rules for password policy, implementors have a wide variety of good choices to choose from.

So I suggest we make a specific ASVS rule like "Implementors should choose 3 or more password rules from the following options: a,b,c,d,e,f,g." to turn the choice into a specific rule.

I think this approach is problematic @jmanico

I think that giving them a mix and match makes the standard hard to implement and verify, I would rather give them solid requirements. Do you have major objection to what I mentioned above?

Verify that a list of context specific words are documented in order to prevent their use in passwords.

Verify that the documented list of context specific words is used to prevent easy to guess passwords being created.

(this second requirement could also be rolled into either 2.1.7 or 2.1.14.

@jmanico
Copy link
Member

jmanico commented Sep 21, 2023

I'm comfortable with your suggestion Josh

@elarlang
Copy link
Collaborator

@tghosth - Ready to move I guess

@tghosth
Copy link
Collaborator

tghosth commented Sep 26, 2023

Created #1732
I believe the new requirement being added to V1 matches the new definition for it

@tghosth tghosth added 6) PR awaiting review and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos labels Sep 26, 2023
elarlang pushed a commit to elarlang/ASVS that referenced this issue Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6) PR awaiting review josh/elar V2.1 passwords Passwords, password storage related issues _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.

5 participants