search

dani-garcia / vaultwarden

Unofficial Bitwarden compatible server written in Rust, formerly known as bitwarden_rs
GNU Affero General Public License v3.0
34.71k stars 1.69k forks source link

v2024.5.0 doesn't allow one to enroll in account recovery #4628

Open realkinetix opened 3 weeks ago

realkinetix commented 3 weeks ago

Running vaultwarden 1.30.5-f05398a6 with bw_web build 2024.5.0 produces the following when trying to enroll in account recovery:

Jun 05 13:02:39 [vaultwarden] [2024-06-05 13:02:39.003][response][INFO] (get_organization_keys) GET /api/organizations/<org_id>/keys => 200 OK
Jun 05 13:02:39 [vaultwarden] [2024-06-05 13:02:39.763][request][INFO] PUT /api/organizations/39e583bf-50fe-41a0-86db-bfeefe01904d/users/246a5
106-ac41-4830-83ec-b64b07511886/reset-password-enrollment
Jun 05 13:02:39 [vaultwarden] [2024-06-05 13:02:39.763][vaultwarden::api][ERROR] No validation provided
Jun 05 13:02:39 [vaultwarden] [2024-06-05 13:02:39.763][response][INFO] (put_reset_password_enrollment) PUT /api/organizations/<org_id>/users/
<org_user_id>/reset-password-enrollment => 400 Bad Request

Switched to bw_web build v2024.3.1 and it works fine.

stefan0xC commented 3 weeks ago

Seems like the client (since web-v2024.4.2 cf. https://github.com/bitwarden/clients/commit/bf11b90c43902ac150eef4e35dbc68a9f7f16273) only sends the "ResetPasswordKey" when enrolling manually into account recovery because they did not validate it server-side.

Should be fixed by https://github.com/bitwarden/clients/pull/8770 (but I have not looked if this requires further changes to our own validation logic).

BlackDex commented 3 weeks ago

Not sure if we should fix this in some way. Almost all those kind of actions send the hash to be verified server side too. And then someone removed that check. I wonder how it works if someone logged-in via device and tries to enroll them self.

I'm also not sure when that PR will be merged and if we can wait for that for example.

addisonbeck commented 3 weeks ago

I'm not all that familiar with this issue in the context of vaultwarden, but I agree that https://github.com/bitwarden/clients/pull/8770 and it's sibling PRs will at least help resolve this since the primary end goal of that work is to do a server side hash verification during account recovery enrollment. Bitwarden clients will be sending a populated MasterPasswordHash to the API again, which seems to be what is expected for this to work.

Mostly chiming in because I can help with this:

I'm also not sure when that PR will be merged and if we can wait for that for example.

I'm expecting to start having this work QA'd in the next week or so. The changes should be in main in the next couple of weeks, and will mostly likely land in production in mid July.

You could fix this a little faster in vaultwarden if you're willing to temporarily remove the server side hash check and put it back later. This is a small security issue, but the amount of things that would already need to have gone wrong for it to be exploited is high.