From 2eb5fc77f56d5f85cba9e07cfa4c727dee3c69e4 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 7 Jul 2022 13:53:46 +0200 Subject: [PATCH] feat(bindings/crypto-nodejs): Remove `Clone` impl for `MediaEncryptionInfo`. We don't want to clone a struct that contains a secret. However, on the Node.js side, we can only receive arguments by references. The problem we have is that we cannot transfer the ownership of `MediaEncryptionInfo` to `AttachmentDecryptor` because we don't own it. To simulate this behavior, we use `Option.take`. A new method then appears: `EncryptedAttachment.hasMediaEncryptionInfoBeenConsumed` to know if the media encryption info has been consumed by `Attachment.decrypt` already or not. That way, we can decrypt only once. It is possible to do a JSON-encoded backup of the media encryption info by calling `EncryptedAttachment.mediaEncryptionInfo` though. --- .../src/attachment.rs | 49 ++++++++++++++----- .../tests/attachment.test.js | 11 +++++ .../src/file_encryption/attachments.rs | 2 +- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/bindings/matrix-sdk-crypto-nodejs/src/attachment.rs b/bindings/matrix-sdk-crypto-nodejs/src/attachment.rs index 6cd60181a..9432c19b2 100644 --- a/bindings/matrix-sdk-crypto-nodejs/src/attachment.rs +++ b/bindings/matrix-sdk-crypto-nodejs/src/attachment.rs @@ -30,7 +30,7 @@ impl Attachment { let mut encrypted_data = Vec::new(); encryptor.read_to_end(&mut encrypted_data).map_err(into_err)?; - let media_encryption_info = encryptor.finish(); + let media_encryption_info = Some(encryptor.finish()); Ok(EncryptedAttachment { encrypted_data: Uint8Array::new(encrypted_data), @@ -42,16 +42,30 @@ impl Attachment { /// /// The encrypted attachment can be created manually, or from the /// `encrypt` method. + /// + /// **Warning**: The encrypted attachment can be used only + /// **once**! The encrypted data will still be present, but the + /// media encryption info (which contain secrets) will be + /// destroyed. It is still possible to get a JSON-encoded backup + /// by calling `EncryptedAttachment.mediaEncryptionInfo`. #[napi] - pub fn decrypt(attachment: &EncryptedAttachment) -> napi::Result { + pub fn decrypt(attachment: &mut EncryptedAttachment) -> napi::Result { + let media_encryption_info = match attachment.media_encryption_info.take() { + Some(media_encryption_info) => media_encryption_info, + None => { + return Err(napi::Error::from_reason( + "The media encryption info are absent from the given encrypted attachment" + .to_string(), + )) + } + }; + let encrypted_data: &[u8] = attachment.encrypted_data.deref(); let mut cursor = Cursor::new(encrypted_data); - let mut decryptor = matrix_sdk_crypto::AttachmentDecryptor::new( - &mut cursor, - attachment.media_encryption_info.clone(), - ) - .map_err(into_err)?; + let mut decryptor = + matrix_sdk_crypto::AttachmentDecryptor::new(&mut cursor, media_encryption_info) + .map_err(into_err)?; let mut decrypted_data = Vec::new(); decryptor.read_to_end(&mut decrypted_data).map_err(into_err)?; @@ -63,7 +77,7 @@ impl Attachment { /// An encrypted attachment, usually created from `Attachment.encrypt`. #[napi] pub struct EncryptedAttachment { - media_encryption_info: matrix_sdk_crypto::MediaEncryptionInfo, + media_encryption_info: Option, /// The actual encrypted data. pub encrypted_data: Uint8Array, @@ -87,15 +101,26 @@ impl EncryptedAttachment { pub fn new(encrypted_data: Uint8Array, media_encryption_info: String) -> napi::Result { Ok(Self { encrypted_data, - media_encryption_info: serde_json::from_str(media_encryption_info.as_str()) - .map_err(into_err)?, + media_encryption_info: Some( + serde_json::from_str(media_encryption_info.as_str()).map_err(into_err)?, + ), }) } /// Return the media encryption info as a JSON-encoded string. The /// structure is fully valid. + /// + /// If the media encryption info have been consumed already, it + /// will return `null`. #[napi(getter)] - pub fn media_encryption_info(&self) -> String { - serde_json::to_string(&self.media_encryption_info).unwrap() + pub fn media_encryption_info(&self) -> Option { + serde_json::to_string(self.media_encryption_info.as_ref()?).ok() + } + + /// Check whether the media encryption info has been consumed by + /// `Attachment.decrypt` already. + #[napi(getter)] + pub fn has_media_encryption_info_been_consumed(&self) -> bool { + self.media_encryption_info.is_none() } } diff --git a/bindings/matrix-sdk-crypto-nodejs/tests/attachment.test.js b/bindings/matrix-sdk-crypto-nodejs/tests/attachment.test.js index 32c5e05c0..86e3eaf8b 100644 --- a/bindings/matrix-sdk-crypto-nodejs/tests/attachment.test.js +++ b/bindings/matrix-sdk-crypto-nodejs/tests/attachment.test.js @@ -32,9 +32,18 @@ describe(Attachment.name, () => { }); test('can decrypt data', () => { + expect(encryptedAttachment.hasMediaEncryptionInfoBeenConsumed).toStrictEqual(false); + const decryptedAttachment = Attachment.decrypt(encryptedAttachment); expect(textDecoder.decode(decryptedAttachment)).toStrictEqual(originalData); + expect(encryptedAttachment.hasMediaEncryptionInfoBeenConsumed).toStrictEqual(true); + }); + + test('can only decrypt once', () => { + expect(encryptedAttachment.hasMediaEncryptionInfoBeenConsumed).toStrictEqual(true); + + expect(() => { textDecoder.decode(decryptedAttachment) }).toThrow() }); }); @@ -61,6 +70,8 @@ describe(EncryptedAttachment.name, () => { }) ); + expect(encryptedAttachment.hasMediaEncryptionInfoBeenConsumed).toStrictEqual(false); expect(textDecoder.decode(Attachment.decrypt(encryptedAttachment))).toStrictEqual(originalData); + expect(encryptedAttachment.hasMediaEncryptionInfoBeenConsumed).toStrictEqual(true); }); }); diff --git a/crates/matrix-sdk-crypto/src/file_encryption/attachments.rs b/crates/matrix-sdk-crypto/src/file_encryption/attachments.rs index afc462bc2..5f9facfe2 100644 --- a/crates/matrix-sdk-crypto/src/file_encryption/attachments.rs +++ b/crates/matrix-sdk-crypto/src/file_encryption/attachments.rs @@ -277,7 +277,7 @@ impl<'a, R: Read + ?Sized + 'a> AttachmentEncryptor<'a, R> { /// Struct holding all the information that is needed to decrypt an encrypted /// file. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] pub struct MediaEncryptionInfo { /// The version of the encryption scheme. #[serde(rename = "v")]