User account key rotation is not atomic #21

New Issue

MrUnknownDE · 2026-04-05T20:28:58+02:00

MrUnknownDE commented

2026-04-05 20:28:58 +02:00

Originally created by @0x484558 on 3/24/2026

I have searched the existing Closed AND Open Issues AND Discussions
I have searched and read the documentation

(most of bug report form is irrelevant)

POST /accounts/key-management/rotate-user-account-keys currently performs a long sequence of database operations without a DB transaction and returns early on error. This allows partial commits across account data categories which can leave account state inconsistent. The endpoint is intended as all-or-nothing key rotation operation, and partial success means some encrypted payloads/key metadata are updated while others are not.

If my understanding of the code is correct, it is possible to run into an exact failure like this:

Have an account with at least one folder, one emergency-access entry / org recovery key, and at least one send;
Trigger key rotation;
Keep folder/emergency/recovery updates value, but set one send.deletionDate to an invalid (bear with me here) far-future value (e.g. now + 31 days);
Submit the request.

Rotate request will fail with 400, folder update will be committed anyway but user key material will not get updated, and send will remain unchanged.

This exact example relies on an invalid request as a precondition, which of course not something clients would normally do, but it is just one way to trigger all sorts of unpredictable consequences from user account keys rotation code path. Transient database errors and process interruptions also happen. The real issue here is lack of defensive programming.

I'm submitting this as an issue because unfortunately, it appears architecturally to be slightly non-trivial to fix.

*Originally created by @0x484558 on 3/24/2026* - [x] I have searched the existing **Closed _AND_ Open** [Issues](https://github.com/dani-garcia/vaultwarden/issues?q=is%3Aissue%20) **_AND_** [Discussions](https://github.com/dani-garcia/vaultwarden/discussions?discussions_q=) - [x] I have searched and read the [documentation](https://github.com/dani-garcia/vaultwarden/wiki/) (most of bug report form is irrelevant) `POST /accounts/key-management/rotate-user-account-keys` currently performs a long sequence of database operations without a DB transaction and returns early on error. This allows partial commits across account data categories which can leave account state inconsistent. The endpoint is intended as all-or-nothing key rotation operation, and partial success means some encrypted payloads/key metadata are updated while others are not. If my understanding of the code is correct, it is possible to run into an exact failure like this: 1. Have an account with at least one folder, one emergency-access entry / org recovery key, and at least one send; 2. Trigger key rotation; 3. Keep folder/emergency/recovery updates value, but set one `send.deletionDate` to an invalid (bear with me here) far-future value (e.g. now + 31 days); 4. Submit the request. Rotate request will fail with 400, folder update will be committed anyway but user key material will not get updated, and send will remain unchanged. This exact example relies on an invalid request as a precondition, which of course not something clients would normally do, but it is just one way to trigger all sorts of unpredictable consequences from user account keys rotation code path. Transient database errors and process interruptions also happen. The real issue here is lack of defensive programming. I'm submitting this as an issue because unfortunately, it appears architecturally to be slightly non-trivial to fix.

MrUnknownDE closed this issue

2026-04-05 20:28:58 +02:00

Sign in to join this conversation.

No Label

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github/vaultwarden#21