-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace crypt with passlib #319
Conversation
Still not completely sure why this isn't working, but maybe it has something to do with the fact that our previous function created a string with pw + salt, even though for SHA512 Dovecot (and I suppose hashlib too) does salt + pw. See https://doc.dovecot.org/configuration_manual/authentication/password_schemes/#salting Gotta go to sleep now :D |
also, with the new implementation, the hashed pw is longer than with the old one.
|
Hm, didn't dive into it deeper. But will passphrases which were hashed with the old function still work? For that the new function needs to return exactly the same hash as the old function, or (a bit ugly) a migration function which detects on login if the passphrase is the old or new hash, uses the appropriate method to authenticate, and updates the hash in the database to the new method on successful login. I'm also unsure where the salt is added; with the old method, it is randomly generated and stored in the database in the same string as the actual hash (i.e. |
Good point, maybe we should only replace crypt on new installations. But even then we are only kicking the bucket down the road, because as soon as they update to Python 3.13, it stops working anyways. So a migration function is our only real option... :/ Regarding the formatting of the hash, the correct way, as mentioned in the dovecot docs should be $6$salt$hash. You are right, the crypt function doesn't say how exactly it does this, and this may even be different on different platforms as crypt uses the systems cryptography functions, if I understand correctly. Anyways, crypt is using a 16 character salt and 86 character hash, have to find out what hashlib does |
Probably useful: https://eighty-twenty.org/2024/01/13/python-crypt-shacrypt Edit: Or not, this does seem overly complicated |
ugh. not sure how complicated it is, but I'd prefer to use a maintained library for our password hashing. hashlib should be able to do this, and to produce a hash exactly like crypt did in the past, no? |
I think this is how salting works with hashlib: https://docs.python.org/3/library/hashlib.html#randomized-hashing |
they are using only hashlib, it's just that their code to reproduce crypt's functionality 1 to 1 is so long that they put it in a seperate file. |
duh :D I've seen that, obviously. Sadly the docs don't explain their defaults for SHA512 salts. |
Another option would be to use passlib (https://passlib.readthedocs.io/en/stable/), which while an additional dependency is recommended as a full replacement for crypt in the python docs (https://docs.python.org/3/library/crypt.html) "The hashlib module is a potential replacement for certain use cases. The passlib package can replace all use cases of this module." |
fyi in #318 is more context |
So, passlib does it, but not in the correct format. This seems to work, but is ugly:
Edit: haha, the § does not help here :D |
linter fails, will fix tomorrow |
The CI fail was very likely due to the staging.testrun.org cert failing, if we re-open this one day, we should re-check, maybe it simply works. |
@missytake you sure that the CI is working now? |
nope, there is still something wrong... :/ |
e5707b7
to
3eae165
Compare
Nope, it's still broken. Account creation works, but authenticating doesn't. Let's close and re-open as soon as we have to support python 3.13 :) |
at least the CI works :) |
fix #318