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

[SR][D9] Add a setting to enable "security by obscurity" on login/password reset forms #4696

Open
ghost opened this issue Oct 12, 2020 · 47 comments

Comments

@ghost
Copy link

ghost commented Oct 12, 2020

This issue has become a feature request for a new setting to be added that allows enabling better privacy on login/password reset forms. This will mean those who prefer usernames/email addresses not be revealed can enable that setting, and those who prefer the better UX of knowing what incorrect information was submitted to these forms can leave it disabled.


If I enter my username in the Reset Password form (/user/password) and submit, I see this message:

Further instructions have been sent to your e-mail address.

If I enter a made-up username in the same form and submit, I get this:

Sorry, [made-up username] is not recognized as a user name or an e-mail address.

This basically tells people whether or not a given username or email address is in-use on the site. I see this as a (low) security issue and possible privacy issue.

I recommend instead giving something like the following message (like I've seen on other sites) whether a valid or invalid username/email address was entered:

If a matching account was found, further instructions have been sent to that account's e-mail address.

@ghost ghost added the type - task label Oct 12, 2020
@stpaultim
Copy link
Member

stpaultim commented Oct 12, 2020

FYI - This is the relevant Drupal issue on the same topic.
https://www.drupal.org/project/drupal/issues/1521996 and this is a follow-up issue re the same issue in the 'Create new account' form: https://www.drupal.org/project/drupal/issues/2346389

@ghost
Copy link
Author

ghost commented Oct 15, 2020

Wow! 200+ comments and 8 years later, and its still not fixed in Drupal... I think there's something very wrong with the way Drupal's issue queue/review system works...

@alanmels
Copy link

I second this suggestion as there is no need for malicious parties trying to hack in to know if certain username is registered on the website or not.

@stpaultim
Copy link
Member

When I first ran into this issue in the Drupal issue queue, I was a bit skeptical that it was necessary (I would have been happy with a contrib option at the time). However, I think that this is becoming a standard way of dealing with password resets and suspect that many users will expect sites to behave this way in the future. I support this change.

@ghost
Copy link
Author

ghost commented Oct 18, 2020

After reading through the Drupal issue a bit more and playing with Backdrop, I realised we'll also need to make a decision about the login form.

If you enter an incorrect username, it'll tell you that the username doesn't exist. Same with the password. So we have the same security/privacy issue here too. But making the message more generic to say that the username or password is incorrect may make it harder for users to know where they made a typo.

@olafgrabienski
Copy link

@BWPanda There already is a decision about the login form, see this discussion from quite a while ago: #111

@stpaultim
Copy link
Member

Thanks @olafgrabienski for the history. That issue was closed 5 years ago and I don't think it is unreasonable to revisit this issue again 5 years later - thinking on issues like this does change over time. At the same time, that old issue leads me to revisit my own original skepticism about this change.

Ultimately, there is a usability issue here vs a small (perceived) increase in security. I'm on the fence and could go either way. I would say that this issue is not a priority for me, but I'm interested in whether or not thinking on this has changed.

@ghost
Copy link
Author

ghost commented Oct 19, 2020

Thanks @olafgrabienski. Reading over that issue, it seems that the argument was that there's no point being obscure about the username/password on the login form when you can just use the password reset form to see if a username exists.

But with my proposal to fix all forms, then that reasoning is out the window and worth discussing again.

@ghost
Copy link
Author

ghost commented Oct 19, 2020

And dont forget, its not just security we're talking about, but privacy too.

@olafgrabienski
Copy link

I realised we'll also need to make a decision about the login form

There already is a decision about the login form, see this discussion from quite a while ago: #111

with my proposal to fix all forms, then that reasoning is out the window and worth discussing again.

Good point, I agree!

@klonos
Copy link
Member

klonos commented Oct 26, 2020

But with my proposal to fix all forms, then that reasoning is out the window and worth discussing again.

We need the issue title and the summary to reflect that then. Care to update those @BWPanda ?

I am supportive of that idea 👍 ...if people think that needs to be a switch in order to provide options for site admins to have it one way or another, then #3624 would be a place for it 😉 (an "Obscure login and password reset messages" checkbox).

@ghost ghost changed the title Change message when resetting password for (slightly) better security/privacy Change message for incorrect login/password reset for (slightly) better security/privacy Oct 26, 2020
@alanmels
Copy link

The fix for similar issue for Drupal was committed 5 days ago: https://www.drupal.org/project/drupal/issues/1521996

@jenlampton
Copy link
Member

jenlampton commented Jan 21, 2021

I just wanted to add a note here that we considered this for Backdrop core way back when, and we instead decided to follow the prevailing recommendations that the user-experience of knowing if you are entering the correct email address (or username) vastly outweighed any security benefit you'd get by hiding that information. At the time we had several blog-posts and articles as references, I'll see if any of those are still online...

edit:

Mailchimp has a great writeup on this: http://blog.mailchimp.com/social-login-buttons-arent-worth-it

The engineering team, ever mindful of security, argued that being generic about username and password errors makes it harder for bad guys to guess usernames by pounding the form with random words or email addresses. But after some further consideration, we decided that it was a false risk, as the username reminder form already tells you if a username exists, and is not a significant security risk for the bajilions of sites that have them.

The end results of Mailchimp offering better error handling and messages resulted in:

From June 12-July 12 we saw 114,239 login failures—that’s a 66% decrease.

edit: also...

This "security feature" only exists for security by obscurity. A true black-hat hacker wouldn't need or rely on this info in the first place.

@ghost
Copy link
Author

ghost commented Jan 21, 2021

Considering the differing opinions on this topic, I'd like to suggest making this a configurable option in Backdrop (with the default setting matching the current functionality). Those who perfer privacy can enable it and the messages will be adjusted accordingly, while those who need the better UX of knowing exactly what information was incorrect can disable it (or leave it disabled, since that'd be the default).

Win-win?

@jenlampton
Copy link
Member

Here's the previous issue: #111

@ghost
Copy link
Author

ghost commented Jan 21, 2021

@jenlampton Yes, @olafgrabienski linked to that above: #4696 (comment)

Edit: See my two comments following his (with that link) as to why this is still a revelant discussion.

@jenlampton
Copy link
Member

jenlampton commented Jan 21, 2021

Can we rephrase this issue as a feature request then? I still don't think it's worth hurting 66% of people for a theoretical (unproven) security enhancement. I also don't see how this relates to privacy. Can you elaborate on that @BWPanda?

There is a contrib solution for this already: https://www.drupal.org/project/username_enumeration_prevention

@ghost
Copy link
Author

ghost commented Jan 21, 2021

I also don't see how this relates to privacy. Can you elaborate on that @BWPanda?

A username (depending on the site and the person using it) could give away information about the site or the person.

For example, someone might prefer to remain anonymous online (not using their real name, or only using their firstname), but then their company website requires all usernames to be [firstname].[lastname], which because of this 'feature' means that information is now effectively public...

Or someone could enter known usernames (or the email address) of a person they know into a particular website to see if that person has registered an account there. Like maybe a support group has a website where people can register to chat privately in a forum, but then other people need only to try logging in as, or resetting the password of, their friends/family members to see if they have an account there (which in and of itself can be a big privacy issue).

Does that explain it better?

@ghost ghost changed the title Change message for incorrect login/password reset for (slightly) better security/privacy Add a setting to enable better security/privacy on login/password reset forms Jan 21, 2021
@ghost
Copy link
Author

ghost commented Jan 21, 2021

Issue title and OP updated.

@alanmels
Copy link

alanmels commented Jan 21, 2021

@jenlampton, thanks for the link and relevant quotes from Mailchimp's write-up.

To say frankly I don't have a strong opinion on this, however what I care about is that Backdrop does not fall behind of any security-related feature Drupal has. I'm pretty sure Drupal community also took into account different opinions and in the end of the day apparently came to conclusion in https://www.drupal.org/project/drupal/issues/1521996

@BWPanda's suggestion to make it configurable is really appealing to me. Having it both ways and leaving for a Backdrop site owner to decide to which policy to follow sounds ideal.

@klonos
Copy link
Member

klonos commented Jan 22, 2021

@BWPanda's suggestion to make it configurable is really appealing to me.

I'm slightly leaning towards this too. If we decide to do it, then perhaps the best place for this checkbox is #3624

@klonos
Copy link
Member

klonos commented Jan 22, 2021

...another thing to note is that the d.org issue is marked as "Needs backport to D7". If/when that happens, and judging by the last 7.x patch available, we may need to provide an upgrade path for the variable that stores the custom text (not required if all we add is a checkbox).

@jenlampton
Copy link
Member

jenlampton commented Jan 22, 2021

There is a contrib solution for this problem, so anyone who needs this feature has a solution. I still don't believe it belongs in core. :)

Reading over that issue, it seems that the argument was that there's no point being obscure about the username/password on the login form when you can just use the password reset form to see if a username exists.

An attacker can also find out on the registration form if a certain username or email address has an account, as both the username and email address need to be unique in order to create an account. (Drupal has not figured out the registration form yet, either) This solution will only help if there is no registration form on the site. -- Please also note that other sites such https://github.com/ also have this "vulnerability". I can check any email address to see if it has an account from the registration form. This is not really a concern for other large sites that do generally care a lot about security.

Or someone could enter known usernames (or the email address) of a person they know into a particular website to see if that person has registered an account there.

I'm still not sure this is a legitimate concern. If I went to ticktok to see if Nate had an account there and searched for quicksketch I'd find the account. But that's not Nate. Knowing a username exists on a site isn't really a privacy concern. Same thing goes for a name. There are plenty of Jennifer Lampton's in the world.

Email address is a larger concern, but considering that Backdrop doesn't validate email address changes, even that's not a good way to tell if I created an account. Someone else could have created an account and changed the email address to my address after the fact.

what I care about is that Backdrop does not fall behind of any security-related feature Drupal has.

Backdrop isn't Drupal. We have a different target audience, and different priorities. Backdrop cares a lot about the user experience, where Drupal doesn't really. Providing a core feature that appears to be helpful at first glance, but in reality will hurt most people is misleading. It's like we're giving Backdrop site owners bad advice. People will turn this on thinking they want it "because, security". Then, when they realize that they are loosing users (or customers) because people can't log in, they are going to be furious with Backdrop. Especially if they learn how there's no real security advantage.

I'm pretty sure Drupal community also took into account different opinions and in the end of the day apparently came to conclusion

That's not really how things work in the Drupal queue. If people keep writing patches, the patches get reviewed and as long as there are enough people to get it RTBC it can get in. There isn't a lot of debating if a change is the right thing for Drupal. That's why we're here, after all :)

I got many of my points above from the Drupal issue, but here are some more cons:

  • This really seems like bad usability to me, and all of it is in service of a level of privacy that I don't think most sites actually want or care about...
  • even if a generic login failure message is used, user flood protection will reveal the validity of a user as it is based on the account ID and IP rather than the actual name entered in the form.
  • Arguably, this is an issue that would be better handled by enhancing the username_enumeration_prevention module for those that need it due to the nature of the site rather than trying to patch core to fix something that the community largely agrees isn't broken.

@klonos
Copy link
Member

klonos commented Sep 26, 2022

Thanks for working on this @BWPanda 🙏🏼 ...I've left a couple of comments in the PR (a nit-pick and a typo), plus:

Leaving the WFM status for this issue/PR, since I've tested this on my local and found it to work as expected.

...noting that this is our take/adaptation of what would have been a crossport of what was implemented in https://www.drupal.org/project/drupal/issues/3200198 for D7 core, which for the record was to add a "Privacy" fieldset, with a text field in it that allows customizing what we are now introducing as a hard-coded set of messages:
image

I feel that the approach we are taking here is the right thing for core, and any customization of the text of the messages should be left to contrib, such as perhaps https://github.com/backdrop-contrib/username_enumeration_prevention (which by the way will need some work after this gets merged into core @jenlampton - happy to help with that when the time comes).

@klonos
Copy link
Member

klonos commented Sep 26, 2022

PS: @BWPanda do you think that using 404s instead of 403s when trying to access user pages belongs in this issue/PR, or file a separate issue for it?

See https://www.drupal.org/project/username_enumeration_prevention/issues/2133887

[edit] Never mind - follow-up created here: #5802

@klonos klonos changed the title Add a setting to enable better security/privacy on login/password reset forms [SR] Add a setting to enable better security/privacy on login/password reset forms Sep 26, 2022
@jenlampton
Copy link
Member

A lot of security issues are being raised about this. We tell people it's not an issue and point them here for the reasoning and discussion, but I think the fact that people keep bringing this up show that there's a legitimate need/desire for this in Backdrop core.

I'd like to disagree with this assumption. Most of the people who are bringing this up are not people who use Backdrop, they are just people out in the wild testing different pieces of software. Most do not know that this is not really an issue, or that it has huge usability implications.

I think our efforts would be better spent working on improving the contrib options. If we ever get a clean solution in the contrib module, we can re-evaluate for core. Our previous decisions based on actual user experience issues vs theoretical security issues are good ones. A bunch of kids hoping to get easy bug bounties shouldn't change that.

@klonos
Copy link
Member

klonos commented Sep 27, 2022

@jenlampton this is a real security/privacy concern - not merely a means for any kids to gain any credit in the security audit world at all.

My understanding from reading https://www.drupal.org/drupal-security-team/security-team-procedures/disclosure-of-usernames-and-user-ids-is-not-considered (*) and https://www.drupal.org/project/drupal/issues/3241232 is that the decision that this is not a security concern was based on certain assumptions made a long time ago, which nowadays feel outdated and overly-generalized.

I am specifically referring to this section here: https://www.drupal.org/drupal-security-team/security-team-procedures/disclosure-of-usernames-and-user-ids-is-not-considered-a-weakness#s-drupals-philosophy, and this assumption here:

Usernames are an important part of online identity. Having a public username helps other users of a site to know the identity of the person they are interacting with, in a forum or a blog. Drupal is primarily intended to be used for sites where identity and interaction are key elements so it is reasonable for that information to be public.

No! Usernames and other information that can potentially be used to identify/target people, or make it easier to compromize their accounts is NOT an important part of an online identity in every Drupal/Backdrop site. Assuming that it is safe/OK or even desired for that information to be public is unacceptable and plain wrong.

That argument assumes that every installation of Drupal is a blog or public forum, where people want their profiles/usernames/information to be public. This is very generalized, and basing the entire software on that assumption is wrong. What we are introducing here is a flip switch that allows sites that should never expose the details of their user accounts (think edu/gov sites, private business sites, e-commerce sites, etc.) to easily harden the security of their site, and increase privacy of registered users by helping reduce the potential attack vector.

I could even go as far as to suggest that the majority of sites that are meant to be used as places where online identities are meant to be discoverable/exposed are most likely to turn to solutions like Wordpress and the likes - not Drupal/Backdrop. This would effectively make this issue here equivalent/similar to #574, in which we changed a long-established default that was also based on the assumption that Drupal/Backdrop sites are meant to be used as forums/blogs and allowed public user registration OOTB. ...but I won't go that far - lets keep the defaults as is now, at least until we know for a fact (with metrics etc.) that people building Backdrop sites prefer it that usernames be obscured in login/password forms. Let's only provide a flip switch to harden this security/privacy aspect, based on the intended use of the site being built.

*PS: Once upon a time I used to be a Military Police Officer, and my first job after I finished my army service was in a data security company. So my security-cautious trained brain gets triggered just by the title of that article! 🤯

@kiamlaluno
Copy link
Member

kiamlaluno commented Sep 28, 2022

I think the key words in https://www.drupal.org/drupal-security-team/security-team-procedures/disclosure-of-usernames-and-user-ids-is-not-considered-a-weakness are the following.

The Drupal Security Team does not consider it a vulnerability that there are ways to determine a registered member's username and/or user ID value (i.e. the numeric uid).

That page doesn't say it could be a vulnerability, but that the Drupal Security team doesn't consider it a vulnerability. If the usernames disclosure were universally consider a non vulnerability, that page would contain a reference to a document showing that.

I am not saying that Backdrop should consider that a vulnerability, or not. I think that what reported in that drupal.org page shows a decision it has been taken from the Drupal Security Team on December 22, 2010. It doesn't mean it was the correct decision to take, or that the same decision should be automatically taken for Backdrop.

@indigoxela
Copy link
Member

Unless I'm missing something, it seems to me, this discussion drifts from course. 🚢

It's labelled as "feature request" and its main goal is to add a new setting to core, that enables admins to easily tweak messages, that have some impact on security / privacy.

I don't think we have to (or should) discuss, if UID enumeration is a vulnerability or not, or if Drupal made the right decision or not. Security evaluation shouldn't happen publicly, and Drupal decisions aren't our job. 😉

@klonos
Copy link
Member

klonos commented Sep 29, 2022

You're making a very good point @indigoxela ...we are not to decide whether determining a registered person's username and/or user ID is a security risk or not - not in this ticket at least. That should happen in a separate ticket (most likely not in the core issue queue, rather than where we decide on policies). What we are to do here is to add a feature that allows site owners/builders to decide whether to treat this as a potential risk or not + then to provide an easy way for them to configure their site to behave accordingly. And I feel that this is a feature worth adding to core.

@ghost ghost removed their assignment Dec 5, 2022
@jenlampton jenlampton changed the title [SR] Add a setting to enable better security/privacy on login/password reset forms [SR] Add a setting to enable "security by obscurity" on login/password reset forms Dec 29, 2022
@jenlampton
Copy link
Member

jenlampton commented Dec 29, 2022

we are not to decide whether determining a registered person's username and/or user ID is a security risk or not

I think that is an important part of this discussion, because if this is not a security improvement, it doesn't belong in core. There are already contrib projects that allow people to enable this feature if they want it, and since core's priority is the user experience, we would only be going against our stated principles by adding this feature to core.

@klonos klonos changed the title [SR] Add a setting to enable "security by obscurity" on login/password reset forms [SR][D9] Add a setting to enable "security by obscurity" on login/password reset forms Mar 4, 2023
@klonos
Copy link
Member

klonos commented Mar 4, 2023

I've added the "contrib candidate" label to this issue, based on @jenlampton's last comment here.

I still find that this feature request has merit and is simple enough to go into core, and is a very good candidate for #3624.

The Australian Government, like many governments around the world, have started taking security even more seriously, and from a "marketing" perspective (trying to convince less technical people in decision-making positions that a feature of one product is "not a security improvement really"), I wouldn't want people to consider Backdrop less secure or lacking in features.

I realize that government/enterprise is not the main audience of our focus, however this seems to be a "game of impressions" if we are to win people over. So yes, I see the point in the previous comment and others, but I am saying that we could make an exception for the sake of feature parity, and because I don't find this change to be introducing any code complexity or maintenance overhead.

In addition, in D7 they are going to deprecate the username_enumeration_prevention contrib module (see Module not needed with D7 core patch in #3200198), and AFAIK this will be an exception to our "you don't need these contrib modules any longer" pitch, and a case where people will actually need a contrib module for an otherwise core feature.

my 2c

@klonos
Copy link
Member

klonos commented Mar 4, 2023

Noting that since Pathauto has been included in Backdrop core, perhaps we should also incorporate https://www.drupal.org/project/pathauto/issues/2943255 as part of this issue here.

  1. On a fresh installation of Backdrop, launch an incognito browser window.
  2. Try to visit /user/1 -> access denied
  3. Try to visit /user/123 -> page not found

Ideally, when the security/obscurity setting we are to introduce here is enabled, both of the above scenarios should lead to a "page not found" (so that valid UIDs are not disclosed).

@klonos
Copy link
Member

klonos commented Mar 4, 2023

@BWPanda I have raised #6018 as a separate issue (good to have a record if people search the queue by the d.org issue ID), but now thinking that we should merge that here and account for that situation when the setting we are to introduce as part of the PR here is enabled. What do you think?

@yorkshire-pudding
Copy link
Member

I'm not convinced that the assertion that there is contrib module so not needed in core really stacks up. There is an issue in that module that in my view makes it unusable.

@izmeez
Copy link

izmeez commented Apr 27, 2024

Is there still a PR for this functionality? I would advocate for this option to become part of core. Whatever perspectives people may have, a choice would be nice.

@stpaultim
Copy link
Member

@izmeez - The previous PR is still available, but it's closed. Someone would need to copy and paste the changes into a new PR and then test it. Or read through the issue to find out how close it was to being usable before it was closed.

I've sometimes adopted abandoned PR's like this one and pushed them through to completion. Maybe you should give this one a go?

@izmeez
Copy link

izmeez commented Apr 28, 2024

@stpaultim Thanks. I looked further and found the PR, backdrop/backdrop#4097 and also the suggestion to use the wording from the drupal commit, https://git.drupalcode.org/project/drupal/-/commit/fcfc6d809db1b4510e91640c6cb045562b299cbb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment