Nobody ever gets password resets right. They're #1 in our list of 7 Deadly Web App Features. Even if you think you understand your password reset, and can fit all its workings in your head, I recommend you go look at it closely again.
No; every implementation fails in some new and creative way.
The implementation I feel most comfortable with:
1. Generate a 256+ bit cryptographically secure random number and base64 it to create a token.
2. Record that token in your database, timestamped, along with the user account for which the token was requested.
3. Mail the token to the user's email address.
4. When the user returns to the site after recovering the token, use that token to look up their account from the database.
5. Expire tokens within single-digit hours so users don't end up accidentally banking password-equivalents in their email accounts.
6. When a user changes their password or requests another password reset, expire all tokens already associated with their account.
I would also recommend:
(a) Not having any in-band administration functionality in your application; instead, have a separate admin application, attached to the same databases, available only on a VPN.
(b) Require 2-factor authentication (such as Duo Security) for both admin VPN access and admin login.
A knock-on benefit of (a): your admin functionality is easier to build, because it doesn't have to do the UI/UX chinups your normal exposed app code has to do; crappy looking admin screens nobody but your employees see are generally fine.
I commented on the other thread, but will repeat here: I really like how Django handles password resets.
No nonce is generated and nothing is stored. The user is emailed a link with her user ID and a token that's a hash of (last login timestamp + the user's ID + the user's (hashed) password + current timestamp). The token is HMAC-signed with the site's secret key.
This way the token automatically expires if the user either successfully changes her password (the password hash will change) or manages to log in (last login timestamp changes).
It seems that in Django password reset tokens are valid forever, but it would be trivial to add the current timestamp to the token and include it when computing the HMAC signature; then the password reset form would check if the token has been generated recently enough.
I like this method because you never need to touch the database and store tokens; it's all fairly stateless.
If you want to get clever to avoid touching the database --- which is why every broken password reset feature ever conceived decided to get clever --- you should have your code assessed professionally. I'm not telling you it's impossible to build a secure password reset that doesn't simply store a token in the database. I'm just saying the cost/benefit payoff is probably not there.
Personally, I have this particular bit of appsec down cold, and if I was building a new app, I wouldn't even think about it: I'd use a random token and save it in the database.
The good thing about taking your web stack's password reset feature, if it has one, is that you're probably going to find out quickly if there's a bug discovered in it.
Note though that that's not the case for 3rd-party password reset libraries or, more likely, the all-purpose security library that provides it. I'd be very wary about using a 3rd party library for password reset unless they've got a credible for story for it having been reviewed.
Django added a timeout period to the password reset in Django 1.2, it defaults to 3 days, the only issue I'm aware of is that it calculates the timeout in days which isn't very convenient if you want to be extra-careful.
Haven't looked too closely at it but Bruno Renié has a Django app that has done most of the heavy-lifting to make password reset customizable by providing as class-based views and changing the timeout period granularity to seconds (it defaults to 172800 seconds or 2 days):
The django-password-reset app appears to be pretty much equivalent to the django.contrib.auth.views version, except, as you say, with CBVs and perhaps some extra customisability in the templates and user lookup methods.
The main criticism of both this and the contrib.auth are that they use the (hashed) user info directly, rather than generating a random code and associating it with the user in a lookup table.
This app actually appears (on my brief inspection) to be less secure than the django.contrib.auth one, since it is using the django.core.signing.loads/dumps methods with a simple static salt, whilst the contrib.auth uses
django.contrib.auth.tokens.PasswordResetTokenGenerator which includes a bunch more state in the token, so that it's auto-invalidated if the user subsequently logs in, changes their password, or other things).
Personally, I wouldn't recommend it.
I'm torn between the "standard" contrib.auth implementation, and the basic {random token, user, expiry} model espoused by tptacek and others throughout this thread.
I am curious as to why the Django devs implemented this the way they did though, given the significant added complexity.
It appears better than most [of the non-DB] implementations I've seen, and as huxley mentions downthread, newer releases do have settings.PASSWORD_RESET_TIMEOUT_DAYS which defaults to 3 to deal with timeouts.
It doesn't handle point 6 of tptacek's list though; that subsequent tokens should invalidate all those prior. You could do that by adding an incrementing 'password_resets' or 'last_reset_issued_at' field to the User model, and including that in the token generator state, but it feels a bit clunky.
I'm not sure why people are placing such an emphasis on DB avoidance; it seems to me that password-reset activities should be a relatively minor source of load for your application in virtually all circumstances.
If you have to use any cryptography other than "generate random number" to implement password reset, you should be ready to cough up $7-14k for a professional security assessment.
I'm not trying to drum up business. I would strongly prefer that you not try to be clever with this feature. Or, you can ignore the audit and just be the subject of a blog post like this sometime in the future.
With respect to (a), for the applications that require some level of admin function, I say to not allow password reset for those higher-level priv accounts.
The difference is that as soon as the token is used, even unsuccessfully, it is whacked from the database. Whether or not password reset is actually completed.
No, this is unnecessary. An attacker who can pull tokens out of the database already has a terrible vulnerability and can probably avoid the need for passwords altogether.
I've tested many many many many applications in the last X years and not one of them has ever done this, nor would I ever recommend that they do it.
> [...] and can probably avoid the need for passwords altogether.
Probably, yeah...
In the case they only have read access to the token DB, and need write access, or read access to another part of the system, it becomes an attack vector.
It is not a cargo-cult measure, and it doesn't add much complexity (just re-use your password encoding logic).
The same goes for session tokens, BTW.
> [...] nor would I ever recommend that they do it.
Maybe you should reconsider that. I know who you are, and how knowledgeable and experienced you are, but in this case I think you're just wrong.
I have never, ever, ever, ever seen an application one-way hash their reset tokens to prevent disclosure of them from their database. That's all I'm going to say here.
What I'll argue is that it's not a best practice, because nobody practices it. I also think it's a silly countermeasure, but I don't feel like I need to defend that argument.
I would not hash reset tokens, but I didn't downmod you for suggesting it. I would get mad at you if you worked on my team and dinged a client for not doing it, though. :)
> What I'll argue is that it's not a best practice, because nobody practices it.
And now you're begging the question!
Actually, some people use it :-).
It shuts down an attack vector, and it's cheap to implement. Why is it silly? My rule of thumb is to treat all passwords equivalents in the same way.
I'm honestly surprised by your hostility towards the idea (not mine, BTW), and by the downvotes for promoting a strategy that provably (in the math sense) increases security.
6. Templating (as an app feature, not as a dev tool).
7. "Advanced Search".
This list is a couple years old. We're going to start tying bug reports to functional areas in targets to get a better empirical list by the end of the year. I expect the new empirical list to be more boring and less useful, though.
We take very hard looks at both inbound and outbound email functionality. Bunch of reasons:
1. Breaks out of the web security domain, requiring devs to think through and implement controls that compensate for things like sessions and access control.
2. New quoting domain creates opportunities for injection to leverage apps to send unexpected messages.
3. Often involves shelling out, with all the attendant risks of that.
4. Inbound mail has different input restrictions than web apps do, creating opportunities for submarined XSS or even SQLI.
It's just a really common place where apps suddenly sprout unexpected moving parts, is what it boils down to.
It is very pleasing to see companies like heroku publicly thanking (we even got a blog link here) the security researcher that initially reported a vulnerability. Hopefully, we'll see more of this in the future.
What am I missing here? This is what I do;
- User resets password, email link to user
- User clicks link, random temporary password emailed to user
- User logs in with temp password, system then asks them for new password.
This way there is no risk of someone somehow guessing the reset link. Even if they do, all it does is email the user, so they gain nothing...
Random temporary password is a terrible idea. It is both less usable and less secure than sending a secure random token and asking the user to provide a new password. Many users will never change the temporary password. All of them will leave the password in their mail spool. Without even more database signaling, the "resets" will never expire, because they are just passwords.
No - the system first sends the secure random token, then instead of asking them for a new password then, it instead auto-generates a random password and emails it to them.
Then the system FORCES them to change the password when they login with the new password.
Oh - and the tempoary password only valid for 24 hours.
So you get the best of both worlds - without anyone being able to 'guess' anything able to do anything...
The system forces them to change the password if they actually log in. If they don't, the password just sits there. And some users will just cut/paste it.
I'm sure there's a million fiddly things you can do to address the weaknesses of temporary password issuance, but you'd be better off sending a semantically meaningless random token. All the countermeasures you're thinking of here apply identically to the token.
I am advocating for the reset scheme that is the hardest to mess up. Yours is not the hardest to mess up. I'm not trying to get you to change yours.
How is this for a "decently secure" password reset method ?
- User enters email address to reset password
- A reset link (uniquely hashed,salted etc.) is generated server side tied to that specific email address. This link can only be used once and also has a expiry date if un-used by that time.
- If this reset link is accessed (hopefully by the intended user), A form is presented to the user where it asks to enter the email address, new password. If entered email address does not match the original email, user gets an error. Immediately, all sessions are invalidated/reset if any. If entered email address matches, then reset the user's password. Again, invalidate/reset any existing sessions.
- After resetting password, never login the user directly. Ask them to login manually again.
The only way this fiddly scheme could be secure is if the randomizing information (that prevents an attacker from brute forcing the hash against all possible email addresses) is different for every token and thus stored somewhere serverside. It is no more secure than simply sending the user a random token, but has all the logistical challenges of storing a token.
> After resetting password, never login the user directly. Ask them to login manually again.
What vulnerability could this close? In order to finish the reset procedure in the previous step, any attacker would need to know the user's email address, and the attacker would obviously know the new password that was just set. I'm probably missing something, but I don't see this extra step adding any security.
I think the large source of problems for password resets is not how the flow is designed from the user perspective, but how it is implemented to handle corner cases that normally rarely occur. Such corner cases are likely to be not well though-out and poorly tested.
For example: A user with a valid session cookie for one account, follows a password reset link for some other account. Or a user generates a password reset link, logs in with an old password, changes the password and follows the reset link.
There are many such tricky cases and the challenge is to explicitly design how they should be handled and cover them with tests.
It is this kind of thing --- the password reset code not expecting ever to be hit from a logged-in session, because it's used in the real world exclusively by people who can't log in --- that create 1/2 the bugs in the real world with password resets. You will find blatant bugs that nobody in the world could possibly miss, except that everyone on the dev team missed them because who tests a password reset from a logged-in account?
"What? Every page gets 'current user' from a common header included in every file that it pulls out of the session, and this page also takes 'email address' from a parameter passed in by the user? OOPS."
"While both Mr. Sclafani and Heroku endeavoured to use test accounts exclusively, a very small number of customer account passwords were reset during the incident"
Likely due to "related issue in the password reset flow that could be used to reset the passwords of a certain subset of users at random"
I would have liked to see more details about what was going on to cause that sort of problem. I can't think of any reasonable code that would cause password resets for an empty ID to be assigned to a random account.
#Start a password reset
@user = User.find_by_email
@user.update_attribute :password_reset_nonce, rand(16 ** 16).to_hex
#mail them the password reset email
#Look up a user for a password reset
@user.find_by_password_reset_nonce params[:password_reset_nonce]
#if nil, above line returns random user on some databases. Oops.
session[:user_id] = @user #Logs in user
#We assume if they know the nonce that proves they own email.
redirect_to passwordReset_url
I've elided additional code which might theoretically be used to make the nonce expire and to prevent re-use for brevity, but it's possible that neither of these measures would fix the issue.
Bugs like this one always make me think "There but for the grace of God go I."
It may be evaluated as "SELECT * FROM USERS WHERE password_reset_nonce IS NULL". If your password_reset_nonce field may contain null values, this will fetch the first row of those it finds. Some databases show quasi-random behavior when fetching that row.
Doing params.fetch(:password_reset_nonce) would get you out of that nil issue. Doing fetch is generally good practice, especially on params, as often without the right params you can't do anything anyway and should be bailing out with a 404 or another error code.
