Skip to content

Commit 9704014

Browse files
authored
[PM-22812] Attachments get corrupted when downgrading from cipherkeys (#328)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-22812 <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective Editing a cipher with attachments causes attachment corruption when switching between cipher-key encryption and user-key encryption in either direction. The SDK's AttachmentView was not providing the decrypted attachment key needed for re-encryption scenarios. When the cipher gets re-encrypted between different encryption contexts (cipher-key ↔ user-key) during save, the client needs the decrypted attachment key to properly re-encrypt it under the new encryption context. Without this, null gets posted for the attachment key, breaking decryption. This is a temporary solution during the migration from TypeScript to the SDK. The decrypted_key field should be removed once all encryption logic is handled within the SDK. Cleanup tracked in: https://bitwarden.atlassian.net/browse/PM-23005 <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes
1 parent 4b80d30 commit 9704014

File tree

2 files changed

+33
-0
lines changed

2 files changed

+33
-0
lines changed

crates/bitwarden-vault/src/cipher/attachment.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@ pub struct AttachmentView {
3737
pub size_name: Option<String>,
3838
pub file_name: Option<String>,
3939
pub key: Option<EncString>,
40+
/// The decrypted attachmentkey in base64 format.
41+
///
42+
/// **TEMPORARY FIELD**: This field is a temporary workaround to provide
43+
/// decrypted attachment keys to the TypeScript client during the migration
44+
/// process. It will be removed once the encryption/decryption logic is
45+
/// fully migrated to the SDK.
46+
///
47+
/// **Ticket**: <https://bitwarden.atlassian.net/browse/PM-23005>
48+
///
49+
/// Do not rely on this field for long-term use.
50+
#[cfg(feature = "wasm")]
51+
pub decrypted_key: Option<String>,
4052
}
4153

4254
#[allow(missing_docs)]
@@ -164,13 +176,27 @@ impl Decryptable<KeyIds, SymmetricKeyId, AttachmentView> for Attachment {
164176
ctx: &mut KeyStoreContext<KeyIds>,
165177
key: SymmetricKeyId,
166178
) -> Result<AttachmentView, CryptoError> {
179+
#[cfg(feature = "wasm")]
180+
let decrypted_key = if let Some(attachment_key) = &self.key {
181+
let content_key_id = ctx.unwrap_symmetric_key(key, ATTACHMENT_KEY, attachment_key)?;
182+
183+
#[allow(deprecated)]
184+
let actual_key = ctx.dangerous_get_symmetric_key(content_key_id)?;
185+
186+
Some(actual_key.to_base64())
187+
} else {
188+
None
189+
};
190+
167191
Ok(AttachmentView {
168192
id: self.id.clone(),
169193
url: self.url.clone(),
170194
size: self.size.clone(),
171195
size_name: self.size_name.clone(),
172196
file_name: self.file_name.decrypt(ctx, key)?,
173197
key: self.key.clone(),
198+
#[cfg(feature = "wasm")]
199+
decrypted_key,
174200
})
175201
}
176202
}
@@ -227,6 +253,7 @@ mod tests {
227253
size_name: Some("100 Bytes".into()),
228254
file_name: Some("Test.txt".into()),
229255
key: None,
256+
decrypted_key: None,
230257
};
231258

232259
let contents = b"This is a test file that we will encrypt. It's 100 bytes long, the encrypted version will be longer!";
@@ -283,6 +310,7 @@ mod tests {
283310
size_name: Some("161 Bytes".into()),
284311
file_name: Some("Test.txt".into()),
285312
key: Some("2.r288/AOSPiaLFkW07EBGBw==|SAmnnCbOLFjX5lnURvoualOetQwuyPc54PAmHDTRrhT0gwO9ailna9U09q9bmBfI5XrjNNEsuXssgzNygRkezoVQvZQggZddOwHB6KQW5EQ=|erIMUJp8j+aTcmhdE50zEX+ipv/eR1sZ7EwULJm/6DY=".parse().unwrap()),
313+
decrypted_key: None,
286314
};
287315

288316
let cipher = Cipher {
@@ -340,6 +368,7 @@ mod tests {
340368
size_name: Some("161 Bytes".into()),
341369
file_name: Some("Test.txt".into()),
342370
key: None,
371+
decrypted_key: None,
343372
};
344373

345374
let cipher = Cipher {

crates/bitwarden-vault/src/cipher/cipher.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,7 @@ mod tests {
993993
size_name: None,
994994
file_name: Some("Attachment test name".into()),
995995
key: None,
996+
decrypted_key: None,
996997
};
997998
cipher.attachments = Some(vec![attachment]);
998999

@@ -1062,6 +1063,7 @@ mod tests {
10621063
size_name: None,
10631064
file_name: Some("Attachment test name".into()),
10641065
key: None,
1066+
decrypted_key: None,
10651067
};
10661068
cipher.attachments = Some(vec![attachment]);
10671069

@@ -1105,6 +1107,7 @@ mod tests {
11051107
size_name: None,
11061108
file_name: Some("Attachment test name".into()),
11071109
key: Some(attachment_key_enc),
1110+
decrypted_key: None,
11081111
};
11091112
cipher.attachments = Some(vec![attachment]);
11101113
let cred = generate_fido2(&mut key_store.context(), SymmetricKeyId::User);
@@ -1180,6 +1183,7 @@ mod tests {
11801183
size_name: None,
11811184
file_name: Some("Attachment test name".into()),
11821185
key: Some(attachment_key_enc.clone()),
1186+
decrypted_key: None,
11831187
};
11841188
cipher.attachments = Some(vec![attachment]);
11851189

0 commit comments

Comments
 (0)