From 3cdf5ccd3b6e071819a0027aaefd596da8de1b02 Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Thu, 28 Sep 2023 10:00:20 -0300 Subject: [PATCH] [PM-115] Cipher key encryption update (#2421) * PM-115 Added new cipher key and encryption/decryption mechanisms on cipher * PM-115 fix format * PM-115 removed ForceKeyRotation from new cipher encryption model given that another approach will be taken * [PM-1690] Added minimum server version restriction to cipher key encryption (#2463) * PM-1690 added minimum server version restriction to cipher key encryption and also change the force key rotation flag * PM-1690 Updated min server version for new cipher encryption key and fixed configService registration * PM-1690 removed forcekeyrotation * PM-115 Temporarily Changed cipher key new encryption config to help testing (this change should be reseted eventually) * PM-2456 Fix attachment encryption on new cipher item encryption model (#2556) * PM-2531 Fix new cipher encryption on adding attachments on ciphers with no item level key (#2559) * PM-115 Changed temporarily cipher key encryption min server version to 2023.6.0 to test * PM-115 Reseted cipher key encryption minimum server version to 2023.5.0 and disable new cipher key on local cipher creation * Added Key value to the cipher export model (#2628) * Update Constants.cs Updated minimum encryption server version to 2023.9.0 so QA can test its behavior * PM-115 Fix file format * PM-115 Changed new encryption off and minimum new encryption server version to 2023.8.0 for testing purposes * PM-115 Changed CIpher key encryption minimum server version to 2023.9.0 * PM-3737 Remove suffix on client version sent to server (#2779) * PM-115 QA testing server min version and enable new cipher key encryption * PM-115 Disable new cipher encryption creation and change minimum server encryption version to 2023.9.1 --------- Co-authored-by: aj-rosado <109146700+aj-rosado@users.noreply.github.com> --- .../Pages/Vault/AttachmentsPageViewModel.cs | 2 +- src/Core/Abstractions/ICipherService.cs | 2 +- src/Core/Constants.cs | 2 + src/Core/Models/Data/CipherData.cs | 2 + src/Core/Models/Domain/Attachment.cs | 35 ++++--- src/Core/Models/Domain/Card.cs | 4 +- src/Core/Models/Domain/Cipher.cs | 58 +++++++---- src/Core/Models/Domain/Fido2Key.cs | 4 +- src/Core/Models/Domain/Field.cs | 4 +- src/Core/Models/Domain/Identity.cs | 4 +- src/Core/Models/Domain/Login.cs | 8 +- src/Core/Models/Domain/LoginUri.cs | 4 +- src/Core/Models/Domain/PasswordHistory.cs | 4 +- src/Core/Models/Domain/SecureNote.cs | 2 +- src/Core/Models/Domain/SymmetricCryptoKey.cs | 8 +- src/Core/Models/Export/Cipher.cs | 4 + src/Core/Models/Request/CipherRequest.cs | 2 + src/Core/Models/Response/CipherResponse.cs | 1 + src/Core/Models/View/CipherView.cs | 1 + src/Core/Services/ApiService.cs | 2 +- src/Core/Services/CipherService.cs | 97 +++++++++++++++---- src/Core/Services/CryptoService.cs | 4 +- src/Core/Utilities/ServiceContainer.cs | 6 +- src/Core/Utilities/VersionHelpers.cs | 34 +++++++ .../Cipher/CipherCustomizations.cs | 26 ++++- test/Core.Test/Services/CipherServiceTests.cs | 12 +-- 26 files changed, 244 insertions(+), 88 deletions(-) create mode 100644 src/Core/Utilities/VersionHelpers.cs diff --git a/src/App/Pages/Vault/AttachmentsPageViewModel.cs b/src/App/Pages/Vault/AttachmentsPageViewModel.cs index 2e4b99b34..a25f93079 100644 --- a/src/App/Pages/Vault/AttachmentsPageViewModel.cs +++ b/src/App/Pages/Vault/AttachmentsPageViewModel.cs @@ -123,7 +123,7 @@ namespace Bit.App.Pages { await _deviceActionService.ShowLoadingAsync(AppResources.Saving); _cipherDomain = await _cipherService.SaveAttachmentRawWithServerAsync( - _cipherDomain, FileName, FileData); + _cipherDomain, Cipher, FileName, FileData); Cipher = await _cipherDomain.DecryptAsync(); await _deviceActionService.HideLoadingAsync(); _platformUtilsService.ShowToast("success", null, AppResources.AttachementAdded); diff --git a/src/Core/Abstractions/ICipherService.cs b/src/Core/Abstractions/ICipherService.cs index 527ce0087..928f79686 100644 --- a/src/Core/Abstractions/ICipherService.cs +++ b/src/Core/Abstractions/ICipherService.cs @@ -33,7 +33,7 @@ namespace Bit.Core.Abstractions Task GetAsync(string id); Task GetLastUsedForUrlAsync(string url); Task ReplaceAsync(Dictionary ciphers); - Task SaveAttachmentRawWithServerAsync(Cipher cipher, string filename, byte[] data); + Task SaveAttachmentRawWithServerAsync(Cipher cipher, CipherView cipherView, string filename, byte[] data); Task SaveCollectionsWithServerAsync(Cipher cipher); Task SaveWithServerAsync(Cipher cipher); Task ShareWithServerAsync(CipherView cipher, string organizationId, HashSet collectionIds); diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 74b415aa7..7fe9fe8f5 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -67,6 +67,8 @@ namespace Bit.Core public const int Argon2MemoryInMB = 64; public const int Argon2Parallelism = 4; public const int MasterPasswordMinimumChars = 12; + public const int CipherKeyRandomBytesLength = 64; + public const string CipherKeyEncryptionMinServerVersion = "2023.9.1"; public const string DefaultFido2KeyType = "public-key"; public const string DefaultFido2KeyAlgorithm = "ECDSA"; public const string DefaultFido2KeyCurve = "P-256"; diff --git a/src/Core/Models/Data/CipherData.cs b/src/Core/Models/Data/CipherData.cs index 4b052b4ee..6009c81a1 100644 --- a/src/Core/Models/Data/CipherData.cs +++ b/src/Core/Models/Data/CipherData.cs @@ -28,6 +28,7 @@ namespace Bit.Core.Models.Data Notes = response.Notes; CollectionIds = collectionIds?.ToList() ?? response.CollectionIds; Reprompt = response.Reprompt; + Key = response.Key; try // Added to address Issue (https://github.com/bitwarden/mobile/issues/1006) { @@ -88,5 +89,6 @@ namespace Bit.Core.Models.Data public List PasswordHistory { get; set; } public List CollectionIds { get; set; } public Enums.CipherRepromptType Reprompt { get; set; } + public string Key { get; set; } } } diff --git a/src/Core/Models/Domain/Attachment.cs b/src/Core/Models/Domain/Attachment.cs index 3e71d6107..b12395a58 100644 --- a/src/Core/Models/Domain/Attachment.cs +++ b/src/Core/Models/Domain/Attachment.cs @@ -1,8 +1,10 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Threading.Tasks; using Bit.Core.Abstractions; using Bit.Core.Models.Data; using Bit.Core.Models.View; +using Bit.Core.Services; using Bit.Core.Utilities; namespace Bit.Core.Models.Domain @@ -11,11 +13,11 @@ namespace Bit.Core.Models.Domain { private HashSet _map = new HashSet { - "Id", - "Url", - "SizeName", - "FileName", - "Key" + nameof(Id), + nameof(Url), + nameof(SizeName), + nameof(FileName), + nameof(Key) }; public Attachment() { } @@ -23,7 +25,7 @@ namespace Bit.Core.Models.Domain public Attachment(AttachmentData obj, bool alreadyEncrypted = false) { Size = obj.Size; - BuildDomainModel(this, obj, _map, alreadyEncrypted, new HashSet { "Id", "Url", "SizeName" }); + BuildDomainModel(this, obj, _map, alreadyEncrypted, new HashSet { nameof(Id), nameof(Url), nameof(SizeName) }); } public string Id { get; set; } @@ -33,25 +35,26 @@ namespace Bit.Core.Models.Domain public EncString Key { get; set; } public EncString FileName { get; set; } - public async Task DecryptAsync(string orgId) + public async Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { var view = await DecryptObjAsync(new AttachmentView(this), this, new HashSet { - "FileName" - }, orgId); + nameof(FileName) + }, orgId, key); if (Key != null) { - var cryptoService = ServiceContainer.Resolve("cryptoService"); try { - var orgKey = await cryptoService.GetOrgKeyAsync(orgId); - var decValue = await cryptoService.DecryptToBytesAsync(Key, orgKey); + var cryptoService = ServiceContainer.Resolve(); + + var decryptKey = key ?? await cryptoService.GetOrgKeyAsync(orgId); + var decValue = await cryptoService.DecryptToBytesAsync(Key, decryptKey); view.Key = new SymmetricCryptoKey(decValue); } - catch + catch (Exception ex) { - // TODO: error? + LoggerHelper.LogEvenIfCantBeResolved(ex); } } return view; @@ -61,7 +64,7 @@ namespace Bit.Core.Models.Domain { var a = new AttachmentData(); a.Size = Size; - BuildDataModel(this, a, _map, new HashSet { "Id", "Url", "SizeName" }); + BuildDataModel(this, a, _map, new HashSet { nameof(Id), nameof(Url), nameof(SizeName) }); return a; } } diff --git a/src/Core/Models/Domain/Card.cs b/src/Core/Models/Domain/Card.cs index 8a7ffff23..dfde0a437 100644 --- a/src/Core/Models/Domain/Card.cs +++ b/src/Core/Models/Domain/Card.cs @@ -31,9 +31,9 @@ namespace Bit.Core.Models.Domain public EncString ExpYear { get; set; } public EncString Code { get; set; } - public Task DecryptAsync(string orgId) + public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return DecryptObjAsync(new CardView(this), this, _map, orgId); + return DecryptObjAsync(new CardView(this), this, _map, orgId, key); } public CardData ToCardData() diff --git a/src/Core/Models/Domain/Cipher.cs b/src/Core/Models/Domain/Cipher.cs index a9c0014db..48aa3a761 100644 --- a/src/Core/Models/Domain/Cipher.cs +++ b/src/Core/Models/Domain/Cipher.cs @@ -2,9 +2,11 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Bit.Core.Abstractions; using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Models.View; +using Bit.Core.Utilities; namespace Bit.Core.Models.Domain { @@ -16,12 +18,12 @@ namespace Bit.Core.Models.Domain { BuildDomainModel(this, obj, new HashSet { - "Id", - "OrganizationId", - "FolderId", - "Name", - "Notes" - }, alreadyEncrypted, new HashSet { "Id", "OrganizationId", "FolderId" }); + nameof(Id), + nameof(OrganizationId), + nameof(FolderId), + nameof(Name), + nameof(Notes) + }, alreadyEncrypted, new HashSet { nameof(Id), nameof(OrganizationId), nameof(FolderId) }); Type = obj.Type; Favorite = obj.Favorite; @@ -34,6 +36,11 @@ namespace Bit.Core.Models.Domain LocalData = localData; Reprompt = obj.Reprompt; + if (obj.Key != null) + { + Key = new EncString(obj.Key); + } + switch (Type) { case Enums.CipherType.Login: @@ -81,29 +88,43 @@ namespace Bit.Core.Models.Domain public List PasswordHistory { get; set; } public HashSet CollectionIds { get; set; } public CipherRepromptType Reprompt { get; set; } + public EncString Key { get; set; } public async Task DecryptAsync() { var model = new CipherView(this); + + if (Key != null) + { + // HACK: I don't like resolving this here but I can't see a better way without + // refactoring a lot of things. + var cryptoService = ServiceContainer.Resolve(); + + var orgKey = await cryptoService.GetOrgKeyAsync(OrganizationId); + + var key = await cryptoService.DecryptToBytesAsync(Key, orgKey); + model.Key = new CipherKey(key); + } + await DecryptObjAsync(model, this, new HashSet { - "Name", - "Notes" - }, OrganizationId); + nameof(Name), + nameof(Notes) + }, OrganizationId, model.Key); switch (Type) { case Enums.CipherType.Login: - model.Login = await Login.DecryptAsync(OrganizationId); + model.Login = await Login.DecryptAsync(OrganizationId, model.Key); break; case Enums.CipherType.SecureNote: - model.SecureNote = await SecureNote.DecryptAsync(OrganizationId); + model.SecureNote = await SecureNote.DecryptAsync(OrganizationId, model.Key); break; case Enums.CipherType.Card: - model.Card = await Card.DecryptAsync(OrganizationId); + model.Card = await Card.DecryptAsync(OrganizationId, model.Key); break; case Enums.CipherType.Identity: - model.Identity = await Identity.DecryptAsync(OrganizationId); + model.Identity = await Identity.DecryptAsync(OrganizationId, model.Key); break; default: break; @@ -115,7 +136,7 @@ namespace Bit.Core.Models.Domain var tasks = new List(); async Task decryptAndAddAttachmentAsync(Attachment attachment) { - var decAttachment = await attachment.DecryptAsync(OrganizationId); + var decAttachment = await attachment.DecryptAsync(OrganizationId, model.Key); model.Attachments.Add(decAttachment); } foreach (var attachment in Attachments) @@ -130,7 +151,7 @@ namespace Bit.Core.Models.Domain var tasks = new List(); async Task decryptAndAddFieldAsync(Field field) { - var decField = await field.DecryptAsync(OrganizationId); + var decField = await field.DecryptAsync(OrganizationId, model.Key); model.Fields.Add(decField); } foreach (var field in Fields) @@ -145,7 +166,7 @@ namespace Bit.Core.Models.Domain var tasks = new List(); async Task decryptAndAddHistoryAsync(PasswordHistory ph) { - var decPh = await ph.DecryptAsync(OrganizationId); + var decPh = await ph.DecryptAsync(OrganizationId, model.Key); model.PasswordHistory.Add(decPh); } foreach (var ph in PasswordHistory) @@ -174,11 +195,12 @@ namespace Bit.Core.Models.Domain CollectionIds = CollectionIds.ToList(), DeletedDate = DeletedDate, Reprompt = Reprompt, + Key = Key?.EncryptedString }; BuildDataModel(this, c, new HashSet { - "Name", - "Notes" + nameof(Name), + nameof(Notes) }); switch (c.Type) { diff --git a/src/Core/Models/Domain/Fido2Key.cs b/src/Core/Models/Domain/Fido2Key.cs index 80a6589f9..504830fd7 100644 --- a/src/Core/Models/Domain/Fido2Key.cs +++ b/src/Core/Models/Domain/Fido2Key.cs @@ -41,9 +41,9 @@ namespace Bit.Core.Models.Domain public EncString UserName { get; set; } public EncString Counter { get; set; } - public async Task DecryptAsync(string orgId) + public async Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return await DecryptObjAsync(new Fido2KeyView(), this, EncryptableProperties, orgId); + return await DecryptObjAsync(new Fido2KeyView(), this, EncryptableProperties, orgId, key); } public Fido2KeyData ToFido2KeyData() diff --git a/src/Core/Models/Domain/Field.cs b/src/Core/Models/Domain/Field.cs index cbfdcbca3..7d962a669 100644 --- a/src/Core/Models/Domain/Field.cs +++ b/src/Core/Models/Domain/Field.cs @@ -28,9 +28,9 @@ namespace Bit.Core.Models.Domain public FieldType Type { get; set; } public LinkedIdType? LinkedId { get; set; } - public Task DecryptAsync(string orgId) + public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return DecryptObjAsync(new FieldView(this), this, _map, orgId); + return DecryptObjAsync(new FieldView(this), this, _map, orgId, key); } public FieldData ToFieldData() diff --git a/src/Core/Models/Domain/Identity.cs b/src/Core/Models/Domain/Identity.cs index 4a705dcb6..5585eeddb 100644 --- a/src/Core/Models/Domain/Identity.cs +++ b/src/Core/Models/Domain/Identity.cs @@ -55,9 +55,9 @@ namespace Bit.Core.Models.Domain public EncString PassportNumber { get; set; } public EncString LicenseNumber { get; set; } - public Task DecryptAsync(string orgId) + public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return DecryptObjAsync(new IdentityView(this), this, _map, orgId); + return DecryptObjAsync(new IdentityView(this), this, _map, orgId, key); } public IdentityData ToIdentityData() diff --git a/src/Core/Models/Domain/Login.cs b/src/Core/Models/Domain/Login.cs index ded94afe7..43900b179 100644 --- a/src/Core/Models/Domain/Login.cs +++ b/src/Core/Models/Domain/Login.cs @@ -31,20 +31,20 @@ namespace Bit.Core.Models.Domain public EncString Totp { get; set; } public List Fido2Keys { get; set; } - public async Task DecryptAsync(string orgId) + public async Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { var view = await DecryptObjAsync(new LoginView(this), this, new HashSet { "Username", "Password", "Totp" - }, orgId); + }, orgId, key); if (Uris != null) { view.Uris = new List(); foreach (var uri in Uris) { - view.Uris.Add(await uri.DecryptAsync(orgId)); + view.Uris.Add(await uri.DecryptAsync(orgId, key)); } } if (Fido2Keys != null) @@ -52,7 +52,7 @@ namespace Bit.Core.Models.Domain view.Fido2Keys = new List(); foreach (var fido2Key in Fido2Keys) { - view.Fido2Keys.Add(await fido2Key.DecryptAsync(orgId)); + view.Fido2Keys.Add(await fido2Key.DecryptAsync(orgId, key)); } } return view; diff --git a/src/Core/Models/Domain/LoginUri.cs b/src/Core/Models/Domain/LoginUri.cs index 31747fd96..e9b26d6d1 100644 --- a/src/Core/Models/Domain/LoginUri.cs +++ b/src/Core/Models/Domain/LoginUri.cs @@ -24,9 +24,9 @@ namespace Bit.Core.Models.Domain public EncString Uri { get; set; } public UriMatchType? Match { get; set; } - public Task DecryptAsync(string orgId) + public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return DecryptObjAsync(new LoginUriView(this), this, _map, orgId); + return DecryptObjAsync(new LoginUriView(this), this, _map, orgId, key); } public LoginUriData ToLoginUriData() diff --git a/src/Core/Models/Domain/PasswordHistory.cs b/src/Core/Models/Domain/PasswordHistory.cs index 6ae1ab1f1..47a969fdd 100644 --- a/src/Core/Models/Domain/PasswordHistory.cs +++ b/src/Core/Models/Domain/PasswordHistory.cs @@ -24,9 +24,9 @@ namespace Bit.Core.Models.Domain public EncString Password { get; set; } public DateTime LastUsedDate { get; set; } - public Task DecryptAsync(string orgId) + public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return DecryptObjAsync(new PasswordHistoryView(this), this, _map, orgId); + return DecryptObjAsync(new PasswordHistoryView(this), this, _map, orgId, key); } public PasswordHistoryData ToPasswordHistoryData() diff --git a/src/Core/Models/Domain/SecureNote.cs b/src/Core/Models/Domain/SecureNote.cs index 55817ab15..b30be2fdb 100644 --- a/src/Core/Models/Domain/SecureNote.cs +++ b/src/Core/Models/Domain/SecureNote.cs @@ -16,7 +16,7 @@ namespace Bit.Core.Models.Domain public SecureNoteType Type { get; set; } - public Task DecryptAsync(string orgId) + public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { return Task.FromResult(new SecureNoteView(this)); } diff --git a/src/Core/Models/Domain/SymmetricCryptoKey.cs b/src/Core/Models/Domain/SymmetricCryptoKey.cs index 2bf9c876c..09d0e9268 100644 --- a/src/Core/Models/Domain/SymmetricCryptoKey.cs +++ b/src/Core/Models/Domain/SymmetricCryptoKey.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using Bit.Core.Enums; namespace Bit.Core.Models.Domain @@ -102,4 +101,11 @@ namespace Bit.Core.Models.Domain : base(key, encType) { } } + + public class CipherKey : SymmetricCryptoKey + { + public CipherKey(byte[] key, EncryptionType? encType = null) + : base(key, encType) + { } + } } diff --git a/src/Core/Models/Export/Cipher.cs b/src/Core/Models/Export/Cipher.cs index 70da4efb2..eaa8ac417 100644 --- a/src/Core/Models/Export/Cipher.cs +++ b/src/Core/Models/Export/Cipher.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.Linq; using Bit.Core.Enums; +using Bit.Core.Models.Domain; using Bit.Core.Models.View; using Newtonsoft.Json; @@ -46,6 +47,7 @@ namespace Bit.Core.Models.Export Name = obj.Name?.EncryptedString; Notes = obj.Notes?.EncryptedString; Favorite = obj.Favorite; + Key = obj.Key?.EncryptedString; Fields = obj.Fields?.Select(f => new Field(f)).ToList(); @@ -82,6 +84,8 @@ namespace Bit.Core.Models.Export public Card Card { get; set; } [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public Identity Identity { get; set; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public string Key { get; set; } public CipherView ToView(Cipher req, CipherView view = null) { diff --git a/src/Core/Models/Request/CipherRequest.cs b/src/Core/Models/Request/CipherRequest.cs index 0e5766bfe..bef11566c 100644 --- a/src/Core/Models/Request/CipherRequest.cs +++ b/src/Core/Models/Request/CipherRequest.cs @@ -19,6 +19,7 @@ namespace Bit.Core.Models.Request Favorite = cipher.Favorite; LastKnownRevisionDate = cipher.RevisionDate; Reprompt = cipher.Reprompt; + Key = cipher.Key?.EncryptedString; switch (Type) { @@ -125,5 +126,6 @@ namespace Bit.Core.Models.Request public Dictionary Attachments2 { get; set; } public DateTime LastKnownRevisionDate { get; set; } public CipherRepromptType Reprompt { get; set; } + public string Key { get; set; } } } diff --git a/src/Core/Models/Response/CipherResponse.cs b/src/Core/Models/Response/CipherResponse.cs index f9c481114..62973af75 100644 --- a/src/Core/Models/Response/CipherResponse.cs +++ b/src/Core/Models/Response/CipherResponse.cs @@ -28,6 +28,7 @@ namespace Bit.Core.Models.Response public List CollectionIds { get; set; } public DateTime? DeletedDate { get; set; } public CipherRepromptType Reprompt { get; set; } + public string Key { get; set; } public DateTime CreationDate { get; set; } } } diff --git a/src/Core/Models/View/CipherView.cs b/src/Core/Models/View/CipherView.cs index 88d2aa137..01c86f450 100644 --- a/src/Core/Models/View/CipherView.cs +++ b/src/Core/Models/View/CipherView.cs @@ -51,6 +51,7 @@ namespace Bit.Core.Models.View public DateTime CreationDate { get; set; } public DateTime? DeletedDate { get; set; } public CipherRepromptType Reprompt { get; set; } + public CipherKey Key { get; set; } public ItemView Item { diff --git a/src/Core/Services/ApiService.cs b/src/Core/Services/ApiService.cs index 8488e4b33..ff2667cfb 100644 --- a/src/Core/Services/ApiService.cs +++ b/src/Core/Services/ApiService.cs @@ -42,7 +42,7 @@ namespace Bit.Core.Services var device = (int)_platformUtilsService.GetDevice(); _httpClient.DefaultRequestHeaders.Add("Device-Type", device.ToString()); _httpClient.DefaultRequestHeaders.Add("Bitwarden-Client-Name", _platformUtilsService.GetClientType().GetString()); - _httpClient.DefaultRequestHeaders.Add("Bitwarden-Client-Version", _platformUtilsService.GetApplicationVersion()); + _httpClient.DefaultRequestHeaders.Add("Bitwarden-Client-Version", VersionHelpers.RemoveSuffix(_platformUtilsService.GetApplicationVersion())); if (!string.IsNullOrWhiteSpace(customUserAgent)) { _httpClient.DefaultRequestHeaders.UserAgent.ParseAdd(customUserAgent); diff --git a/src/Core/Services/CipherService.cs b/src/Core/Services/CipherService.cs index 5033a01d6..cdbc4aa46 100644 --- a/src/Core/Services/CipherService.cs +++ b/src/Core/Services/CipherService.cs @@ -1,4 +1,6 @@ -using System; +//#define ENABLE_NEW_CIPHER_KEY_ENCRYPTION_ON_CREATION + +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -30,6 +32,7 @@ namespace Bit.Core.Services private readonly IStorageService _storageService; private readonly II18nService _i18nService; private readonly Func _searchService; + private readonly IConfigService _configService; private readonly string _clearCipherCacheKey; private readonly string[] _allClearCipherCacheKeys; private Dictionary> _domainMatchBlacklist = new Dictionary> @@ -48,6 +51,7 @@ namespace Bit.Core.Services IStorageService storageService, II18nService i18nService, Func searchService, + IConfigService configService, string clearCipherCacheKey, string[] allClearCipherCacheKeys) { @@ -59,6 +63,7 @@ namespace Bit.Core.Services _storageService = storageService; _i18nService = i18nService; _searchService = searchService; + _configService = configService; _clearCipherCacheKey = clearCipherCacheKey; _allClearCipherCacheKeys = allClearCipherCacheKeys; } @@ -181,6 +186,26 @@ namespace Bit.Core.Services Reprompt = model.Reprompt }; + key = await UpdateCipherAndGetCipherKeyAsync(cipher, model, key); + + var tasks = new List + { + EncryptObjPropertyAsync(model, cipher, new HashSet + { + nameof(CipherView.Name), + nameof(CipherView.Notes) + }, key), + EncryptCipherDataAsync(cipher, model, key), + EncryptFieldsAsync(model.Fields, key, cipher), + EncryptPasswordHistoriesAsync(model.PasswordHistory, key, cipher), + EncryptAttachmentsAsync(model.Attachments, key, cipher) + }; + await Task.WhenAll(tasks); + return cipher; + } + + private async Task UpdateCipherAndGetCipherKeyAsync(Cipher cipher, CipherView cipherView, SymmetricCryptoKey key = null, bool shouldCreateNewCipherKeyIfNeeded = true) + { if (key == null && cipher.OrganizationId != null) { key = await _cryptoService.GetOrgKeyAsync(cipher.OrganizationId); @@ -190,20 +215,42 @@ namespace Bit.Core.Services } } - var tasks = new List + if (!await ShouldUseCipherKeyEncryptionAsync()) { - EncryptObjPropertyAsync(model, cipher, new HashSet - { - "Name", - "Notes" - }, key), - EncryptCipherDataAsync(cipher, model, key), - EncryptFieldsAsync(model.Fields, key, cipher), - EncryptPasswordHistoriesAsync(model.PasswordHistory, key, cipher), - EncryptAttachmentsAsync(model.Attachments, key, cipher) - }; - await Task.WhenAll(tasks); - return cipher; + return key; + } + + if (cipherView.Key != null) + { + cipher.Key = await _cryptoService.EncryptAsync(cipherView.Key.Key, key); + return cipherView.Key; + } + + if (!shouldCreateNewCipherKeyIfNeeded) + { + return key; + } + +#if ENABLE_NEW_CIPHER_KEY_ENCRYPTION_ON_CREATION + // turned on, only on debug to check that the enc/decryption is working fine at the cipher level. + // this will be allowed on production on a later release. + var cfs = ServiceContainer.Resolve(); + var newKey = new SymmetricCryptoKey(await cfs.RandomBytesAsync(Core.Constants.CipherKeyRandomBytesLength)); + cipher.Key = await _cryptoService.EncryptAsync(newKey.Key, key); + + return newKey; +#else + return key; +#endif + } + + private async Task ShouldUseCipherKeyEncryptionAsync() + { + var config = await _configService.GetAsync(); + + return config != null + && + VersionHelpers.IsServerVersionGreaterThanOrEqualTo(config.Version, Constants.CipherKeyEncryptionMinServerVersion); } public async Task GetAsync(string id) @@ -511,6 +558,7 @@ namespace Bit.Core.Services var request = new CipherRequest(cipher); response = await _apiService.PutCipherAsync(cipher.Id, request); } + var userId = await _stateService.GetActiveUserIdAsync(); var data = new CipherData(response, userId, cipher.CollectionIds); await UpsertAsync(data); @@ -560,9 +608,9 @@ namespace Bit.Core.Services .Any(c => !cipher.Login.MainFido2Key.IsUniqueAgainst(c.Login?.MainFido2Key)); } - public async Task SaveAttachmentRawWithServerAsync(Cipher cipher, string filename, byte[] data) + public async Task SaveAttachmentRawWithServerAsync(Cipher cipher, CipherView cipherView, string filename, byte[] data) { - var (attachmentKey, protectedAttachmentKey, encKey) = await MakeAttachmentKeyAsync(cipher.OrganizationId); + var (attachmentKey, protectedAttachmentKey, encKey) = await MakeAttachmentKeyAsync(cipher.OrganizationId, cipher, cipherView); var encFileName = await _cryptoService.EncryptAsync(filename, encKey); var encFileData = await _cryptoService.EncryptToBytesAsync(data, attachmentKey); @@ -579,6 +627,7 @@ namespace Bit.Core.Services var uploadDataResponse = await _apiService.PostCipherAttachmentAsync(cipher.Id, request); response = uploadDataResponse.CipherResponse; + await _fileUploadService.UploadCipherAttachmentFileAsync(uploadDataResponse, encFileName, encFileData); } catch (ApiException e) when (e.Error.StatusCode == System.Net.HttpStatusCode.NotFound || e.Error.StatusCode == System.Net.HttpStatusCode.MethodNotAllowed) @@ -801,10 +850,18 @@ namespace Bit.Core.Services // Helpers - private async Task> MakeAttachmentKeyAsync(string organizationId) + private async Task> MakeAttachmentKeyAsync(string organizationId, Cipher cipher = null, CipherView cipherView = null) { - var encryptionKey = await _cryptoService.GetOrgKeyAsync(organizationId) - ?? (SymmetricCryptoKey)await _cryptoService.GetUserKeyWithLegacySupportAsync(); + var orgKey = await _cryptoService.GetOrgKeyAsync(organizationId); + + SymmetricCryptoKey encryptionKey = orgKey; + if (cipher != null && cipherView != null) + { + encryptionKey = await UpdateCipherAndGetCipherKeyAsync(cipher, cipherView, orgKey, false); + } + + encryptionKey ??= await _cryptoService.GetUserKeyWithLegacySupportAsync(); + var (attachmentKey, protectedAttachmentKey) = await _cryptoService.MakeDataEncKeyAsync(encryptionKey); return new Tuple(attachmentKey, protectedAttachmentKey, encryptionKey); } @@ -1069,7 +1126,7 @@ namespace Bit.Core.Services { await EncryptObjPropertyAsync(model, attachment, new HashSet { - "FileName" + nameof(AttachmentView.FileName) }, key); if (model.Key != null) { diff --git a/src/Core/Services/CryptoService.cs b/src/Core/Services/CryptoService.cs index 534db688d..5bd3c2129 100644 --- a/src/Core/Services/CryptoService.cs +++ b/src/Core/Services/CryptoService.cs @@ -236,9 +236,9 @@ namespace Bit.Core.Services { throw new ArgumentNullException(nameof(key)); } - if (!(key is UserKey) && !(key is OrgKey)) + if (!(key is UserKey) && !(key is OrgKey) && !(key is CipherKey)) { - throw new ArgumentException($"Data encryption keys must be of type UserKey or OrgKey. {key.GetType().FullName} unsupported."); + throw new ArgumentException($"Data encryption keys must be of type UserKey or OrgKey or CipherKey. {key.GetType().FullName} unsupported."); } var newSymKey = await _cryptoFunctionService.RandomBytesAsync(64); diff --git a/src/Core/Utilities/ServiceContainer.cs b/src/Core/Utilities/ServiceContainer.cs index f2eada5be..197fa4668 100644 --- a/src/Core/Utilities/ServiceContainer.cs +++ b/src/Core/Utilities/ServiceContainer.cs @@ -44,8 +44,9 @@ namespace Bit.Core.Utilities var organizationService = new OrganizationService(stateService, apiService); var settingsService = new SettingsService(stateService); var fileUploadService = new FileUploadService(apiService); + var configService = new ConfigService(apiService, stateService, logger); var cipherService = new CipherService(cryptoService, stateService, settingsService, apiService, - fileUploadService, storageService, i18nService, () => searchService, clearCipherCacheKey, + fileUploadService, storageService, i18nService, () => searchService, configService, clearCipherCacheKey, allClearCipherCacheKeys); var folderService = new FolderService(cryptoService, stateService, apiService, i18nService, cipherService); var collectionService = new CollectionService(cryptoService, stateService, i18nService); @@ -88,7 +89,6 @@ namespace Bit.Core.Utilities var environmentService = new EnvironmentService(apiService, stateService, conditionedRunner); var eventService = new EventService(apiService, stateService, organizationService, cipherService); var usernameGenerationService = new UsernameGenerationService(cryptoService, apiService, stateService); - var configService = new ConfigService(apiService, stateService, logger); Register(conditionedRunner); Register("tokenService", tokenService); @@ -96,6 +96,7 @@ namespace Bit.Core.Utilities Register("appIdService", appIdService); Register("organizationService", organizationService); Register("settingsService", settingsService); + Register(configService); Register("cipherService", cipherService); Register("folderService", folderService); Register("collectionService", collectionService); @@ -114,7 +115,6 @@ namespace Bit.Core.Utilities Register("environmentService", environmentService); Register("eventService", eventService); Register(usernameGenerationService); - Register(configService); Register(deviceTrustCryptoService); Register(passwordResetEnrollmentService); } diff --git a/src/Core/Utilities/VersionHelpers.cs b/src/Core/Utilities/VersionHelpers.cs new file mode 100644 index 000000000..a56fc5126 --- /dev/null +++ b/src/Core/Utilities/VersionHelpers.cs @@ -0,0 +1,34 @@ +using System; + +namespace Bit.Core.Utilities +{ + public static class VersionHelpers + { + private const char SUFFIX_SEPARATOR = '-'; + + /// + /// Compares two server versions and gets whether the + /// is greater than or equal to . + /// WARNING: This doesn't take into account hotfix suffix. + /// + /// Version to compare + /// Version to compare against + /// + /// True if is greater than or equal to ; False otherwise. + /// + public static bool IsServerVersionGreaterThanOrEqualTo(string targetVersion, string compareToVersion) + { + return new Version(RemoveSuffix(targetVersion)).CompareTo(new Version(RemoveSuffix(compareToVersion))) >= 0; + } + + public static string RemoveSuffix(string version) + { + if (string.IsNullOrWhiteSpace(version)) + { + throw new ArgumentNullException(nameof(version)); + } + + return version.Split(SUFFIX_SEPARATOR)[0]; + } + } +} diff --git a/test/Core.Test/AutoFixture/Cipher/CipherCustomizations.cs b/test/Core.Test/AutoFixture/Cipher/CipherCustomizations.cs index 6d4f47aad..57cec73b2 100644 --- a/test/Core.Test/AutoFixture/Cipher/CipherCustomizations.cs +++ b/test/Core.Test/AutoFixture/Cipher/CipherCustomizations.cs @@ -1,6 +1,8 @@ using System; +using System.Text; using AutoFixture; using Bit.Core.Models.Domain; +using Bit.Core.Models.View; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -26,16 +28,36 @@ namespace Bit.Core.Test.AutoFixture } } + internal class UserCipherView : ICustomization + { + public void Customize(IFixture fixture) + { + byte[] getRandomBytes(int size) + { + Random random = new Random(); + + byte[] bytes = new byte[size]; + random.NextBytes(bytes); + return bytes; + }; + + fixture.Customize(composer => composer + .Without(c => c.OrganizationId) + .Without(c => c.Attachments) + .With(c => c.Key, new SymmetricCryptoKey(getRandomBytes(32), Enums.EncryptionType.AesCbc128_HmacSha256_B64))); + } + } + internal class UserCipherAutoDataAttribute : CustomAutoDataAttribute { public UserCipherAutoDataAttribute() : base(new SutProviderCustomization(), - new UserCipher()) + new UserCipher(), new UserCipherView()) { } } internal class InlineUserCipherAutoDataAttribute : InlineCustomAutoDataAttribute { public InlineUserCipherAutoDataAttribute(params object[] values) : base(new[] { typeof(SutProviderCustomization), - typeof(UserCipher) }, values) + typeof(UserCipher), typeof(UserCipherView) }, values) { } } diff --git a/test/Core.Test/Services/CipherServiceTests.cs b/test/Core.Test/Services/CipherServiceTests.cs index f63d115c7..ab436c9d1 100644 --- a/test/Core.Test/Services/CipherServiceTests.cs +++ b/test/Core.Test/Services/CipherServiceTests.cs @@ -22,7 +22,7 @@ namespace Bit.Core.Test.Services { [Theory, UserCipherAutoData] public async Task SaveWithServerAsync_PrefersFileUploadService(SutProvider sutProvider, - Cipher cipher, string fileName, EncByteArray data, AttachmentUploadDataResponse uploadDataResponse, EncString encKey) + Cipher cipher, CipherView cipherView, string fileName, EncByteArray data, AttachmentUploadDataResponse uploadDataResponse, EncString encKey) { var encFileName = new EncString(fileName); sutProvider.GetDependency().EncryptAsync(fileName, Arg.Any()) @@ -33,7 +33,7 @@ namespace Bit.Core.Test.Services sutProvider.GetDependency().PostCipherAttachmentAsync(cipher.Id, Arg.Any()) .Returns(uploadDataResponse); - await sutProvider.Sut.SaveAttachmentRawWithServerAsync(cipher, fileName, data.Buffer); + await sutProvider.Sut.SaveAttachmentRawWithServerAsync(cipher, cipherView, fileName, data.Buffer); await sutProvider.GetDependency().Received(1) .UploadCipherAttachmentFileAsync(uploadDataResponse, encFileName, data); @@ -43,7 +43,7 @@ namespace Bit.Core.Test.Services [InlineUserCipherAutoData(HttpStatusCode.NotFound)] [InlineUserCipherAutoData(HttpStatusCode.MethodNotAllowed)] public async Task SaveWithServerAsync_FallsBackToLegacyFormData(HttpStatusCode statusCode, - SutProvider sutProvider, Cipher cipher, string fileName, EncByteArray data, + SutProvider sutProvider, Cipher cipher, CipherView cipherView, string fileName, EncByteArray data, CipherResponse response, EncString encKey) { sutProvider.GetDependency().EncryptAsync(fileName, Arg.Any()) @@ -56,7 +56,7 @@ namespace Bit.Core.Test.Services sutProvider.GetDependency().PostCipherAttachmentLegacyAsync(cipher.Id, Arg.Any()) .Returns(response); - await sutProvider.Sut.SaveAttachmentRawWithServerAsync(cipher, fileName, data.Buffer); + await sutProvider.Sut.SaveAttachmentRawWithServerAsync(cipher, cipherView, fileName, data.Buffer); await sutProvider.GetDependency().Received(1) .PostCipherAttachmentLegacyAsync(cipher.Id, Arg.Any()); @@ -64,7 +64,7 @@ namespace Bit.Core.Test.Services [Theory, UserCipherAutoData] public async Task SaveWithServerAsync_ThrowsOnBadRequestApiException(SutProvider sutProvider, - Cipher cipher, string fileName, EncByteArray data, EncString encKey) + Cipher cipher, CipherView cipherView, string fileName, EncByteArray data, EncString encKey) { sutProvider.GetDependency().EncryptAsync(fileName, Arg.Any()) .Returns(new EncString(fileName)); @@ -77,7 +77,7 @@ namespace Bit.Core.Test.Services .Throws(expectedException); var actualException = await Assert.ThrowsAsync(async () => - await sutProvider.Sut.SaveAttachmentRawWithServerAsync(cipher, fileName, data.Buffer)); + await sutProvider.Sut.SaveAttachmentRawWithServerAsync(cipher, cipherView, fileName, data.Buffer)); Assert.Equal(expectedException.Error.StatusCode, actualException.Error.StatusCode); }