diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs index da361e5a0c7c..a4419c79accf 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataAndAuthenticationModel.cs @@ -1,6 +1,4 @@ -#nullable enable - -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Utilities; @@ -22,6 +20,8 @@ public class MasterPasswordUnlockAndAuthenticationDataModel : IValidatableObject [EncryptedString] public required string MasterKeyEncryptedUserKey { get; set; } [StringLength(50)] public string? MasterPasswordHint { get; set; } + [MaxLength(256)] + public string? MasterPasswordSalt { get; set; } public IEnumerable Validate(ValidationContext validationContext) { @@ -58,9 +58,9 @@ public MasterPasswordUnlockAndAuthenticationData ToUnlockData() MasterKeyAuthenticationHash = MasterKeyAuthenticationHash, MasterKeyEncryptedUserKey = MasterKeyEncryptedUserKey, - MasterPasswordHint = MasterPasswordHint + MasterPasswordHint = MasterPasswordHint, + MasterPasswordSalt = MasterPasswordSalt }; return data; } - } diff --git a/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs index cb66540a6bb4..030319a864ee 100644 --- a/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs +++ b/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs @@ -72,8 +72,7 @@ public User ToUser() // KdfMemory and KdfParallelism are optional (only used for Argon2id) KdfMemory = MasterPasswordUnlock?.Kdf.Memory ?? KdfMemory, KdfParallelism = MasterPasswordUnlock?.Kdf.Parallelism ?? KdfParallelism, - // PM-28827 To be added when MasterPasswordSalt is added to the user column - // MasterPasswordSalt = MasterPasswordUnlock?.Salt ?? Email.ToLower().Trim(), + MasterPasswordSalt = MasterPasswordUnlock?.Salt, Key = MasterPasswordUnlock?.MasterKeyWrappedUserKey ?? UserSymmetricKey }; diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs index b79ce8bce10c..0cf59140bec2 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs @@ -18,6 +18,7 @@ public class MasterPasswordUnlockAndAuthenticationData /// public required string MasterKeyEncryptedUserKey { get; set; } public string? MasterPasswordHint { get; set; } + public string? MasterPasswordSalt { get; set; } public bool ValidateForUser(User user) { @@ -25,6 +26,10 @@ public bool ValidateForUser(User user) { return false; } + else if (MasterPasswordSalt != null && MasterPasswordSalt != user.GetMasterPasswordSalt()) + { + return false; + } else if (Email != user.Email) { return false; diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 43b0f064a7d9..2b6e32fb0dd1 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -197,7 +197,7 @@ private void BuildMasterPasswordUnlock() Parallelism = _user.KdfParallelism }, MasterKeyEncryptedUserKey = _user.Key!, - Salt = _user.Email.ToLowerInvariant() + Salt = _user.GetMasterPasswordSalt() }; } else diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 4dd5df514fe8..b7517f4b2225 100644 --- a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -801,6 +801,7 @@ private void SetupRotateUserAccountUnlockData( request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory = user.KdfMemory; request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism = user.KdfParallelism; request.AccountUnlockData.MasterPasswordUnlockData.Email = user.Email; + request.AccountUnlockData.MasterPasswordUnlockData.MasterPasswordSalt = user.GetMasterPasswordSalt(); request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey = _mockEncryptedString; // Unlock data arrays diff --git a/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs b/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs index 3c099ce9626d..1ab0709d64eb 100644 --- a/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs +++ b/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs @@ -181,6 +181,37 @@ public void ToUser_Returns_User(string email, string masterPasswordHash, string Assert.Equal(userSymmetricKey, result.Key); Assert.Equal(userAsymmetricKeys.PublicKey, result.PublicKey); Assert.Equal(userAsymmetricKeys.EncryptedPrivateKey, result.PrivateKey); + Assert.Null(result.MasterPasswordSalt); + } + + [Theory] + [BitAutoData] + public void ToUser_WithMasterPasswordUnlock_MapsSalt(string email, string masterPasswordHint, + KeysRequestModel userAsymmetricKeys) + { + // Arrange + var model = new RegisterFinishRequestModel + { + Email = email, + MasterPasswordHint = masterPasswordHint, + UserAsymmetricKeys = userAsymmetricKeys, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = AuthConstants.PBKDF2_ITERATIONS.Default + }, + MasterKeyWrappedUserKey = "wrapped-key", + Salt = "explicit-salt-value" + } + }; + + // Act + var resultUser = model.ToUser(); + + // Assert + Assert.Equal("explicit-salt-value", resultUser.MasterPasswordSalt); } [Fact] diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs index 40406509549b..de4a61b563a0 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs @@ -139,6 +139,48 @@ public async Task SetInitialMasterPassword_InvalidSalt_ThrowsBadRequestException Assert.Equal("Invalid master password salt.", exception.Message); } + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_NullSalt_UsesEmailFallback( + SutProvider sutProvider, + User user, UserAccountKeysData accountKeys, KdfSettings kdfSettings, + Organization org, OrganizationUser orgUser, string serverSideHash, string masterPasswordHint) + { + // Arrange + user.Key = null; + user.MasterPasswordSalt = null; + var expectedSalt = user.Email.ToLowerInvariant().Trim(); + var model = CreateValidModel(user, accountKeys, kdfSettings, org.Identifier, masterPasswordHint); + + // Verify the model uses the email-derived salt + Assert.Equal(expectedSalt, model.MasterPasswordUnlock.Salt); + Assert.Equal(expectedSalt, model.MasterPasswordAuthentication.Salt); + + sutProvider.GetDependency() + .GetByIdentifierAsync(org.Identifier) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + sutProvider.GetDependency>() + .HashPassword(user, model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) + .Returns(serverSideHash); + + UpdateUserData mockUpdateUserData = (connection, transaction) => Task.CompletedTask; + sutProvider.GetDependency() + .SetMasterPassword(user.Id, model.MasterPasswordUnlock, serverSideHash, model.MasterPasswordHint) + .Returns(mockUpdateUserData); + + // Act — should not throw since email fallback provides a valid salt + await sutProvider.Sut.SetInitialMasterPasswordAsync(user, model); + + // Assert + await sutProvider.GetDependency().Received(1) + .LogUserEventAsync(user.Id, EventType.User_ChangedPassword); + } + [Theory] [BitAutoData] public async Task SetInitialMasterPassword_InvalidOrgSsoIdentifier_ThrowsBadRequestException( diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs index 5a144b7042be..6aea42970063 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs @@ -65,6 +65,50 @@ await sutProvider.GetDependency().Received(1) .LogUserEventAsync(user.Id, EventType.User_ChangedPassword); } + [Theory] + [BitAutoData] + public async Task OnboardMasterPassword_NullSalt_UsesEmailFallback( + SutProvider sutProvider, + User user, KdfSettings kdfSettings, + Organization org, OrganizationUser orgUser, string serverSideHash, string masterPasswordHint) + { + // Arrange + user.Key = null; + user.PublicKey = "public-key"; + user.PrivateKey = "private-key"; + user.MasterPasswordSalt = null; + var expectedSalt = user.Email.ToLowerInvariant().Trim(); + var model = CreateValidModel(user, kdfSettings, org.Identifier, masterPasswordHint); + + // Verify the model uses the email-derived salt + Assert.Equal(expectedSalt, model.MasterPasswordUnlock.Salt); + Assert.Equal(expectedSalt, model.MasterPasswordAuthentication.Salt); + + sutProvider.GetDependency() + .GetByIdentifierAsync(org.Identifier) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + sutProvider.GetDependency>() + .HashPassword(user, model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) + .Returns(serverSideHash); + + UpdateUserData mockUpdateUserData = (connection, transaction) => Task.CompletedTask; + sutProvider.GetDependency() + .SetMasterPassword(user.Id, model.MasterPasswordUnlock, serverSideHash, model.MasterPasswordHint) + .Returns(mockUpdateUserData); + + // Act — should not throw since email fallback provides a valid salt + await sutProvider.Sut.SetMasterPasswordAsync(user, model); + + // Assert + await sutProvider.GetDependency().Received(1) + .LogUserEventAsync(user.Id, EventType.User_ChangedPassword); + } + [Theory] [BitAutoData] public async Task OnboardMasterPassword_UserAlreadyHasPassword_ThrowsBadRequestException( diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index 161300719f5e..0e000fba4bf4 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -600,7 +600,9 @@ private static void SetTestKdfAndSaltForUserAndModel(User user, RotateUserAccoun model.MasterPasswordUnlockData.KdfIterations = 3; model.MasterPasswordUnlockData.KdfMemory = 64; model.MasterPasswordUnlockData.KdfParallelism = 4; - // The email is the salt for the KDF and is validated currently. + model.MasterPasswordUnlockData.MasterPasswordSalt = user.GetMasterPasswordSalt(); + // The email used to be the salt for the KDF and is validated currently. + // TODO: This can be removed once the email is no longer used as part of the KDF salt and validation is updated to reflect that. user.Email = model.MasterPasswordUnlockData.Email; } diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index a8cb99e962f5..0e29d353c52c 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -335,6 +335,7 @@ public async Task Build_WhenUserHasNoMasterPassword_ShouldReturnNoMasterPassword public async Task Build_WhenUserHasMasterPassword_ShouldReturnMasterPasswordUnlock(User user) { user.Email = "test@example.COM"; + user.MasterPasswordSalt = null; var result = await _builder.ForUser(user).BuildAsync(); @@ -344,7 +345,19 @@ public async Task Build_WhenUserHasMasterPassword_ShouldReturnMasterPasswordUnlo Assert.Equal(user.KdfIterations, result.MasterPasswordUnlock.Kdf.Iterations); Assert.Equal(user.KdfMemory, result.MasterPasswordUnlock.Kdf.Memory); Assert.Equal(user.KdfParallelism, result.MasterPasswordUnlock.Kdf.Parallelism); - Assert.Equal("test@example.com", result.MasterPasswordUnlock.Salt); + Assert.Equal(user.GetMasterPasswordSalt(), result.MasterPasswordUnlock.Salt); Assert.Equal(user.Key, result.MasterPasswordUnlock.MasterKeyEncryptedUserKey); } + + [Theory, BitAutoData] + public async Task Build_WhenUserHasExplicitSalt_ShouldReturnExplicitSalt(User user) + { + user.MasterPasswordSalt = "explicit-salt-value"; + + var result = await _builder.ForUser(user).BuildAsync(); + + Assert.True(result.HasMasterPassword); + Assert.NotNull(result.MasterPasswordUnlock); + Assert.Equal("explicit-salt-value", result.MasterPasswordUnlock.Salt); + } }