-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add option to enable encryption by default for new rooms #7639
Conversation
Is it possible to add a second switch to have also an option to enable encryption by default for public rooms? |
Good call - I could see a homeserver wanting all rooms encrypted. |
@anoadragon453 I think this could use a rebase now that the other PR has been merged? |
84beb13
to
ee77f2c
Compare
ee77f2c
to
5b6471d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got some other bikesheds to paint here, but before we get there, I think there is a problem:
as noted in https://github.com/matrix-org/matrix-doc/issues/2612#issuecomment-639577866, by basing this on the presets, we're not applying it to what other people seem to mean by "public room" :/.
@richvdh Fun, but thanks for noting down the current situation, this PR could hopefully be a step in the right direction then rather than one in the opposite. The reason this one initially went for presets was due to it being based off the dinsic PR which did the same thing. So I suppose we want to follow Riot's lead and base things on the join rules? That doesn't sound too bad in theory :) |
After discussing with @richvdh and @ara4n on matrix, we plan to move forward with using presets to determine what defines a "public" room vs. a "private" one, which is what this PR currently does. More context in https://github.com/matrix-org/matrix-doc/issues/2612 |
90fb7f2
to
1468df2
Compare
1468df2
to
d95174f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. couple of additional thoughts
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
23de5dc
to
04c3455
Compare
Biggest requested option since Riot turned on forced e2ee is the ability to disable e2ee in Synapse. Why wasn't that considered in this change? This PR is much newer than all the others requesting the ability to fully disable e2ee. |
Fixes #7821, introduced in #7639 Turns out PyYAML translates `off` into a `False` boolean if it's unquoted (see https://stackoverflow.com/questions/36463531/pyyaml-automatically-converting-certain-keys-to-boolean-values), which seems to be a liberal interpretation of this bit of the YAML spec: https://yaml.org/spec/1.1/current.html#id864510 An alternative fix would be to implement the solution mentioned in the SO post linked above, but I'm aware it might break existing setups (which might use these values in the configuration file) so it's probably better just to add an extra check for this one. We should be aware that this is a thing for the next times we do that though. I didn't find any other occurrence of this bug elsewhere in the codebase.
…7822) Fixes #7821, introduced in #7639 Turns out PyYAML translates `off` into a `False` boolean if it's unquoted (see https://stackoverflow.com/questions/36463531/pyyaml-automatically-converting-certain-keys-to-boolean-values), which seems to be a liberal interpretation of this bit of the YAML spec: https://yaml.org/spec/1.1/current.html#id864510 An alternative fix would be to implement the solution mentioned in the SO post linked above, but I'm aware it might break existing setups (which might use these values in the configuration file) so it's probably better just to add an extra check for this one. We should be aware that this is a thing for the next times we do that though. I didn't find any other occurrence of this bug elsewhere in the codebase.
* commit '03619324f': Create a ListenerConfig object (#7681) Fix changelog wording 1.15.1 Wrap register_device coroutine in an ensureDeferred (#7684) Ensure the body is a string before comparing push rules. (#7701) Ensure etag is a string for GET room_keys/version response (#7691) Update m.id.phone to use 'phone' instead of 'number' (#7687) Fix "There was no active span when trying to log." error (#7698) Enable 3PID add/bind/unbind endpoints on r0 routes Discard RDATA from already seen positions. (#7648) Replace iteritems/itervalues/iterkeys with native versions. (#7692) Fix warnings about losing log context during UI auth. (#7688) Fix a typo when comparing the URI & method during UI Auth. (#7689) Remove "user_id" from GET /presence. (#7606) Increase the default SAML session expirary time to 15 minutes. (#7664) fix typo in sample_config.yaml (#7652) Take out a lock before modifying _CACHES (#7663) Add option to enable encryption by default for new rooms (#7639) Clean-up the fallback login code. (#7657)
Fixes #2431 (
however, I'm not sure from the issue whether it was desired to have this option affect public rooms)Adds config option
encryption_enabled_by_default_for_room_type
, which determines whether encryption should be enabled with the default encryption algorithm in private or public rooms upon creation. Whether the room is private or public is decided based upon the room creation preset that is used.Part of this PR is also pulling out all of the individual instances of
m.megolm.v1.aes-sha2
into a constant variable to eliminate typos ala #7637Based on #7637