[Django] #33691: Deprecate CryptPasswordHasher.
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz | Owner: Mariusz Felisiak
Felisiak |
Type: | Status: assigned
Cleanup/optimization |
Component: | Version: 4.0
contrib.auth |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33691>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:2 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 :)
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15678 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"02dbf1667c6da61ea9346f7c9f174a158b896811" 02dbf166]:
{{{
#!CommitTicketReference repository=""
revision="02dbf1667c6da61ea9346f7c9f174a158b896811"
Fixed #33691 -- Deprecated
django.contrib.auth.hashers.CryptPasswordHasher.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
Replying to [comment:3 Mariusz Felisiak]:
> Replying to [comment:2 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
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Agreed, we can at least deprecate `SHA1PasswordHasher`,
`UnsaltedMD5PasswordHasher`, `UnsaltedSHA1PasswordHasher` before the
feature freeze.
Please `Refs #33691 -- ...` in your patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
[https://github.com/django/django/pull/15871 PR] for `SHA1PasswordHasher`,
`UnsaltedMD5PasswordHasher`, `UnsaltedSHA1PasswordHasher` deprecation.
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
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.
+ """
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"3b79dab19a2300a4884a3d81baa6c7c1f2dee059" 3b79dab1]:
{{{
#!CommitTicketReference repository=""
revision="3b79dab19a2300a4884a3d81baa6c7c1f2dee059"
Refs #33691 -- Deprecated insecure password hashers.
SHA1PasswordHasher, UnsaltedSHA1PasswordHasher, and
UnsaltedMD5PasswordHasher
are now deprecated.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b5ac6e78f838376b61f3f7dfccd66f906ad6d394" b5ac6e78]:
{{{
#!CommitTicketReference repository=""
revision="b5ac6e78f838376b61f3f7dfccd66f906ad6d394"
Refs #33691 -- Removed django.contrib.auth.hashers.CryptPasswordHasher per
deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: Mariusz
Type: | Felisiak
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"6e4e5523a8f40b63f3e74889266a7d638f6364dc" 6e4e5523]:
{{{
#!CommitTicketReference repository=""
revision="6e4e5523a8f40b63f3e74889266a7d638f6364dc"
Refs #33691 -- Removed insecure password hashers per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33691#comment:12>