#31842 closed Bug (fixed)
django.core.signing.dumps() and loads() not backwards compatible
Reported by: | Markus Holtermann | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | Core (Other) | Version: | 3.1 |
Severity: | Release blocker | Keywords: | |
Cc: | אורי | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In Django 3.1, the default algorithm for django.core.signing.Signer
was changed from sha1 to sha256. That's good!
However, the django.core.signing.dumps()
and django.core.signing.loads()
functions don't expose the algorithm. I think, that that makes an upgrade from 3.0 to 3.1 impossible when one uses the dumps()
function in a multi-node Django setup.
Let's say there are two servers. Both run an application with Django 3.0. and make use of dumps()
and loads()
. Then server 1 is upgraded to use Django 3.1. At this point, because of to the backwards compatibility parts in the decoding/signature validation, a token signed by server 2 can be loaded on server 1 and 2. However, a token signed by server 1 cannot be loaded on server 2.
This problem could be mitigated by exposing the algorithm in the dumps()
method and setting it to sha1
. Once all servers run Django 3.1, the algorithm could be changed to sha256
.
Change History (14)
comment:1 by , 4 years ago
comment:2 by , 4 years ago
I'm skeptical that Django should facilitate running multiple Django versions concurrently.
I think we should allow find a way to multi-node upgrades to happen just like we do with pickle_version
for anything that uses pickle.dumps
. Large project Django upgrades are often tentatively deployed through canarying and eventually rolled out to multiple nodes gradually (pods, dynos, etc.).
Not having a way to do a graceful roll out of a version should be considered a release blocker IMO.
comment:3 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Adding the algorithm='sha256'
parameter to loads()
and dumps()
sounds reasonable, it's not much of a burden. However, I'm not sure if that's all you will need.
comment:4 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 4 years ago
Patch needs improvement: | set |
---|
comment:7 by , 4 years ago
Patch needs improvement: | unset |
---|
comment:10 by , 4 years ago
Does DEFAULT_HASHING_ALGORITHM = 'sha1'
work for you with several servers with Django 3.1 and 3.0? I checked on my staging server and it seems not to work - exceptions are raised if I revert from Django 3.1 to 3.0 after users log in.
comment:11 by , 4 years ago
Cc: | added |
---|
comment:12 by , 4 years ago
אורי this is a separate issue, sessions respect DEFAULT_HASHING_ALGORITHM
, but format has changed, see #31864.
comment:14 by , 3 years ago
Was there a reason the DEFAULT_HASHING_ALGORITHM
solution was used in preference to adding the algorithm
parameter to the two methods?
The project I'm working on needs to be able to select the hashing algorithm so we can progressively upgrade several systems that all use these methods. Updating them all at once is not feasible and we have a requirement to maintain compatibility for a short period while the upgrade is happening. Exposing the algorithm
parameter would allow us to do this easily and provide the same functionality if the default algorithm changes again in the future.
Exposing the algorithm
parameter would also allow users to opt into more secure hashing algorithms ahead of Django making it the default.
If possible, I'd like to see this reopened (or another ticket if that's the preference) so we can add the algorithm
parameter to these methods. I'd also be willing to put a PR together, I've not contributed to Django before but this seems straight forward enough.
I'm skeptical that Django should facilitate running multiple Django versions concurrently. I can't imagine there won't be other issues. Schema migrations are the first that come to mind.