matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.79k stars 2.13k forks source link

Upgrading a public room erroneously makes it E2EE when E2EE is required for private rooms #9246

Open joepie91 opened 3 years ago

joepie91 commented 3 years ago

Description

(Note: I'm filing this issue on behalf of someone else, who can't currently file issues, so I don't have direct access to the homeserver in question.)

When Synapse is configured to require E2EE for private rooms, then when a public room is upgraded through that homeserver, it is erroneously marked as end-to-end encrypted. According to TravisR:

looks like a synapse bug. encryption_enabled_by_default_for_room_type doesn't seem to work with room upgrades (/upgrade creates a room without a preset, which is by default a private room until it later updates the state)

Automatically enabling E2EE for the upgraded room should only occur for private rooms, instead.

Steps to reproduce

Version information

If not matrix.org:

jonaharagon commented 3 years ago

Run homeserver that requires E2EE for private rooms

Not required per se, but encryption_enabled_by_default_for_room_type: invite (as opposed to off).

clokep commented 3 years ago

Run homeserver that requires E2EE for private rooms

Not required per se, but encryption_enabled_by_default_for_room_type: invite (as opposed to off).

This setting isn't actually used by Synapse, it just gets passed through to the clients in the /versions response. If this is the setting that causes an issue, it sounds like a client bug to me.

anoadragon453 commented 3 years ago

Actually, we do use those config options when generating a room, so the ball may in fact be in Synapse's court:

https://github.com/matrix-org/synapse/blob/9dde9c9f01ff8ed4c60314f10d97261739ea0547/synapse/handlers/room.py#L106-L112

The default value is off, but when set to invite will indeed end up generating an encrypted room due to explicitly using PRIVATE_CHAT as a preset: https://github.com/matrix-org/synapse/blob/9dde9c9f01ff8ed4c60314f10d97261739ea0547/synapse/handlers/room.py#L435-L445

We note that the preset setting is quite arbitrary, as we'll overlay the state from the previous room. We could solve this specific case by switching the preset from PRIVATE to PUBLIC, though I think a better solution would just be to create a room without any state other than the bare minimum - then overlay the old room's relevant state on top.

jonaharagon commented 3 years ago

That would be a better solution for upgrades. However having looked at this a bit more since posting my first reply, this is clearly a bug that affects more than just room upgrades. Creating any sort of private room in Element with this setting set to to invite creates a room with E2EE enabled, even if Encryption is specifically switched off in the room creation dialog in Element. This is not the behavior I'd expect from a setting that just sets a "default", at least. Is that a separate issue?

anoadragon453 commented 3 years ago

@jonaharagon Hrm, you're right. Clients can only indicate that they want encryption by sending m.room.encryption as part of the initial state during /createRoom. They can't currently explicitly indicate that they don't want encryption.

Turning this option on will add a m.room.encryption state event to the room if one is not already present. That is effectively forcing it on, and you're right in that the option's name and description don't currently reflect that. It sounds like the branding of it needs to be changed a bit, and that would be a separate issue yes.

jahway603 commented 1 year ago

Since Issue https://github.com/matrix-org/synapse/issues/14719 was determined to be a duplicate of this issue, I am proposing that the /upgraderoom command have a flag to keep a room in its current state of un-Encrypted after it is upgraded when this is the desired result of upgrading an un-Encrypted room.

Also this issue might need a title revision as public & private rooms can BOTH be E2EE.