#32130 closed Bug (fixed)
Password reset token incompatibility.
| Reported by: | Gordon Wrigley | Owned by: | Mariusz Felisiak |
|---|---|---|---|
| Component: | contrib.auth | Version: | 3.1 |
| Severity: | Release blocker | Keywords: | |
| Cc: | Adam Johnson, Hasan Ramezani | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
As noted here https://docs.djangoproject.com/en/3.1/releases/3.1/#django-contrib-auth the hashing for password reset tokens has changed between 3.0 and 3.1 and work has been done to ensure existing tokens will still work (at least until 4.0).
However the encoding of the token creation time has also changed. Specifically from days since 1/1/01 to seconds since 1/1/01. And it appears no work has been done to support tokens with the older values. So a token generated on Oct 1, 2020 will come through as 7213 days which will then get interpreted as 7213 seconds, aka 2am Jan 1, 2001.
So while exiting tokens in the wild will pass crypto validation they will all show as expired if your PASSWORD_RESET_TIMEOUT is less than ~20 years.
The code base I'm working on uses these tokens (perhaps unwisely) in some email links that are expected to have a 3 month lifetime and an upgrade from 3.0 to 3.1 looks likely to render all the tokens in the wild expired which is suboptimal.
Change History (13)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
| Cc: | Adam Johnson added |
|---|
comment:3 Changed 3 years ago by
We've been discussing using patchy to do something to that effect.
With free code access you can easily tell because the prefix is 3 characters in the old scheme and 6 in the other so you could switch off that.
If you were to switch off value anything in the 10k (~27*365) - 600m(~19*365*60*60) range should be a safe cutoff.
comment:4 Changed 3 years ago by
Great catch, we missed this. However, I'm not sure what we can do, any heuristic will be clunky, IMO. 1 second, 1 day, 300k seconds, or 300k days are all valid.
comment:5 Changed 3 years ago by
It’s seconds/days since January 2001, so I don’t think it’s clunky... I doubt anyone is running an app with the clock set between 2001 and 2002 for example.
comment:6 Changed 3 years ago by
I think the prefix length might be a better check, it's totally unambiguous
comment:7 Changed 3 years ago by
| Cc: | Hasan Ramezani added |
|---|---|
| Component: | Uncategorized → contrib.auth |
| Owner: | changed from nobody to Mariusz Felisiak |
| Severity: | Normal → Release blocker |
| Status: | new → assigned |
| Summary: | 3.0 -> 3.1 password token incompatibility. → Password reset token incompatibility. |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
I think the prefix length might be a better check, it's totally unambiguous
Ahh, yes. That's bulletproof, thanks!
Regression in 45304e444e0d780ceeb5fc03e6761569dfe17ab2.
comment:9 Changed 3 years ago by
| Triage Stage: | Accepted → Ready for checkin |
|---|
This seems like an oversight. Would it make sense to reinterpret tokens with a timeout less than some small-in-seconds but large-in-days cutoff, such as 3600 * 365, as days instead of seconds?