#21398 closed Bug (fixed)
Fix py-bcrypt compatibility on Python 3
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Python 3 | Version: | 1.6 |
Severity: | Release blocker | Keywords: | |
Cc: | Donald Stufft | 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
When using the BCryptSHA256PasswordHasher or derivatives, the verification of passwords fails when on Python 3.
This is due to the following line:
return constant_time_compare(data, bcrypt.hashpw(password, data))
In BCryptSHA256PasswordHasher.verify(), 'data' is forced to the 'bytes' type, but the output of bcrypt.hashpw() is of type 'str'. The hashpw() output should just like 'data' be passed through force_bytes() before comparison, because the comparison is now always returning False on Python3.
Change History (11)
comment:1 by , 11 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 11 years ago
I can't reproduce (using Django's existing tests in django/contrib/auth/tests/test_hashers.py bcrypt.hashpw()
returning str
as reported. Arjan, can you provide more details (ideally a failing test case for Django's test suite)?
comment:4 by , 11 years ago
I was indeed using py-bcrypt. When I switch to bcrypt everything works perfectly.
Please put the incompatibility with py-bcrypt in the releasenotes, as I was using it with Django 1.5 and Python 3 before without any problems. (py-bcrypt supports Python 3 since version 0.4 according to their website)
I actually did read the releasenotes before upgrading, so an entry in there would have saved me a lot of headaches.
comment:5 by , 11 years ago
Donald, what would you like to do here? I can draft a doc patch for the release notes if necessary.
comment:7 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Summary: | BCryptSHA256PasswordHasher verify fails on Python 3 → Fix py-bcrypt compatibility on Python 3 |
All right, I'll mark this as a release blocker so we backport it to 1.6. Not sure if we can integrate the verification with Jenkins easily since presumably bcrypt and py-bcrypt can't be easily installed side by side.
comment:8 by , 11 years ago
Has patch: | set |
---|
Pull request that simply adds a force_bytes()
call as suggested. This does fix the tests on Python 3 with py-bcrypt 0.4.
comment:9 by , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:10 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Are you using py-bcrypt? Django now recommends using bcrypt, however, if we are no longer compatible with py-bcrypt and haven't put that in our docs, we have a problem. I'm not sure if that was the intention or not. This should be in the 1.6 release notes as well.
https://github.com/django/django/commit/c792c83cad54f064b6ba13e285e95a90e2c61f09