search

element-hq / element-meta

Shared/meta documentation and project artefacts for Element clients
66 stars 11 forks source link

Expected UTDs: Hide UTDs for messages sent while you were not in the room ("Pre-join UTDs") #2317

Closed BillCarsonFr closed 3 weeks ago

BillCarsonFr commented 4 months ago

Problem

Sometimes, a client will receive messages that were sent before they were a member of the room. This can happen for a number of reasons:

In this situation, sometimes the receiving client will be unable to decrypt (UTD) the message because it does not have the room key needed (because no-one sent it when they send the message, because they didn't know this user needed it).

We want to make it clear why this UTD happened, because in many circumstances it is expected, and not an error.

(Note: this is different from UTDs where someone was a member, but the message was not encrypted for this device. That is tracked under https://github.com/element-hq/element-meta/issues/2313.)

Solution

To be able to identify this type of message and inform the user appropriately, we need to know whether the user was a member of the room when the message event was created.

The server is well-placed to know this because it has access to the DAG, whereas the client does not.

So we propose to write and implement MSC4115, allowing the server to annotate events with information about their membership of the room at the time the event was created.

Work to do

Spec:

Synapse:

matrix-rust-sdk:

matrix-analytics-events

Element X Android:

Element X iOS:

Element Web/matrix-js-sdk:

(We expect to implement this a little later in Web.)

Notes

Further work

This is a partial solution: there is still a race on the sender side. That has solutions discussed at https://github.com/element-hq/element-meta/issues/2268.

We might also want to consider preventing setting history_visibility: shared on encrypted rooms, which would stop the server sending us the events in the first place.

Previous attempts to hide these UTDs

Element Web attempted to hide these UTDs in the past: https://github.com/matrix-org/matrix-react-sdk/pull/3881 but various problems with this approach have been identified e.g. https://github.com/element-hq/element-web/issues/22671 .

Sharing room history, MSC3061

MSC3061 Share room keys for past messages allows providing keys to people who are allowed to decrypt messages, even if they were not a member when they were created.

If/when this is implemented, it will allow decrypting this type of UTD later. But:

richvdh commented 4 months ago

Expected UTDs: Hide pre-invite expected UTDs when room allows unless MSC3061 is supported and you have been invited (not join by yourself)?

I'm really confused by this title. Is this specific to pre-invite UTDs, or does it also cover UTDs sent between the invite and the join? "when room allows" --- when room allows what?

BillCarsonFr commented 4 months ago

I'm really confused by this title. Is this specific to pre-invite UTDs, or does it also cover UTDs sent between the invite and the join? "when room allows" --- when room allows what?

It's related to the room history visibility. If you are invited in an invite only room you will not get any historical message anyhow. So it is if the room is e2e with history_visibility set to shared or world readable there will be things to hide. I am not sure if the sdk encrypts for invited users or not for these settings, if it does then we need to hide pre-invite. Thus if there is an UTD between in invite and join it should not be hidden.

There are edge cases like if it's a 3pid invite, but maybe in this case we should warn the sender that there is no point sending a message in that case.

richvdh commented 4 months ago

If you are invited in an invite only room

what do you mean by an invite only room?

I am not sure if the sdk encrypts for invited users or not for these settings, if it does then we need to hide pre-invite.

matrix-js-sdk has this logic:

    public async getEncryptionTargetMembers(): Promise<RoomMember[]> {
        await this.loadMembersIfNeeded();
        let members = this.getMembersWithMembership("join");
        if (this.shouldEncryptForInvitedMembers()) {
            members = members.concat(this.getMembersWithMembership("invite"));
        }
        return members;
    }

    public shouldEncryptForInvitedMembers(): boolean {
        const ev = this.currentState.getStateEvents(EventType.RoomHistoryVisibility, "");
        return ev?.getContent()?.history_visibility !== "joined";
    }
BillCarsonFr commented 4 months ago

If you are invited in an invite only room

what do you mean by an invite only room?

I am not sure if the sdk encrypts for invited users or not for these settings, if it does then we need to hide pre-invite.

matrix-js-sdk has this logic:

    public async getEncryptionTargetMembers(): Promise<RoomMember[]> {
        await this.loadMembersIfNeeded();
        let members = this.getMembersWithMembership("join");
        if (this.shouldEncryptForInvitedMembers()) {
            members = members.concat(this.getMembersWithMembership("invite"));
        }
        return members;
    }

    public shouldEncryptForInvitedMembers(): boolean {
        const ev = this.currentState.getStateEvents(EventType.RoomHistoryVisibility, "");
        return ev?.getContent()?.history_visibility !== "joined";
    }

Interesting on android there is also a global setting enableEncryptionForInvitedMembers that is true by default. So it's encrypting to invited if global setting is true and room history visibility is not joined.

IOS is like web

richvdh commented 3 months ago

Proposed solution:

This is a partial solution: there is still a race on the sender side. That has solutions discussed at https://github.com/element-hq/element-meta/issues/2268.

We might also want to consider preventing setting history_visibility: shared on encrypted rooms, which would stop the server sending us the events in the first place.

andybalaam commented 2 months ago

Old description, for the record:

There was already a tentative to fix that problem https://github.com/matrix-org/matrix-react-sdk/pull/3881 But it has bugs https://github.com/element-hq/element-web/issues/22671

Rooms have different history_visibility settings. It is possible to create an encrypted room, that allows a user to access messages sent before he was part of that room. Given that the user was not in the room when the historical messages were sent, the key to decrypt them has not been distributed to him. This will create expected UTDs.

image

For other history_visibility settings like join/invite the encryption layer will adapt to share the keys correctly, and the server would not serve events that couldn't be decrypted.

The only way this user could access this history, is that a previous member of the room shares the keys, as per MSC3061 Share room keys for past messages.

For MSC3061 to work properly, all devices in the room should support MSC3061 (otherwise the key generated by the devices won't be marked as sharable), the current device should support it (otherwise it will reject the forwarded keys as they were not requested), and the user should be invited by another user that supports MSC3061. Notice that the user has to be invited first, if he join the room by himself (space restricted room) it won't work.

In any cases, we know for sure that:

### Tasks
- [ ] If a client doesn't support MSC3061, the room history can be hidden (showing a tile in the timeline that there is history but you can't access it)
- [ ] If a client supports MSC3061, and the user joined by itself, the room history can be hidden

Some technical points

There is no reliable way to know exactly if an event was sent before the user as joined the room.

see https://github.com/matrix-org/matrix-spec-proposals/blob/rav/proposal/membership_on_events/proposals/4115-membership-on-events.md

So a first approach based on hiding events just based on the invite event position in the timeline might lead to some errors.

richvdh commented 3 weeks ago

I think this should be done, as of the next releases of Synapse and all the clients.