Skip to content

Commit

Permalink
fix: Prefer to encrypt if E2eeEnabled even if peers have EncryptPrefe…
Browse files Browse the repository at this point in the history
…rence::NoPreference

First of all, chatmail servers normally forbid to send unencrypted mail, so if we know the peer's
key, we should encrypt to it. Chatmail setups have `E2eeEnabled=1` by default and this isn't
possible to change in UIs, so this change fixes the chatmail case. Additionally, for chatmail, if a
peer has `EncryptPreference::Reset`, let's handle it as `EncryptPreference::NoPreference` for the
reason above. Still, if `E2eeEnabled` is 0 for a chatmail setup somehow, e.g. the user set it via
environment, let's assume that the user knows what they do and ignore `IsChatmail` flag.

NB:
- If we don't know the peer's key, we should try to send an unencrypted message as before for a
  chatmail setup.
- This change doesn't remove the "majority rule", but now the majority with
  `EncryptPreference::NoPreference` can't disable encryption if the local preference is `Mutual`. To
  disable encryption, some peer should have a missing peerstate or, for the non-chatmail case, the
  majority should have `EncryptPreference::Reset`.
  • Loading branch information
iequidoo committed Jan 4, 2025
1 parent f31e86d commit 2bce446
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 27 deletions.
12 changes: 8 additions & 4 deletions python/tests/test_1_online.py
Original file line number Diff line number Diff line change
Expand Up @@ -1253,7 +1253,10 @@ def test_no_old_msg_is_fresh(acfactory, lp):

def test_prefer_encrypt(acfactory, lp):
"""Test quorum rule for encryption preference in 1:1 and group chat."""
ac1, ac2, ac3 = acfactory.get_online_accounts(3)
ac1 = acfactory.new_online_configuring_account(fix_is_chatmail=True)
ac2 = acfactory.new_online_configuring_account(fix_is_chatmail=True)
ac3 = acfactory.new_online_configuring_account(fix_is_chatmail=True)
acfactory.bring_accounts_online()
ac1.set_config("e2ee_enabled", "0")
ac2.set_config("e2ee_enabled", "1")
ac3.set_config("e2ee_enabled", "0")
Expand All @@ -1276,7 +1279,8 @@ def test_prefer_encrypt(acfactory, lp):
lp.sec("ac2: sending message to ac1")
chat2 = ac2.create_chat(ac1)
msg2 = chat2.send_text("message2")
assert not msg2.is_encrypted()
# Own preference is `Mutual` and we have the peer's key.
assert msg2.is_encrypted()
ac1._evtracker.wait_next_incoming_message()

lp.sec("ac1: sending message to group chat with ac2 and ac3")
Expand All @@ -1292,8 +1296,8 @@ def test_prefer_encrypt(acfactory, lp):
ac3.set_config("e2ee_enabled", "1")
chat3 = ac3.create_chat(ac1)
msg4 = chat3.send_text("message4")
# ac1 still does not prefer encryption
assert not msg4.is_encrypted()
# Own preference is `Mutual` and we have the peer's key.
assert msg4.is_encrypted()
ac1._evtracker.wait_next_incoming_message()

lp.sec("ac1: sending another message to group chat with ac2 and ac3")
Expand Down
128 changes: 106 additions & 22 deletions src/e2ee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,17 @@ impl EncryptHelper {

/// Determines if we can and should encrypt.
///
/// For encryption to be enabled, `e2ee_guaranteed` should be true, or strictly more than a half
/// of peerstates should prefer encryption. Own preference is counted equally to peer
/// preferences, even if message copy is not sent to self.
///
/// `e2ee_guaranteed` should be set to true for replies to encrypted messages (as required by
/// Autocrypt Level 1, version 1.1) and for messages sent in protected groups.
///
/// Returns an error if `e2ee_guaranteed` is true, but one or more keys are missing.
pub fn should_encrypt(
pub(crate) async fn should_encrypt(
&self,
context: &Context,
e2ee_guaranteed: bool,
peerstates: &[(Option<Peerstate>, String)],
) -> Result<bool> {
let is_chatmail = context.is_chatmail().await?;
let mut prefer_encrypt_count = if self.prefer_encrypt == EncryptPreference::Mutual {
1
} else {
Expand All @@ -64,10 +61,15 @@ impl EncryptHelper {
Some(peerstate) => {
let prefer_encrypt = peerstate.prefer_encrypt;
info!(context, "Peerstate for {addr:?} is {prefer_encrypt}.");
match peerstate.prefer_encrypt {
EncryptPreference::NoPreference | EncryptPreference::Reset => {}
EncryptPreference::Mutual => prefer_encrypt_count += 1,
};
if match peerstate.prefer_encrypt {
EncryptPreference::NoPreference | EncryptPreference::Reset => {
(peerstate.prefer_encrypt != EncryptPreference::Reset || is_chatmail)
&& self.prefer_encrypt == EncryptPreference::Mutual
}
EncryptPreference::Mutual => true,
} {
prefer_encrypt_count += 1;
}
}
None => {
let msg = format!("Peerstate for {addr:?} missing, cannot encrypt");
Expand Down Expand Up @@ -170,9 +172,11 @@ pub async fn ensure_secret_key_exists(context: &Context) -> Result<()> {
#[cfg(test)]
mod tests {
use super::*;
use crate::chat::send_text_msg;
use crate::key::DcKey;
use crate::message::{Message, Viewtype};
use crate::param::Param;
use crate::receive_imf::receive_imf;
use crate::test_utils::{bob_keypair, TestContext, TestContextManager};

mod ensure_secret_key_exists {
Expand Down Expand Up @@ -320,29 +324,109 @@ Sent with my Delta Chat Messenger: https://delta.chat";
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_should_encrypt() {
async fn test_should_encrypt() -> Result<()> {
let t = TestContext::new_alice().await;
assert!(t.get_config_bool(Config::E2eeEnabled).await?);
let encrypt_helper = EncryptHelper::new(&t).await.unwrap();

// test with EncryptPreference::NoPreference:
// if e2ee_eguaranteed is unset, there is no encryption as not more than half of peers want encryption
let ps = new_peerstates(EncryptPreference::NoPreference);
assert!(encrypt_helper.should_encrypt(&t, true, &ps).unwrap());
assert!(!encrypt_helper.should_encrypt(&t, false, &ps).unwrap());
assert!(encrypt_helper.should_encrypt(&t, true, &ps).await?);
// Own preference is `Mutual` and we have the peer's key.
assert!(encrypt_helper.should_encrypt(&t, false, &ps).await?);

// test with EncryptPreference::Reset
let ps = new_peerstates(EncryptPreference::Reset);
assert!(encrypt_helper.should_encrypt(&t, true, &ps).unwrap());
assert!(!encrypt_helper.should_encrypt(&t, false, &ps).unwrap());
assert!(encrypt_helper.should_encrypt(&t, true, &ps).await?);
assert!(!encrypt_helper.should_encrypt(&t, false, &ps).await?);

// test with EncryptPreference::Mutual (self is also Mutual)
let ps = new_peerstates(EncryptPreference::Mutual);
assert!(encrypt_helper.should_encrypt(&t, true, &ps).unwrap());
assert!(encrypt_helper.should_encrypt(&t, false, &ps).unwrap());
assert!(encrypt_helper.should_encrypt(&t, true, &ps).await?);
assert!(encrypt_helper.should_encrypt(&t, false, &ps).await?);

// test with missing peerstate
let ps = vec![(None, "[email protected]".to_string())];
assert!(encrypt_helper.should_encrypt(&t, true, &ps).is_err());
assert!(!encrypt_helper.should_encrypt(&t, false, &ps).unwrap());
assert!(encrypt_helper.should_encrypt(&t, true, &ps).await.is_err());
assert!(!encrypt_helper.should_encrypt(&t, false, &ps).await?);
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_should_encrypt_e2ee_disabled() -> Result<()> {
let t = &TestContext::new_alice().await;
t.set_config_bool(Config::E2eeEnabled, false).await?;
let encrypt_helper = EncryptHelper::new(t).await.unwrap();

let ps = new_peerstates(EncryptPreference::NoPreference);
assert!(!encrypt_helper.should_encrypt(t, false, &ps).await?);

let ps = new_peerstates(EncryptPreference::Reset);
assert!(encrypt_helper.should_encrypt(t, true, &ps).await?);

let mut ps = new_peerstates(EncryptPreference::Mutual);
// Own preference is `NoPreference` and there's no majority with `Mutual`.
assert!(!encrypt_helper.should_encrypt(t, false, &ps).await?);
// Now the majority wants to encrypt. Let's encrypt, anyway there are other cases when we
// can't send unencrypted, e.g. protected groups.
ps.push(ps[0].clone());
assert!(encrypt_helper.should_encrypt(t, false, &ps).await?);

// Test with missing peerstate.
let ps = vec![(None, "[email protected]".to_string())];
assert!(encrypt_helper.should_encrypt(t, true, &ps).await.is_err());
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_chatmail_prefers_to_encrypt() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;
let bob = &tcm.bob().await;
bob.set_config_bool(Config::IsChatmail, true).await?;

let bob_chat_id = tcm
.send_recv_accept(alice, bob, "Hello from DC")
.await
.chat_id;
receive_imf(
bob,
b"From: [email protected]\n\
To: [email protected]\n\
Message-ID: <[email protected]>\n\
Date: Sun, 22 Mar 3000 22:37:58 +0000\n\
\n\
Hello from another MUA\n",
false,
)
.await?;
send_text_msg(bob, bob_chat_id, "hi".to_string()).await?;
let sent_msg = bob.pop_sent_msg().await;
let msg = Message::load_from_db(bob, sent_msg.sender_msg_id).await?;
assert!(msg.get_showpadlock());
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_chatmail_can_send_unencrypted() -> Result<()> {
let mut tcm = TestContextManager::new();
let bob = &tcm.bob().await;
bob.set_config_bool(Config::IsChatmail, true).await?;
let bob_chat_id = receive_imf(
bob,
b"From: [email protected]\n\
To: [email protected]\n\
Message-ID: <[email protected]>\n\
Date: Sun, 22 Mar 3000 22:37:58 +0000\n\
\n\
Hello\n",
false,
)
.await?
.unwrap()
.chat_id;
bob_chat_id.accept(bob).await?;
send_text_msg(bob, bob_chat_id, "hi".to_string()).await?;
let sent_msg = bob.pop_sent_msg().await;
let msg = Message::load_from_db(bob, sent_msg.sender_msg_id).await?;
assert!(!msg.get_showpadlock());
Ok(())
}
}
4 changes: 3 additions & 1 deletion src/mimefactory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,9 @@ impl MimeFactory {

let peerstates = self.peerstates_for_recipients(context).await?;
let is_encrypted = !self.should_force_plaintext()
&& encrypt_helper.should_encrypt(context, e2ee_guaranteed, &peerstates)?;
&& encrypt_helper
.should_encrypt(context, e2ee_guaranteed, &peerstates)
.await?;
let is_securejoin_message = if let Loaded::Message { msg, .. } = &self.loaded {
msg.param.get_cmd() == SystemMessage::SecurejoinMessage
} else {
Expand Down

0 comments on commit 2bce446

Please sign in to comment.