-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-16603] Add userkey rotation v2 #5204
Open
quexten
wants to merge
38
commits into
main
Choose a base branch
from
km/userkey-rotation-v2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+906
โ9
Open
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
5dd74db
Implement userkey rotation v2
quexten 26175b7
Update request models
quexten ba489db
Cleanup
quexten ff44902
Update tests
quexten 4b7667a
Improve test
quexten cd40987
Add tests
quexten 73f6f67
Fix formatting
quexten a939132
Fix test
quexten 58b984f
Merge branch 'main' into km/userkey-rotation-v2
quexten 2fe3c27
Remove whitespace
quexten 414775c
Fix namespace
quexten b96bfad
Merge branch 'km/userkey-rotation-v2' of github.com:bitwarden/server โฆ
quexten e5c4c9e
Enable nullable on models
quexten fb952da
Fix build
quexten 2495d07
Add tests and enable nullable on masterpasswordunlockdatamodel
quexten f462ef2
Fix test
quexten bcfe852
Remove rollback
quexten 0567da7
Add tests
quexten de5b914
Make masterpassword hint optional
quexten 2820055
Update user query
quexten 0958762
Add EF test
quexten e95e4a2
Improve test
quexten 5262611
Cleanup
quexten 7cbc7a5
Set masterpassword hint
quexten 8f9252d
Merge branch 'main' into km/userkey-rotation-v2
quexten a8a59da
Remove connection close
quexten 853e226
Merge branch 'km/userkey-rotation-v2' of github.com:bitwarden/server โฆ
quexten 0ca2caa
Add tests for invalid kdf types
quexten 39bb255
Update test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommโฆ
quexten 159cb21
Merge branch 'main' into km/userkey-rotation-v2
quexten b8e8d27
Fix formatting
quexten 39201fb
Update src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataโฆ
quexten ab4afa0
Update src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataMโฆ
quexten c64b007
Update src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataMโฆ
quexten 3b767d8
Update src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs
quexten d67b4d6
Fix imports
quexten 49a6a82
Fix tests
quexten 0716858
Merge branch 'main' into km/userkey-rotation-v2
quexten File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
๏ปฟ#nullable enable | ||
|
||
using System.ComponentModel.DataAnnotations; | ||
using Bit.Core.Enums; | ||
using Bit.Core.KeyManagement.Models.Data; | ||
using Bit.Core.Utilities; | ||
|
||
namespace Bit.Api.Auth.Models.Request.Accounts; | ||
|
||
public class MasterPasswordUnlockDataModel : IValidatableObject | ||
{ | ||
public required KdfType KdfType { get; set; } | ||
public required int KdfIterations { get; set; } | ||
public int? KdfMemory { get; set; } | ||
public int? KdfParallelism { get; set; } | ||
|
||
[StrictEmailAddress] | ||
[StringLength(256)] | ||
public required string Email { get; set; } | ||
quexten marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[StringLength(300)] | ||
public required string MasterKeyAuthenticationHash { get; set; } | ||
[EncryptedString] public required string MasterKeyEncryptedUserKey { get; set; } | ||
[StringLength(50)] | ||
public string? MasterPasswordHint { get; set; } | ||
|
||
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext) | ||
Thomas-Avery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
if (KdfType == KdfType.PBKDF2_SHA256) | ||
{ | ||
if (KdfMemory.HasValue || KdfParallelism.HasValue) | ||
{ | ||
yield return new ValidationResult("KdfMemory and KdfParallelism must be null for PBKDF2_SHA256", new[] { nameof(KdfMemory), nameof(KdfParallelism) }); | ||
} | ||
} | ||
else if (KdfType == KdfType.Argon2id) | ||
{ | ||
if (!KdfMemory.HasValue || !KdfParallelism.HasValue) | ||
{ | ||
yield return new ValidationResult("KdfMemory and KdfParallelism must have values for Argon2id", new[] { nameof(KdfMemory), nameof(KdfParallelism) }); | ||
} | ||
} | ||
else | ||
{ | ||
yield return new ValidationResult("Invalid KdfType", new[] { nameof(KdfType) }); | ||
} | ||
} | ||
|
||
public MasterPasswordUnlockData ToUnlockData() | ||
{ | ||
var data = new MasterPasswordUnlockData | ||
{ | ||
KdfType = KdfType, | ||
KdfIterations = KdfIterations, | ||
KdfMemory = KdfMemory, | ||
KdfParallelism = KdfParallelism, | ||
|
||
Email = Email, | ||
|
||
MasterKeyAuthenticationHash = MasterKeyAuthenticationHash, | ||
MasterKeyEncryptedUserKey = MasterKeyEncryptedUserKey, | ||
MasterPasswordHint = MasterPasswordHint | ||
}; | ||
return data; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 changes: 10 additions & 0 deletions
10
src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
๏ปฟ#nullable enable | ||
using Bit.Core.Utilities; | ||
|
||
namespace Bit.Api.KeyManagement.Models.Requests; | ||
|
||
public class AccountKeysRequestModel | ||
{ | ||
[EncryptedString] public required string UserKeyEncryptedAccountPrivateKey { get; set; } | ||
public required string AccountPublicKey { get; set; } | ||
} |
13 changes: 13 additions & 0 deletions
13
src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
๏ปฟ#nullable enable | ||
using System.ComponentModel.DataAnnotations; | ||
|
||
namespace Bit.Api.KeyManagement.Models.Requests; | ||
|
||
public class RotateUserAccountKeysAndDataRequestModel | ||
{ | ||
[StringLength(300)] | ||
public required string OldMasterKeyAuthenticationHash { get; set; } | ||
quexten marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public required UnlockDataRequestModel AccountUnlockData { get; set; } | ||
public required AccountKeysRequestModel AccountKeys { get; set; } | ||
public required AccountDataRequestModel AccountData { get; set; } | ||
} |
16 changes: 16 additions & 0 deletions
16
src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
๏ปฟ#nullable enable | ||
using Bit.Api.AdminConsole.Models.Request.Organizations; | ||
using Bit.Api.Auth.Models.Request; | ||
using Bit.Api.Auth.Models.Request.Accounts; | ||
using Bit.Api.Auth.Models.Request.WebAuthn; | ||
|
||
namespace Bit.Api.KeyManagement.Models.Requests; | ||
|
||
public class UnlockDataRequestModel | ||
{ | ||
// All methods to get to the userkey | ||
public required MasterPasswordUnlockDataModel MasterPasswordUnlockData { get; set; } | ||
public required IEnumerable<EmergencyAccessWithIdRequestModel> EmergencyAccessUnlockData { get; set; } | ||
public required IEnumerable<ResetPasswordWithOrgIdRequestModel> OrganizationAccountRecoveryUnlockData { get; set; } | ||
public required IEnumerable<WebAuthnLoginRotateKeyRequestModel> PasskeyUnlockData { get; set; } | ||
} |
12 changes: 12 additions & 0 deletions
12
src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
๏ปฟ#nullable enable | ||
using Bit.Api.Tools.Models.Request; | ||
using Bit.Api.Vault.Models.Request; | ||
|
||
namespace Bit.Api.KeyManagement.Models.Requests; | ||
|
||
public class AccountDataRequestModel | ||
{ | ||
public required IEnumerable<CipherWithIdRequestModel> Ciphers { get; set; } | ||
public required IEnumerable<FolderWithIdRequestModel> Folders { get; set; } | ||
public required IEnumerable<SendWithIdRequestModel> Sends { get; set; } | ||
Thomas-Avery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
๏ปฟ#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 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; | ||
} | ||
} | ||
} |
28 changes: 28 additions & 0 deletions
28
src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
๏ปฟusing Bit.Core.Auth.Entities; | ||
using Bit.Core.Auth.Models.Data; | ||
using Bit.Core.Entities; | ||
using Bit.Core.Tools.Entities; | ||
using Bit.Core.Vault.Entities; | ||
|
||
namespace Bit.Core.KeyManagement.Models.Data; | ||
|
||
public class RotateUserAccountKeysData | ||
{ | ||
// Authentication for this requests | ||
public string OldMasterKeyAuthenticationHash { get; set; } | ||
|
||
// Other keys encrypted by the userkey | ||
public string UserKeyEncryptedAccountPrivateKey { get; set; } | ||
public string AccountPublicKey { get; set; } | ||
|
||
// All methods to get to the userkey | ||
public MasterPasswordUnlockData MasterPasswordUnlockData { get; set; } | ||
public IEnumerable<EmergencyAccess> EmergencyAccesses { get; set; } | ||
public IReadOnlyList<OrganizationUser> OrganizationUsers { get; set; } | ||
public IEnumerable<WebAuthnLoginRotateKeyData> WebAuthnKeys { get; set; } | ||
|
||
// User vault data encrypted by the userkey | ||
public IEnumerable<Cipher> Ciphers { get; set; } | ||
public IEnumerable<Folder> Folders { get; set; } | ||
public IReadOnlyList<Send> Sends { get; set; } | ||
} |
20 changes: 20 additions & 0 deletions
20
src/Core/KeyManagement/UserKey/IRotateUserAccountKeysCommand.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
๏ปฟusing Bit.Core.Entities; | ||
using Bit.Core.KeyManagement.Models.Data; | ||
using Microsoft.AspNetCore.Identity; | ||
|
||
namespace Bit.Core.KeyManagement.UserKey; | ||
|
||
/// <summary> | ||
/// Responsible for rotation of a user key and updating database with re-encrypted data | ||
/// </summary> | ||
public interface IRotateUserAccountKeysCommand | ||
{ | ||
/// <summary> | ||
/// Sets a new user key and updates all encrypted data. | ||
/// </summary> | ||
/// <param name="model">All necessary information for rotation. If data is not included, this will lead to the change being rejected.</param> | ||
/// <returns>An IdentityResult for verification of the master password hash</returns> | ||
/// <exception cref="ArgumentNullException">User must be provided.</exception> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should also mention that |
||
/// <exception cref="InvalidOperationException">User KDF settings and email must match the model provided settings.</exception> | ||
Task<IdentityResult> RotateUserAccountKeysAsync(User user, RotateUserAccountKeysData model); | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add custom validator instead to cover this scenario for Argon2. Example