[PM-3565] Enforce higher minimum KDF (#3304)

Extract KDF logic into a new Range class. Increase minimum iterations for PBKDF.

src/Api/Auth/Models/Request/Accounts/SetKeyConnectorKeyRequestModel.cs

@@ -2,11 +2,10 @@
 using Bit.Core.Auth.Models.Api.Request.Accounts;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
-using Bit.Core.Utilities;
 
 namespace Bit.Api.Auth.Models.Request.Accounts;
 
-public class SetKeyConnectorKeyRequestModel : IValidatableObject
+public class SetKeyConnectorKeyRequestModel
 {
     [Required]
     public string Key { get; set; }
@@ -31,9 +30,4 @@ public class SetKeyConnectorKeyRequestModel : IValidatableObject
         Keys.ToUser(existingUser);
         return existingUser;
     }
-
-    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
-    {
-        return KdfSettingsValidator.Validate(Kdf, KdfIterations, KdfMemory, KdfParallelism);
-    }
 }

src/Api/Auth/Models/Request/Accounts/SetPasswordRequestModel.cs

@@ -2,11 +2,10 @@
 using Bit.Core.Auth.Models.Api.Request.Accounts;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
-using Bit.Core.Utilities;
 
 namespace Bit.Api.Auth.Models.Request.Accounts;
 
-public class SetPasswordRequestModel : IValidatableObject
+public class SetPasswordRequestModel
 {
     [Required]
     [StringLength(300)]
@@ -35,9 +34,4 @@ public class SetPasswordRequestModel : IValidatableObject
         Keys?.ToUser(existingUser);
         return existingUser;
     }
-
-    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
-    {
-        return KdfSettingsValidator.Validate(Kdf, KdfIterations, KdfMemory, KdfParallelism);
-    }
 }

src/Api/Controllers/AccountsController.cs

@@ -113,7 +113,7 @@ public class AccountsController : Controller
             kdfInformation = new UserKdfInformation
             {
                 Kdf = KdfType.PBKDF2_SHA256,
-                KdfIterations = 100000,
+                KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default,
             };
         }
         return new PreloginResponseModel(kdfInformation);

src/Core/Constants.cs

@@ -31,6 +31,45 @@ public static class Constants
     public const string IdentityProvider = "bitwarden";
 }
 
+public static class AuthConstants
+{
+    public static readonly RangeConstant PBKDF2_ITERATIONS = new(600_000, 2_000_000, 600_000);
+
+    public static readonly RangeConstant ARGON2_ITERATIONS = new(2, 10, 3);
+    public static readonly RangeConstant ARGON2_MEMORY = new(15, 1024, 64);
+    public static readonly RangeConstant ARGON2_PARALLELISM = new(1, 16, 4);
+
+}
+
+public class RangeConstant
+{
+    public int Default { get; }
+    public int Min { get; }
+    public int Max { get; }
+
+    public RangeConstant(int min, int max, int defaultValue)
+    {
+        Default = defaultValue;
+        Min = min;
+        Max = max;
+
+        if (Min > Max)
+        {
+            throw new ArgumentOutOfRangeException($"{Min} is larger than {Max}.");
+        }
+
+        if (!InsideRange(defaultValue))
+        {
+            throw new ArgumentOutOfRangeException($"{Default} is outside allowed range of {Min}-{Max}.");
+        }
+    }
+
+    public bool InsideRange(int number)
+    {
+        return Min <= number && number <= Max;
+    }
+}
+
 public static class TokenPurposes
 {
     public const string LinkSso = "LinkSso";

src/Core/Utilities/KdfSettingsValidator.cs

@@ -10,23 +10,23 @@ public static class KdfSettingsValidator
         switch (kdfType)
         {
             case KdfType.PBKDF2_SHA256:
-                if (kdfIterations < 5000 || kdfIterations > 2_000_000)
+                if (!AuthConstants.PBKDF2_ITERATIONS.InsideRange(kdfIterations))
                 {
-                    yield return new ValidationResult("KDF iterations must be between 5000 and 2000000.");
+                    yield return new ValidationResult($"KDF iterations must be between {AuthConstants.PBKDF2_ITERATIONS.Min} and {AuthConstants.PBKDF2_ITERATIONS.Max}.");
                 }
                 break;
             case KdfType.Argon2id:
-                if (kdfIterations <= 0)
+                if (!AuthConstants.ARGON2_ITERATIONS.InsideRange(kdfIterations))
                 {
-                    yield return new ValidationResult("Argon2 iterations must be greater than 0.");
+                    yield return new ValidationResult($"Argon2 iterations must be between {AuthConstants.ARGON2_ITERATIONS.Min} and {AuthConstants.ARGON2_ITERATIONS.Max}.");
                 }
-                else if (!kdfMemory.HasValue || kdfMemory.Value < 15 || kdfMemory.Value > 1024)
+                else if (!kdfMemory.HasValue || !AuthConstants.ARGON2_MEMORY.InsideRange(kdfMemory.Value))
                 {
-                    yield return new ValidationResult("Argon2 memory must be between 15mb and 1024mb.");
+                    yield return new ValidationResult($"Argon2 memory must be between {AuthConstants.ARGON2_MEMORY.Min}mb and {AuthConstants.ARGON2_MEMORY.Max}mb.");
                 }
-                else if (!kdfParallelism.HasValue || kdfParallelism.Value < 1 || kdfParallelism.Value > 16)
+                else if (!kdfParallelism.HasValue || !AuthConstants.ARGON2_PARALLELISM.InsideRange(kdfParallelism.Value))
                 {
-                    yield return new ValidationResult("Argon2 parallelism must be between 1 and 16.");
+                    yield return new ValidationResult($"Argon2 parallelism must be between {AuthConstants.ARGON2_PARALLELISM.Min} and {AuthConstants.ARGON2_PARALLELISM.Max}.");
                 }
                 break;
 

src/Identity/Controllers/AccountsController.cs

@@ -75,7 +75,7 @@ public class AccountsController : Controller
             kdfInformation = new UserKdfInformation
             {
                 Kdf = KdfType.PBKDF2_SHA256,
-                KdfIterations = 100000,
+                KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default,
             };
         }
         return new PreloginResponseModel(kdfInformation);

test/Api.Test/Controllers/AccountsControllerTests.cs

@@ -1,6 +1,7 @@
 using System.Security.Claims;
 using Bit.Api.Auth.Models.Request.Accounts;
 using Bit.Api.Controllers;
+using Bit.Core;
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.AdminConsole.Services;
 using Bit.Core.Auth.Models.Api.Request.Accounts;
@@ -111,14 +112,14 @@ public class AccountsControllerTests : IDisposable
     }
 
     [Fact]
-    public async Task PostPrelogin_WhenUserDoesNotExist_ShouldDefaultToSha256And100000Iterations()
+    public async Task PostPrelogin_WhenUserDoesNotExist_ShouldDefaultToPBKDF()
     {
         _userRepository.GetKdfInformationByEmailAsync(Arg.Any<string>()).Returns(Task.FromResult((UserKdfInformation)null));
 
         var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" });
 
         Assert.Equal(KdfType.PBKDF2_SHA256, response.Kdf);
-        Assert.Equal(100000, response.KdfIterations);
+        Assert.Equal(AuthConstants.PBKDF2_ITERATIONS.Default, response.KdfIterations);
     }
 
     [Fact]

test/Api.Test/Models/Request/Accounts/KdfRequestModelTests.cs

@@ -9,11 +9,11 @@ public class KdfRequestModelTests
 {
     [Theory]
     [InlineData(KdfType.PBKDF2_SHA256, 1_000_000, null, null)] // Somewhere in the middle
-    [InlineData(KdfType.PBKDF2_SHA256, 5000, null, null)] // Right on the lower boundary
+    [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, 10, 500, 8)] // Somewhere in the middle
-    [InlineData(KdfType.Argon2id, 1, 15, 1)] // Right on the lower boundary
-    [InlineData(KdfType.Argon2id, 5000, 1024, 16)] // 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
@@ -32,7 +32,7 @@ public class KdfRequestModelTests
 
     [Theory]
     [InlineData(null, 350_000, null, null, 1)] // Although KdfType is nullable, it's marked as [Required]
-    [InlineData(KdfType.PBKDF2_SHA256, 1000, null, null, 1)] // Too few iterations
+    [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

test/Core.Test/ConstantsTests.cs

@@ -0,0 +1,69 @@
+using Xunit;
+
+namespace Bit.Core.Test;
+
+public class ConstantsTests
+{
+    public class RangeConstantTests
+    {
+        [Fact]
+        public void Constructor_WithValidValues_SetsProperties()
+        {
+            // Arrange
+            const int min = 0;
+            const int max = 10;
+            const int defaultValue = 5;
+
+            // Act
+            var rangeConstant = new RangeConstant(min, max, defaultValue);
+
+            // Assert
+            Assert.Equal(min, rangeConstant.Min);
+            Assert.Equal(max, rangeConstant.Max);
+            Assert.Equal(defaultValue, rangeConstant.Default);
+        }
+
+        [Fact]
+        public void Constructor_WithInvalidValues_ThrowsArgumentOutOfRangeException()
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() => new RangeConstant(10, 0, 5));
+        }
+
+        [Fact]
+        public void Constructor_WithDefaultValueOutsideRange_ThrowsArgumentOutOfRangeException()
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() => new RangeConstant(0, 10, 20));
+        }
+
+        [Theory]
+        [InlineData(5)]
+        [InlineData(0)]
+        [InlineData(10)]
+        public void InsideRange_WithValidValues_ReturnsTrue(int number)
+        {
+            // Arrange
+            var rangeConstant = new RangeConstant(0, 10, 5);
+
+            // Act
+            bool result = rangeConstant.InsideRange(number);
+
+            // Assert
+            Assert.True(result);
+        }
+
+        [Theory]
+        [InlineData(-1)]
+        [InlineData(11)]
+        public void InsideRange_WithInvalidValues_ReturnsFalse(int number)
+        {
+            // Arrange
+            var rangeConstant = new RangeConstant(0, 10, 5);
+
+            // Act
+            bool result = rangeConstant.InsideRange(number);
+
+            // Assert
+            Assert.False(result);
+        }
+    }
+}

test/Identity.Test/Controllers/AccountsControllerTests.cs

@@ -1,4 +1,5 @@
-using Bit.Core.Auth.Models.Api.Request.Accounts;
+using Bit.Core;
+using Bit.Core.Auth.Models.Api.Request.Accounts;
 using Bit.Core.Auth.Models.Business.Tokenables;
 using Bit.Core.Auth.Services;
 using Bit.Core.Entities;
@@ -64,14 +65,14 @@ public class AccountsControllerTests : IDisposable
     }
 
     [Fact]
-    public async Task PostPrelogin_WhenUserDoesNotExist_ShouldDefaultToSha256And100000Iterations()
+    public async Task PostPrelogin_WhenUserDoesNotExist_ShouldDefaultToPBKDF()
     {
         _userRepository.GetKdfInformationByEmailAsync(Arg.Any<string>()).Returns(Task.FromResult<UserKdfInformation>(null!));
 
         var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" });
 
         Assert.Equal(KdfType.PBKDF2_SHA256, response.Kdf);
-        Assert.Equal(100000, response.KdfIterations);
+        Assert.Equal(AuthConstants.PBKDF2_ITERATIONS.Default, response.KdfIterations);
     }
 
     [Fact]