feat(bindings/crypto-nodejs): Implement an Attachment API.
#818
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch provides a new API to encrypt and decrypt attachment, i.e. big buffer of type `Uint8Array`. It's based on `matrix_sdk_crypto::AttachmentEncryptor` and `AttachmentDecryptor`.
Codecov Report
@@ Coverage Diff @@
## main #818 +/- ##
=======================================
Coverage 77.92% 77.92%
=======================================
Files 92 92
Lines 13919 13930 +11
=======================================
+ Hits 10846 10855 +9
- Misses 3073 3075 +2
Continue to review full report at Codecov.
|
turt2live
requested changes
Jul 5, 2022
…ning `Uint8Array`. The new `napi-rs` release includes a patch that avoids cloning and copying data inside a `Uint8Array` (napi-rs/napi-rs#1224), it now returns a “Node.js reference” of it. This new `napi-rs` release also includes one of our patch, napi-rs/napi-rs#1200, which means we no longer need to depend on our fork.
Based on the [Section 11.11.1.6.1 Extensions to `m.room.message` msgtypes](https://spec.matrix.org/v1.2/client-server-api/#extensions-to-mroommessage-msgtypes), the parameter for the JSON Web Key is named `key`, not `web_key`. This patch fixes that by renaming the field when serializing and deserializing.
poljar
requested changes
Jul 7, 2022
…nInfo`. 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.
poljar
approved these changes
Jul 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch fixes #796.
This patch provides a new API to encrypt and decrypt attachment, i.e. big buffer of type
Uint8Array.This patch also fixes a bug in
matrix_sdk_crypto::MediaEncryptedInfowhere itsweb_keyfield should serialize to and deserialize fromkeyto respect the specification.It's based on
matrix_sdk_crypto::AttachmentEncryptorandAttachmentDecryptor.Note: I'm really not happy with the factThis is now fixed. Data are no longer copied!EncryptedAttachment.encryptedDatareturns a copy. But we cannot return a&Twithnapi-rs. I tried to create a new buffer that references data from elsewhere, like withUint8Array::with_external_data, but the data are collected by the GC (which notifies us before dropping the data); that's not what we want. I then tried withReferenceorRef, butnapi-rsdoesn't allow to return them from a function, so it's useless. I'll investigate that. In the meantime, as mentionned in #796, right now, there is a copy in the previousmatrix-sdk-crypto-nodejsAPI. This patch is actually better as it doesn't need to serialize and deserialize the entire data, it just computes a newUint8Array, which is an improvement.