Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Messages shown as unencrypted with red shield in encrypted room search UI #11831

Closed
dkasak opened this issue Jan 13, 2020 · 10 comments · Fixed by matrix-org/matrix-react-sdk#4738 or matrix-org/matrix-react-sdk#4788
Labels

Comments

@dkasak
Copy link
Member

dkasak commented Jan 13, 2020

Description

Searching for messages in encrypted rooms (using the new encrypted room search functionality implemented via seshat), the results are shown as unencrypted with a red shield even though the original message was properly encrypted and verified. This happens because seshat reinjects the decrypted messages but the verified status of the message gets lost along the way.

Steps to reproduce

  1. Turn on encrypted room search in labs.
  2. Send a message to an encrypted room.
  3. Search for the message and observe the red shield.

Version information

  • Platform: desktop
  • OS: Arch Linux
  • Version: develop (9e8358d), with matrix-js-sdk and matrix-react-sdk also on develop
@aaronraimist aaronraimist added the A-Indexing Indexing messages via Seshat label Feb 16, 2020
@aaronraimist
Copy link
Collaborator

Just so you know, you can upload images directly to GitHub issues so you don't have to worry about other services deleting them.

Unencrypted

turt2live added a commit to matrix-org/matrix-react-sdk that referenced this issue Apr 22, 2020
Fixes element-hq/element-web#13262

This is part of the cross-signing shipping master plan. Known issues relating to this feature are:
* element-hq/element-web#12896
* https://github.com/vector-im/riot-web/issues/12385
* element-hq/element-web#11831
* element-hq/element-web#11155

In theory, these are issues we're comfortable with shipping as we're already enabling it by default. This just makes it easier on everyone by removing the flag (making it still enabled by default).
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this issue Apr 22, 2020
Fixes element-hq/element-web#13262

This is part of the cross-signing shipping master plan. Known issues relating to this feature are:
* element-hq/element-web#12896
* https://github.com/vector-im/riot-web/issues/12385
* element-hq/element-web#11831
* element-hq/element-web#11155

In theory, these are issues we're comfortable with shipping as we're already enabling it by default. This just makes it easier on everyone by removing the flag (making it still enabled by default).
@tredondo
Copy link

tredondo commented May 6, 2020

Just wanted to confirm I see this in v1.6 as well.

@pwr22
Copy link

pwr22 commented May 8, 2020

Me too, running the latest flatpak on Manjaro.

@t3chguy
Copy link
Member

t3chguy commented May 13, 2020

Not sure what the plan here is for bite-sized given as it looks, Seshat only stores the decrypted event so we lose the encryption information. Asking Synapse for each event by ID would be slow but possible I guess. Hiding the icons is bad because some of these messages might be from unverified devices and whatnot.

@jryans ideas?

@jryans
Copy link
Collaborator

jryans commented May 21, 2020

Not sure what the plan here is for bite-sized given as it looks, Seshat only stores the decrypted event so we lose the encryption information. Asking Synapse for each event by ID would be slow but possible I guess. Hiding the icons is bad because some of these messages might be from unverified devices and whatnot.

Hmm, my suggestion as a quick fix would be to hide the icons in search results for now. The trust status is based on dynamic info, so we can't save it in Seshat and it would also be expensive to check every message as you say, so I think for now we should bias towards removing the warning overload (which isn't even correct in this case) by hiding the icons.

I presume in many cases, people may click on a search result to view the message in context in the room, and then the correct status should be visible there.

@t3chguy
Copy link
Member

t3chguy commented May 21, 2020

The trust status is based on dynamic info

Sure but if Seshat stored the deviceId of the [verified] sending session then we could check if we have trust for that session for rendering

@jryans
Copy link
Collaborator

jryans commented May 21, 2020

Sure but if Seshat stored the deviceId of the [verified] sending session then we could check if we have trust for that session for rendering

Ah okay, fair enough then. Let's summon @poljar to estimate the complexity of doing that.

@poljar
Copy link
Contributor

poljar commented May 21, 2020

Seshat allows you to store arbitrary json, it only cares that the fields required to index an event (event id, sender, content...) are there. Any additional json fields are left as is, or if you will, json comes in, json comes out.

Not sure how to add that info without re-crawling a room though.

@jryans
Copy link
Collaborator

jryans commented May 21, 2020

Okay, sounds like we would like to additionally persist some bits of the encrypted event as well then, as in at least the device ID, and then somehow trigger all rooms to be re-indexed (as otherwise we'd only have that for some messages).

I'll remove bite-sized as the scope is larger than I guessed, but it would be good to work on this as part cross-signing release feedback over the next few weeks if possible.

@t3chguy
Copy link
Member

t3chguy commented May 22, 2020

Note, we'd have to check that the signature is consistent with the device id the payload claims to be sent by and then we can just persist the now "verified" deviceId for checking later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants