Skip to content

Commit 303ac1c

Browse files
Patrick-Pimentel-BitwardenPatrick-Pimentel-Bitwarden
committed
feat(validation): [PM-32626] Standardize Unlock and Authentication Validation - Initial pass at standardizing the validation for the RegisterFinishRequestModel.
1 parent 3dbd17f commit 303ac1c

File tree

4 files changed

+180
-12
lines changed

4 files changed

+180
-12
lines changed

‎src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,11 @@ public IEnumerable<ValidationResult> Validate(ValidationContext validationContex
133133
[nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash), nameof(MasterPasswordHash)]);
134134
}
135135

136-
// 2. Validate kdf settings.
137-
if (MasterPasswordUnlock != null)
136+
// 2. Validate kdf settings, kdf equality, and salt equality between authentication and unlock.
137+
if (MasterPasswordUnlock != null && MasterPasswordAuthentication != null)
138138
{
139-
foreach (var validationResult in KdfSettingsValidator.Validate(MasterPasswordUnlock.ToData().Kdf))
140-
{
141-
yield return validationResult;
142-
}
143-
}
144-
145-
if (MasterPasswordAuthentication != null)
146-
{
147-
foreach (var validationResult in KdfSettingsValidator.Validate(MasterPasswordAuthentication.ToData().Kdf))
139+
foreach (var validationResult in KdfSettingsValidator.ValidateAuthenticationAndUnlockData(
140+
MasterPasswordAuthentication.ToData(), MasterPasswordUnlock.ToData()))
148141
{
149142
yield return validationResult;
150143
}
@@ -188,7 +181,7 @@ public IEnumerable<ValidationResult> Validate(ValidationContext validationContex
188181
yield return new ValidationResult($"{nameof(MasterPasswordAuthentication)} not found on RequestModel", [nameof(MasterPasswordAuthentication)]);
189182
}
190183

191-
// 3. Lastly, validate access token type and presence. Must be done last because of yield break.
184+
// 4. Lastly, validate access token type and presence. Must be done last because of yield break.
192185
RegisterFinishTokenType tokenType;
193186
var tokenTypeResolved = true;
194187
try

‎src/Core/Utilities/KdfSettingsValidator.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,37 @@ namespace Bit.Core.Utilities;
66

77
public static class KdfSettingsValidator
88
{
9+
/// <summary>
10+
/// Validates that authentication and unlock data have matching KDF settings and salts,
11+
/// then validates the KDF settings themselves.
12+
/// </summary>
13+
public static IEnumerable<ValidationResult> ValidateAuthenticationAndUnlockData(
14+
MasterPasswordAuthenticationData authentication,
15+
MasterPasswordUnlockData unlock)
16+
{
17+
// Currently KDF settings are not saved separately for authentication and unlock and must therefore be equal
18+
if (!authentication.Kdf.Equals(unlock.Kdf))
19+
{
20+
yield return new ValidationResult(
21+
"KDF settings must be equal for authentication and unlock.",
22+
[nameof(authentication.Kdf)]);
23+
// KDF settings diverge; remaining validation is not meaningful
24+
yield break;
25+
}
26+
27+
if (authentication.Salt != unlock.Salt)
28+
{
29+
yield return new ValidationResult(
30+
"Salt must be equal for authentication and unlock.",
31+
[nameof(authentication.Salt)]);
32+
}
33+
34+
foreach (var validationResult in Validate(authentication.Kdf))
35+
{
36+
yield return validationResult;
37+
}
38+
}
39+
940
// PM-28143 - Remove below when fixing ticket
1041
public static IEnumerable<ValidationResult> Validate(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism)
1142
{

‎test/Api.Test/Utilities/KdfSettingsValidatorTests.cs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using Bit.Core.Enums;
2+
using Bit.Core.KeyManagement.Models.Data;
23
using Bit.Core.Utilities;
34
using Xunit;
45

@@ -33,4 +34,93 @@ public void Validate_Fails(KdfType kdfType, int kdfIterations, int? kdfMemory, i
3334
Assert.NotEmpty(results);
3435
Assert.Equal(expectedFailures, results.Count());
3536
}
37+
38+
[Fact]
39+
public void ValidateAuthenticationAndUnlockData_WhenMatchingKdfAndSalt_ReturnsNoErrors()
40+
{
41+
var kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600_000 };
42+
var authentication = new MasterPasswordAuthenticationData
43+
{
44+
Kdf = kdf,
45+
MasterPasswordAuthenticationHash = "hash",
46+
Salt = "salt"
47+
};
48+
var unlock = new MasterPasswordUnlockData
49+
{
50+
Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600_000 },
51+
MasterKeyWrappedUserKey = "wrapped",
52+
Salt = "salt"
53+
};
54+
55+
var results = KdfSettingsValidator.ValidateAuthenticationAndUnlockData(authentication, unlock).ToList();
56+
57+
Assert.Empty(results);
58+
}
59+
60+
[Fact]
61+
public void ValidateAuthenticationAndUnlockData_WhenKdfMismatch_ReturnsKdfEqualityError()
62+
{
63+
var authentication = new MasterPasswordAuthenticationData
64+
{
65+
Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600_000 },
66+
MasterPasswordAuthenticationHash = "hash",
67+
Salt = "salt"
68+
};
69+
var unlock = new MasterPasswordUnlockData
70+
{
71+
Kdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 3, Memory = 64, Parallelism = 4 },
72+
MasterKeyWrappedUserKey = "wrapped",
73+
Salt = "salt"
74+
};
75+
76+
var results = KdfSettingsValidator.ValidateAuthenticationAndUnlockData(authentication, unlock).ToList();
77+
78+
Assert.Single(results);
79+
Assert.Equal("KDF settings must be equal for authentication and unlock.", results[0].ErrorMessage);
80+
}
81+
82+
[Fact]
83+
public void ValidateAuthenticationAndUnlockData_WhenSaltMismatch_ReturnsSaltEqualityError()
84+
{
85+
var authentication = new MasterPasswordAuthenticationData
86+
{
87+
Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600_000 },
88+
MasterPasswordAuthenticationHash = "hash",
89+
Salt = "salt-auth"
90+
};
91+
var unlock = new MasterPasswordUnlockData
92+
{
93+
Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600_000 },
94+
MasterKeyWrappedUserKey = "wrapped",
95+
Salt = "salt-unlock"
96+
};
97+
98+
var results = KdfSettingsValidator.ValidateAuthenticationAndUnlockData(authentication, unlock).ToList();
99+
100+
Assert.Single(results);
101+
Assert.Equal("Salt must be equal for authentication and unlock.", results[0].ErrorMessage);
102+
}
103+
104+
[Fact]
105+
public void ValidateAuthenticationAndUnlockData_WhenKdfInvalid_ReturnsKdfValidationError()
106+
{
107+
// Matching but out-of-range KDF (iterations below minimum for PBKDF2)
108+
var authentication = new MasterPasswordAuthenticationData
109+
{
110+
Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 100 },
111+
MasterPasswordAuthenticationHash = "hash",
112+
Salt = "salt"
113+
};
114+
var unlock = new MasterPasswordUnlockData
115+
{
116+
Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 100 },
117+
MasterKeyWrappedUserKey = "wrapped",
118+
Salt = "salt"
119+
};
120+
121+
var results = KdfSettingsValidator.ValidateAuthenticationAndUnlockData(authentication, unlock).ToList();
122+
123+
Assert.Single(results);
124+
Assert.Contains("KDF iterations must be between", results[0].ErrorMessage);
125+
}
36126
}

‎test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,60 @@ public void Validate_WhenAllFieldsValidWithSubModels_IsValid()
326326
Assert.Empty(results);
327327
}
328328

329+
[Fact]
330+
public void Validate_WhenKdfMismatchBetweenAuthAndUnlock_ReturnsKdfEqualityError()
331+
{
332+
var model = new RegisterFinishRequestModel
333+
{
334+
Email = "user@example.com",
335+
UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" },
336+
MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel
337+
{
338+
Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default },
339+
MasterKeyWrappedUserKey = "wrapped",
340+
Salt = "salt"
341+
},
342+
MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel
343+
{
344+
Kdf = new KdfRequestModel { KdfType = KdfType.Argon2id, Iterations = 3, Memory = 64, Parallelism = 4 },
345+
MasterPasswordAuthenticationHash = "auth-hash",
346+
Salt = "salt"
347+
},
348+
EmailVerificationToken = "token"
349+
};
350+
351+
var results = Validate(model);
352+
353+
Assert.Contains(results, r => r.ErrorMessage == "KDF settings must be equal for authentication and unlock.");
354+
}
355+
356+
[Fact]
357+
public void Validate_WhenSaltMismatchBetweenAuthAndUnlock_ReturnsSaltEqualityError()
358+
{
359+
var model = new RegisterFinishRequestModel
360+
{
361+
Email = "user@example.com",
362+
UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" },
363+
MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel
364+
{
365+
Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default },
366+
MasterKeyWrappedUserKey = "wrapped",
367+
Salt = "salt-unlock"
368+
},
369+
MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel
370+
{
371+
Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default },
372+
MasterPasswordAuthenticationHash = "auth-hash",
373+
Salt = "salt-auth"
374+
},
375+
EmailVerificationToken = "token"
376+
};
377+
378+
var results = Validate(model);
379+
380+
Assert.Contains(results, r => r.ErrorMessage == "Salt must be equal for authentication and unlock.");
381+
}
382+
329383
[Fact]
330384
public void Validate_WhenNoValidRegistrationTokenProvided_ReturnsTokenErrorOnly()
331385
{

0 commit comments

Comments
 (0)