Issue #350: Password Reset: CSRF check failed
Reported by: | Sylwester Kardziejonek |
State: | resolved |
Created on: | 2019-12-23 10:42 |
Updated on: | 2020-01-21 20:09 |
Description
Hi.
I’ve just updated Kallithea from 0.3.2 to 0.5.0. The first issue is that most users can’t login (bcrypt complains about salt), so the next thing I’ve tried is password reset, but it also fails after you submit the password change form.
CSRF check failed
Thankfully, somehow, my admin account worked and I’m able to reset password for everyone.
It was a fresh 0.5.0 installation, I’ve moved the files and database file (SQLite) from 0.3.2 installation, performed DB upgrade and made sure I’ve set all config values correctly.
Attachments
Comments
Comment by Mads Kiilerich, on 2019-12-23 12:24
Thanks for the report.
Can you confirm the password reset problem will be solved with
login: fix "Reset Your Password" form /_admin/password_reset_confirmation is the only place where we allow request parameters to be used as "default" in the form using htmlfill.render . But by default, htmlfill will clear all input fields that doesn't have a new "default" value provided. That would clear the CSRF protection cookie already inserted as a hidden field in the form. Fixed by setting force_defaults to False - see http://www.formencode.org/en/1.2-branch/modules/htmlfill.html diff --git a/kallithea/controllers/login.py b/kallithea/controllers/login.py --- a/kallithea/controllers/login.py +++ b/kallithea/controllers/login.py @@ -215,6 +215,7 @@ class LoginController(BaseController): return htmlfill.render( render('/password_reset_confirmation.html'), defaults=dict(request.params), + force_defaults=False, # don't clear form _session_csrf_secret_token encoding='UTF-8') form = PasswordResetConfirmationForm()()
?
Can you show more details about how bcrypt complains about salt? Like exact error message and log snippet?
What OS are you using?
Comment by Sylwester Kardziejonek, on 2019-12-23 13:07
The patch you suggested helped with password reset throwing a 403. Thanks
Error related to salt is not very helpful.
kallithea_1 | 2019-12-23 10:10:06.830 ERROR [kallithea.lib.auth_modules.auth_internal] user slykar had a bad password kallithea_1 | 2019-12-23 10:10:06.831 WARNI [kallithea.lib.auth_modules] User `slykar` failed to authenticate against kallithea.lib.auth_modules.auth_internal kallithea_1 | 2019-12-23 10:13:39.613 ERROR [kallithea.lib.auth] error from bcrypt checking password: Invalid salt
I’m also getting just a lot of repeated messages like this:
kallithea_1 | 2019-12-23 10:10:06.830 ERROR [kallithea.lib.auth_modules.auth_internal] user slykar had a bad password kallithea_1 | 2019-12-23 10:10:06.831 WARNI [kallithea.lib.auth_modules] User `slykar` failed to authenticate against kallithea.lib.auth_modules.auth_internal
Even tough users can now login and move around the app.
I’m running inside python:2.7 docker container, so it’s debian stretch, but I also had the same issues using ubuntu:16.04 image.
EDIT:
Maybe I should create a separate Issue for the bcrypt problem?
It’s really weird, because when I did the upgrade for the first time I could not login, but when I’ve tried to upgrade from scratch one more time, then my account worked. After deploying my account was still working, but others had issues.
Coud it be related to a config key session.secret
? It was named beaker.session.secret
and I think only when I’ve set both keys I could login. I then removed the beaker.session.secret
before deploying, as I assumed it’s not used anymore.
Comment by Mads Kiilerich, on 2019-12-23 13:28
bcrypt authentication is selfcontained - I cannot imagine the other secrets would have any influence.
And you say some of them works?
Could the problem be related to using unicode characters?
Testing bcrypt in a python2
prompt manually:
>>> import bcrypt >>> bcrypt.checkpw('password', '$2a$10$ycHAzdCG0rxam9oiyLiHROG2QnVOupSQ3r1nSEnYLWAFCLCAtvVXq') True >>> bcrypt.checkpw('PASSWORD', '$2a$10$ycHAzdCG0rxam9oiyLiHROG2QnVOupSQ3r1nSEnYLWAFCLCAtvVXq') False >>> bcrypt.checkpw('password', '$2a$10$ycHAzdCG0rxam9oiyLiHRO') False >>> bcrypt.checkpw('password', '$2a$10$ycHAzdCG0rxam9oiyLiHR') Traceback (most recent call last): ValueError: Invalid salt >>> bcrypt.checkpw('password', '$3a$10$ycHAzdCG0rxam9oiyLiHROG2QnVOupSQ3r1nSEnYLWAFCLCAtvVXq') ValueError: Invalid salt >>> bcrypt.checkpw('password', '$1a$10$ycHAzdCG0rxam9oiyLiHROG2QnVOupSQ3r1nSEnYLWAFCLCAtvVXq') ValueError: Invalid salt >>>
Many kind of invalid input just returns false - it must be very wrong before we get Invalid salt.
You can perhaps try with your own password and the value from the users
table in the database to get a basic idea of how bcrypt works.
How does the invalid passwords look like? Can you share a password/hash combination that has been replaced and is worthless?
Comment by Sylwester Kardziejonek, on 2019-12-23 13:59
And you say some of them works?
I don’t have any issues with my hash when I’m using bcrypt.checkpw
. When I login I do not generate any errors, but I can see many logs like this for multiple other users.
kallithea_1 | 2019-12-23 11:31:05.643 ERROR [kallithea.lib.auth_modules.auth_internal] user manuel had a bad password kallithea_1 | 2019-12-23 11:31:05.643 WARNI [kallithea.lib.auth_modules] User `manuel` failed to authenticate against kallithea.lib.auth_modules.auth_internal
They do not report any issues tough. Things seem to work. I’ve tried to take their hash and use it directly in bcrypt.checkpw
and I get False
(I don't know their passwords). Those users do not generate the salt error anymore, only the above errors appear.
I’ve also noticed some users still have the old password hash in database. Looks like some kind of sha
hash.
They can’t login and get the above error (including bad salt error), so I would assume their old password hash is used and bcrypt does throw an invalid salt error for those hashes. Was there any upgrade note about required password resets I’ve missed or should they be updated during db migration?
It’s hard for me to get any more information from other users and I myself don’t experience any issues right now, so I’m unable to provide anything more useful than this, sorry
Comment by Mads Kiilerich, on 2019-12-23 14:44
In what way do the old password hashes look like sha hashes? Do they mention sha? Or is it because it is 64 hex “digits”?
Do you only see problems for users with such sha “passwords”?
I am not aware of any relevant related changes or missing upgrade steps.
Comment by Sylwester Kardziejonek, on 2019-12-23 16:51
The password
column in the database for some users looks like this. Only after password is reset by the admin it is a proper bcrypt hash.
5e4a8757024b7c1bc634c1a95d951a3911419d7fe8c1c8cbb718275723dc312e
Do you only see problems for users with such sha “passwords”?
Users who have this 64 hex digits can’t login. In this case it’s obviously because of the hash format. The bcrypt
simply states that the hash is invalid.
For some reason during the migration user passwords were not updated (I’m not even sure if they were supposed to be updated automatically?).
I was able to figure out the source of other errors.
kallithea_1 | 2019-12-23 11:31:05.643 ERROR [kallithea.lib.auth_modules.auth_internal] user manuel had a bad password kallithea_1 | 2019-12-23 11:31:05.643 WARNI [kallithea.lib.auth_modules] User `manuel` failed to authenticate against kallithea.lib.auth_modules.auth_internal
Some script used old credentials and it generated a lot of entries. At first I saw this for many users (sourcetree pooling for changes maybe), now it’s only for one. So I assume it is working correctly as soon as the bcrypt hash is correct.
So I think the only real issue left is why passwords were not updated to be compatible with bcrypt.
Comment by Mads Kiilerich, on 2019-12-23 19:04
For odd historical reasons, Kallithea uses sha256 on windows, and bcrypt elsewhere. I think it has been like that forever. What you describe thus sounds like Windows is involved … but you say you are using docker with ubuntu?
Comment by Sylwester Kardziejonek, on 2019-12-24 13:50
I understand. When I’ve inherited the Kallithea installation I’ve found it running inside a docker container that was based on Ubuntu. For upgrade purposes I’ve build a docker container based on debian.
There could have been Windows involved long time before, but it was working without any issues with those sha256 hashes inside dockerized Ubuntu.
With the information you provided I have to believe that someone has modified the source code of Kallithea inside the container, completely ignoring good practices, forcing it to work with sha256 after migrating from Windows.
In this case, sorry for the trouble.
Is the patch you provided in the first comment gonna end up in some release I could install or build from sources? I could not find the related commit using BitBucket.
Comment by Thomas De Schampheleire, on 2019-12-29 20:39
@Mads Kiilerich Will you push the patch you posted above to the stable branch?
Comment by Mads Kiilerich, on 2020-01-03 21:52
The fix above is a part of https://kallithea-scm.org/repos/kallithea/pull-request/221/_/Fixes_for_stable . I’m not sure if it would be better to keep the defaults dict under control and also set _session_csrf_secret_token
…
This series also propose changes to make windows/unix migration more smooth.
Comment by Thomas De Schampheleire, on 2020-01-21 20:09
Fixed on stable as 8b47181750a8, will be part of 0.5.1. Thanks for reporting!