Skip to content
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

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

Closed
realkinetix opened this issue Jun 7, 2024 · 3 comments · Fixed by #4823
Closed

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

realkinetix opened this issue Jun 7, 2024 · 3 comments · Fixed by #4823

Comments

@realkinetix
Copy link

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.

@BlackDex BlackDex transferred this issue from dani-garcia/bw_web_builds Jun 7, 2024
@stefan0xC
Copy link
Contributor

stefan0xC commented Jun 7, 2024

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

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

@BlackDex
Copy link
Collaborator

BlackDex commented Jun 8, 2024

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
Copy link

I'm not all that familiar with this issue in the context of vaultwarden, but I agree that bitwarden/clients#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.

BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Aug 7, 2024
- Updated crates
- Updated web-vault to v2024.6.2
  This version is currently the latest version compatible with our API implementation.
  For newer versions we need more code updates to make it compatible.
  Thanks to @stefan0xC this version fixes dani-garcia#4628
- Added a small fix to prevent errors in the Vaultwarden and Client logs.
  The v2024.6.2 web-vault calls an endpoint with invalid arguments.
  If this happens we ignore the call and just return an Ok.
- Added the bulk-collection endpoint (Though not yet available in v2024.6.2)

Fixes dani-garcia#4628
BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Aug 7, 2024
- Updated crates
- Updated web-vault to v2024.6.2
  This version is currently the latest version compatible with our API implementation.
  For newer versions we need more code updates to make it compatible.
  Thanks to @stefan0xC this version fixes dani-garcia#4628
- Added a small fix to prevent errors in the Vaultwarden and Client logs.
  The v2024.6.2 web-vault calls an endpoint with invalid arguments.
  If this happens we ignore the call and just return an Ok.
- Added the bulk-collection endpoint (Though not yet available in v2024.6.2)

Fixes dani-garcia#4628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants