Encryption key rotation fails with 500 error

[BlackDex]

Steps To Reproduce

1. Go to Settings > Security > Master password
2. Enable Also rotate my account's encryption key
3. Provide current and new master password
4. Click on Change master password

It produces a 500 error.

{
    "message": "An unhandled server error has occurred.",
    "validationErrors": null,
    "exceptionMessage": null,
    "exceptionStackTrace": null,
    "innerExceptionMessage": null,
    "object": "error"
}

Expected Result

Rotated encryption key succeeded.

Actual Result

It fails.

Screenshots or Videos

No response

Additional Context

The folders array contains an id with null. The folder count is also 1 more than the account actually has.

Operating System

Linux

Operating System Version

n/a

Web Browser

Firefox

Browser Version

125.0b3

Build Version

2024.3.0

Issue Tracking Info

• I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.

[BlackDex]

As an addition, i tested this with a Self-Hosted 6a0f6e1d where it seems to break.
With the current Cloud solution, which is using b64b75ff, it seems to work just fine, but it does contain the null id folder there though.

[SergeantConfused]

Hello @BlackDex,

Thank you for reporting this. I tested this in the cloud Web App 2024.3.0 using Firefox 123.0.1, and I was able to successfully rotate the account's encryption key. I recommend that you create a Support ticket so we'd have a look at this together, and please also include a link to this GitHub report in the body of your ticket.

Alternatively, you can seek assistance from other Bitwarden users in our Community Forums, if you wish.

We use GitHub to track bugs and other development related matters; This GitHub report will be closed at this point.

Thank you again,

[BlackDex]

@SergeantConfused I am a bit confused here. I do not need support. I'm reporting an issue/bug here.
I just now tested with the latest bitwarden/self-host:beta available, and that uses the Bitwarden web client 2024.2.5, and it also fails. And, i then tested v2024.2.3, and it also fails.

I did some more testing. Used an empty account which is part of an organization, and when i try to rotate the key, it seems to also rotate the organization owned ciphers and sends those too. That in the end causes a failure i think.

When i create a user which is not part of an organization, it works just fine.

Looking at the logs below, it doesn't seem to have anything to do with the folders, but rather the ciphers not owned by the user, but by the organization.

fail: Bit.Api.Utilities.ExceptionHandlerFilterAttribute[0]
      => SpanId:889484141903e16c, TraceId:d37107375005afc43faad4e42bd4c421, ParentId:0000000000000000 => ConnectionId:0HN2BD0AO4QIE => RequestPath:/accounts/key RequestId:0HN2BD0AO4QIE:00000001 => Bit.Api.Auth.Controllers.AccountsController.PostKey (Api)
      SQLite Error 19: 'UNIQUE constraint failed: Cipher.Id'.
      Microsoft.Data.Sqlite.SqliteException (0x80004005): SQLite Error 19: 'UNIQUE constraint failed: Cipher.Id'.
         at Microsoft.Data.Sqlite.SqliteException.ThrowExceptionForRC(Int32 rc, sqlite3 db)
         at Microsoft.Data.Sqlite.SqliteDataReader.NextResult()
         at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader(CommandBehavior behavior)
         at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader()
         at Microsoft.Data.Sqlite.SqliteCommand.ExecuteNonQuery()
         at System.Data.Common.DbCommand.ExecuteNonQueryAsync(CancellationToken cancellationToken)
      --- End of stack trace from previous location ---
         at LinqToDB.Data.DataConnection.ExecuteNonQueryAsync(CancellationToken cancellationToken)
         at LinqToDB.Data.DataConnection.ExecuteNonQueryDataAsync(CancellationToken cancellationToken)
         at LinqToDB.Data.DataConnection.ExecuteNonQueryDataAsync(CancellationToken cancellationToken)
         at LinqToDB.Data.CommandInfo.ExecuteAsync(CancellationToken cancellationToken)
         at LinqToDB.DataProvider.MultipleRowsHelper.ExecuteAsync(CancellationToken cancellationToken)
         at LinqToDB.DataProvider.BasicBulkCopy.MultipleRowsCopyHelperAsync(MultipleRowsHelper helper, IEnumerable source, String from, Action`1 prepFunction, Action`3 addFunction, Action`1 finishFunction, CancellationToken cancellationToken, Int32 maxParameters, Int32 maxSqlLength)
         at LinqToDB.EntityFrameworkCore.LinqToDBForEFTools.BulkCopyAsync[T](DbContext context, BulkCopyOptions options, IEnumerable`1 source, CancellationToken cancellationToken)
         at Bit.Infrastructure.EntityFramework.Vault.Repositories.CipherRepository.UpdateUserKeysAndCiphersAsync(User user, IEnumerable`1 ciphers, IEnumerable`1 folders, IEnumerable`1 sends) in /source/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs:line 876
         at Bit.Core.Services.UserService.UpdateKeyAsync(User user, String masterPassword, String key, String privateKey, IEnumerable`1 ciphers, IEnumerable`1 folders, IEnumerable`1 sends)
         at Bit.Api.Auth.Controllers.AccountsController.PostKey(UpdateKeyRequestModel model) in /source/src/Api/Auth/Controllers/AccountsController.cs:line 484
         at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Logged|12_1(ControllerActionInvoker invoker)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|26_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)

[BlackDex]

v2023.12.x seems to not send org ciphers, v2024.1.x seems to, and it fails using a Self-Hosted instance (unified).

[quexten]

Not having debugged it thoroughly, but just having done brief triage, it seems to be (accidentally?) introduced in #6881.

Before:

clients/apps/web/src/app/auth/settings/change-password.component.ts

Lines 244 to 252 in 28de943

	const ciphers = await this.cipherService.getAllDecrypted();
	for (let i = 0; i < ciphers.length; i++) {
	if (ciphers[i].organizationId != null) {
	continue;
	}
	const cipher = await this.cipherService.encrypt(ciphers[i], newUserKey);
	request.ciphers.push(new CipherWithIdRequest(cipher));
	}

After:

clients/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts

Lines 104 to 116 in f987153

	private async encryptCiphers(newUserKey: UserKey): Promise<CipherWithIdRequest[]> {
	const ciphers = await this.cipherService.getAllDecrypted();
	if (!ciphers) {
	// Must return an empty array for backwards compatibility
	return [];
	}
	return await Promise.all(
	ciphers.map(async (cipher) => {
	const encryptedCipher = await this.cipherService.encrypt(cipher, newUserKey);
	return new CipherWithIdRequest(encryptedCipher);
	}),
	);
	}

which seems to be missing a check for the orgId, which lines up with @BlackDex's observation.

[stefan0xC]

I tested this in the cloud Web App 2024.3.0 using Firefox 123.0.1, and I was able to successfully rotate the account's encryption key.

To replicate the issue with the folders I think you need a self-hosted instance that has the feature flag KeyRotationImprovements turned off (which is most likely why it works on the cloud) and by creating at least 1 custom folder.

Until web-v2023.12.0 the folderViews were filtered

clients/apps/web/src/app/auth/settings/change-password.component.ts

Lines 235 to 242 in 8d90085

	const folders = await firstValueFrom(this.folderService.folderViews$);
	for (let i = 0; i < folders.length; i++) {
	if (folders[i].id == null) {
	continue;
	}
	const folder = await this.folderService.encrypt(folders[i], newUserKey);
	request.folders.push(new FolderWithIdRequest(folder));
	}

but since web-v2024.1.0 the empty folder is sent

clients/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts

Lines 118 to 130 in dbf0f1e

	private async encryptFolders(newUserKey: UserKey): Promise<FolderWithIdRequest[]> {
	const folders = await firstValueFrom(this.folderService.folderViews$);
	if (!folders) {
	// Must return an empty array for backwards compatibility
	return [];
	}
	return await Promise.all(
	folders.map(async (folder) => {
	const encryptedFolder = await this.folderService.encrypt(folder, newUserKey);
	return new FolderWithIdRequest(encryptedFolder);
	}),
	);
	}

and it sounds like the old server code cannot handle the missing id.

[BlackDex]

No need for any folder too be created. It does this on a fresh account to. The problem probably is that the self hosted do not have that feature flag in the config, but it still receives the data in that format. And therefore it probably breaks by trying to update or actually add the org ciphers as a user cipher, and that causes a duplicate index/foreign key error.