feat(vault): add key id validation

src/Api/Vault/Controllers/CiphersController.cs

@@ -165,15 +165,8 @@ public async Task<CipherResponseModel> Post([FromBody] CipherRequestModel model)
     {
         var user = await _userService.GetUserByPrincipalAsync(User);
 
-        // Validate the model was encrypted for the posting user
-        if (model.EncryptedFor != null)
-        {
-            if (model.EncryptedFor != user.Id)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", user.Id, model.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
-        }
+        // Validate the model was encrypted by the posting user
+        ValidateCipherEncryptedByUser(model, user);
 
         var cipher = model.ToCipherDetails(user.Id);
         if (cipher.OrganizationId.HasValue && !await _currentContext.OrganizationUser(cipher.OrganizationId.Value))
@@ -191,15 +184,8 @@ public async Task<CipherResponseModel> PostCreate([FromBody] CipherCreateRequest
     {
         var user = await _userService.GetUserByPrincipalAsync(User);
 
-        // Validate the model was encrypted for the posting user
-        if (model.Cipher.EncryptedFor != null)
-        {
-            if (model.Cipher.EncryptedFor != user.Id)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", user.Id, model.Cipher.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
-        }
+        // Validate the model was encrypted by the posting user
+        ValidateCipherEncryptedByUser(model.Cipher, user);
 
         var cipher = model.Cipher.ToCipherDetails(user.Id);
         if (cipher.OrganizationId.HasValue && !await _currentContext.OrganizationUser(cipher.OrganizationId.Value))
@@ -225,14 +211,7 @@ public async Task<CipherMiniResponseModel> PostAdmin([FromBody] CipherCreateRequ
         var userId = _userService.GetProperUserId(User).Value;
 
         // Validate the model was encrypted for the posting user
-        if (model.Cipher.EncryptedFor != null)
-        {
-            if (model.Cipher.EncryptedFor != userId)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", userId, model.Cipher.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
-        }
+        ValidateCipherEncryptedForUser(model.Cipher, userId);
 
         await _cipherService.SaveAsync(cipher, userId, model.Cipher.LastKnownRevisionDate, model.CollectionIds, true, false);
 
@@ -250,15 +229,8 @@ public async Task<CipherResponseModel> Put(Guid id, [FromBody] CipherRequestMode
             throw new NotFoundException();
         }
 
-        // Validate the model was encrypted for the posting user
-        if (model.EncryptedFor != null)
-        {
-            if (model.EncryptedFor != user.Id)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CipherId: {CipherId}, CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", id, user.Id, model.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
-        }
+        // Validate the model was encrypted by the posting user
+        ValidateCipherEncryptedByUser(model, user, id);
 
         ValidateClientVersionForFido2CredentialSupport(cipher);
 
@@ -291,14 +263,7 @@ public async Task<CipherMiniResponseModel> PutAdmin(Guid id, [FromBody] CipherRe
         var cipher = await _cipherRepository.GetOrganizationDetailsByIdAsync(id);
 
         // Validate the model was encrypted for the posting user
-        if (model.EncryptedFor != null)
-        {
-            if (model.EncryptedFor != userId)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CipherId: {CipherId}, CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", id, userId, model.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
-        }
+        ValidateCipherEncryptedForUser(model, userId, id);
 
         ValidateClientVersionForFido2CredentialSupport(cipher);
 
@@ -732,15 +697,8 @@ public async Task<CipherResponseModel> PutShare(Guid id, [FromBody] CipherShareR
             throw new NotFoundException();
         }
 
-        // Validate the model was encrypted for the posting user
-        if (model.Cipher.EncryptedFor != null)
-        {
-            if (model.Cipher.EncryptedFor != user.Id)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CipherId: {CipherId} CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", id, user.Id, model.Cipher.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
-        }
+        // Validate the model was encrypted by the posting user
+        ValidateCipherEncryptedByUser(model.Cipher, user, id);
 
         ValidateClientVersionForFido2CredentialSupport(cipher);
 
@@ -1237,14 +1195,10 @@ public async Task<ListResponseModel<CipherMiniResponseModel>> PutShareMany([From
         var ciphers = await _cipherRepository.GetManyByUserIdAsync(userId, withOrganizations: false);
         var ciphersDict = ciphers.ToDictionary(c => c.Id);
 
-        // Validate the model was encrypted for the posting user
+        // Validate the models were encrypted for the posting user
         foreach (var cipher in model.Ciphers)
         {
-            if (cipher.EncryptedFor.HasValue && cipher.EncryptedFor.Value != userId)
-            {
-                _logger.LogError("Cipher was not encrypted for the current user. CipherId: {CipherId}, CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}", cipher.Id, userId, cipher.EncryptedFor);
-                throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
-            }
+            ValidateCipherEncryptedForUser(cipher, userId, cipher.Id);
         }
 
         var shareCiphers = new List<(CipherDetails, DateTime?)>();
@@ -1661,6 +1615,58 @@ private void ValidateClientVersionForFido2CredentialSupport(Cipher cipher)
         }
     }
 
+    /// <summary>
+    /// Validates that the cipher in <paramref name="model"/> was encrypted by the acting user.
+    /// <para>
+    /// Deprecated in favor of <see cref="ValidateCipherEncryptedByUser"/>, which identifies the key
+    /// rather than the user. Only checked when the client sends the field.
+    /// </para>
+    /// </summary>
+    private void ValidateCipherEncryptedForUser(CipherRequestModel model, Guid userId, Guid? cipherId = null)
+    {
+#pragma warning disable CS0618 // EncryptedFor is deprecated, but is still honored for clients that send it.
+        var encryptedFor = model.EncryptedFor;
+#pragma warning restore CS0618
+
+        if (encryptedFor != null && encryptedFor != userId)
+        {
+            _logger.LogError(
+                "Cipher was not encrypted for the current user. CipherId: {CipherId}, CurrentUser: {CurrentUserId}, EncryptedFor: {EncryptedFor}",
+                cipherId, userId, encryptedFor);
+            throw new BadRequestException("Cipher was not encrypted for the current user. Please try again.");
+        }
+    }
+
+    /// <summary>
+    /// Validates that the cipher in <paramref name="model"/> was encrypted by the acting user, with that
+    /// user's current user key.
+    /// <para>
+    /// The key id is only compared when both sides are present: the client may predate the field, and the
+    /// user may not have a key id recorded yet. This mirrors
+    /// <see cref="Core.KeyManagement.Models.Data.MasterPasswordUnlockData.ValidateKeyIdUnchangedForUser"/>.
+    /// </para>
+    /// </summary>
+    private void ValidateCipherEncryptedByUser(CipherRequestModel model, User user, Guid? cipherId = null)
+    {
+        ValidateCipherEncryptedForUser(model, user.Id, cipherId);
+
+        var currentUserKeyId = user.GetUserKeyId();
+        var encryptedByKeyId = model.GetEncryptedByKeyId();
+        if (currentUserKeyId is null || encryptedByKeyId is null)
+        {
+            // Either the user has no key id recorded yet, or the client predates the field; nothing to compare.
+            return;
+        }
+
+        if (!currentUserKeyId.Equals(encryptedByKeyId))
+        {
+            _logger.LogError(
+                "Cipher was not encrypted with the current user key. CipherId: {CipherId}, CurrentUser: {CurrentUserId}, EncryptedByKeyId: {EncryptedByKeyId}",
+                cipherId, user.Id, encryptedByKeyId);
+            throw new BadRequestException("Cipher was not encrypted with the current user key. Please try again.");
+        }
+    }
+
     private async Task<CipherOrganizationDetails> GetByIdAsyncAdmin(Guid cipherId)
     {
         return await _cipherRepository.GetOrganizationDetailsByIdAsync(cipherId);

src/Api/Vault/Models/Request/CipherRequestModel.cs

@@ -3,6 +3,7 @@
 
 using System.ComponentModel.DataAnnotations;
 using System.Text.Json;
+using Bit.Core.KeyManagement.Models.Data;
 using Bit.Core.Utilities;
 using Bit.Core.Vault.Entities;
 using Bit.Core.Vault.Enums;
@@ -15,7 +16,16 @@ public class CipherRequestModel
     /// <summary>
     /// The Id of the user that encrypted the cipher. It should always represent a UserId.
     /// </summary>
+    [Obsolete("Use EncryptedByKeyId instead, which identifies the key the cipher was encrypted with.")]
     public Guid? EncryptedFor { get; set; }
+
+    /// <summary>
+    /// Hex-encoded key id of the user key the client held when it encrypted this cipher. Absent for
+    /// clients that predate the field. When present, it must match the acting user's current user key id.
+    /// </summary>
+    [KeyId]
+    public string EncryptedByKeyId { get; set; }
+
     public CipherType Type { get; set; }
 
     [StringLength(36)]
@@ -67,6 +77,12 @@ public class CipherRequestModel
     public DateTime? LastKnownRevisionDate { get; set; } = null;
     public DateTime? ArchivedDate { get; set; }
 
+    /// <summary>
+    /// The key the client encrypted this cipher with, or null when it did not supply one.
+    /// </summary>
+    public KeyId GetEncryptedByKeyId() =>
+        KeyId.FromHexEncodedString(string.IsNullOrEmpty(EncryptedByKeyId) ? null : EncryptedByKeyId);
+
     public CipherDetails ToCipherDetails(Guid userId, bool allowOrgIdSet = true)
     {
         var hasOrgId = !string.IsNullOrWhiteSpace(OrganizationId);