New Feature: Allow password reset token to expire in under a day

[Zhiqiang Liu]

I need general consensus on how to proceed with supporting password expire time to be under a day. Currently it is not possible because we use PASSWORD_RESET_TIMEOUT_DAYS.

In ticket 28622 we have two options. 

One is to continue to use the same setting PASSWORD_RESET_TIMEOUT_DAYS, but change the value to non-integer (such as timedelta) so we can send hours, minutes, etc to it.

The other one is to create a new setting like PASSWORD_RESET_TIMEOUT which takes seconds.To support backward compatibility, I think we should keep PASSWORD_RESET_TIMEOUT_DAYS and its default value of 3. Only use PASSWORD_RESET_TIMEOUT when provided.

I'm unsure which one is better, so inputs are welcome.

[Tom Forbes]

I think we shouldn't shoe-horn a timedelta into the existing setting, so my vote is with the second option, but I think a timedelta is much more readable than just an integer.

Also, the existing 3 day timeout for password links is quite surprising from a security point of view. The consultants I work with would flag up a token that lasts longer than 12 hours as an issue during a pentest. 

IMO a new, far shorter default should be added to this setting.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/c8e96008-eb95-4924-8e5e-9b02d6b90c99%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Sjoerd Job Postmus]

To be honest, I'm quite surprised that the password reset feature does not use `TimestampSigner` which already supports timedeltas explicitly.

Is the Signing backend overkill for this? Probably yes. But I think using the signing backend still makes sense since it's already there. So if one were to move from day-based timeouts to second/timedelta based timeouts, one might as well use the TimestampSigner.

On Thursday, September 21, 2017 at 10:22:20 AM UTC+2, Tom Forbes wrote:

I think we shouldn't shoe-horn a timedelta into the existing setting, so my vote is with the second option, but I think a timedelta is much more readable than just an integer.

Also, the existing 3 day timeout for password links is quite surprising from a security point of view. The consultants I work with would flag up a token that lasts longer than 12 hours as an issue during a pentest. 

IMO a new, far shorter default should be added to this setting.

On 21 Sep 2017 03:56, "Zhiqiang Liu" <zachl...@gmail.com> wrote:

I need general consensus on how to proceed with supporting password expire time to be under a day. Currently it is not possible because we use PASSWORD_RESET_TIMEOUT_DAYS.

In ticket 28622 we have two options. 

One is to continue to use the same setting PASSWORD_RESET_TIMEOUT_DAYS, but change the value to non-integer (such as timedelta) so we can send hours, minutes, etc to it.

The other one is to create a new setting like PASSWORD_RESET_TIMEOUT which takes seconds.To support backward compatibility, I think we should keep PASSWORD_RESET_TIMEOUT_DAYS and its default value of 3. Only use PASSWORD_RESET_TIMEOUT when provided.

I'm unsure which one is better, so inputs are welcome.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

[Eddy C]

I think Minute, with default value 30 or 60, is the best unit for this setting.

3 minutes (even 1) is short enough for edge case and 720 (12 hours) also looks good.

On Thursday, September 21, 2017 at 6:22:20 PM UTC+10, Tom Forbes wrote:

I think we shouldn't shoe-horn a timedelta into the existing setting, so my vote is with the second option, but I think a timedelta is much more readable than just an integer.

Also, the existing 3 day timeout for password links is quite surprising from a security point of view. The consultants I work with would flag up a token that lasts longer than 12 hours as an issue during a pentest. 

IMO a new, far shorter default should be added to this setting.

On 21 Sep 2017 03:56, "Zhiqiang Liu" <zachl...@gmail.com> wrote:

I need general consensus on how to proceed with supporting password expire time to be under a day. Currently it is not possible because we use PASSWORD_RESET_TIMEOUT_DAYS.

In ticket 28622 we have two options. 

One is to continue to use the same setting PASSWORD_RESET_TIMEOUT_DAYS, but change the value to non-integer (such as timedelta) so we can send hours, minutes, etc to it.

The other one is to create a new setting like PASSWORD_RESET_TIMEOUT which takes seconds.To support backward compatibility, I think we should keep PASSWORD_RESET_TIMEOUT_DAYS and its default value of 3. Only use PASSWORD_RESET_TIMEOUT when provided.

I'm unsure which one is better, so inputs are welcome.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

[Adam Johnson]

Why not just keep PASSWORD_RESET_TIMEOUT_DAYS and allow floats? Then you can just do 1/24 for an hour.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6d0d4251-64bc-40a0-b191-9cf3dfe8c91b%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Dylan Reinhold]

I agree about deprecating PASSWORD_RESET_TIMEOUT_DAYS, with no rush to remove. Then if PASSWORD_RESET_TIMEOUT it takes precedent.

Now for the input to PASSWORD_RESET_TIMEOUT looking at current settings in django, anything I found that is time/age based is expressed in integer seconds.

I would say stay with seconds,

Dylan

--

[charettes]

That's what I proposed on the ticket but I feel like it felt odd to me, the setting name does't suggest this is possible and it might be hard to achieve exact second precious because of float rounding?

In my opinion introducing PASSWORD_RESET_TIMEOUT with timedelta support would be the best option.

Simon

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6d0d4251-64bc-40a0-b191-9cf3dfe8c91b%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Zhiqiang Liu]

Yeah I don't think float number of days is a good choice because the calculation will be weird with precision issues.

I think it makes sense to use PASSWORD_RESET_TIMEOUT. For timedelta vs. integer seconds. Timedelta has the benefit of readability, but integer has the benefit of simplicity. I think in SETTINGS everything should be as simple as possible, so I think integer seconds is a better choice here. And it is used in most applications too.

[Dylan Reinhold]

I still think seconds are the way to go, but maybe the documentation could give a clue that timedelta().seconds can be used for readability
PASSWORD_RESET_TIMEOUT = datetime.timedelta(hours=6, minutes=30).seconds

Dylan

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2857fb9f-4959-4d08-91ca-1287fd3d8246%40googlegroups.com.

[Zhiqiang Liu]

If most agree, I will proceed with using seconds.

It is a good idea for the potential documentation Dylan!

Zach

[Tom Forbes]

I would still vote for a timedelta, im not sure if there is a strong consensus in the thread. 

Representing the time as seconds always irks me, you can make it more readable by using multiplication but you often end up with a comment anyway and it doesn't scan nearly as well. Having to do 'timedelta.seconds' is OK, but it seems a bit like busywork. 

It's a small thing but I don't see any practical problem with just accepting a timedelta, they are nicer to work with in the settings file itself and within Django, especially if the TimestampSigner accepts them natively and we start to use that.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d2c48f2a-7017-4ffc-9f33-0cfdb1db31de%40googlegroups.com.

[Tom Forbes]

It also seems odd to express it as seconds, it's often going to be a large value between an hour and several days and the lowest resolution for the value anyone would need is minutes.

[Collin Anderson]

Seconds is consistent with all of the other settings, even for long ones like CSRF_COOKIE_AGE and SESSION_COOKIE_AGE. It also means you can avoid importing datetime in your settings file.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFNZOJMLiFj0csHPJ1_cNfxBizG2T_UzHX7oV9%3DS4zhkRfA7DQ%40mail.gmail.com.

[Florian Apolloner]

+1 for consistency

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d2c48f2a-7017-4ffc-9f33-0cfdb1db31de%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

[Adam Johnson]

+1 for consistency too. You can always use a timedelta if you want, with total_seconds() (not seconds as mentioned before😉).

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/56d501f3-6bc2-4869-b935-0cda273b78c9%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Zhiqiang Liu]

+1, also it is a good point that all things in settings should be simple, complex date object does not belong there, at least should not be required.

It should be for the majority of users as possible.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/56d501f3-6bc2-4869-b935-0cda273b78c9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Adam

[Luke Plant]

I would be +1 to what Adam wrote from me i.e. just allow the value to accept floats.

However, I don't think it will work due to the way that we round the precision of timestamps to days. This was done partly to reduce the number of characters needed to express the timestamp, to keep URLs as short as possible. We would  have to change the mechanism to store more precision into the timestamp. This would result in an upgrade 'bump' for users (i.e. links generated before the upgrade would become invalid after upgrade).

However, I really question whether we need any change here, and whether it would be a good idea.

Having a short expiration time (less than 1 hour) could cause major problems for some people - plenty of systems introduce 5 or 10 minute delays in mail delivery, and with some people's internet connection it can take several minutes to open a web page. This also means that some people end up finishing the process of whatever they were doing the next day (I know I've done this several times on various sites), so a timeout of at least 1 or 2 days is a good default. If you want to come back after the weekend and carry on, 3 days makes more sense as a minimum.

In terms of security, I don't think there is really any need for anyone to reduce below the default at all (see below). So I'm very unconvinced about the need for changing to PASSWORD_RESET_TIMEOUT - it is just unnecessary upgrade work for some existing projects (this is the biggest consideration for me), and it could encourage people to set the value to something low that would decrease usability.

Security:

The security of the password reset feature is almost entirely independent of the value of the timeout setting. There are 3 attack vectors I can see:

1) Someone's email account is compromised, and they then do a password reset on a Django site.

We simply can't protect against this AFAICS.

2) Someone's email account is compromised, and they find/use a password reset email in the person's inbox.

This is the only scenario for which having a shorter timeout makes a difference. It is somewhat unlikely, because in 99% of cases the attacker would be able to generate a password reset email themselves after compromising the account. For this narrow case where the attacker is unwilling/unable to trigger/receive a new password reset email, it is worth having some protection against them being able to use old tokens, but 3 days seems plenty short enough for this situation, especially given the fact that a *used* password reset token immediately becomes invalid due to the way we hash on internal state of the user record.

3) A brute force attack.

To do this, the attacker has to:

1. Supply a user ID (let's assume this is easy)

2. **Choose** a timestamp (very easy, just choose the current time)

3. Create a 20 character hexadecimal hmac that matches both the timestamp and the internal state of the user (see https://github.com/django/django/blob/master/django/contrib/auth/tokens.py ).

Since the attacker can choose the timestamp, the probability of guessing correctly depends *only* on:

1. The number of bits in the hash (20*4 = 80)

2. The number of attempts (or, the number of requests per second possible and the time available)

It does *not* depend on the value of the reset timeout *at all*.

If we assume they can make 100 req/s, and they try continuously for 10 years, they've got a chance of around 1 in 10^13.

In other words, I reject the premise of the ticket, which is that to improve security some people need to reduce the timeout. It makes virtually no difference to the security of this feature, and in fact you would be protected against almost all realistic attacks if there was no timeout. I imagine that the requirement of "meeting security requirements" mentioned on the ticket is due to people who think this works like a short, 6 digit OTP, for which 3 days would be far too long ( see https://sakurity.com/blog/2015/07/18/2fa.html ). We could put a note in the docs about this, I don't know how to do that in a succinct way apart from to link to a copy of this email or something.

However, if we really do 'need' this change, we should at least keep the default to what it is now, and put a notice in the docs saying that reducing it hurts usability and makes no practical difference to security. Since we would be causing an upgrade bump and breaking existing links, we may as well also switch to TimestampSigner (the password reset code was originally written before that existed), which would also mean changing urlconfs I imagine. This would also require a significant section in the upgrade notes. (In my book, this is a further argument against doing this change at all).

Regards,

Luke

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM2Y-krvLkKwxvNp%3DEuLa-oMDhuB%2BkxABwE5Ae76LOPPdw%40mail.gmail.com.

[Zhiqiang Liu]

Luke,

thanks for the long explanation. I see your points here. I actually saw the make token function and was thinking about it what is the best way to do with that. I think most people here feel there's need to at least allow some flexibility for the time out since there will be cases under a day is needed.

I will keep this discussion for a couple of more days to see if we can get consensus and how we should implemented if needed.

Zach

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6d0d4251-64bc-40a0-b191-9cf3dfe8c91b%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Luke Plant]

Hi Zach,

To be clear, I'm not totally opposed to the change. There could certainly be advantages to updating the code, especially if we move to TimestampSigner.

However, from a quick look, I'm not sure if TimestampSigner will be possible - we want the hashed value to incorporate some internal data, but don't want that internal data to be part of the signed message that Signer produces (for various reasons, including the fact that we want to keep the URL as short as possible for maximum compatibility). I think we should also be careful not to change the public interface of PasswordResetTokenGenerator, since people may have subclassed it, which limits the amount of cleanup you can do here.

I imagine that changing the timestamp to support better precision will also create work for people upgrading (the setting change, plus URLconf changes).

What I am suggesting is that we should seriously consider whether all this is worth it given the extremely marginal security benefits.

Regards,

Luke

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d82d874b-a076-44ee-a091-ee3924570053%40googlegroups.com.

[Nick Zaccardi]

There are cases where corporate security policies require (regardless of the actual security they provide) which require a <24hr link. Here is a x/post from this ticket:

I do want to explain why this doesn't meet the 1% of use cases. When I originally reported this I was working on a password reset feature in a different app (a large corporate financial application) which has very specific policies on passwords, password resets, and the validity time of both. From a contractual perspective (regardless of user experience) >24hr link would be a break in policy or worse a violation of contractual obligation to implement a <24hr link. For most up to 2 days is fine, for some, regardless of the real-life implications of the policy, it is a big deal.

[nleb...@fibrenoire.ca]

+1

Would also be nice if there was a way to "clear / invalidate" current token for a given user