Opened 7 months ago
Closed 5 months ago
#35492 closed Cleanup/optimization (wontfix)
Replace call to User.set_password with make_password in authenticate
Reported by: | Natalia Bidart | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | אורי | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In the current implementation of ModelBackend.authenticate()
, set_password()
is invoked on an empty User model to conceal timing differences between existing and non-existing users, thereby preventing password timing attacks.
However, relying on set_password()
in this context may lead to unintended consequences, given it is a public and overridable method of the model. The Security Team suggested to directly call make_password()
instead to achieve the same desired timing effect.
Change History (5)
comment:1 by , 7 months ago
Cc: | added |
---|
follow-up: 3 comment:2 by , 7 months ago
comment:3 by , 7 months ago
Replying to Claude Paroz:
The call in
authenticate
is counting on the fact thatcheck_password
(itself callingset_password
) ...
I think check_password
only calls set_password
after validating the password, and only if the password is correct (is_correct
is true), and also if must_update
is true, so if the password is not valid and is validated by set_password
, it will raise an exception only if the password is correct, and not always when there is no such user (if the username doesn't exist in the database).
comment:4 by , 6 months ago
Hi,
We patched Django with such a change on Speedy Net:
https://github.com/speedy-net/speedy-net/commit/3c98b76ffdadb636664aebacfa126db33b5b78f5
If you want you can apply a similar change to Django itself.
Thanks,
Uri (אורי).
comment:5 by , 5 months ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
I did a follow up about this with the members Security Team and there is an inclination to close this as wontfix
for the reasons outlined by Claude above. Doing so now, if there is a desire for a further/broader discussion, a new topic should be started in the Forum.
I'm still a bit undecided on this. The call in
authenticate
is counting on the fact thatcheck_password
(itself callingset_password
) is using more or less the same time thanset_password
. If we callmake_password
instead, wouldn't this open a timing effect attack vector for any project doing any not-negligible work in a customizedset_password
? Hence I'm not sure that changing the function to call is not simply moving a potential problem elsewhere.