#33691 closed Cleanup/optimization (fixed)
Deprecate CryptPasswordHasher.
Reported by: | Mariusz Felisiak | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | contrib.auth | Version: | 4.0 |
Severity: | Normal | Keywords: | |
Cc: | Florian Apolloner, Carlton Gibson | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
CryptPasswordHasher
was added 15 years ago mainly to support legacy UNIX apps. It's almost undocumented, not recommended, and supported only on UNIX. Moreover crypt
module was deprecated in Python 3.11 (see https://github.com/python/cpython/commit/f45aa8f304a12990c2ca687f2088f04b07906033).
We should deprecate it in Django 4.1 and remove in Django 5.0.
Change History (12)
comment:1 by , 2 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 3 comment:2 by , 2 years ago
follow-up: 6 comment:3 by , 2 years ago
Replying to Florian Apolloner:
ACK, while we are on it I wonder if we should deprecate the unsalted & sha/md5 hashers as well. It is time to face reality, if you haven't upgraded Django by now and are still on one of those old algorithms your installation is probably 10 years or older?
MD5PasswordHasher
is still a nice hook for faster testing :)
comment:6 by , 2 years ago
Replying to Mariusz Felisiak:
Replying to Florian Apolloner:
ACK, while we are on it I wonder if we should deprecate the unsalted & sha/md5 hashers as well. It is time to face reality, if you haven't upgraded Django by now and are still on one of those old algorithms your installation is probably 10 years or older?
MD5PasswordHasher
is still a nice hook for faster testing :)
Like Florian, I also think that now is the time to get rid of the following hashers:
MD5PasswordHasher
SHA1PasswordHasher
UnsaltedMD5PasswordHasher
UnsaltedSHA1PasswordHasher
It is long past time that these were gone, and they were removed from the default PASSWORD_HASHERS
setting in Django 1.10 along with CryptPasswordHasher
. In addition, they will not be fully removed until December 2023, so that gives Django 4.1 and 4.2 to move of these, and 4.2, being an LTS, gives plenty of time beyond that.
Yes, it'd be nice to keep something fast around for testing purposes and we've been recommending MD5PasswordHasher
up to now, but I don't think that should prevent removal of the other three at the very least...
However, I propose that we add an explicit new TestPasswordHasher
that has something in place to scream about not using it outside of local development environments and continuous integration pipelines for the purpose of speeding up testing. Maybe this could be achieved with the check framework (disabled when using testing tools). Or disabled when Django is executed via normal ASGI/WSGI stuff as would be done in production, but not when running tests. Or that TestPasswordHasher
will only work when it is the only hasher.
Another question would be whether we make it still use MD5 as the algorithm, or whether, now that we're making it explicitly for testing, we make it even faster by making it totally insecure? (Although this would maybe be a step to far in case of people still ignoring all the warnings...)
I started messing around very roughly with this idea on the following branch, but I don't really have all that much time to follow it up right now. It needs thought about how we'd flag up and/or block use of TestPasswordHasher
in the wrong places.
https://github.com/ngnpope/django/tree/test-password-hasher
comment:7 by , 2 years ago
Agreed, we can at least deprecate SHA1PasswordHasher
, UnsaltedMD5PasswordHasher
, UnsaltedSHA1PasswordHasher
before the feature freeze.
Please Refs #33691 -- ...
in your patch.
comment:8 by , 2 years ago
PR for SHA1PasswordHasher
, UnsaltedMD5PasswordHasher
, UnsaltedSHA1PasswordHasher
deprecation.
comment:9 by , 2 years ago
What about something like this (TestPasswordHasher being imported from django.test.utils
is also a rather strong indication not to use it in production):
diff --git a/django/contrib/auth/hashers.py b/django/contrib/auth/hashers.py index 432c624483..f409882d70 100644 --- a/django/contrib/auth/hashers.py +++ b/django/contrib/auth/hashers.py @@ -675,7 +675,7 @@ class SHA1PasswordHasher(BasePasswordHasher): pass -class MD5PasswordHasher(BasePasswordHasher): +class _MD5UnsecurePasswordHasher(BasePasswordHasher): """ The Salted MD5 password hashing algorithm (not recommended) """ @@ -717,6 +717,17 @@ class MD5PasswordHasher(BasePasswordHasher): pass +# RemovedInDjango51Warning. +class MD5PasswordHasher(_MD5UnsecurePasswordHasher): + def __init__(self, *args, **kwargs): + warnings.warn( + "django.contrib.auth.hashers.MD5PasswordHasher is deprecated.", + RemovedInDjango51Warning, + stacklevel=2, + ) + super().__init__(*args, **kwargs) + + # RemovedInDjango51Warning. class UnsaltedSHA1PasswordHasher(BasePasswordHasher): """ diff --git a/django/test/utils.py b/django/test/utils.py index 270e34b69d..1e87bcaa9f 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -17,6 +17,7 @@ from xml.dom.minidom import Node, parseString from django.apps import apps from django.apps.registry import Apps from django.conf import UserSettingsHolder, settings +from django.contrib.auth.hashers import _MD5UnsecurePasswordHasher from django.core import mail from django.core.exceptions import ImproperlyConfigured from django.core.signals import request_started, setting_changed @@ -999,3 +1000,9 @@ def register_lookup(field, *lookups, lookup_name=None): finally: for lookup in lookups: field._unregister_lookup(lookup, lookup_name) + + +class TestPasswordHasher(_MD5UnsecurePasswordHasher): + """ + An unsecure but fast Salted MD5 password hashing algorithm for speedier tests. + """
ACK, while we are on it I wonder if we should deprecate the unsalted & sha/md5 hashers as well. It is time to face reality, if you haven't upgraded Django by now and are still on one of those old algorithms your installation is probably 10 years or older?