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

Sensitive Data: Mitigate against password guessing attacks #3153

Closed
pdurbin opened this issue Jun 1, 2016 · 32 comments
Closed

Sensitive Data: Mitigate against password guessing attacks #3153

pdurbin opened this issue Jun 1, 2016 · 32 comments

Comments

@pdurbin
Copy link
Member

pdurbin commented Jun 1, 2016

http://policy.security.harvard.edu/sb2-password-guessing says the following as of 2016-06-01:

Password guessing

SB2: Servers or applications must implement a mechanism that inhibits password guessing attacks on user accounts if the server or application does its own authentication.

How to Comply

Block excessive logins

Block user from logging in for a period of time after no more than 10 successive invalid login attempts.

Dataverse does do its own authentication in the case of local/builtin accounts so we need to meet this requirement to be able to host level 3 data as defined at http://policy.security.harvard.edu/level-3

When Shibboleth accounts are used, we're trusting that the Identity Provider (IdP) is mitigating against password guessing attacks.

We should decide what "block user from logging in for a period of time" means. My initial thought would that this would be disabling the account for an hour or whatever (configurable) but this is not yet supported (#2419) and from reading https://www.owasp.org/index.php/Blocking_Brute_Force_Attacks I'm realizing that there's the potential for abuse. An attacker who knows your username or email address for your local/builtin account would be able to lock you out of your account temporarily simply by guessing random passwords 10 times. That OWASP page suggests blocking the IP of the attacker, which may be problematic if many users are behind one IP address. It also suggests a CAPTCHA but I don't think that would meet the "block user" requirement. Their suggestion to introduce a delay against the IP of the attacker reminds me strongly of IQSS/dataverse-pm#23 which is about adding rate limiting to the API.

@pdurbin
Copy link
Member Author

pdurbin commented Jun 9, 2016

I posted a link to this issue to https://groups.google.com/d/msg/dataverse-dev/hxnA_Wumo_M/J41aNceCBAAJ and http://irclog.iq.harvard.edu/dataverse/2016-06-08#i_36462 and http://irclog.perlgeek.de/crimsonfu/2016-06-09 for discussion and I like the idea from @pameyer and @bpeisenbraun of what seems to be called "exponential backoff". The screenshot below is from https://devcentral.f5.com/articles/implementing-the-exponential-backoff-algorithm-to-thwart-dictionary-attacks and shows how the lockout time increases with each subsequent failed login attempt:

screen shot 2016-06-09 at 9 28 40 am

If we happened to be using Ruby and Rack, we could take advantage of the exponential backoff feature of rack-attack: https://github.com/kickstarter/rack-attack/wiki/Advanced-Configuration#exponential-backoff

I suppose the thing we would be locking out is an IP address. I'm actually not sure that we need to implement account disabling in #2419 first. The requirement is to block a user from logging in, which is different from disabling an account.

@pdurbin pdurbin changed the title Sensitive Data: Mitigate Against Password Guessing Attacks Sensitive Data: Mitigate against password guessing attacks Jun 9, 2016
@pdurbin
Copy link
Member Author

pdurbin commented Jun 17, 2016

Obviously, we'd like to avoid the scenario at https://twitter.com/esttorhe/status/743623557352022020 in the screenshot below:

screen shot 2016-06-17 at 9 48 27 am

@pdurbin
Copy link
Member Author

pdurbin commented Jun 28, 2016

This morning @djbrooke and I met to discuss this issue. I presented two approaches:

  • Approach 1: Lock account. After 10 (or fewer, configurable) successive invalid logins, lock the account (User account cannot be disabled/deactivated #2419) for a period of time (configurable).
  • Approach 2: Block IP. After 10 (or fewer, configurable) successive invalid logins, block the IP address for a period of time (configurable).

@djbrooke indicated that I should go ahead work on #2419 so that locking an account is possible and then implement Approach 1: Lock account. So this issue is dependent on that one.

As I indicated above, locking the account was my initial thought anyway (perhaps because of how the requirement is written) and @landreev seems to agree with this approach. Yesterday I wrote to abdc-security and a friend reacted with, "you are overthinking your password lockout problem... N bad passwords in Y interval locks out the account for 24 hours."

I've been discussing Approach 2 (Block IP) with @whorka and he was gracious enough to show me the code at https://accounts.hmdc.harvard.edu which uses this approach but again, we'll focus on account locking instead (Approach 1).

@michbarsinai
Copy link
Member

Important heads-up. Thanks, Phil!

My comments:

  1. Disabling account is very different from blocking a login. For example, if someone tries to guess user X's password, while user X is logged in from another device or is using the API via a script, X's legitimate usage would be stopped. Moreover - if someone has a password and tries to guess the username, this would go under the radar of account disabling.
  2. Blocking an IP would probably not work either, especially with our target audience, as institutions tend to use a few IP addresses for outside access.
  3. Also - blocking and tracking done on the server-side add overhead to the server, sometime for each HTTP request.

So - I assume it's best not to allow people to automatically guess password, which is what CAPTCHA does. We could use reCAPTCHA (or similar project, as reCAPTCHA was acquired by Google) to help digitize books while we're at it. So maybe present a CAPTCHA after 3 failed tries from same IP address in a 3-4 minute period. That's still added some burden on the server, but we could use an in-memory asynchronous map, and not have to go to the DB again. Also - this would effect only the login screen.

@pdurbin
Copy link
Member Author

pdurbin commented Jun 29, 2016

@michbarsinai thanks for your thoughts on all this. I'm not sure a CAPTCHA fulfills the requirement of "Block user from logging in for a period of time after no more than 10 successive invalid login attempts."

As I mentioned at #3153 (comment) I'm focusing on locking the account for now. Here's a screenshot to get @mheppler @eaquigley and @mcrosas thinking about what the UI/UX should be:

log_in_-root_dataverse-_2016-06-29_16 03 32

I'm also adding logic in EjbDataverseEngine to prevent Commands from being executed when an account is locked. Once I have something worth demo'ing I'll push a war file to a test server.

@michbarsinai
Copy link
Member

But any disaffected student could lock his professor out this way... maybe the rule needs updating or refinement.

What does Harvard's SHIB/HarvardKey servers do?

On 29 Jun 2016, at 23:10, Philip Durbin notifications@github.com wrote:

@michbarsinai https://github.com/michbarsinai thanks for your thoughts on all this. I'm not sure a CAPTCHA fulfills the requirement of "Block user from logging in for a period of time after no more than 10 successive invalid login attempts."

As I mentioned at #3153 (comment) #3153 (comment) I'm focusing on locking the account for now. Here's a screenshot to get @mheppler https://github.com/mheppler @eaquigley https://github.com/eaquigley and @mcrosas https://github.com/mcrosas thinking about what the UI/UX should be:

https://cloud.githubusercontent.com/assets/21006/16467071/ad76d914-3e13-11e6-919d-cfcca0ed2d98.png
I'm also adding logic in EjbDataverseEngine to prevent Commands from being executed when an account is locked. Once I have something worth demo'ing I'll push a war file to a test server.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #3153 (comment), or mute the thread https://github.com/notifications/unsubscribe/AB2UJJraDbV5Sqi56CH0G-NB8NDyM5zPks5qQtEdgaJpZM4Iri3k.

pdurbin added a commit to pdurbin/dataverse that referenced this issue Jun 29, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 1, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 1, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 1, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 5, 2016
@djbrooke
Copy link
Contributor

djbrooke commented Jul 5, 2016

Notes from 7/7 meeting:

  • 10 tries before an account is locked
  • We need better error message visibility (it was unclear that the account lock message was triggered as it looked so similar to the "wrong password" message)
  • The lockout period should be configurable
  • Open question - does the superuser have the ability to unlock an account?

@pdurbin
Copy link
Member Author

pdurbin commented Jul 8, 2016

The code I demoed to @djbrooke @mheppler @eaquigley @scolapasta @kcondon and @bsilverstein95 on 2016-07-05 was 6852a96 from https://github.com/pdurbin/dataverse/tree/3153-passwordguess

To echo what @djbrooke wrote, @eaquigley felt that the change from "wrong password" to "locked" was too subtle.

@eaquigley also asked that I test to make sure that resetting one's password doesn't unlock the account. It shouldn't but I'll be happy to test this.

@scolapasta opined that perhaps when the account becomes locked the user should be forced to reset their password. I'm not particularly fond of this idea but I wanted to at least mention it.

In the code I demo'ed a superuser can already unlock an account via API:

. There are already tests around this functionality. I guess I could rip out this feature if we really shouldn't have it.

We also discussed the user story of a user locking themselves out. They are told to contact Support. What should Support do? Should they say, "Just wait until your account is unlocked." Should a superuser have a UI for seeing which accounts are locked? In the harvesting branch a dashboard is being developed and #2211 is related.

pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 19, 2016
Conflicts:
src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java
src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java
src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 19, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 19, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 19, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 20, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 20, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 20, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 20, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 20, 2016
pdurbin added a commit to pdurbin/dataverse that referenced this issue Jul 20, 2016
@pdurbin
Copy link
Member Author

pdurbin commented Jul 25, 2016

I just pushed some code to a branch at https://github.com/IQSS/dataverse/tree/3153-2419-lockuser which is more or less a cleaned up version of what I demo'ed on 2016-07-05. In order to run this code without dropping your database, you need to run the SQL script at https://github.com/IQSS/dataverse/blob/3153-2419-lockuser/scripts/database/upgrades/lockuser.sql

@pdurbin
Copy link
Member Author

pdurbin commented Oct 10, 2017

I pushed an experiment to pdurbin@bad1b7f but I think it would be extremely messy to reuse the existing SendFeedbackDialog.java/contactFormFragment.xhtml code since it's so oriented toward sending email. @TaniaSchlatter wants a pop up, which is fine but it will be smaller, maybe something like this:

screen shot 2017-10-10 at 4 36 51 pm

@oscardssmith
Copy link
Contributor

I'd like to quickly point out that it would be a significantly less trivial captca if the problem was a picture (unobscured), than if it is just text.

@pdurbin
Copy link
Member Author

pdurbin commented Oct 12, 2017

I just made pull request #4193 and am putting this issue into Code Review at https://waffle.io/IQSS/dataverse

I believe @mheppler and I have delivered the non-popup math challenge after three failed login attempts as specified but I'd like to remind everyone the it's also possible to conduct password guessing attack via API like this:

curl 'http://localhost:8080/api/builtin-users/dataverseAdmin/api-token?password=dictionaryAttack1'

curl 'http://localhost:8080/api/builtin-users/dataverseAdmin/api-token?password=dictionaryAttack2'

etc.

This was implemented in #1818 and is documented in http://guides.dataverse.org/en/4.8/api/native-api.html#retrieving-the-api-token-of-a-builtin-user but unless we hear strong objections from @astrofrog (who opened that issue) or anyone at @CenterForOpenScience I think we should consider disabling this API out of the box and providing a mechanism to turn it back on if Dataverse installations really want it and more API tests. Thoughts?

@pdurbin
Copy link
Member Author

pdurbin commented Oct 12, 2017

It wasn't hard for @mheppler to convince me to go ahead disable to ability to look up one's API token via API using your username and password, which I did in fd743e2 and added a new database setting to revert to the old behavior.

@eunices
Copy link
Contributor

eunices commented Feb 10, 2022

UI IMPACT

Log In

  • add "Log In Disabled" error msg (third frame of the animated GIF below...)

login-locked

Hi devs, to check, will this feature be implemented anytime soon?

@pdurbin
Copy link
Member Author

pdurbin commented Feb 10, 2022

@eunices thanks for your interest in this. In the future, please create a new issue because we don't always notice comments on old, closed issues.

It looks like we never implemented this exact "Log In Disabled" message as part of dealing with password guessing attacks. (We do show a "Sorry, your account has been deactivated" message if a superuser has deactivated your account (pull request #7629) but these are very different things.) Instead, if the user enters the password wrong 3 times, we start showing a math problem. This is to slow the user down (mitigate against password guessing attacks). Users cannot lock themselves out. Again, please feel free to open a new issue if you're interested in different behavior.

@eunices
Copy link
Contributor

eunices commented Feb 11, 2022

@pdurbin will create a new issue if needed.

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

No branches or pull requests