Gophish: Password policy and login management

[eickeh]

What version of Gophish are you using?: 0.7.1

Brief description of the issue:
Thanks again for your great work!
We have several suggestions to improve the password policy and login management of GoPhish. In this thread we would like to discuss several ways to implement it. We are also open to provide some pull requests after some final decisions have been made.
If you're more comfortable with splitting this into several issues, I'll be happy to open them.

Here are our findings:
1) At the moment, there are no restrictions concerning the password complexity. I suggest a minimum length of 12 characters optionally also with enforced numbers.

2) To prevent session abuse one could enhance the functionality to log out a user automatically after some time of inactivity. At the moment, the standard value is at 5 days, which is a lot. A standard value could be 30 minutes. Moreover one could add an option to change this value or to deactivate it completely.

3) This is the most complicated point. At the moment, there is no protection against password brute forcing. To make such attacks more difficult, one could add progressive delays upon failed logins.
An example implementation could do the following: After e.g. 3 failed login attempts for an user, one has to wait a minute to try again. If he/she fails 3 times again afterwards, he has to wait twice the time before trying again. This should have an upper bound (lets say 3 hours) to prevent that the account is practically locked forever.
With a view to the upcoming RBAC support, one could also think about locking (non admin) accounts and an option for the admin to re enable them.

[jordan-wright]

Hi @eickeh,

I'd be fine accepting PR's for these, thanks for sending in the suggestions!

Let's shoot for the first two to start. I agree that we'll want to consider account lockout more carefully, and I think taking care of 1 and 2 will mitigate many of the threats that would require account lockouts.

For the password policy, I'd prefer to not be too prescriptive. I think just a reasonable length would be sufficient - 12 seems perhaps a bit much to me (don't get me wrong - I use a password manager so I wouldn't care, I'm just thinking about the average admin) but I'm open to hear more about your reasoning.

If we're on this subject, I'd also like to make it so the default password must be changed after the first login. If that's something you'd like to take on while you're in this space, it'd be appreciated! I'm just thinking of a flow that's something like:

• Add a new database schema that adds a boolean password_change_required field to users
• Set it to true for the initial admin
• On a successful login, if it's true, then set a session variable indicating a password reset needs to happen and redirect the admin to the change password page (I forget if we have one of these already)
• When the password is reset, update the database and remove that session flag
• If the admin tries to navigate away before resetting the password, catch the session flag and redirect them to the change password page

Let me know if you have any questions about any of this, where to look for the code, etc. Always happy to help!

[eickeh]

Hi @jordan-wright ,

Thanks for the quick reply!
Alright, so we will try create pull requests some time soon.

According to the password length. Just 8 characters is not up to date in my opinion. If you just google "password brute force attacks calculator" and play a bit around with different password lengths and complexities, you will see that a weak (but possibly average) password of just 8 lower case ASCII letters will be broken in a few hours by an average pc in an offline attack. 12 characters take much much longer (weeks to years depending on the computer used).
My personal preference are at least 15 characters, but as you say, that would maybe annoy admins a bit too much, so 12 can be a good trade off. Sure, password managers would be the best option, but unfortunately they are not used everywhere.
Of course, the average scenario for GoPhish will be an online cracking scenario which can be mitigated by some of the techniques I mentioned above. But in my opinion one should always assume a quite strong attacker.

So, the final decision is up to you, but I'd personally recommend at least 12 characters.

[edermi]

Rather than increasing password length, increasing the bcrypt cost may be something to consider in order to keep usability as well as offline cracking resistance at a reasonable level:

https://github.com/gophish/gophish/blob/master/auth/auth.go#L60

https://godoc.org/golang.org/x/crypto/bcrypt#pkg-constants

The cost may also be configurable or even generated dynamically by running some speedtests on the actual hardware when starting gophish and storing the information in the database.

https://security.stackexchange.com/questions/3959/recommended-of-iterations-when-using-pkbdf2-sha256/3993#3993

[S0larflare]

I’d argue that for something lik gophish, speedtests are irrelevant. The only use of bcrypt will be for logging in to the admin console - I’d say it would be rare in most cases to have more than one user try to log in at once.

Personally, I’d say that if you want to add password complexity/length, it should be configurable rather than forced arbitrarily. 8 might be a bit on the weak side, but that would mainly be against a brute force attack on the web interface. The best protection there would be to add in some rate limiting

Sent from my iPhone

On 21 Aug 2019, at 11:11, Michael Eder <[email protected][e-mail]> wrote:

Rather than increasing password length, increasing the bcrypt cost may be something to consider in order to keep usability as well as offline cracking resistance at a reasonable level:

https://github.com/gophish/gophish/blob/master/auth/auth.go#L60

https://godoc.org/golang.org/x/crypto/bcrypt#pkg-constants

The cost may also be configurable or even generated dynamically by running some speedtests on the actual hardware when starting gophish and storing the information in the database.

https://security.stackexchange.com/questions/3959/recommended-of-iterations-when-using-pkbdf2-sha256/3993#3993

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://github.com/gophish/gophish/issues/1538?email_source=notifications&email_token=ACEGR3MP7BV7XHMTAHI634TQFT2BLA5CNFSM4IL4VBS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4Y2OKA#issuecomment-523347752, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACEGR3LDOSI7DTNCOUZHO4DQFT2BLANCNFSM4IL4VBSQ.

[eickeh]

Rather than increasing password length, increasing the bcrypt cost may be something to consider in order to keep usability as well as offline cracking resistance at a reasonable level:

That sounds indeed quite interesting to me. Maybe that could be an option as well.

The cost may also be configurable or even generated dynamically by running some speedtests on the actual hardware when starting gophish and storing the information in the database.

https://security.stackexchange.com/questions/3959/recommended-of-iterations-when-using-pkbdf2-sha256/3993#3993

That's an interesting calculation described in that link. Nevertheless, I'd say one shouldn't base the bcrypt cost on the machine GoPhish is running on. That could also be a crappy vServer taking ages to compute a proper password hash. On the other side, the attacker has lots of powerful machines, cracking such a password relatively quick. Also, as @S0larflare mentioned, GoPhish probably won't have many users logging in at the same time, which allows us to increase the bcrypt cost.
So, maybe one should assume a moderate server and perform such a calculation to determine a bcrypt cost which will be hard coded in GoPhish. Regularly, when computing power increases, one can also increase this cost.
What do you guys think about that?

I’d argue that for something lik gophish, speedtests are irrelevant. The only use of bcrypt will be for logging in to the admin console - I’d say it would be rare in most cases to have more than one user try to log in at once.

The bcrypt cost is definitely nothing you have to bother very much when you only consider an online attacker. There, as you said, one has better countermeasures like the approach I described in point 3 of my initial post (i.e. some kind of rate limiting).
The attacker tackled by the bcrypt cost is an offline attacker, who got the password database somehow and is able to crack that on her local computer.

Personally, I’d say that if you want to add password complexity/length, it should be configurable rather than forced arbitrarily. 8 might be a bit on the weak side, but that would mainly be against a brute force attack on the web interface. The best protection there would be to add in some rate limiting

I totally agree with you. So, the question is what the default minimum password length should be. I'd still argue for 12. If the admin is annoyed by that, it's configurable.

[edermi]

12 is not really user-friendly. Apart from weak hash functions (which we haven't in this case), I don't see a good reason for using such long passwords. If the bcrypt cost is high enough, even brute forcing 6 alphanumerical characters may become unfeasible for attackers with high computing resources.

[jordan-wright]

I don't think we should make a configurable password policy. I'm very picky when it comes to adding options to Gophish, but I promise it's for a good reason: while it's the "flexible" thing to do, it very easily results in an overwhelming UX.

Instead, I try to choose reasonable defaults and be opinionated, at the benefit of making the tool easy to use. To that end, let's just go with 8 characters for now (of course, with no max length), with no other restrictions. I'd also like to add reasonable rate limiting middleware to prevent brute force on the web server (though bcrypt should already be helping with that a little). I'm open to suggestions on what that rate limit should be.

This is a step up from what we have now, and an encouragement to use a decent password. It's already recommended to restrict access to the Gophish dashboard to a whitelist of IP addresses (if not just on localhost, which is the default) so if someone is willing to expose the dashboard to the broader Internet and use a weak password, they will. I'm not convinced an 8 or 12 character limit will make a huge difference there (compared to simply rate limiting the login page), and it would cost usability.

I appreciate the great discussion and the perspectives being offered. @eickeh if you need any help getting these PR's in shape, please don't hesitate to let me know. Happy to help any way I can.

[glennzw]

I think it's good to take advice from industry standards like OWASP or NIST:

OWASP Authentication Cheat Sheet
OWAP Password Storage Cheat Sheet
NIST Digital Identity Guidelines

Some thoughts I have:

Default Admin Creds
The default 'admin/gophish' credentials could be problematic. A googledork or ShodanHQ search finds a bunch of internet exposed GoPhish login prompts, possibly with default creds. Two easy solutions I can think of that won't get in the way of people testing locally and not wanting to change passwords:

• A nag at the top of the screen when logging in about changing admin password
• Generating a random password on first run of the gophish executable, and printing it out in the console.

Jordan's suggestion for user accounts requiring password change with the boolean flag sounds reasonable.

Password Hashes
GoPhish currently uses bcrypt with bcrypt.DefaultCost. The DefaultCost is 10, and could be raised to a maximum of 31 (MaxCost). The OWASP and NIST guidelines are intentionally vague on what value is 'best' and we can debate it all day here (or read various debates online).

The cost value is typically 'what you can afford on your hardware', so my suggestion here would be to leave it in the hands of the user. Either they can edit gophish/util themselves and compile, or we could have an option in config.json. Either way this is power user territory, and in my opinion the default is fine for this use case and threat model.

A bonus of the bcrypt.GenerateFromPassword function is that is creates a salt for us, mitigating offline / rainbow table attacks, so our attack scenario here is a compromised database and brute force. Increasing the cost would make this harder, but also open the server to Denial-of-Service attacks if set too high.

If this was a new application and we were at the specifications stage, I'd suggest using Argon2, winner of the Password Hashing Competition.

Password Complexity
I don't like prescriptive password requirements, especially requiring certain characters. Minimum password length might be worth considering, as it's currently one character minimum. NIST would recommend 8 characters.

I'd favour giving the user advice, rather than being prescriptive. e.g. using DropBox's zxcvbn and/or check against haveibeenpwned.com with hibp-js.

To protect against online brute forcing (against the web login) I'd suggest a short delay in controllers/route.go#L277 for incorrectly entering a password, with a maximum value to avoid account lockout DoS (the three hour suggestion would allow for a DoS condition):

sleepFor := math.Max(consecutiveBadLogins, 5) time.Sleep(time.Duration(sleepFor) * time.Second)

The consecutiveBadLogins value could be stored in the database, and reset to 0 after a successful login.

If we're being pedantic we could also have a maximum password length of 1,000 to avoid DoS attacks whereby bcrypt continuously computes million length passwords.

Automatic Logout / Session Expiration
Again I'd rather not be prescriptive. I'd favour a 'Keep me Logged In' button on login if anything. The other consideration is do we want to allow simultaneous logins. I don't think it's a problem with GoPhish, but the recommendation is usually to terminate the older one.

2FA
While we're on the topic of authentication, why not a bit of 2FA. I hear Duo as some nice solutions, Jordan ;)

[jordan-wright]

Thanks for sharing these perspectives @glennzw!

Default Admin Creds

I agree that the default credentials are a problem. While my initial solution was simply forcing the admin to change the password the first time they hit the admin dashboard, I do think there's value in closing that short gap by printing a random password to the console (but note: still requiring the admin to change the password on first use).

Password Hashes

Let's leave bcrypt alone for now. The default cost should be fine for our threat model. I see no reason to increase it at this time, and don't see that as important enough to warrant a configuration option.

Password Complexity

I'm with you. I'd be fine incorporating zxcvbn on the frontend, though I see that as a next step after this first round of changes in terms of priorities.

Otherwise, I'd like to just set an 8 character limit and call it a day. If people want to choose a bad password, they will. They are hosting the service, they own that responsibility. We can give guiderails, but I don't want them to be so tight that usability suffers.

I do agree about setting a rate limit. I think this would best be handled via a middleware layer, though the exact implementation would be tbd.

Automatic Logout / Session Expiration

I'm with you here. I wouldn't do anything in this space for now.

2FA

Heh, I'd be all for adding 2FA support. I don't personally have the cycles to knock that out right now compared to other work on my plate, and I would also see this as a next step after the more tactical fixes.

Thanks for taking the time to send in those suggestions! It seems that we are largely very much on the same page.

[eickeh]

Sorry for the delay but I finally found some time to work on this issue (PRs will follow). Currently, I'm implementing the rate limiting. For this, I added two new fields to the users table:
1) failedLogins: integer counting the number of consecutive failed logings. Is set to 0 on successful login
2) lockedUntilDate: Datetime until the user is locked after a number of consecutive failed logins (I picked 5).

So, with these numbers, after a user failed 5 times in a row, he/she is blocked for a configured timeout (I picked 30 seconds). After 5 additional attempts, that doubles to 1 minute and so on until a maximum timeout of roughly 4 hours .

While this approach is pretty simple to implement, we can't give the user feedback about the lock. If we did that, we'd admit that a user with this name exists and therefore give an attacker a tool to find out existing usernames. But without feedback, it can be pretty frustrating for the user.

Another approach would be to create a new table in the database counting login attempts for arbitrary user names (not only registered usernames) and performs the same logic as above. That would mitigate the above problem but introduces the possiblity to flood our database with rate limiting entries.

A possible solution could be the following. Increase initial timeout and the number of failed logins before triggering a lock to really high values. Let's say 100 logins and 2 hours. A normal user would practically never reach that and machines have to perform 100 times the work to figure out a single username.

What do you guys think about that?

[jordan-wright]

Hey @eickeh!

Thanks for following up! My apologies for not mentioning this in the thread, but I have a local branch that has about 90% of this implemented (rate limiting, password policy, etc.).

I can get the local branch uploaded today and add a note here with what’s done and what’s left to do if that’s something you’d be willing to pick up and take over the finish line!

[jordan-wright]

Hey @eickeh! Just a heads up I've pushed the updates (2 commits) to the 1538-password-policy. A bunch of that will follow exactly what we talked about above. If I have time later this evening, I'll drop in some more comments here to better describe what's left to do.

[eickeh]

Hi @jordan-wright!

Thanks for following up! My apologies for not mentioning this in the thread, but I have a local branch that has about 90% of this implemented (rate limiting, password policy, etc.).

Well, shit happens. But don't worry, you did a much better job anyway ;)

Hey @eickeh! Just a heads up I've pushed the updates (2 commits) to the 1538-password-policy. A bunch of that will follow exactly what we talked about above. If I have time later this evening, I'll drop in some more comments here to better describe what's left to do.

Wow, nice work, looks relly neat! For me it seems like the features are all implemented except some minor TODOs. Is that right?
Maybe I can finish some of these to get these features into production.