-
Notifications
You must be signed in to change notification settings - Fork 96
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
feat: ignore encryption preferences #6612
base: main
Are you sure you want to change the base?
Conversation
7ad7fcc
to
c4d8826
Compare
c8b1f04
to
f2e3f44
Compare
f2e3f44
to
f92606b
Compare
1e31f3e
to
aba9544
Compare
723e691
to
40e477e
Compare
aba9544
to
491d6ab
Compare
91d5151
to
69c2277
Compare
Encryption preference is sent in Autocrypt header, but otherwise ignored. Delta Chat always prefers encryption if it is available.
69c2277
to
859b5b8
Compare
We need to think about hermes.radio usecase, it sets the config but the config is not going to have any effect. hermes.radio currently relies on stripping Autocrypt to downgrade everyone to unencrypted. Maybe they don't want to have encryption key at all, or maybe could be fine with encryption if we build high compression mode into the core so the server does not need to recompress. I am not removing the setting completely because we have it set in provider database for hermes.radio, but we are definitely breaking this usecase so need to discuss it separately. |
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.
Technically this is "ignore own encryption preference", peer's encryption preference is already ignored currently, so maybe clarify this in the comment message?
@@ -442,7 +442,6 @@ async fn configure(ctx: &Context, param: &EnteredLoginParam) -> Result<Configure | |||
ctx.set_config(Config::MvboxMove, Some("0")).await?; | |||
ctx.set_config(Config::OnlyFetchMvbox, None).await?; | |||
ctx.set_config(Config::ShowEmails, None).await?; | |||
ctx.set_config(Config::E2eeEnabled, Some("1")).await?; |
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.
So if encryption preference is still sent in the Autocrypt header, can't this lead to other Autocrypt-capable MUA or old Delta Chat sending messages unencrypted?
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 assume that it's enabled anyway, so, no need to enable it again?
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.
Why does this break the hermes.radio usecase? If they strip the Autocrypt headers, then DC can't encrypt, so it won't.
What might break their usecase is the new QR codes that will be encrypted with a symmetric secret and don't leak Autocrypt header, but we can think of this when it's time (I mean, for now they are only a rough idea, nothing concrete).
If this makes problems for them, we can always hardcode non-encryption (e.g. add if self.get_config(Config::E2eeEnabled).await? { return Ok(false); }
to the beginning of should_encrypt()
).
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.
FTR, the important semantic change is in this file here, everything else is just tests
Always prefer encryption if it is available.
This is in preparation for PGP-contacts as we want to clearly separate encrypted and unencrypted chats instead of having them switch between encrypted and unencrypted based on majority vote.