psi-im / plugins

Officially supported Psi plugins
56 stars 24 forks source link

allow omemo to be disabled/enabled when not available #106

Closed nullobsi closed 2 years ago

nullobsi commented 2 years ago

Fix #54 Fix #57 Fix psi-im/psi#630

Allow users to disable OMEMO even if it is not available to solve issues involving OMEMO forcing encryption

Neustradamus commented 2 years ago

@nullobsi: Thanks, I can not wait to test it! :)

I am not sure if it solves 1:1 problem too.

cc: @Ri0n, @Vitozz, @tehnick, @stigger.

Ri0n commented 2 years ago

I could be wrong but this may result in sending an unencrypted message when only encrypted is desired. Please check.

nullobsi commented 2 years ago

i think you're right; it will send an unencrypted message with OMEMO is not available even if the option "Always Enabled is on" at least that's what I think. I don't really have a way of testing that, but I will look into that later

nullobsi commented 2 years ago

@Ri0n , could you please elaborate on when only encrypted would be desired? It seems that if OMEMO is not available for a MUC, the OMEMO button would be disabled which indicates to the user that OMEMO does not work. Are you talking about the option to always enable OMEMO?

nullobsi commented 2 years ago

hm, now that i think of it, it would make more sense to allow OMEMO to be toggled on or off, even if it is not "available" for the MUC.

I think this would also fix the issue of -> https://github.com/psi-im/psi/issues/630 and #57 if that was the case, since you could simply disable it; that would also keep the security, if it doesn't work with encryption enabled, it won't send it at all

Ri0n commented 2 years ago

I had just a quick look. the availability is computed from participants. But what if there is no participants in the room except self user... If this disables the encryption it may become a problem. Assuming we have a room where we want only encrypted messages then the first participant can't send anything until more participants are joined (like leaving a message in MAM for later read by other participants).

it won't send it at all

I think a MUC message should be sent in any case. But it should never behave some unexpected way. Like sending unencrypted message when encryption is enabled. Theoretically the client can warn like nobody will be able to read the message but send it anyway. The warning could be considered as a noisy garbage though. If no sessions were built since there are no trusted keys with any of the MUC's participant, it doesn't new-joiners doesn't have trusted keys too or we can't start trusting.

Ri0n commented 2 years ago

but I'm not a plugin author and hardly understand omemo. So maybe I'm wrong. We need @stigger here.

nullobsi commented 2 years ago

Okay, I changed the PR to instead allow OMEMO to be enabled/disabled when no sessions can be built/it is not available. This keeps the current behavior where unencrypted messages are not sent until the user explicitly disabled encryption

stigger commented 2 years ago

@Ri0n Your assessment is correct

Neustradamus commented 2 years ago

Currently, there is not an option to enable or disable OMEMO.

Example, we are in a MUC Room, two known people, it is possible to enable or disable OMEMO RED icon. We can disable, send a message, enable again send a message, no problem all works.

The problem arrives when one (on two in this example) leaves the mucroom, the OMEMO icon is now GREY: OMEMO encryption is not available for this group It is impossible to disable again because it is in GREY. When we try to send a new message, we can see (without the sent message): *** [OMEMO] Unable to build any sessions, the message was not sent in MUC Room.

The contact comeback, the OMEMO red icon is here, not enabled, but when we send a new message, it is sent and it is encrypted:

[XX:XX:55] ***  OMEMO encryption is enabled
[XX:XX:55] <Me> test

The contact replies to this message, OMEMO red icon is here, not enabled, it is sent:

[XX:XX:30] ***  Encryption is disabled
[XX:XX:30] <Contact> return test

New message, no change to reply:

[XX:XX:37] ***  OMEMO encryption is enabled
[XX:XX:37] <Me> return test reply

The contact leaves again, try to send a message (without the sent message): [XX:XX:48] *** [OMEMO] Unable to build any sessions, the message was not sent

Neustradamus commented 2 years ago

@nullobsi: Thanks for your PR!

@milada-horakova, @Asquator: I know that you have the same bug than me "*** [OMEMO] Unable to build any sessions, the message was not sent", what do you think?

cc: @ritzmann, @ulfi, @rapgro, @kssytsrk.