mujx / nheko

No longer maintained - Desktop client for the Matrix protocol (active fork https://github.com/Nheko-Reborn)
GNU General Public License v3.0
394 stars 65 forks source link

E2EE: Image from clipboard data / upload is sent unencrypted #348

Open lbeltrame opened 6 years ago

lbeltrame commented 6 years ago

System:

Actual behavior

Pasting an image from the clipboard will cause the attached image to be sent unencrypted. Uploading the image via the attachment icon does the same. Checking the raw source of the image shows indeed that it was sent without being encrypted.

Expected behavior

Image is sent encrypted.

Steps to reproduce

  1. Access an E2EE-enabled room
  2. Paste / attach an image so that it is shown inline
  3. Send the message
  4. Check from Riot the state of the message
albjeremias commented 6 years ago

user content is tottaly broken on riot...

checkout this issue: https://github.com/vector-im/riot-web/issues/6173#issuecomment-366040915

here some comments on the code: https://github.com/matrix-org/matrix-react-sdk/blob/master/src/components/views/messages/MFileBody.js#L66-L116

mujx commented 6 years ago

Encrypted attachments are not implemented yet.

albjeremias commented 6 years ago

@mujx in matrix or in nheko?

mujx commented 6 years ago

In nheko, Riot sends them encrypted.

lbeltrame commented 6 years ago

Can the attachment be disabled (only if it doesn't require workarounds) in E2EE rooms? Right now there's no way to tell from nheko that they're sent unencrypted.

mujx commented 6 years ago

It would be better IMHO to wait for proper support than trying to add now an option to disable them which most likely be removed in the future.

lbeltrame commented 6 years ago

Sounds reasonable to me. FTR, I'm perfectly fine with this feature not being present yet, but there should be at least a warning somewhere (and IMO the README.md is the best place for it) that not all content is encrypted (or conversely, indicating what is supported, like "attachments not encrypted yet").

If you think it makes sense, I can make a PR against the README.md.

mujx commented 6 years ago

@lbeltrame Yup that's reasonable, go ahead.