5 ms·

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

by wulczer 14y ago

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.

tptacek 14y ago
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.
- wulczer 14y ago
  I just gave django.contrib.auth.tokens a read and shared my opinion on it :) I wouldn't roll my own password reset feature if I can just take the builtin one from Django, which is what I did.
  - tptacek 14y ago
    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: Good. 3rd Party Library: Less Good Just Using A Random Token: Good Cryptography: You Will Perish In Flames
    - thibaut_barrere 14y ago
      Does Devise [1] qualifies as a properly reviewed 3rd Party Library, to your knowledge? Asking this since it's probably the most widely used authentication gem in Rails etc... [1] https://github.com/plataformatec/devise https://github.com/plataformatec/devise
huxley 14y ago
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. https://docs.djangoproject.com/en/1.4/ref/settings/#std:setting-PASSWORD_RESET_TIMEOUT_DAYS https://docs.djangoproject.com/en/1.4/ref/settings/#std:sett... 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): https://github.com/brutasse/django-password-reset/ https://github.com/brutasse/django-password-reset/ http://django-password-reset.readthedocs.org/en/latest/ http://django-password-reset.readthedocs.org/en/latest/
- shabble 14y ago
  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.
wglb 14y ago
This makes me a bit uncomfortable. I am more comfortable with a semantically-meaningless cryptographically random token. Additionally, this token could be valid for a very long time. I would probably flag this approach in an assessment.
shabble 14y ago
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.

by wulczer 14y ago

tptacek 14y ago

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.

wulczer 14y ago
I just gave django.contrib.auth.tokens a read and shared my opinion on it :) I wouldn't roll my own password reset feature if I can just take the builtin one from Django, which is what I did.
- tptacek 14y ago
  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: Good. 3rd Party Library: Less Good Just Using A Random Token: Good Cryptography: You Will Perish In Flames
  - thibaut_barrere 14y ago
    Does Devise [1] qualifies as a properly reviewed 3rd Party Library, to your knowledge? Asking this since it's probably the most widely used authentication gem in Rails etc... [1] https://github.com/plataformatec/devise https://github.com/plataformatec/devise

huxley 14y ago

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. https://docs.djangoproject.com/en/1.4/ref/settings/#std:setting-PASSWORD_RESET_TIMEOUT_DAYS https://docs.djangoproject.com/en/1.4/ref/settings/#std:sett... 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): https://github.com/brutasse/django-password-reset/ https://github.com/brutasse/django-password-reset/ http://django-password-reset.readthedocs.org/en/latest/ http://django-password-reset.readthedocs.org/en/latest/

shabble 14y ago
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.

wglb 14y ago

This makes me a bit uncomfortable. I am more comfortable with a semantically-meaningless cryptographically random token. Additionally, this token could be valid for a very long time. I would probably flag this approach in an assessment.

shabble 14y ago

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.