From 287b3a0580cdccd1b63d2f1108e734de2f42fc8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gonc=CC=A7alves?= Date: Tue, 12 Dec 2023 12:22:28 +0000 Subject: [PATCH] PM-4739 Fix PR comments --- src/Core/Abstractions/ICryptoService.cs | 1 + src/Core/Models/Domain/Cipher.cs | 4 +--- src/Core/Models/Domain/Login.cs | 5 ++++- src/Core/Models/Domain/LoginUri.cs | 30 ++++--------------------- src/Core/Models/Export/LoginUri.cs | 2 ++ src/Core/Services/CryptoService.cs | 14 ++++++++++++ 6 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/Core/Abstractions/ICryptoService.cs b/src/Core/Abstractions/ICryptoService.cs index cdd5915ba..ffb9f4c31 100644 --- a/src/Core/Abstractions/ICryptoService.cs +++ b/src/Core/Abstractions/ICryptoService.cs @@ -64,5 +64,6 @@ namespace Bit.Core.Abstractions Task GetOrDeriveMasterKeyAsync(string password, string userId = null); Task UpdateMasterKeyAndUserKeyAsync(MasterKey masterKey); Task HashAsync(string value, CryptoHashAlgorithm hashAlgorithm); + Task ValidateUriChecksumAsync(EncString remoteUriChecksum, string rawUri, string orgId, SymmetricCryptoKey key); } } diff --git a/src/Core/Models/Domain/Cipher.cs b/src/Core/Models/Domain/Cipher.cs index 9a818519d..a7a2d6943 100644 --- a/src/Core/Models/Domain/Cipher.cs +++ b/src/Core/Models/Domain/Cipher.cs @@ -93,7 +93,6 @@ namespace Bit.Core.Models.Domain public async Task DecryptAsync() { var model = new CipherView(this); - var bypassValidation = true; if (Key != null) { @@ -105,7 +104,6 @@ namespace Bit.Core.Models.Domain var key = await cryptoService.DecryptToBytesAsync(Key, orgKey); model.Key = new CipherKey(key); - bypassValidation = false; } await DecryptObjAsync(model, this, new HashSet @@ -117,7 +115,7 @@ namespace Bit.Core.Models.Domain switch (Type) { case Enums.CipherType.Login: - model.Login = await Login.DecryptAsync(OrganizationId, bypassValidation, model.Key); + model.Login = await Login.DecryptAsync(OrganizationId, Key == null, model.Key); break; case Enums.CipherType.SecureNote: model.SecureNote = await SecureNote.DecryptAsync(OrganizationId, model.Key); diff --git a/src/Core/Models/Domain/Login.cs b/src/Core/Models/Domain/Login.cs index e9df86924..58eee3939 100644 --- a/src/Core/Models/Domain/Login.cs +++ b/src/Core/Models/Domain/Login.cs @@ -2,8 +2,10 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Bit.Core.Abstractions; using Bit.Core.Models.Data; using Bit.Core.Models.View; +using Bit.Core.Utilities; namespace Bit.Core.Models.Domain { @@ -41,11 +43,12 @@ namespace Bit.Core.Models.Domain }, orgId, key); if (Uris != null) { + var cryptoService = ServiceContainer.Resolve(); view.Uris = new List(); foreach (var uri in Uris) { var loginUriView = await uri.DecryptAsync(orgId, key); - if (bypassValidation || (await uri.ValidateChecksum(loginUriView.Uri, orgId, key))) + if (bypassValidation || await cryptoService.ValidateUriChecksumAsync(uri.UriChecksum, loginUriView.Uri, orgId, key)) { view.Uris.Add(loginUriView); } diff --git a/src/Core/Models/Domain/LoginUri.cs b/src/Core/Models/Domain/LoginUri.cs index 6c45af91e..896570d73 100644 --- a/src/Core/Models/Domain/LoginUri.cs +++ b/src/Core/Models/Domain/LoginUri.cs @@ -1,10 +1,9 @@ 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 { @@ -12,7 +11,8 @@ namespace Bit.Core.Models.Domain { private HashSet _map = new HashSet { - "Uri" + nameof(Uri), + nameof(UriChecksum) }; public LoginUri() { } @@ -20,12 +20,6 @@ namespace Bit.Core.Models.Domain public LoginUri(LoginUriData obj, bool alreadyEncrypted = false) { Match = obj.Match; - - if (obj.UriChecksum != null) - { - UriChecksum = new EncString(obj.UriChecksum); - } - BuildDomainModel(this, obj, _map, alreadyEncrypted); } @@ -35,7 +29,7 @@ namespace Bit.Core.Models.Domain public Task DecryptAsync(string orgId, SymmetricCryptoKey key = null) { - return DecryptObjAsync(new LoginUriView(this), this, _map, orgId, key); + return DecryptObjAsync(new LoginUriView(this), this, _map.Where(m => m != nameof(UriChecksum)).ToHashSet(), orgId, key); } public LoginUriData ToLoginUriData() @@ -44,21 +38,5 @@ namespace Bit.Core.Models.Domain BuildDataModel(this, u, _map, new HashSet { "Match" }); return u; } - - public async Task ValidateChecksum(string clearTextUri, string orgId, SymmetricCryptoKey key) - { - if (this.UriChecksum == null) - { - return false; - } - - // 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 localChecksum = await cryptoService.HashAsync(clearTextUri, CryptoHashAlgorithm.Sha256); - - var remoteChecksum = await this.UriChecksum.DecryptAsync(orgId, key); - return remoteChecksum == localChecksum; - } } } diff --git a/src/Core/Models/Export/LoginUri.cs b/src/Core/Models/Export/LoginUri.cs index e3f215ff7..a4e161fed 100644 --- a/src/Core/Models/Export/LoginUri.cs +++ b/src/Core/Models/Export/LoginUri.cs @@ -17,10 +17,12 @@ namespace Bit.Core.Models.Export { Match = obj.Match; Uri = obj.Uri?.EncryptedString; + UriChecksum = obj.UriChecksum?.EncryptedString; } public UriMatchType? Match { get; set; } public string Uri { get; set; } + public string UriChecksum { get; set; } public static LoginUriView ToView(LoginUri req, LoginUriView view = null) { diff --git a/src/Core/Services/CryptoService.cs b/src/Core/Services/CryptoService.cs index 02c7542e0..e2a1d2efa 100644 --- a/src/Core/Services/CryptoService.cs +++ b/src/Core/Services/CryptoService.cs @@ -736,6 +736,20 @@ namespace Bit.Core.Services return Convert.ToBase64String(hashArray); } + public async Task ValidateUriChecksumAsync(EncString remoteUriChecksum, string rawUri, string orgId, SymmetricCryptoKey key) + { + if (remoteUriChecksum == null) + { + return false; + } + + //var cryptoService = ServiceContainer.Resolve(); + var localChecksum = await HashAsync(rawUri, CryptoHashAlgorithm.Sha256); + + var remoteChecksum = await remoteUriChecksum.DecryptAsync(orgId, key); + return remoteChecksum == localChecksum; + } + // --HELPER METHODS-- private async Task StoreAdditionalKeysAsync(UserKey userKey, string userId = null)