From ff092a031eb0489672095a76c8f4d4564222b835 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 24 Sep 2025 05:10:46 +0900 Subject: [PATCH] [PM-23229] Add extra validation to kdf changes + authentication data + unlock data (#6121) * Added MasterPasswordUnlock to UserDecryptionOptions as part of identity response * Implement support for authentication data and unlock data in kdf change * Extract to kdf command and add tests * Fix namespace * Delete empty file * Fix build * Clean up tests * Fix tests * Add comments * Cleanup * Cleanup * Cleanup * Clean-up and fix build * Address feedback; force new parameters on KDF change request * Clean-up and add tests * Re-add logger * Update logger to interface * Clean up, remove Kdf Request Model * Remove kdf request model tests * Fix types in test * Address feedback to rename request model and re-add tests * Fix namespace * Move comments * Rename InnerKdfRequestModel to KdfRequestModel --------- Co-authored-by: Maciej Zieniuk --- .../Auth/Controllers/AccountsController.cs | 18 +- .../Request/Accounts/KdfRequestModel.cs | 25 -- ...sswordUnlockDataAndAuthenticationModel.cs} | 6 +- .../Request/Accounts/PasswordRequestModel.cs | 14 +- .../Models/Requests/KdfRequestModel.cs | 26 ++ ...rPasswordAuthenticationDataRequestModel.cs | 21 ++ .../MasterPasswordUnlockDataRequestModel.cs | 22 ++ .../Models/Requests/UnlockDataRequestModel.cs | 2 +- src/Core/Entities/User.cs | 5 + .../KeyManagement/Kdf/IChangeKdfCommand.cs | 15 + .../Kdf/Implementations/ChangeKdfCommand.cs | 94 +++++ ...eyManagementServiceCollectionExtensions.cs | 3 + .../KeyManagement/Models/Data/KdfSettings.cs | 38 +++ .../Data/MasterPasswordAuthenticationData.cs | 18 + ...sterPasswordUnlockAndAuthenticationData.cs | 34 ++ .../Models/Data/MasterPasswordUnlockData.cs | 28 +- .../Models/Data/RotateUserAccountKeysData.cs | 2 +- src/Core/Services/IUserService.cs | 2 - .../Services/Implementations/UserService.cs | 33 -- src/Core/Utilities/KdfSettingsValidator.cs | 6 + .../Controllers/AccountsControllerTests.cs | 61 +++- .../Request/MasterPasswordUnlockDataModel.cs | 6 +- .../Request/Accounts/KdfRequestModelTests.cs | 65 ---- .../Utilities/KdfSettingsValidatorTests.cs | 36 ++ .../Kdf/ChangeKdfCommandTests.cs | 322 ++++++++++++++++++ 25 files changed, 729 insertions(+), 173 deletions(-) delete mode 100644 src/Api/Auth/Models/Request/Accounts/KdfRequestModel.cs rename src/Api/Auth/Models/Request/Accounts/{MasterPasswordUnlockDataModel.cs => MasterPasswordUnlockDataAndAuthenticationModel.cs} (90%) create mode 100644 src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs create mode 100644 src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs create mode 100644 src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs create mode 100644 src/Core/KeyManagement/Kdf/IChangeKdfCommand.cs create mode 100644 src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs create mode 100644 src/Core/KeyManagement/Models/Data/KdfSettings.cs create mode 100644 src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs create mode 100644 src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs delete mode 100644 test/Api.Test/Models/Request/Accounts/KdfRequestModelTests.cs create mode 100644 test/Api.Test/Utilities/KdfSettingsValidatorTests.cs create mode 100644 test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 0bed7c29c4..501399db38 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -16,6 +16,7 @@ using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Kdf; using Bit.Core.Models.Api.Response; using Bit.Core.Repositories; using Bit.Core.Services; @@ -39,7 +40,7 @@ public class AccountsController : Controller private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IFeatureService _featureService; private readonly ITwoFactorEmailService _twoFactorEmailService; - + private readonly IChangeKdfCommand _changeKdfCommand; public AccountsController( IOrganizationService organizationService, @@ -51,7 +52,8 @@ public class AccountsController : Controller ITdeOffboardingPasswordCommand tdeOffboardingPasswordCommand, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IFeatureService featureService, - ITwoFactorEmailService twoFactorEmailService + ITwoFactorEmailService twoFactorEmailService, + IChangeKdfCommand changeKdfCommand ) { _organizationService = organizationService; @@ -64,7 +66,7 @@ public class AccountsController : Controller _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _featureService = featureService; _twoFactorEmailService = twoFactorEmailService; - + _changeKdfCommand = changeKdfCommand; } @@ -256,7 +258,7 @@ public class AccountsController : Controller } [HttpPost("kdf")] - public async Task PostKdf([FromBody] KdfRequestModel model) + public async Task PostKdf([FromBody] PasswordRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -264,8 +266,12 @@ public class AccountsController : Controller throw new UnauthorizedAccessException(); } - var result = await _userService.ChangeKdfAsync(user, model.MasterPasswordHash, - model.NewMasterPasswordHash, model.Key, model.Kdf.Value, model.KdfIterations.Value, model.KdfMemory, model.KdfParallelism); + if (model.AuthenticationData == null || model.UnlockData == null) + { + throw new BadRequestException("AuthenticationData and UnlockData must be provided."); + } + + var result = await _changeKdfCommand.ChangeKdfAsync(user, model.MasterPasswordHash, model.AuthenticationData.ToData(), model.UnlockData.ToData()); if (result.Succeeded) { return; diff --git a/src/Api/Auth/Models/Request/Accounts/KdfRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/KdfRequestModel.cs deleted file mode 100644 index fc62f22bab..0000000000 --- a/src/Api/Auth/Models/Request/Accounts/KdfRequestModel.cs +++ /dev/null @@ -1,25 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using Bit.Core.Enums; -using Bit.Core.Utilities; - -namespace Bit.Api.Auth.Models.Request.Accounts; - -public class KdfRequestModel : PasswordRequestModel, IValidatableObject -{ - [Required] - public KdfType? Kdf { get; set; } - [Required] - public int? KdfIterations { get; set; } - public int? KdfMemory { get; set; } - public int? KdfParallelism { get; set; } - - public override IEnumerable Validate(ValidationContext validationContext) - { - if (Kdf.HasValue && KdfIterations.HasValue) - { - return KdfSettingsValidator.Validate(Kdf.Value, KdfIterations.Value, KdfMemory, KdfParallelism); - } - - return Enumerable.Empty(); - } -} diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs similarity index 90% rename from src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs rename to src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs index ba57788cec..da361e5a0c 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs @@ -7,7 +7,7 @@ using Bit.Core.Utilities; namespace Bit.Api.Auth.Models.Request.Accounts; -public class MasterPasswordUnlockDataModel : IValidatableObject +public class MasterPasswordUnlockAndAuthenticationDataModel : IValidatableObject { public required KdfType KdfType { get; set; } public required int KdfIterations { get; set; } @@ -45,9 +45,9 @@ public class MasterPasswordUnlockDataModel : IValidatableObject } } - public MasterPasswordUnlockData ToUnlockData() + public MasterPasswordUnlockAndAuthenticationData ToUnlockData() { - var data = new MasterPasswordUnlockData + var data = new MasterPasswordUnlockAndAuthenticationData { KdfType = KdfType, KdfIterations = KdfIterations, diff --git a/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs index 01da1f0f9f..8fa51e9f34 100644 --- a/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs @@ -1,7 +1,7 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable +#nullable enable using System.ComponentModel.DataAnnotations; +using Bit.Api.KeyManagement.Models.Requests; namespace Bit.Api.Auth.Models.Request.Accounts; @@ -9,9 +9,13 @@ public class PasswordRequestModel : SecretVerificationRequestModel { [Required] [StringLength(300)] - public string NewMasterPasswordHash { get; set; } + public required string NewMasterPasswordHash { get; set; } [StringLength(50)] - public string MasterPasswordHint { get; set; } + public string? MasterPasswordHint { get; set; } [Required] - public string Key { get; set; } + public required string Key { get; set; } + + // Note: These will eventually become required, but not all consumers are moved over yet. + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } } diff --git a/src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs b/src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs new file mode 100644 index 0000000000..904304a633 --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs @@ -0,0 +1,26 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Api.KeyManagement.Models.Requests; + +public class KdfRequestModel +{ + [Required] + public required KdfType KdfType { get; init; } + [Required] + public required int Iterations { get; init; } + public int? Memory { get; init; } + public int? Parallelism { get; init; } + + public KdfSettings ToData() + { + return new KdfSettings + { + KdfType = KdfType, + Iterations = Iterations, + Memory = Memory, + Parallelism = Parallelism + }; + } +} diff --git a/src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs new file mode 100644 index 0000000000..d65dc8fcb7 --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs @@ -0,0 +1,21 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Api.KeyManagement.Models.Requests; + +public class MasterPasswordAuthenticationDataRequestModel +{ + public required KdfRequestModel Kdf { get; init; } + public required string MasterPasswordAuthenticationHash { get; init; } + [StringLength(256)] public required string Salt { get; init; } + + public MasterPasswordAuthenticationData ToData() + { + return new MasterPasswordAuthenticationData + { + Kdf = Kdf.ToData(), + MasterPasswordAuthenticationHash = MasterPasswordAuthenticationHash, + Salt = Salt + }; + } +} diff --git a/src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs new file mode 100644 index 0000000000..ce7a2b343f --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs @@ -0,0 +1,22 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Utilities; + +namespace Bit.Api.KeyManagement.Models.Requests; + +public class MasterPasswordUnlockDataRequestModel +{ + public required KdfRequestModel Kdf { get; init; } + [EncryptedString] public required string MasterKeyWrappedUserKey { get; init; } + [StringLength(256)] public required string Salt { get; init; } + + public MasterPasswordUnlockData ToData() + { + return new MasterPasswordUnlockData + { + Kdf = Kdf.ToData(), + MasterKeyWrappedUserKey = MasterKeyWrappedUserKey, + Salt = Salt + }; + } +} diff --git a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs index 23c3eb95d0..3af944110c 100644 --- a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs @@ -10,7 +10,7 @@ namespace Bit.Api.KeyManagement.Models.Requests; public class UnlockDataRequestModel { // All methods to get to the userkey - public required MasterPasswordUnlockDataModel MasterPasswordUnlockData { get; set; } + public required MasterPasswordUnlockAndAuthenticationDataModel MasterPasswordUnlockData { get; set; } public required IEnumerable EmergencyAccessUnlockData { get; set; } public required IEnumerable OrganizationAccountRecoveryUnlockData { get; set; } public required IEnumerable PasskeyUnlockData { get; set; } diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index b92d22b0e3..12c527ed78 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -78,6 +78,11 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac public DateTime? LastEmailChangeDate { get; set; } public bool VerifyDevices { get; set; } = true; + public string GetMasterPasswordSalt() + { + return Email.ToLowerInvariant().Trim(); + } + public void SetNewId() { Id = CoreHelpers.GenerateComb(); diff --git a/src/Core/KeyManagement/Kdf/IChangeKdfCommand.cs b/src/Core/KeyManagement/Kdf/IChangeKdfCommand.cs new file mode 100644 index 0000000000..081ae5248d --- /dev/null +++ b/src/Core/KeyManagement/Kdf/IChangeKdfCommand.cs @@ -0,0 +1,15 @@ +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.KeyManagement.Kdf; + +/// +/// Command to change the Key Derivation Function (KDF) settings for a user. This includes +/// changing the masterpassword authentication hash, and the masterkey encrypted userkey. +/// The salt must not change during the KDF change. +/// +public interface IChangeKdfCommand +{ + public Task ChangeKdfAsync(User user, string masterPasswordAuthenticationHash, MasterPasswordAuthenticationData authenticationData, MasterPasswordUnlockData unlockData); +} diff --git a/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs b/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs new file mode 100644 index 0000000000..fe736f9ac6 --- /dev/null +++ b/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs @@ -0,0 +1,94 @@ +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.KeyManagement.Kdf.Implementations; + +/// +public class ChangeKdfCommand : IChangeKdfCommand +{ + private readonly IUserService _userService; + private readonly IPushNotificationService _pushService; + private readonly IUserRepository _userRepository; + private readonly IdentityErrorDescriber _identityErrorDescriber; + private readonly ILogger _logger; + + public ChangeKdfCommand(IUserService userService, IPushNotificationService pushService, IUserRepository userRepository, IdentityErrorDescriber describer, ILogger logger) + { + _userService = userService; + _pushService = pushService; + _userRepository = userRepository; + _identityErrorDescriber = describer; + _logger = logger; + } + + public async Task ChangeKdfAsync(User user, string masterPasswordAuthenticationHash, MasterPasswordAuthenticationData authenticationData, MasterPasswordUnlockData unlockData) + { + ArgumentNullException.ThrowIfNull(user); + if (!await _userService.CheckPasswordAsync(user, masterPasswordAuthenticationHash)) + { + return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); + } + + // Validate to prevent user account from becoming un-decryptable from invalid parameters + // + // Prevent a de-synced salt value from creating an un-decryptable unlock method + authenticationData.ValidateSaltUnchangedForUser(user); + unlockData.ValidateSaltUnchangedForUser(user); + + // Currently KDF settings are not saved separately for authentication and unlock and must therefore be equal + if (!authenticationData.Kdf.Equals(unlockData.Kdf)) + { + throw new BadRequestException("KDF settings must be equal for authentication and unlock."); + } + var validationErrors = KdfSettingsValidator.Validate(unlockData.Kdf); + if (validationErrors.Any()) + { + throw new BadRequestException("KDF settings are invalid."); + } + + // Update the user with the new KDF settings + // This updates the authentication data and unlock data for the user separately. Currently these still + // use shared values for KDF settings and salt. + // The authentication hash, and the unlock data each are dependent on: + // - The master password (entered by the user every time) + // - The KDF settings (iterations, memory, parallelism) + // - The salt + // These combinations - (password, authentication hash, KDF settings, salt) and (password, unlock data, KDF settings, salt) + // must remain consistent to unlock correctly. + + // Authentication + // Note: This mutates the user but does not yet save it to DB. That is done atomically, later. + // This entire operation MUST be atomic to prevent a user from being locked out of their account. + // Salt is ensured to be the same as unlock data, and the value stored in the account and not updated. + // KDF is ensured to be the same as unlock data above and updated below. + var result = await _userService.UpdatePasswordHash(user, authenticationData.MasterPasswordAuthenticationHash); + if (!result.Succeeded) + { + _logger.LogWarning("Change KDF failed for user {userId}.", user.Id); + return result; + } + + // Salt is ensured to be the same as authentication data, and the value stored in the account, and is not updated. + // Kdf - These will be seperated in the future, but for now are ensured to be the same as authentication data above. + user.Key = unlockData.MasterKeyWrappedUserKey; + user.Kdf = unlockData.Kdf.KdfType; + user.KdfIterations = unlockData.Kdf.Iterations; + user.KdfMemory = unlockData.Kdf.Memory; + user.KdfParallelism = unlockData.Kdf.Parallelism; + + var now = DateTime.UtcNow; + user.RevisionDate = user.AccountRevisionDate = now; + user.LastKdfChangeDate = now; + + await _userRepository.ReplaceAsync(user); + await _pushService.PushLogOutAsync(user.Id); + return IdentityResult.Success; + } +} diff --git a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs index cacf3d4140..e4ebdb4860 100644 --- a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs +++ b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs @@ -1,5 +1,7 @@ using Bit.Core.KeyManagement.Commands; using Bit.Core.KeyManagement.Commands.Interfaces; +using Bit.Core.KeyManagement.Kdf; +using Bit.Core.KeyManagement.Kdf.Implementations; using Microsoft.Extensions.DependencyInjection; namespace Bit.Core.KeyManagement; @@ -15,5 +17,6 @@ public static class KeyManagementServiceCollectionExtensions private static void AddKeyManagementCommands(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); } } diff --git a/src/Core/KeyManagement/Models/Data/KdfSettings.cs b/src/Core/KeyManagement/Models/Data/KdfSettings.cs new file mode 100644 index 0000000000..cc1e465330 --- /dev/null +++ b/src/Core/KeyManagement/Models/Data/KdfSettings.cs @@ -0,0 +1,38 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; + +namespace Bit.Core.KeyManagement.Models.Data; + +public class KdfSettings +{ + public required KdfType KdfType { get; init; } + public required int Iterations { get; init; } + public int? Memory { get; init; } + public int? Parallelism { get; init; } + + public void ValidateUnchangedForUser(User user) + { + if (user.Kdf != KdfType || user.KdfIterations != Iterations || user.KdfMemory != Memory || user.KdfParallelism != Parallelism) + { + throw new ArgumentException("Invalid KDF settings."); + } + } + + public override bool Equals(object? obj) + { + if (obj is not KdfSettings other) + { + return false; + } + + return KdfType == other.KdfType && + Iterations == other.Iterations && + Memory == other.Memory && + Parallelism == other.Parallelism; + } + + public override int GetHashCode() + { + return HashCode.Combine(KdfType, Iterations, Memory, Parallelism); + } +} diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs new file mode 100644 index 0000000000..c0ae949a3f --- /dev/null +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs @@ -0,0 +1,18 @@ +using Bit.Core.Entities; + +namespace Bit.Core.KeyManagement.Models.Data; + +public class MasterPasswordAuthenticationData +{ + public required KdfSettings Kdf { get; init; } + public required string MasterPasswordAuthenticationHash { get; init; } + public required string Salt { get; init; } + + public void ValidateSaltUnchangedForUser(User user) + { + if (user.GetMasterPasswordSalt() != Salt) + { + throw new ArgumentException("Invalid master password salt."); + } + } +} diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs new file mode 100644 index 0000000000..e305d92fec --- /dev/null +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs @@ -0,0 +1,34 @@ +#nullable enable +using Bit.Core.Entities; +using Bit.Core.Enums; + +namespace Bit.Core.KeyManagement.Models.Data; + +public class MasterPasswordUnlockAndAuthenticationData +{ + public KdfType KdfType { get; set; } + public int KdfIterations { get; set; } + public int? KdfMemory { get; set; } + public int? KdfParallelism { get; set; } + + public required string Email { get; set; } + public required string MasterKeyAuthenticationHash { get; set; } + public required string MasterKeyEncryptedUserKey { get; set; } + public string? MasterPasswordHint { get; set; } + + public bool ValidateForUser(User user) + { + if (KdfType != user.Kdf || KdfMemory != user.KdfMemory || KdfParallelism != user.KdfParallelism || KdfIterations != user.KdfIterations) + { + return false; + } + else if (Email != user.Email) + { + return false; + } + else + { + return true; + } + } +} diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs index 0ddfc03190..d1ab6f645b 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs @@ -1,34 +1,20 @@ #nullable enable + using Bit.Core.Entities; -using Bit.Core.Enums; namespace Bit.Core.KeyManagement.Models.Data; public class MasterPasswordUnlockData { - public KdfType KdfType { get; set; } - public int KdfIterations { get; set; } - public int? KdfMemory { get; set; } - public int? KdfParallelism { get; set; } + public required KdfSettings Kdf { get; init; } + public required string MasterKeyWrappedUserKey { get; init; } + public required string Salt { get; init; } - public required string Email { get; set; } - public required string MasterKeyAuthenticationHash { get; set; } - public required string MasterKeyEncryptedUserKey { get; set; } - public string? MasterPasswordHint { get; set; } - - public bool ValidateForUser(User user) + public void ValidateSaltUnchangedForUser(User user) { - if (KdfType != user.Kdf || KdfMemory != user.KdfMemory || KdfParallelism != user.KdfParallelism || KdfIterations != user.KdfIterations) + if (user.GetMasterPasswordSalt() != Salt) { - return false; - } - else if (Email != user.Email) - { - return false; - } - else - { - return true; + throw new ArgumentException("Invalid master password salt."); } } } diff --git a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs index b89f19797f..557fb56ff3 100644 --- a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs +++ b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs @@ -19,7 +19,7 @@ public class RotateUserAccountKeysData public string AccountPublicKey { get; set; } // All methods to get to the userkey - public MasterPasswordUnlockData MasterPasswordUnlockData { get; set; } + public MasterPasswordUnlockAndAuthenticationData MasterPasswordUnlockData { get; set; } public IEnumerable EmergencyAccesses { get; set; } public IReadOnlyList OrganizationUsers { get; set; } public IEnumerable WebAuthnKeys { get; set; } diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index ef602be93a..412f9db36e 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -38,8 +38,6 @@ public interface IUserService Task ConvertToKeyConnectorAsync(User user); Task AdminResetPasswordAsync(OrganizationUserType type, Guid orgId, Guid id, string newMasterPassword, string key); Task UpdateTempPasswordAsync(User user, string newMasterPassword, string key, string hint); - Task ChangeKdfAsync(User user, string masterPassword, string newMasterPassword, string key, - KdfType kdf, int kdfIterations, int? kdfMemory, int? kdfParallelism); Task RefreshSecurityStampAsync(User user, string masterPasswordHash); Task UpdateTwoFactorProviderAsync(User user, TwoFactorProviderType type, bool setEnabled = true, bool logEvent = true); Task DisableTwoFactorProviderAsync(User user, TwoFactorProviderType type); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 386cb8c3d2..a36b9e37cc 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -777,39 +777,6 @@ public class UserService : UserManager, IUserService return IdentityResult.Success; } - public async Task ChangeKdfAsync(User user, string masterPassword, string newMasterPassword, - string key, KdfType kdf, int kdfIterations, int? kdfMemory, int? kdfParallelism) - { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - if (await CheckPasswordAsync(user, masterPassword)) - { - var result = await UpdatePasswordHash(user, newMasterPassword); - if (!result.Succeeded) - { - return result; - } - - var now = DateTime.UtcNow; - user.RevisionDate = user.AccountRevisionDate = now; - user.LastKdfChangeDate = now; - user.Key = key; - user.Kdf = kdf; - user.KdfIterations = kdfIterations; - user.KdfMemory = kdfMemory; - user.KdfParallelism = kdfParallelism; - await _userRepository.ReplaceAsync(user); - await _pushService.PushLogOutAsync(user.Id); - return IdentityResult.Success; - } - - Logger.LogWarning("Change KDF failed for user {userId}.", user.Id); - return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); - } - public async Task RefreshSecurityStampAsync(User user, string secret) { if (user == null) diff --git a/src/Core/Utilities/KdfSettingsValidator.cs b/src/Core/Utilities/KdfSettingsValidator.cs index db7936acff..f89e8ddb66 100644 --- a/src/Core/Utilities/KdfSettingsValidator.cs +++ b/src/Core/Utilities/KdfSettingsValidator.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Data; namespace Bit.Core.Utilities; @@ -34,4 +35,9 @@ public static class KdfSettingsValidator break; } } + + public static IEnumerable Validate(KdfSettings settings) + { + return Validate(settings.KdfType, settings.Iterations, settings.Memory, settings.Parallelism); + } } diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index ce870c0860..e81d51281d 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -10,6 +10,7 @@ using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Kdf; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture.Attributes; @@ -33,6 +34,7 @@ public class AccountsControllerTests : IDisposable private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand; private readonly IFeatureService _featureService; private readonly ITwoFactorEmailService _twoFactorEmailService; + private readonly IChangeKdfCommand _changeKdfCommand; public AccountsControllerTests() @@ -47,7 +49,7 @@ public class AccountsControllerTests : IDisposable _tdeOffboardingPasswordCommand = Substitute.For(); _featureService = Substitute.For(); _twoFactorEmailService = Substitute.For(); - + _changeKdfCommand = Substitute.For(); _sut = new AccountsController( _organizationService, @@ -59,7 +61,8 @@ public class AccountsControllerTests : IDisposable _tdeOffboardingPasswordCommand, _twoFactorIsEnabledQuery, _featureService, - _twoFactorEmailService + _twoFactorEmailService, + _changeKdfCommand ); } @@ -242,12 +245,18 @@ public class AccountsControllerTests : IDisposable { var user = GenerateExampleUser(); ConfigureUserServiceToReturnValidPrincipalFor(user); - _userService.ChangePasswordAsync(user, default, default, default, default) + _userService.ChangePasswordAsync(user, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(IdentityResult.Success)); - await _sut.PostPassword(new PasswordRequestModel()); + await _sut.PostPassword(new PasswordRequestModel + { + MasterPasswordHash = "masterPasswordHash", + NewMasterPasswordHash = "newMasterPasswordHash", + MasterPasswordHint = "masterPasswordHint", + Key = "key" + }); - await _userService.Received(1).ChangePasswordAsync(user, default, default, default, default); + await _userService.Received(1).ChangePasswordAsync(user, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } [Fact] @@ -256,7 +265,13 @@ public class AccountsControllerTests : IDisposable ConfigureUserServiceToReturnNullPrincipal(); await Assert.ThrowsAsync( - () => _sut.PostPassword(new PasswordRequestModel()) + () => _sut.PostPassword(new PasswordRequestModel + { + MasterPasswordHash = "masterPasswordHash", + NewMasterPasswordHash = "newMasterPasswordHash", + MasterPasswordHint = "masterPasswordHint", + Key = "key" + }) ); } @@ -265,11 +280,17 @@ public class AccountsControllerTests : IDisposable { var user = GenerateExampleUser(); ConfigureUserServiceToReturnValidPrincipalFor(user); - _userService.ChangePasswordAsync(user, default, default, default, default) + _userService.ChangePasswordAsync(user, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(IdentityResult.Failed())); await Assert.ThrowsAsync( - () => _sut.PostPassword(new PasswordRequestModel()) + () => _sut.PostPassword(new PasswordRequestModel + { + MasterPasswordHash = "masterPasswordHash", + NewMasterPasswordHash = "newMasterPasswordHash", + MasterPasswordHint = "masterPasswordHint", + Key = "key" + }) ); } @@ -593,6 +614,30 @@ public class AccountsControllerTests : IDisposable await _twoFactorEmailService.Received(1).SendNewDeviceVerificationEmailAsync(user); } + [Theory] + [BitAutoData] + public async Task PostKdf_WithNullAuthenticationData_ShouldFail( + User user, PasswordRequestModel model) + { + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + model.AuthenticationData = null; + + // Act + await Assert.ThrowsAsync(() => _sut.PostKdf(model)); + } + + [Theory] + [BitAutoData] + public async Task PostKdf_WithNullUnlockData_ShouldFail( + User user, PasswordRequestModel model) + { + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + model.UnlockData = null; + + // Act + await Assert.ThrowsAsync(() => _sut.PostKdf(model)); + } + // Below are helper functions that currently belong to this // test class, but ultimately may need to be split out into // something greater in order to share common test steps with diff --git a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs index 4c78c7015a..254f57a128 100644 --- a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs +++ b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs @@ -18,7 +18,7 @@ public class MasterPasswordUnlockDataModelTests [InlineData(KdfType.Argon2id, 3, 64, 4)] public void Validate_Success(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) { - var model = new MasterPasswordUnlockDataModel + var model = new MasterPasswordUnlockAndAuthenticationDataModel { KdfType = kdfType, KdfIterations = kdfIterations, @@ -43,7 +43,7 @@ public class MasterPasswordUnlockDataModelTests [InlineData((KdfType)2, 2, 64, 4)] public void Validate_Failure(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) { - var model = new MasterPasswordUnlockDataModel + var model = new MasterPasswordUnlockAndAuthenticationDataModel { KdfType = kdfType, KdfIterations = kdfIterations, @@ -59,7 +59,7 @@ public class MasterPasswordUnlockDataModelTests Assert.NotNull(result.First().ErrorMessage); } - private static List Validate(MasterPasswordUnlockDataModel model) + private static List Validate(MasterPasswordUnlockAndAuthenticationDataModel model) { var results = new List(); Validator.TryValidateObject(model, new ValidationContext(model), results, true); diff --git a/test/Api.Test/Models/Request/Accounts/KdfRequestModelTests.cs b/test/Api.Test/Models/Request/Accounts/KdfRequestModelTests.cs deleted file mode 100644 index 612b7ad442..0000000000 --- a/test/Api.Test/Models/Request/Accounts/KdfRequestModelTests.cs +++ /dev/null @@ -1,65 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using Bit.Api.Auth.Models.Request.Accounts; -using Bit.Core.Enums; -using Xunit; - -namespace Bit.Api.Test.Models.Request.Accounts; - -public class KdfRequestModelTests -{ - [Theory] - [InlineData(KdfType.PBKDF2_SHA256, 1_000_000, null, null)] // Somewhere in the middle - [InlineData(KdfType.PBKDF2_SHA256, 600_000, null, null)] // Right on the lower boundary - [InlineData(KdfType.PBKDF2_SHA256, 2_000_000, null, null)] // Right on the upper boundary - [InlineData(KdfType.Argon2id, 5, 500, 8)] // Somewhere in the middle - [InlineData(KdfType.Argon2id, 2, 15, 1)] // Right on the lower boundary - [InlineData(KdfType.Argon2id, 10, 1024, 16)] // Right on the upper boundary - public void Validate_IsValid(KdfType kdfType, int? kdfIterations, int? kdfMemory, int? kdfParallelism) - { - var model = new KdfRequestModel - { - Kdf = kdfType, - KdfIterations = kdfIterations, - KdfMemory = kdfMemory, - KdfParallelism = kdfParallelism, - Key = "TEST", - NewMasterPasswordHash = "TEST", - }; - - var results = Validate(model); - Assert.Empty(results); - } - - [Theory] - [InlineData(null, 350_000, null, null, 1)] // Although KdfType is nullable, it's marked as [Required] - [InlineData(KdfType.PBKDF2_SHA256, 500_000, null, null, 1)] // Too few iterations - [InlineData(KdfType.PBKDF2_SHA256, 2_000_001, null, null, 1)] // Too many iterations - [InlineData(KdfType.Argon2id, 0, 30, 8, 1)] // Iterations must be greater than 0 - [InlineData(KdfType.Argon2id, 10, 14, 8, 1)] // Too little memory - [InlineData(KdfType.Argon2id, 10, 14, 0, 1)] // Too small of a parallelism value - [InlineData(KdfType.Argon2id, 10, 1025, 8, 1)] // Too much memory - [InlineData(KdfType.Argon2id, 10, 512, 17, 1)] // Too big of a parallelism value - public void Validate_Fails(KdfType? kdfType, int? kdfIterations, int? kdfMemory, int? kdfParallelism, int expectedFailures) - { - var model = new KdfRequestModel - { - Kdf = kdfType, - KdfIterations = kdfIterations, - KdfMemory = kdfMemory, - KdfParallelism = kdfParallelism, - Key = "TEST", - NewMasterPasswordHash = "TEST", - }; - - var results = Validate(model); - Assert.NotEmpty(results); - Assert.Equal(expectedFailures, results.Count); - } - - public static List Validate(KdfRequestModel model) - { - var results = new List(); - Validator.TryValidateObject(model, new ValidationContext(model), results); - return results; - } -} diff --git a/test/Api.Test/Utilities/KdfSettingsValidatorTests.cs b/test/Api.Test/Utilities/KdfSettingsValidatorTests.cs new file mode 100644 index 0000000000..5c556979c7 --- /dev/null +++ b/test/Api.Test/Utilities/KdfSettingsValidatorTests.cs @@ -0,0 +1,36 @@ +using Bit.Core.Enums; +using Bit.Core.Utilities; +using Xunit; + +namespace Bit.Api.Test.Utilities; + +public class KdfSettingsValidatorTests +{ + [Theory] + [InlineData(KdfType.PBKDF2_SHA256, 1_000_000, null, null)] // Somewhere in the middle + [InlineData(KdfType.PBKDF2_SHA256, 600_000, null, null)] // Right on the lower boundary + [InlineData(KdfType.PBKDF2_SHA256, 2_000_000, null, null)] // Right on the upper boundary + [InlineData(KdfType.Argon2id, 5, 500, 8)] // Somewhere in the middle + [InlineData(KdfType.Argon2id, 2, 15, 1)] // Right on the lower boundary + [InlineData(KdfType.Argon2id, 10, 1024, 16)] // Right on the upper boundary + public void Validate_IsValid(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) + { + var results = KdfSettingsValidator.Validate(kdfType, kdfIterations, kdfMemory, kdfParallelism); + Assert.Empty(results); + } + + [Theory] + [InlineData(KdfType.PBKDF2_SHA256, 500_000, null, null, 1)] // Too few iterations + [InlineData(KdfType.PBKDF2_SHA256, 2_000_001, null, null, 1)] // Too many iterations + [InlineData(KdfType.Argon2id, 0, 30, 8, 1)] // Iterations must be greater than 0 + [InlineData(KdfType.Argon2id, 10, 14, 8, 1)] // Too little memory + [InlineData(KdfType.Argon2id, 10, 14, 0, 1)] // Too small of a parallelism value + [InlineData(KdfType.Argon2id, 10, 1025, 8, 1)] // Too much memory + [InlineData(KdfType.Argon2id, 10, 512, 17, 1)] // Too big of a parallelism value + public void Validate_Fails(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism, int expectedFailures) + { + var results = KdfSettingsValidator.Validate(kdfType, kdfIterations, kdfMemory, kdfParallelism); + Assert.NotEmpty(results); + Assert.Equal(expectedFailures, results.Count()); + } +} diff --git a/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs b/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs new file mode 100644 index 0000000000..02e04b9ce9 --- /dev/null +++ b/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs @@ -0,0 +1,322 @@ +#nullable enable + +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Kdf.Implementations; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.KeyManagement.Kdf; + +[SutProviderCustomize] +public class ChangeKdfCommandTests +{ + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_ChangesKdfAsync(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()).Returns(Task.FromResult(IdentityResult.Success)); + + var kdf = new KdfSettings + { + KdfType = Enums.KdfType.Argon2id, + Iterations = 4, + Memory = 512, + Parallelism = 4 + }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "newMasterPassword", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "masterKeyWrappedUserKey", + Salt = user.GetMasterPasswordSalt() + }; + + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); + + await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => + u.Id == user.Id + && u.Kdf == Enums.KdfType.Argon2id + && u.KdfIterations == 4 + && u.KdfMemory == 512 + && u.KdfParallelism == 4 + )); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_UserIsNull_ThrowsArgumentNullException(SutProvider sutProvider) + { + var kdf = new KdfSettings + { + KdfType = Enums.KdfType.Argon2id, + Iterations = 4, + Memory = 512, + Parallelism = 4 + }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "newMasterPassword", + Salt = "salt" + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "masterKeyWrappedUserKey", + Salt = "salt" + }; + + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ChangeKdfAsync(null!, "masterPassword", authenticationData, unlockData)); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_WrongPassword_ReturnsPasswordMismatch(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(false)); + + var kdf = new KdfSettings + { + KdfType = Enums.KdfType.Argon2id, + Iterations = 4, + Memory = 512, + Parallelism = 4 + }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "newMasterPassword", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "masterKeyWrappedUserKey", + Salt = user.GetMasterPasswordSalt() + }; + + var result = await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); + Assert.False(result.Succeeded); + Assert.Contains(result.Errors, e => e.Code == "PasswordMismatch"); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_WithAuthenticationAndUnlockData_UpdatesUserCorrectly(SutProvider sutProvider, User user) + { + var constantKdf = new KdfSettings + { + KdfType = Enums.KdfType.Argon2id, + Iterations = 5, + Memory = 1024, + Parallelism = 4 + }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = constantKdf, + MasterPasswordAuthenticationHash = "new-auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = constantKdf, + MasterKeyWrappedUserKey = "new-wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()).Returns(Task.FromResult(IdentityResult.Success)); + + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); + + await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => + u.Id == user.Id + && u.Kdf == constantKdf.KdfType + && u.KdfIterations == constantKdf.Iterations + && u.KdfMemory == constantKdf.Memory + && u.KdfParallelism == constantKdf.Parallelism + && u.Key == "new-wrapped-key" + )); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_KdfNotEqualBetweenAuthAndUnlock_ThrowsBadRequestException(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }, + MasterPasswordAuthenticationHash = "new-auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = new KdfSettings { KdfType = Enums.KdfType.PBKDF2_SHA256, Iterations = 100000 }, + MasterKeyWrappedUserKey = "new-wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_AuthDataSaltMismatch_Throws(SutProvider sutProvider, User user, KdfSettings kdf) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "new-auth-hash", + Salt = "different-salt" + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "new-wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_UnlockDataSaltMismatch_Throws(SutProvider sutProvider, User user, KdfSettings kdf) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "new-auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "new-wrapped-key", + Salt = "different-salt" + }; + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_UpdatePasswordHashFails_ReturnsFailure(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + var failedResult = IdentityResult.Failed(new IdentityError { Code = "TestFail", Description = "Test fail" }); + sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()).Returns(Task.FromResult(failedResult)); + + var kdf = new KdfSettings + { + KdfType = Enums.KdfType.Argon2id, + Iterations = 4, + Memory = 512, + Parallelism = 4 + }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "newMasterPassword", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "masterKeyWrappedUserKey", + Salt = user.GetMasterPasswordSalt() + }; + + var result = await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); + + Assert.False(result.Succeeded); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_InvalidKdfSettings_ThrowsBadRequestException(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + + // Create invalid KDF settings (iterations too low for PBKDF2) + var invalidKdf = new KdfSettings + { + KdfType = Enums.KdfType.PBKDF2_SHA256, + Iterations = 1000, // This is below the minimum of 600,000 + Memory = null, + Parallelism = null + }; + + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = invalidKdf, + MasterPasswordAuthenticationHash = "new-auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = invalidKdf, + MasterKeyWrappedUserKey = "new-wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + + var exception = await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); + + Assert.Equal("KDF settings are invalid.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_InvalidArgon2Settings_ThrowsBadRequestException(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(true)); + + // Create invalid Argon2 KDF settings (memory too high) + var invalidKdf = new KdfSettings + { + KdfType = Enums.KdfType.Argon2id, + Iterations = 3, // Valid + Memory = 2048, // This is above the maximum of 1024 + Parallelism = 4 // Valid + }; + + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = invalidKdf, + MasterPasswordAuthenticationHash = "new-auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = invalidKdf, + MasterKeyWrappedUserKey = "new-wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + + var exception = await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); + + Assert.Equal("KDF settings are invalid.", exception.Message); + } + +}