From 05398ad8a4ba3329cdf9c0aed3e4b35fa150a4a9 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Thu, 24 Jul 2025 12:49:15 -0400 Subject: [PATCH] [PM-22736] Send password hasher (#6112) * feat: - Add SendPasswordHasher class and interface - DI for SendPasswordHasher to use Marker class allowing us to use custom options for the SendPasswordHasher without impacting other PasswordHashers. * test: Unit tests for SendPasswordHasher implementation * doc: docs for interface and comments Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> --- .../PasswordValidationConstants.cs | 6 + .../Sends/ISendPasswordHasher.cs | 21 ++++ .../KeyManagement/Sends/SendPasswordHasher.cs | 35 ++++++ .../Sends/SendPasswordHasherMarker.cs | 10 ++ ...sswordHasherServiceCollectionExtensions.cs | 31 ++++++ .../Utilities/ServiceCollectionExtensions.cs | 3 +- .../KeyManagement/SendPasswordHasherTests.cs | 103 ++++++++++++++++++ 7 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 src/Core/Auth/UserFeatures/PasswordValidation/PasswordValidationConstants.cs create mode 100644 src/Core/KeyManagement/Sends/ISendPasswordHasher.cs create mode 100644 src/Core/KeyManagement/Sends/SendPasswordHasher.cs create mode 100644 src/Core/KeyManagement/Sends/SendPasswordHasherMarker.cs create mode 100644 src/Core/KeyManagement/Sends/SendPasswordHasherServiceCollectionExtensions.cs create mode 100644 test/Core.Test/KeyManagement/SendPasswordHasherTests.cs diff --git a/src/Core/Auth/UserFeatures/PasswordValidation/PasswordValidationConstants.cs b/src/Core/Auth/UserFeatures/PasswordValidation/PasswordValidationConstants.cs new file mode 100644 index 0000000000..b0803cd3cd --- /dev/null +++ b/src/Core/Auth/UserFeatures/PasswordValidation/PasswordValidationConstants.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.Auth.UserFeatures.PasswordValidation; + +public static class PasswordValidationConstants +{ + public const int PasswordHasherKdfIterations = 100000; +} diff --git a/src/Core/KeyManagement/Sends/ISendPasswordHasher.cs b/src/Core/KeyManagement/Sends/ISendPasswordHasher.cs new file mode 100644 index 0000000000..63bb7f5499 --- /dev/null +++ b/src/Core/KeyManagement/Sends/ISendPasswordHasher.cs @@ -0,0 +1,21 @@ +namespace Bit.Core.KeyManagement.Sends; + +public interface ISendPasswordHasher +{ + /// + /// Matches the send password hash against the user provided client password hash. The send password is server hashed and the client + /// password hash is hashed by the server for comparison in this method. + /// + /// The send password that is hashed by the server. + /// The user provided password hash that has not yet been hashed by the server for comparison. + /// true if hashes match false otherwise + /// Thrown if the server password hash or client password hash is null or empty. + bool PasswordHashMatches(string sendPasswordHash, string clientPasswordHash); + + /// + /// Accepts a client hashed send password and returns a server hashed password. + /// + /// + /// server hashed password + string HashOfClientPasswordHash(string clientHashedPassword); +} diff --git a/src/Core/KeyManagement/Sends/SendPasswordHasher.cs b/src/Core/KeyManagement/Sends/SendPasswordHasher.cs new file mode 100644 index 0000000000..abe57d3cc6 --- /dev/null +++ b/src/Core/KeyManagement/Sends/SendPasswordHasher.cs @@ -0,0 +1,35 @@ +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.KeyManagement.Sends; + +internal class SendPasswordHasher(IPasswordHasher passwordHasher) : ISendPasswordHasher +{ + private readonly IPasswordHasher _passwordHasher = passwordHasher; + + /// + /// + /// + public bool PasswordHashMatches(string sendPasswordHash, string inputPasswordHash) + { + if (string.IsNullOrWhiteSpace(sendPasswordHash) || string.IsNullOrWhiteSpace(inputPasswordHash)) + { + return false; + } + + var passwordResult = _passwordHasher.VerifyHashedPassword(SendPasswordHasherMarker.Instance, sendPasswordHash, inputPasswordHash); + + /* + In our use-case we input a high-entropy, pre-hashed secret sent by the client. Thus, we don't really care + about if the hash needs to be rehashed. Sends also only live for 30 days max. + */ + return passwordResult is PasswordVerificationResult.Success or PasswordVerificationResult.SuccessRehashNeeded; + } + + /// + /// + /// + public string HashOfClientPasswordHash(string clientHashedPassword) + { + return _passwordHasher.HashPassword(SendPasswordHasherMarker.Instance, clientHashedPassword); + } +} diff --git a/src/Core/KeyManagement/Sends/SendPasswordHasherMarker.cs b/src/Core/KeyManagement/Sends/SendPasswordHasherMarker.cs new file mode 100644 index 0000000000..d4b80a09a2 --- /dev/null +++ b/src/Core/KeyManagement/Sends/SendPasswordHasherMarker.cs @@ -0,0 +1,10 @@ +namespace Bit.Core.KeyManagement.Sends; + +// This should not be used except for DI as open generic marker class for use with +// the SendPasswordHasher. +public class SendPasswordHasherMarker +{ + // We know we will pass a single instance that isn't used to the PasswordHasher so we + // gain an efficiency benefit of not creating multiple marker classes. + public static readonly SendPasswordHasherMarker Instance = new(); +} diff --git a/src/Core/KeyManagement/Sends/SendPasswordHasherServiceCollectionExtensions.cs b/src/Core/KeyManagement/Sends/SendPasswordHasherServiceCollectionExtensions.cs new file mode 100644 index 0000000000..22939ce60c --- /dev/null +++ b/src/Core/KeyManagement/Sends/SendPasswordHasherServiceCollectionExtensions.cs @@ -0,0 +1,31 @@ +using Bit.Core.Auth.UserFeatures.PasswordValidation; +using Bit.Core.KeyManagement.Sends; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection.Extensions; + +namespace Microsoft.Extensions.DependencyInjection; + +using Microsoft.Extensions.Options; + +public static class SendPasswordHasherServiceCollectionExtensions +{ + public static void AddSendPasswordServices(this IServiceCollection services) + { + const string sendPasswordHasherMarkerName = "SendPasswordHasherMarker"; + + services.AddOptions(sendPasswordHasherMarkerName) + .Configure(options => options.IterationCount = PasswordValidationConstants.PasswordHasherKdfIterations); + + services.TryAddScoped>(sp => + { + var opts = sp + .GetRequiredService>() + .Get(sendPasswordHasherMarkerName); + + var optionsAccessor = Options.Create(opts); + + return new PasswordHasher(optionsAccessor); + }); + services.TryAddScoped(); + } +} diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 0768aa7060..0c3f0cbca1 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -23,6 +23,7 @@ using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Services; using Bit.Core.Auth.Services.Implementations; using Bit.Core.Auth.UserFeatures; +using Bit.Core.Auth.UserFeatures.PasswordValidation; using Bit.Core.Billing.Services; using Bit.Core.Billing.Services.Implementations; using Bit.Core.Billing.TrialInitiation; @@ -381,7 +382,7 @@ public static class ServiceCollectionExtensions services.TryAddTransient(typeof(IOtpTokenProvider<>), typeof(OtpTokenProvider<>)); services.AddScoped(); - services.Configure(options => options.IterationCount = 100000); + services.Configure(options => options.IterationCount = PasswordValidationConstants.PasswordHasherKdfIterations); services.Configure(options => { options.TokenLifespan = TimeSpan.FromDays(30); diff --git a/test/Core.Test/KeyManagement/SendPasswordHasherTests.cs b/test/Core.Test/KeyManagement/SendPasswordHasherTests.cs new file mode 100644 index 0000000000..53c518b03f --- /dev/null +++ b/test/Core.Test/KeyManagement/SendPasswordHasherTests.cs @@ -0,0 +1,103 @@ +using Bit.Core.KeyManagement.Sends; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.KeyManagement.Sends; + +[SutProviderCustomize] +public class SendPasswordHasherTests +{ + [Theory] + [BitAutoData(PasswordVerificationResult.Success)] + [BitAutoData(PasswordVerificationResult.SuccessRehashNeeded)] + void VerifyPasswordHash_WithValidMatching_ReturnsTrue( + PasswordVerificationResult passwordVerificationResult, + SutProvider sutProvider, + string sendPasswordHash, + string inputPasswordHash) + { + // Arrange + sutProvider.GetDependency>() + .VerifyHashedPassword(Arg.Any(), sendPasswordHash, inputPasswordHash) + .Returns(passwordVerificationResult); + + // Act + var result = sutProvider.Sut.PasswordHashMatches(sendPasswordHash, inputPasswordHash); + + // Assert + Assert.True(result); + sutProvider.GetDependency>() + .Received(1) + .VerifyHashedPassword(Arg.Any(), sendPasswordHash, inputPasswordHash); + } + + [Theory, BitAutoData] + void VerifyPasswordHash_WithNonMatchingPasswords_ReturnsFalse( + SutProvider sutProvider, + string sendPasswordHash, + string inputPasswordHash) + { + // Arrange + sutProvider.GetDependency>() + .VerifyHashedPassword(Arg.Any(), sendPasswordHash, inputPasswordHash) + .Returns(PasswordVerificationResult.Failed); + + // Act + var result = sutProvider.Sut.PasswordHashMatches(sendPasswordHash, inputPasswordHash); + + // Assert + Assert.False(result); + sutProvider.GetDependency>() + .Received(1) + .VerifyHashedPassword(Arg.Any(), sendPasswordHash, inputPasswordHash); + } + + [Theory] + [InlineData(null, "inputPassword")] + [InlineData("", "inputPassword")] + [InlineData(" ", "inputPassword")] + [InlineData("sendPassword", null)] + [InlineData("sendPassword", "")] + [InlineData("sendPassword", " ")] + [InlineData(null, null)] + [InlineData("", "")] + public void VerifyPasswordHash_WithNullOrEmptyParameters_ReturnsFalse( + string? sendPasswordHash, + string? inputPasswordHash) + { + // Arrange + var passwordHasher = Substitute.For>(); + var sut = new SendPasswordHasher(passwordHasher); + + // Act + var result = sut.PasswordHashMatches(sendPasswordHash, inputPasswordHash); + + // Assert + Assert.False(result); + passwordHasher.DidNotReceive().VerifyHashedPassword(Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + void HashPasswordHash_WithValidInput_ReturnsHashedPassword( + SutProvider sutProvider, + string clientHashedPassword, + string expectedHashedResult) + { + // Arrange + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), clientHashedPassword) + .Returns(expectedHashedResult); + + // Act + var result = sutProvider.Sut.HashOfClientPasswordHash(clientHashedPassword); + + // Assert + Assert.Equal(expectedHashedResult, result); + sutProvider.GetDependency>() + .Received(1) + .HashPassword(Arg.Any(), clientHashedPassword); + } +}