[PM-2199] Implement userkey rotation for all TDE devices (#5446)

* Implement userkey rotation v2

* Update request models

* Cleanup

* Update tests

* Improve test

* Add tests

* Fix formatting

* Fix test

* Remove whitespace

* Fix namespace

* Enable nullable on models

* Fix build

* Add tests and enable nullable on masterpasswordunlockdatamodel

* Fix test

* Remove rollback

* Add tests

* Make masterpassword hint optional

* Update user query

* Add EF test

* Improve test

* Cleanup

* Set masterpassword hint

* Remove connection close

* Add tests for invalid kdf types

* Update test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Fix formatting

* Update src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Update src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Update src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Update src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Fix imports

* Fix tests

* Add poc for tde rotation

* Improve rotation transaction safety

* Add validator tests

* Clean up validator

* Add newline

* Add devicekey unlock data to integration test

* Fix tests

* Fix tests

* Remove null check

* Remove null check

* Fix IsTrusted returning wrong result

* Add rollback

* Cleanup

* Address feedback

* Further renames

---------

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

src/Api/Controllers/DevicesController.cs

@@ -1,6 +1,5 @@
 using System.ComponentModel.DataAnnotations;
 using Bit.Api.Auth.Models.Request;
-using Bit.Api.Auth.Models.Request.Accounts;
 using Bit.Api.Models.Request;
 using Bit.Api.Models.Response;
 using Bit.Core.Auth.Models.Api.Request;
@@ -125,7 +124,7 @@ public class DevicesController : Controller
     }
 
     [HttpPost("{identifier}/retrieve-keys")]
-    public async Task<ProtectedDeviceResponseModel> GetDeviceKeys(string identifier, [FromBody] SecretVerificationRequestModel model)
+    public async Task<ProtectedDeviceResponseModel> GetDeviceKeys(string identifier)
     {
         var user = await _userService.GetUserByPrincipalAsync(User);
 
@@ -134,14 +133,7 @@ public class DevicesController : Controller
             throw new UnauthorizedAccessException();
         }
 
-        if (!await _userService.VerifySecretAsync(user, model.Secret))
-        {
-            await Task.Delay(2000);
-            throw new BadRequestException(string.Empty, "User verification failed.");
-        }
-
         var device = await _deviceRepository.GetByIdentifierAsync(identifier, user.Id);
-
         if (device == null)
         {
             throw new NotFoundException();

src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs

@@ -8,6 +8,7 @@ using Bit.Api.Tools.Models.Request;
 using Bit.Api.Vault.Models.Request;
 using Bit.Core;
 using Bit.Core.Auth.Entities;
+using Bit.Core.Auth.Models.Api.Request;
 using Bit.Core.Auth.Models.Data;
 using Bit.Core.Entities;
 using Bit.Core.Exceptions;
@@ -43,6 +44,7 @@ public class AccountsKeyManagementController : Controller
         _organizationUserValidator;
     private readonly IRotationValidator<IEnumerable<WebAuthnLoginRotateKeyRequestModel>, IEnumerable<WebAuthnLoginRotateKeyData>>
         _webauthnKeyValidator;
+    private readonly IRotationValidator<IEnumerable<OtherDeviceKeysUpdateRequestModel>, IEnumerable<Device>> _deviceValidator;
 
     public AccountsKeyManagementController(IUserService userService,
         IFeatureService featureService,
@@ -57,7 +59,8 @@ public class AccountsKeyManagementController : Controller
             emergencyAccessValidator,
         IRotationValidator<IEnumerable<ResetPasswordWithOrgIdRequestModel>, IReadOnlyList<OrganizationUser>>
             organizationUserValidator,
-        IRotationValidator<IEnumerable<WebAuthnLoginRotateKeyRequestModel>, IEnumerable<WebAuthnLoginRotateKeyData>> webAuthnKeyValidator)
+        IRotationValidator<IEnumerable<WebAuthnLoginRotateKeyRequestModel>, IEnumerable<WebAuthnLoginRotateKeyData>> webAuthnKeyValidator,
+        IRotationValidator<IEnumerable<OtherDeviceKeysUpdateRequestModel>, IEnumerable<Device>> deviceValidator)
     {
         _userService = userService;
         _featureService = featureService;
@@ -71,6 +74,7 @@ public class AccountsKeyManagementController : Controller
         _emergencyAccessValidator = emergencyAccessValidator;
         _organizationUserValidator = organizationUserValidator;
         _webauthnKeyValidator = webAuthnKeyValidator;
+        _deviceValidator = deviceValidator;
     }
 
     [HttpPost("regenerate-keys")]
@@ -109,6 +113,7 @@ public class AccountsKeyManagementController : Controller
             EmergencyAccesses = await _emergencyAccessValidator.ValidateAsync(user, model.AccountUnlockData.EmergencyAccessUnlockData),
             OrganizationUsers = await _organizationUserValidator.ValidateAsync(user, model.AccountUnlockData.OrganizationAccountRecoveryUnlockData),
             WebAuthnKeys = await _webauthnKeyValidator.ValidateAsync(user, model.AccountUnlockData.PasskeyUnlockData),
+            DeviceKeys = await _deviceValidator.ValidateAsync(user, model.AccountUnlockData.DeviceKeyUnlockData),
 
             Ciphers = await _cipherValidator.ValidateAsync(user, model.AccountData.Ciphers),
             Folders = await _folderValidator.ValidateAsync(user, model.AccountData.Folders),

src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs

@@ -3,6 +3,7 @@ using Bit.Api.AdminConsole.Models.Request.Organizations;
 using Bit.Api.Auth.Models.Request;
 using Bit.Api.Auth.Models.Request.Accounts;
 using Bit.Api.Auth.Models.Request.WebAuthn;
+using Bit.Core.Auth.Models.Api.Request;
 
 namespace Bit.Api.KeyManagement.Models.Requests;
 
@@ -13,4 +14,5 @@ public class UnlockDataRequestModel
     public required IEnumerable<EmergencyAccessWithIdRequestModel> EmergencyAccessUnlockData { get; set; }
     public required IEnumerable<ResetPasswordWithOrgIdRequestModel> OrganizationAccountRecoveryUnlockData { get; set; }
     public required IEnumerable<WebAuthnLoginRotateKeyRequestModel> PasskeyUnlockData { get; set; }
+    public required IEnumerable<OtherDeviceKeysUpdateRequestModel> DeviceKeyUnlockData { get; set; }
 }

src/Api/KeyManagement/Validators/DeviceRotationValidator.cs

@@ -0,0 +1,53 @@
+using Bit.Core.Auth.Models.Api.Request;
+using Bit.Core.Auth.Utilities;
+using Bit.Core.Entities;
+using Bit.Core.Exceptions;
+using Bit.Core.Repositories;
+
+namespace Bit.Api.KeyManagement.Validators;
+
+/// <summary>
+/// Device implementation for <see cref="IRotationValidator{T,R}"/>
+/// </summary>
+public class DeviceRotationValidator : IRotationValidator<IEnumerable<OtherDeviceKeysUpdateRequestModel>, IEnumerable<Device>>
+{
+    private readonly IDeviceRepository _deviceRepository;
+
+    /// <summary>
+    /// Instantiates a new <see cref="DeviceRotationValidator"/>
+    /// </summary>
+    /// <param name="deviceRepository">Retrieves all user <see cref="Device"/>s</param>
+    public DeviceRotationValidator(IDeviceRepository deviceRepository)
+    {
+        _deviceRepository = deviceRepository;
+    }
+
+    public async Task<IEnumerable<Device>> ValidateAsync(User user, IEnumerable<OtherDeviceKeysUpdateRequestModel> devices)
+    {
+        var result = new List<Device>();
+
+        var existingTrustedDevices = (await _deviceRepository.GetManyByUserIdAsync(user.Id)).Where(d => d.IsTrusted()).ToList();
+        if (existingTrustedDevices.Count == 0)
+        {
+            return result;
+        }
+
+        foreach (var existing in existingTrustedDevices)
+        {
+            var device = devices.FirstOrDefault(c => c.DeviceId == existing.Id);
+            if (device == null)
+            {
+                throw new BadRequestException("All existing trusted devices must be included in the rotation.");
+            }
+
+            if (device.EncryptedUserKey == null || device.EncryptedPublicKey == null)
+            {
+                throw new BadRequestException("Rotated encryption keys must be provided for all devices that are trusted.");
+            }
+
+            result.Add(device.ToDevice(existing));
+        }
+
+        return result;
+    }
+}

src/Api/Startup.cs

@@ -31,7 +31,7 @@ using Bit.Core.Auth.Models.Data;
 using Bit.Core.Auth.Identity.TokenProviders;
 using Bit.Core.Tools.ImportFeatures;
 using Bit.Core.Tools.ReportFeatures;
-
+using Bit.Core.Auth.Models.Api.Request;
 
 #if !OSS
 using Bit.Commercial.Core.SecretsManager;
@@ -168,6 +168,9 @@ public class Startup
         services
             .AddScoped<IRotationValidator<IEnumerable<WebAuthnLoginRotateKeyRequestModel>, IEnumerable<WebAuthnLoginRotateKeyData>>,
                 WebAuthnLoginKeyRotationValidator>();
+        services
+            .AddScoped<IRotationValidator<IEnumerable<OtherDeviceKeysUpdateRequestModel>, IEnumerable<Device>>,
+                DeviceRotationValidator>();
 
         // Services
         services.AddBaseServices(globalSettings);

src/Core/Auth/Models/Api/Request/DeviceKeysUpdateRequestModel.cs

@@ -1,4 +1,5 @@
 using System.ComponentModel.DataAnnotations;
+using Bit.Core.Entities;
 using Bit.Core.Utilities;
 
 namespace Bit.Core.Auth.Models.Api.Request;
@@ -7,6 +8,13 @@ public class OtherDeviceKeysUpdateRequestModel : DeviceKeysUpdateRequestModel
 {
     [Required]
     public Guid DeviceId { get; set; }
+
+    public Device ToDevice(Device existingDevice)
+    {
+        existingDevice.EncryptedPublicKey = EncryptedPublicKey;
+        existingDevice.EncryptedUserKey = EncryptedUserKey;
+        return existingDevice;
+    }
 }
 
 public class DeviceKeysUpdateRequestModel

src/Core/Auth/Models/Api/Response/DeviceAuthRequestResponseModel.cs

@@ -1,5 +1,4 @@
 using Bit.Core.Auth.Models.Data;
-using Bit.Core.Auth.Utilities;
 using Bit.Core.Enums;
 using Bit.Core.Models.Api;
 
@@ -19,7 +18,7 @@ public class DeviceAuthRequestResponseModel : ResponseModel
             Type = deviceAuthDetails.Type,
             Identifier = deviceAuthDetails.Identifier,
             CreationDate = deviceAuthDetails.CreationDate,
-            IsTrusted = deviceAuthDetails.IsTrusted()
+            IsTrusted = deviceAuthDetails.IsTrusted,
         };
 
         if (deviceAuthDetails.AuthRequestId != null && deviceAuthDetails.AuthRequestCreatedAt != null)

src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs

@@ -20,6 +20,7 @@ public class RotateUserAccountKeysData
     public IEnumerable<EmergencyAccess> EmergencyAccesses { get; set; }
     public IReadOnlyList<OrganizationUser> OrganizationUsers { get; set; }
     public IEnumerable<WebAuthnLoginRotateKeyData> WebAuthnKeys { get; set; }
+    public IEnumerable<Device> DeviceKeys { get; set; }
 
     // User vault data encrypted by the userkey
     public IEnumerable<Cipher> Ciphers { get; set; }

src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs

@@ -20,6 +20,7 @@ public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand
     private readonly ISendRepository _sendRepository;
     private readonly IEmergencyAccessRepository _emergencyAccessRepository;
     private readonly IOrganizationUserRepository _organizationUserRepository;
+    private readonly IDeviceRepository _deviceRepository;
     private readonly IPushNotificationService _pushService;
     private readonly IdentityErrorDescriber _identityErrorDescriber;
     private readonly IWebAuthnCredentialRepository _credentialRepository;
@@ -42,6 +43,7 @@ public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand
     public RotateUserAccountKeysCommand(IUserService userService, IUserRepository userRepository,
         ICipherRepository cipherRepository, IFolderRepository folderRepository, ISendRepository sendRepository,
         IEmergencyAccessRepository emergencyAccessRepository, IOrganizationUserRepository organizationUserRepository,
+        IDeviceRepository deviceRepository,
         IPasswordHasher<User> passwordHasher,
         IPushNotificationService pushService, IdentityErrorDescriber errors, IWebAuthnCredentialRepository credentialRepository)
     {
@@ -52,6 +54,7 @@ public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand
         _sendRepository = sendRepository;
         _emergencyAccessRepository = emergencyAccessRepository;
         _organizationUserRepository = organizationUserRepository;
+        _deviceRepository = deviceRepository;
         _pushService = pushService;
         _identityErrorDescriber = errors;
         _credentialRepository = credentialRepository;
@@ -127,6 +130,11 @@ public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand
             saveEncryptedDataActions.Add(_credentialRepository.UpdateKeysForRotationAsync(user.Id, model.WebAuthnKeys));
         }
 
+        if (model.DeviceKeys.Any())
+        {
+            saveEncryptedDataActions.Add(_deviceRepository.UpdateKeysForRotationAsync(user.Id, model.DeviceKeys));
+        }
+
         await _userRepository.UpdateUserKeyAndEncryptedDataV2Async(user, saveEncryptedDataActions);
         await _pushService.PushLogOutAsync(user.Id);
         return IdentityResult.Success;

src/Core/Repositories/IDeviceRepository.cs

@@ -1,5 +1,6 @@
 using Bit.Core.Auth.Models.Data;
 using Bit.Core.Entities;
+using Bit.Core.KeyManagement.UserKey;
 
 #nullable enable
 
@@ -16,4 +17,5 @@ public interface IDeviceRepository : IRepository<Device, Guid>
     // other requests.
     Task<ICollection<DeviceAuthDetails>> GetManyByUserIdWithDeviceAuth(Guid userId);
     Task ClearPushTokenAsync(Guid id);
+    UpdateEncryptedDataForKeyRotation UpdateKeysForRotationAsync(Guid userId, IEnumerable<Device> devices);
 }

src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

@@ -1,8 +1,10 @@
 using System.Data;
 using Bit.Core.Auth.Models.Data;
 using Bit.Core.Entities;
+using Bit.Core.KeyManagement.UserKey;
 using Bit.Core.Repositories;
 using Bit.Core.Settings;
+using Bit.Core.Utilities;
 using Dapper;
 using Microsoft.Data.SqlClient;
 
@@ -109,4 +111,35 @@ public class DeviceRepository : Repository<Device, Guid>, IDeviceRepository
                 commandType: CommandType.StoredProcedure);
         }
     }
+
+    public UpdateEncryptedDataForKeyRotation UpdateKeysForRotationAsync(Guid userId, IEnumerable<Device> devices)
+    {
+        return async (SqlConnection connection, SqlTransaction transaction) =>
+        {
+            const string sql = @"
+                UPDATE D
+                SET
+                    D.[EncryptedPublicKey] = UD.[encryptedPublicKey],
+                    D.[EncryptedUserKey] = UD.[encryptedUserKey]
+                FROM
+                    [dbo].[Device] D
+                INNER JOIN
+                    OPENJSON(@DeviceCredentials)
+                    WITH (
+                        id UNIQUEIDENTIFIER,
+                        encryptedPublicKey NVARCHAR(MAX),
+                        encryptedUserKey NVARCHAR(MAX)
+                    ) UD
+                    ON UD.[id] = D.[Id]
+                WHERE
+                    D.[UserId] = @UserId";
+            var deviceCredentials = CoreHelpers.ClassToJsonData(devices);
+
+            await connection.ExecuteAsync(
+                sql,
+                new { UserId = userId, DeviceCredentials = deviceCredentials },
+                transaction: transaction,
+                commandType: CommandType.Text);
+        };
+    }
 }

src/Infrastructure.EntityFramework/Repositories/DeviceRepository.cs

@@ -1,5 +1,6 @@
 using AutoMapper;
 using Bit.Core.Auth.Models.Data;
+using Bit.Core.KeyManagement.UserKey;
 using Bit.Core.Repositories;
 using Bit.Core.Settings;
 using Bit.Infrastructure.EntityFramework.Auth.Repositories.Queries;
@@ -91,4 +92,30 @@ public class DeviceRepository : Repository<Core.Entities.Device, Device, Guid>,
             return await query.GetQuery(dbContext, userId, expirationMinutes).ToListAsync();
         }
     }
+
+    public UpdateEncryptedDataForKeyRotation UpdateKeysForRotationAsync(Guid userId, IEnumerable<Core.Entities.Device> devices)
+    {
+        return async (_, _) =>
+        {
+            var deviceUpdates = devices.ToList();
+            using var scope = ServiceScopeFactory.CreateScope();
+            var dbContext = GetDatabaseContext(scope);
+            var userDevices = await GetDbSet(dbContext)
+                .Where(device => device.UserId == userId)
+                .ToListAsync();
+            var userDevicesWithUpdatesPending = userDevices
+                .Where(existingDevice => deviceUpdates.Any(updatedDevice => updatedDevice.Id == existingDevice.Id))
+                .ToList();
+
+            foreach (var deviceToUpdate in userDevicesWithUpdatesPending)
+            {
+                var deviceUpdate = deviceUpdates.First(deviceUpdate => deviceUpdate.Id == deviceToUpdate.Id);
+                deviceToUpdate.EncryptedPublicKey = deviceUpdate.EncryptedPublicKey;
+                deviceToUpdate.EncryptedUserKey = deviceUpdate.EncryptedUserKey;
+            }
+
+            await dbContext.SaveChangesAsync();
+        };
+    }
+
 }

test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs

@@ -29,6 +29,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
     private readonly ApiApplicationFactory _factory;
     private readonly LoginHelper _loginHelper;
     private readonly IUserRepository _userRepository;
+    private readonly IDeviceRepository _deviceRepository;
     private readonly IPasswordHasher<User> _passwordHasher;
     private string _ownerEmail = null!;
 
@@ -40,6 +41,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
         _client = factory.CreateClient();
         _loginHelper = new LoginHelper(_factory, _client);
         _userRepository = _factory.GetService<IUserRepository>();
+        _deviceRepository = _factory.GetService<IDeviceRepository>();
         _emergencyAccessRepository = _factory.GetService<IEmergencyAccessRepository>();
         _organizationUserRepository = _factory.GetService<IOrganizationUserRepository>();
         _passwordHasher = _factory.GetService<IPasswordHasher<User>>();
@@ -238,10 +240,12 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
         ];
         request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey = _mockEncryptedString;
         request.AccountUnlockData.PasskeyUnlockData = [];
+        request.AccountUnlockData.DeviceKeyUnlockData = [];
         request.AccountUnlockData.EmergencyAccessUnlockData = [];
         request.AccountUnlockData.OrganizationAccountRecoveryUnlockData = [];
 
         var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request);
+        var responseMessage = await response.Content.ReadAsStringAsync();
         response.EnsureSuccessStatusCode();
 
         var userNewState = await _userRepository.GetByEmailAsync(_ownerEmail);

test/Api.Test/KeyManagement/Validators/DeviceRotationValidatorTests.cs

@@ -0,0 +1,49 @@
+using Bit.Api.KeyManagement.Validators;
+using Bit.Core.Auth.Models.Api.Request;
+using Bit.Core.Entities;
+using Bit.Core.Exceptions;
+using Bit.Core.Repositories;
+using Bit.Test.Common.AutoFixture;
+using Bit.Test.Common.AutoFixture.Attributes;
+using NSubstitute;
+using Xunit;
+
+namespace Bit.Api.Test.KeyManagement.Validators;
+
+[SutProviderCustomize]
+public class DeviceRotationValidatorTests
+{
+    [Theory, BitAutoData]
+    public async Task ValidateAsync_SentDevicesAreEmptyButDatabaseDevicesAreNot_Throws(
+        SutProvider<DeviceRotationValidator> sutProvider, User user, IEnumerable<OtherDeviceKeysUpdateRequestModel> devices)
+    {
+        var userCiphers = devices.Select(c => new Device { Id = c.DeviceId, EncryptedPrivateKey = "EncryptedPrivateKey", EncryptedPublicKey = "EncryptedPublicKey", EncryptedUserKey = "EncryptedUserKey" }).ToList();
+        sutProvider.GetDependency<IDeviceRepository>().GetManyByUserIdAsync(user.Id)
+            .Returns(userCiphers);
+        await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.ValidateAsync(user, Enumerable.Empty<OtherDeviceKeysUpdateRequestModel>()));
+    }
+
+    [Theory, BitAutoData]
+    public async Task ValidateAsync_SentDevicesTrustedButDatabaseUntrusted_Throws(
+        SutProvider<DeviceRotationValidator> sutProvider, User user, IEnumerable<OtherDeviceKeysUpdateRequestModel> devices)
+    {
+        var userCiphers = devices.Select(c => new Device { Id = c.DeviceId, EncryptedPrivateKey = "Key", EncryptedPublicKey = "Key", EncryptedUserKey = "Key" }).ToList();
+        sutProvider.GetDependency<IDeviceRepository>().GetManyByUserIdAsync(user.Id)
+            .Returns(userCiphers);
+        await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.ValidateAsync(user, [
+            new OtherDeviceKeysUpdateRequestModel { DeviceId = userCiphers.First().Id, EncryptedPublicKey = null, EncryptedUserKey = null }
+        ]));
+    }
+
+    [Theory, BitAutoData]
+    public async Task ValidateAsync_Validates(
+        SutProvider<DeviceRotationValidator> sutProvider, User user, IEnumerable<OtherDeviceKeysUpdateRequestModel> devices)
+    {
+        var userCiphers = devices.Select(c => new Device { Id = c.DeviceId, EncryptedPrivateKey = "Key", EncryptedPublicKey = "Key", EncryptedUserKey = "Key" }).ToList().Slice(0, 1);
+        sutProvider.GetDependency<IDeviceRepository>().GetManyByUserIdAsync(user.Id)
+            .Returns(userCiphers);
+        Assert.NotEmpty(await sutProvider.Sut.ValidateAsync(user, [
+            new OtherDeviceKeysUpdateRequestModel { DeviceId = userCiphers.First().Id, EncryptedPublicKey = "Key", EncryptedUserKey = "Key" }
+        ]));
+    }
+}