[PR #2421] [MERGED] [PM-115] Cipher key encryption update #3558

Closed
opened 2025-11-26 23:34:23 -06:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/2421
Author: @fedemkr
Created: 3/20/2023
Status: Merged
Merged: 9/28/2023
Merged by: @fedemkr

Base: masterHead: feature/PM-115-encryption-update-new-model


📝 Commits (10+)

  • 2bb7207 PM-115 Added new cipher key and encryption/decryption mechanisms on cipher
  • c8555ef Merge branch 'master' into feature/PM-115-encryption-update-new-model
  • a3b18dc PM-115 fix format
  • 74190e0 Merge branch 'master' into feature/PM-115-encryption-update-new-model
  • 7cdc5a6 Merge branch 'master' into feature/PM-115-encryption-update-new-model
  • 1a0e9e9 PM-115 removed ForceKeyRotation from new cipher encryption model given that another approach will be taken
  • b150d88 [PM-1690] Added minimum server version restriction to cipher key encryption (#2463)
  • bcd47b3 Merge branch 'master' into feature/PM-115-encryption-update-new-model
  • e19b86e PM-115 Temporarily Changed cipher key new encryption config to help testing (this change should be reseted eventually)
  • 74196f6 Merge branch 'master' into feature/PM-115-encryption-update-new-model

📊 Changes

26 files changed (+241 additions, -85 deletions)

View changed files

📝 src/App/Pages/Vault/AttachmentsPageViewModel.cs (+1 -1)
📝 src/Core/Abstractions/ICipherService.cs (+1 -1)
📝 src/Core/Constants.cs (+2 -0)
📝 src/Core/Models/Data/CipherData.cs (+2 -0)
📝 src/Core/Models/Domain/Attachment.cs (+19 -16)
📝 src/Core/Models/Domain/Card.cs (+2 -2)
📝 src/Core/Models/Domain/Cipher.cs (+40 -18)
📝 src/Core/Models/Domain/Fido2Key.cs (+2 -2)
📝 src/Core/Models/Domain/Field.cs (+2 -2)
📝 src/Core/Models/Domain/Identity.cs (+2 -2)
📝 src/Core/Models/Domain/Login.cs (+4 -4)
📝 src/Core/Models/Domain/LoginUri.cs (+2 -2)
📝 src/Core/Models/Domain/PasswordHistory.cs (+2 -2)
📝 src/Core/Models/Domain/SecureNote.cs (+1 -1)
📝 src/Core/Models/Domain/SymmetricCryptoKey.cs (+7 -1)
📝 src/Core/Models/Export/Cipher.cs (+4 -0)
📝 src/Core/Models/Request/CipherRequest.cs (+2 -0)
📝 src/Core/Models/Response/CipherResponse.cs (+1 -0)
📝 src/Core/Models/View/CipherView.cs (+1 -0)
📝 src/Core/Services/ApiService.cs (+1 -1)

...and 6 more files

📄 Description

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

The cipher will now have its own Key to encrypt/decrypt its content which will be encrypted using the master password or organization key.

This needs server changes to work and clients changes to have the key assigned (given that for now the mobile client won't create the cipher key):

Code changes

  • Cipher: Added new properties for the key. Added logic to decrypt using the new Key if available, passing such key to the different item types. And updated it to not use hardcoded strings and use nameof().

  • CipherService: Updated logic to encrypt the Cipher using the new Key.
    Note: for now the logic of the new Key gets applied only if the Key already exists on the Cipher. The creation of such Key locally will be enabled on a later release to avoid conflicts between clients.

  • CipherData: Added new properties for the key.

  • CipherRequest, CipherResponse, CipherView: Added new properties for the key.

  • Attachment: Updated it to not use hardcoded strings and use nameof() and also added the key as a parameter to support the new approach.

  • Card, Field, Identity, Login, LoginUri, PasswordHistory, SecureNote: Added the key as a parameter to support the new approach.

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/bitwarden/android/pull/2421 **Author:** [@fedemkr](https://github.com/fedemkr) **Created:** 3/20/2023 **Status:** ✅ Merged **Merged:** 9/28/2023 **Merged by:** [@fedemkr](https://github.com/fedemkr) **Base:** `master` ← **Head:** `feature/PM-115-encryption-update-new-model` --- ### 📝 Commits (10+) - [`2bb7207`](https://github.com/bitwarden/android/commit/2bb72076ae97b18b3d96209c1633458192720014) PM-115 Added new cipher key and encryption/decryption mechanisms on cipher - [`c8555ef`](https://github.com/bitwarden/android/commit/c8555ef1e70390865d390dabcc0d1b5d922fceba) Merge branch 'master' into feature/PM-115-encryption-update-new-model - [`a3b18dc`](https://github.com/bitwarden/android/commit/a3b18dced6a5e7ac7c8a09d447c40d5c43ca2b1b) PM-115 fix format - [`74190e0`](https://github.com/bitwarden/android/commit/74190e0125aa07819159716fdc30f97b74007d70) Merge branch 'master' into feature/PM-115-encryption-update-new-model - [`7cdc5a6`](https://github.com/bitwarden/android/commit/7cdc5a62331eca9b93e887dde1000057e3e12d42) Merge branch 'master' into feature/PM-115-encryption-update-new-model - [`1a0e9e9`](https://github.com/bitwarden/android/commit/1a0e9e961d36a5d1818f2398a9f5d99a717dd7ac) PM-115 removed ForceKeyRotation from new cipher encryption model given that another approach will be taken - [`b150d88`](https://github.com/bitwarden/android/commit/b150d883c03b68da2d0737fb1de915ff7701bfb8) [PM-1690] Added minimum server version restriction to cipher key encryption (#2463) - [`bcd47b3`](https://github.com/bitwarden/android/commit/bcd47b3b3e14b19580a006e9af7b15ed58a5fbbd) Merge branch 'master' into feature/PM-115-encryption-update-new-model - [`e19b86e`](https://github.com/bitwarden/android/commit/e19b86ee003358cdc81318dad429bc6572bffe90) PM-115 Temporarily Changed cipher key new encryption config to help testing (this change should be reseted eventually) - [`74196f6`](https://github.com/bitwarden/android/commit/74196f6ecf1d14ec7506f2db4c81f7718b86adfb) Merge branch 'master' into feature/PM-115-encryption-update-new-model ### 📊 Changes **26 files changed** (+241 additions, -85 deletions) <details> <summary>View changed files</summary> 📝 `src/App/Pages/Vault/AttachmentsPageViewModel.cs` (+1 -1) 📝 `src/Core/Abstractions/ICipherService.cs` (+1 -1) 📝 `src/Core/Constants.cs` (+2 -0) 📝 `src/Core/Models/Data/CipherData.cs` (+2 -0) 📝 `src/Core/Models/Domain/Attachment.cs` (+19 -16) 📝 `src/Core/Models/Domain/Card.cs` (+2 -2) 📝 `src/Core/Models/Domain/Cipher.cs` (+40 -18) 📝 `src/Core/Models/Domain/Fido2Key.cs` (+2 -2) 📝 `src/Core/Models/Domain/Field.cs` (+2 -2) 📝 `src/Core/Models/Domain/Identity.cs` (+2 -2) 📝 `src/Core/Models/Domain/Login.cs` (+4 -4) 📝 `src/Core/Models/Domain/LoginUri.cs` (+2 -2) 📝 `src/Core/Models/Domain/PasswordHistory.cs` (+2 -2) 📝 `src/Core/Models/Domain/SecureNote.cs` (+1 -1) 📝 `src/Core/Models/Domain/SymmetricCryptoKey.cs` (+7 -1) 📝 `src/Core/Models/Export/Cipher.cs` (+4 -0) 📝 `src/Core/Models/Request/CipherRequest.cs` (+2 -0) 📝 `src/Core/Models/Response/CipherResponse.cs` (+1 -0) 📝 `src/Core/Models/View/CipherView.cs` (+1 -0) 📝 `src/Core/Services/ApiService.cs` (+1 -1) _...and 6 more files_ </details> ### 📄 Description ## Type of change - [ ] Bug fix - [X] New feature development - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective <!--Describe what the purpose of this PR is. For example: what bug you're fixing or what new feature you're adding--> The cipher will now have its own `Key` to encrypt/decrypt its content which will be encrypted using the master password or organization key. This needs server changes to work and clients changes to have the key assigned (given that for now the mobile client won't create the cipher key): - Server: [https://github.com/bitwarden/server/pull/2681](https://github.com/bitwarden/server/pull/2681 ) - Clients: [https://github.com/bitwarden/clients/pull/5114](https://github.com/bitwarden/clients/pull/5114) ## Code changes <!--Explain the changes you've made to each file or major component. This should help the reviewer understand your changes--> <!--Also refer to any related changes or PRs in other repositories--> * **Cipher:** Added new properties for the key. Added logic to decrypt using the new `Key` if available, passing such key to the different item types. And updated it to not use hardcoded strings and use `nameof()`. * **CipherService:** Updated logic to encrypt the `Cipher` using the new `Key`. _Note: for now the logic of the new `Key` gets applied only if the `Key` already exists on the `Cipher`. The creation of such `Key` locally will be enabled on a later release to avoid conflicts between clients._ * **CipherData:** Added new properties for the key. * **CipherRequest, CipherResponse, CipherView:** Added new properties for the key. * **Attachment:** Updated it to not use hardcoded strings and use `nameof()` and also added the `key` as a parameter to support the new approach. * **Card, Field, Identity, Login, LoginUri, PasswordHistory, SecureNote:** Added the `key` as a parameter to support the new approach. ## Before you submit - Please check for formatting errors (`dotnet format --verify-no-changes`) (required) - Please add **unit tests** where it makes sense to do so (encouraged but not required) - If this change requires a **documentation update** - notify the documentation team - If this change has particular **deployment requirements** - notify the DevOps team --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2025-11-26 23:34:23 -06:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#3558