Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#21398 closed Bug (fixed)

Fix py-bcrypt compatibility on Python 3

Reported by: arjan@… 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 Tim Graham, 11 years ago

Cc: Donald Stufft added
Triage Stage: UnreviewedAccepted

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

comment:2 by Donald Stufft, 11 years ago

py-bcrypt does not support Python3.

comment:3 by Tim Graham, 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 arjan@…, 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 Tim Graham, 11 years ago

Donald, what would you like to do here? I can draft a doc patch for the release notes if necessary.

comment:6 by Donald Stufft, 11 years ago

I don't have a problem with forcing the data through force_bytes.

comment:7 by Tim Graham, 11 years ago

Severity: NormalRelease blocker
Summary: BCryptSHA256PasswordHasher verify fails on Python 3Fix 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 Tim Graham, 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 Claude Paroz, 11 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

In d15985d81ff1c3b353a48a87189b7847798214c0:

Fixed #21398 -- Fixed BCryptSHA256PasswordHasher with py-bcrypt and Python 3.

Thanks arjan at anymore.nl for the report.

comment:11 by Tim Graham <timograham@…>, 11 years ago

In 4b9e932fd46eaac4774d229c40c2ee75f8fb759b:

[1.6.x] Fixed #21398 -- Fixed BCryptSHA256PasswordHasher with py-bcrypt and Python 3.

Thanks arjan at anymore.nl for the report.

Backport of d15985d81f from master

Note: See TracTickets for help on using tickets.
Back to Top