diff --git a/benches/decrypting.rs b/benches/decrypting.rs index 5d233cde5a..b998754a5d 100644 --- a/benches/decrypting.rs +++ b/benches/decrypting.rs @@ -8,43 +8,47 @@ //! cargo bench --bench decrypting --features="internals" //! ``` //! -//! or, if you want to only run e.g. the 'Decrypt a symmetrically encrypted message' benchmark: +//! or, if you want to only run e.g. the 'Decrypt and parse a symmetrically encrypted message' benchmark: //! //! ```text -//! cargo bench --bench decrypting --features="internals" -- 'Decrypt a symmetrically encrypted message' +//! cargo bench --bench decrypting --features="internals" -- 'Decrypt and parse a symmetrically encrypted message' //! ``` //! -//! You can also pass a substring. -//! So, you can run all 'Decrypt and parse' benchmarks with: +//! You can also pass a substring: //! //! ```text -//! cargo bench --bench decrypting --features="internals" -- 'Decrypt and parse' +//! cargo bench --bench decrypting --features="internals" -- 'symmetrically' //! ``` //! //! Symmetric decryption has to try out all known secrets, //! You can benchmark this by adapting the `NUM_SECRETS` variable. use std::hint::black_box; +use std::sync::LazyLock; use criterion::{Criterion, criterion_group, criterion_main}; use deltachat::internals_for_benches::create_broadcast_secret; -use deltachat::internals_for_benches::create_dummy_keypair; use deltachat::internals_for_benches::save_broadcast_secret; +use deltachat::securejoin::get_securejoin_qr; use deltachat::{ - Events, - chat::ChatId, - config::Config, - context::Context, - internals_for_benches::key_from_asc, - internals_for_benches::parse_and_get_text, - internals_for_benches::store_self_keypair, - pgp::{KeyPair, SeipdVersion, decrypt, pk_encrypt, symm_encrypt_message}, - stock_str::StockStrings, + Events, chat::ChatId, config::Config, context::Context, internals_for_benches::key_from_asc, + internals_for_benches::parse_and_get_text, internals_for_benches::store_self_keypair, + pgp::KeyPair, stock_str::StockStrings, }; -use rand::{Rng, rng}; use tempfile::tempdir; -const NUM_SECRETS: usize = 500; +static NUM_BROADCAST_SECRETS: LazyLock = LazyLock::new(|| { + std::env::var("NUM_BROADCAST_SECRETS") + .unwrap_or("500".to_string()) + .parse() + .unwrap() +}); +static NUM_AUTH_TOKENS: LazyLock = LazyLock::new(|| { + std::env::var("NUM_AUTH_TOKENS") + .unwrap_or("5000".to_string()) + .parse() + .unwrap() +}); async fn create_context() -> Context { let dir = tempdir().unwrap(); @@ -70,66 +74,6 @@ async fn create_context() -> Context { fn criterion_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("Decrypt"); - // =========================================================================================== - // Benchmarks for decryption only, without any other parsing - // =========================================================================================== - - group.sample_size(10); - - group.bench_function("Decrypt a symmetrically encrypted message", |b| { - let plain = generate_plaintext(); - let secrets = generate_secrets(); - let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { - let secret = secrets[NUM_SECRETS / 2].clone(); - symm_encrypt_message( - plain.clone(), - create_dummy_keypair("alice@example.org").unwrap().secret, - black_box(&secret), - true, - ) - .await - .unwrap() - }); - - b.iter(|| { - let mut msg = - decrypt(encrypted.clone().into_bytes(), &[], black_box(&secrets)).unwrap(); - let decrypted = msg.as_data_vec().unwrap(); - - assert_eq!(black_box(decrypted), plain); - }); - }); - - group.bench_function("Decrypt a public-key encrypted message", |b| { - let plain = generate_plaintext(); - let key_pair = create_dummy_keypair("alice@example.org").unwrap(); - let secrets = generate_secrets(); - let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { - pk_encrypt( - plain.clone(), - vec![black_box(key_pair.public.clone())], - key_pair.secret.clone(), - true, - true, - SeipdVersion::V2, - ) - .await - .unwrap() - }); - - b.iter(|| { - let mut msg = decrypt( - encrypted.clone().into_bytes(), - std::slice::from_ref(&key_pair.secret), - black_box(&secrets), - ) - .unwrap(); - let decrypted = msg.as_data_vec().unwrap(); - - assert_eq!(black_box(decrypted), plain); - }); - }); - // =========================================================================================== // Benchmarks for the whole parsing pipeline, incl. decryption (but excl. receive_imf()) // =========================================================================================== @@ -139,7 +83,7 @@ fn criterion_benchmark(c: &mut Criterion) { // "secret" is the shared secret that was used to encrypt text_symmetrically_encrypted.eml. // Put it into the middle of our secrets: - secrets[NUM_SECRETS / 2] = "secret".to_string(); + secrets[*NUM_BROADCAST_SECRETS / 2] = "secret".to_string(); let context = rt.block_on(async { let context = create_context().await; @@ -148,6 +92,10 @@ fn criterion_benchmark(c: &mut Criterion) { .await .unwrap(); } + for _i in 0..*NUM_AUTH_TOKENS { + get_securejoin_qr(&context, None).await.unwrap(); + } + println!("NUM_AUTH_TOKENS={}", *NUM_AUTH_TOKENS); context }); @@ -161,7 +109,7 @@ fn criterion_benchmark(c: &mut Criterion) { ) .await .unwrap(); - assert_eq!(text, "Symmetrically encrypted message"); + assert_eq!(black_box(text), "Symmetrically encrypted message"); } }); }); @@ -176,7 +124,7 @@ fn criterion_benchmark(c: &mut Criterion) { ) .await .unwrap(); - assert_eq!(text, "hi"); + assert_eq!(black_box(text), "hi"); } }); }); @@ -185,17 +133,12 @@ fn criterion_benchmark(c: &mut Criterion) { } fn generate_secrets() -> Vec { - let secrets: Vec = (0..NUM_SECRETS) + let secrets: Vec = (0..*NUM_BROADCAST_SECRETS) .map(|_| create_broadcast_secret()) .collect(); + println!("NUM_BROADCAST_SECRETS={}", *NUM_BROADCAST_SECRETS); secrets } -fn generate_plaintext() -> Vec { - let mut plain: Vec = vec![0; 500]; - rng().fill(&mut plain[..]); - plain -} - criterion_group!(benches, criterion_benchmark); criterion_main!(benches); diff --git a/deltachat-jsonrpc/src/api/types/qr.rs b/deltachat-jsonrpc/src/api/types/qr.rs index de05e79c6a..5b58a06d85 100644 --- a/deltachat-jsonrpc/src/api/types/qr.rs +++ b/deltachat-jsonrpc/src/api/types/qr.rs @@ -19,6 +19,8 @@ pub enum QrObject { invitenumber: String, /// Authentication code. authcode: String, + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask the user whether to join the group. AskVerifyGroup { @@ -34,6 +36,8 @@ pub enum QrObject { invitenumber: String, /// Authentication code. authcode: String, + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask the user whether to join the broadcast channel. AskJoinBroadcast { @@ -54,6 +58,8 @@ pub enum QrObject { invitenumber: String, /// Authentication code. authcode: String, + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Contact fingerprint is verified. /// @@ -229,6 +235,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); @@ -237,6 +244,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } } Qr::AskVerifyGroup { @@ -246,6 +254,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); @@ -256,6 +265,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } } Qr::AskJoinBroadcast { @@ -265,6 +275,7 @@ impl From for QrObject { fingerprint, authcode, invitenumber, + is_v3, } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); @@ -275,6 +286,7 @@ impl From for QrObject { fingerprint, authcode, invitenumber, + is_v3, } } Qr::FprOk { contact_id } => { diff --git a/src/aheader.rs b/src/aheader.rs index 65db5248d5..cd2d494df8 100644 --- a/src/aheader.rs +++ b/src/aheader.rs @@ -47,11 +47,11 @@ pub struct Aheader { pub public_key: SignedPublicKey, pub prefer_encrypt: EncryptPreference, - // Whether `_verified` attribute is present. - // - // `_verified` attribute is an extension to `Autocrypt-Gossip` - // header that is used to tell that the sender - // marked this key as verified. + /// Whether `_verified` attribute is present. + /// + /// `_verified` attribute is an extension to `Autocrypt-Gossip` + /// header that is used to tell that the sender + /// marked this key as verified. pub verified: bool, } diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index e5e5a05407..e3529369b6 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2731,27 +2731,24 @@ async fn test_broadcast_members_cant_see_each_other() -> Result<()> { join_securejoin(charlie, &qr).await.unwrap(); let request = charlie.pop_sent_msg().await; - assert_eq!(request.recipients, "alice@example.org charlie@example.net"); + assert_eq!(request.recipients, "alice@example.org"); alice.recv_msg_trash(&request).await; } - tcm.section("Alice sends auth-required"); + tcm.section("Alice sends vc-pubkey"); { - let auth_required = alice.pop_sent_msg().await; - assert_eq!( - auth_required.recipients, - "charlie@example.net alice@example.org" - ); - let parsed = charlie.parse_msg(&auth_required).await; - assert!(parsed.get_header(HeaderDef::AutocryptGossip).is_some()); - assert!(parsed.decoded_data_contains("charlie@example.net")); + let vc_pubkey = alice.pop_sent_msg().await; + assert_eq!(vc_pubkey.recipients, "charlie@example.net"); + let parsed = charlie.parse_msg(&vc_pubkey).await; + assert!(parsed.get_header(HeaderDef::AutocryptGossip).is_none()); + assert_eq!(parsed.decoded_data_contains("charlie@example.net"), false); assert_eq!(parsed.decoded_data_contains("bob@example.net"), false); - let parsed_by_bob = bob.parse_msg(&auth_required).await; + let parsed_by_bob = bob.parse_msg(&vc_pubkey).await; assert!(parsed_by_bob.decrypting_failed); - charlie.recv_msg_trash(&auth_required).await; + charlie.recv_msg_trash(&vc_pubkey).await; } tcm.section("Charlie sends request-with-auth"); @@ -2992,9 +2989,8 @@ async fn test_broadcast_recipients_sync1() -> Result<()> { alice1.recv_msg_trash(&request).await; alice2.recv_msg_trash(&request).await; - let auth_required = alice1.pop_sent_msg().await; - charlie.recv_msg_trash(&auth_required).await; - alice2.recv_msg_trash(&auth_required).await; + let vc_pubkey = alice1.pop_sent_msg().await; + charlie.recv_msg_trash(&vc_pubkey).await; let request_with_auth = charlie.pop_sent_msg().await; alice1.recv_msg_trash(&request_with_auth).await; @@ -3299,14 +3295,17 @@ async fn test_broadcast_joining_golden() -> Result<()> { .await; let alice_bob_contact = alice.add_or_lookup_contact_no_key(bob).await; - let private_chat = ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) - .await? - .unwrap(); // The 1:1 chat with Bob should not be visible to the user: - assert_eq!(private_chat.blocked, Blocked::Yes); + assert!( + ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) + .await? + .is_none() + ); + let private_chat_id = + ChatId::create_for_contact_with_blocked(alice, alice_bob_contact.id, Blocked::Not).await?; alice .golden_test_chat( - private_chat.id, + private_chat_id, "test_broadcast_joining_golden_private_chat", ) .await; @@ -3583,16 +3582,13 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { join_securejoin(bob0, &qr).await.unwrap(); let request = bob0.pop_sent_msg().await; - assert_eq!(request.recipients, "alice@example.org bob@example.net"); + assert_eq!(request.recipients, "alice@example.org"); alice.recv_msg_trash(&request).await; - let auth_required = alice.pop_sent_msg().await; - assert_eq!( - auth_required.recipients, - "bob@example.net alice@example.org" - ); + let vc_pubkey = alice.pop_sent_msg().await; + assert_eq!(vc_pubkey.recipients, "bob@example.net"); - bob0.recv_msg_trash(&auth_required).await; + bob0.recv_msg_trash(&vc_pubkey).await; let request_with_auth = bob0.pop_sent_msg().await; assert_eq!( request_with_auth.recipients, @@ -3608,7 +3604,7 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { assert_eq!(rcvd.param.get_cmd(), SystemMessage::MemberAddedToGroup); tcm.section("Bob's second device also receives these messages"); - bob1.recv_msg_trash(&auth_required).await; + bob1.recv_msg_trash(&vc_pubkey).await; bob1.recv_msg_trash(&request_with_auth).await; bob1.recv_msg(&member_added).await; diff --git a/src/decrypt.rs b/src/decrypt.rs index 4012a8f41e..c85a17e361 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -1,34 +1,28 @@ -//! End-to-end decryption support. +//! Helper functions for decryption. +//! The actual decryption is done in the [`crate::pgp`] module. use std::collections::HashSet; +use std::io::Cursor; +use ::pgp::composed::Message; use anyhow::Result; use mailparse::ParsedMail; -use crate::key::{Fingerprint, SignedPublicKey, SignedSecretKey}; +use crate::key::{Fingerprint, SignedPublicKey}; use crate::pgp; -/// Tries to decrypt a message, but only if it is structured as an Autocrypt message. -/// -/// If successful and the message was encrypted, -/// returns the decrypted and decompressed message. -pub fn try_decrypt<'a>( - mail: &'a ParsedMail<'a>, - private_keyring: &'a [SignedSecretKey], - shared_secrets: &[String], -) -> Result>> { +pub fn get_encrypted_pgp_message<'a>(mail: &'a ParsedMail<'a>) -> Result>> { let Some(encrypted_data_part) = get_encrypted_mime(mail) else { return Ok(None); }; - let data = encrypted_data_part.get_body_raw()?; - let msg = pgp::decrypt(data, private_keyring, shared_secrets)?; - + let cursor = Cursor::new(data); + let (msg, _headers) = Message::from_armor(cursor)?; Ok(Some(msg)) } /// Returns a reference to the encrypted payload of a message. -pub(crate) fn get_encrypted_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail<'b>> { +pub fn get_encrypted_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail<'b>> { get_autocrypt_mime(mail) .or_else(|| get_mixed_up_mime(mail)) .or_else(|| get_attachment_mime(mail)) diff --git a/src/e2ee.rs b/src/e2ee.rs index 5c3ce91dcb..e98785bd20 100644 --- a/src/e2ee.rs +++ b/src/e2ee.rs @@ -70,8 +70,13 @@ impl EncryptHelper { shared_secret: &str, mail_to_encrypt: MimePart<'static>, compress: bool, + sign: bool, ) -> Result { - let sign_key = load_self_secret_key(context).await?; + let sign_key = if sign { + Some(load_self_secret_key(context).await?) + } else { + None + }; let mut raw_message = Vec::new(); let cursor = Cursor::new(&mut raw_message); diff --git a/src/message.rs b/src/message.rs index 65c591361a..09cbc2461d 100644 --- a/src/message.rs +++ b/src/message.rs @@ -2064,6 +2064,22 @@ pub(crate) async fn set_msg_failed( Ok(()) } +/// Inserts a tombstone into `msgs` table +/// to prevent downloading the same message in the future. +/// +/// Returns tombstone database row ID. +pub(crate) async fn insert_tombstone(context: &Context, rfc724_mid: &str) -> Result { + let row_id = context + .sql + .insert( + "INSERT INTO msgs(rfc724_mid, chat_id) VALUES (?,?)", + (rfc724_mid, DC_CHAT_ID_TRASH), + ) + .await?; + let msg_id = MsgId::new(u32::try_from(row_id)?); + Ok(msg_id) +} + /// The number of messages assigned to unblocked chats pub async fn get_unblocked_msg_cnt(context: &Context) -> usize { match context diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 71fdf0d7df..931aaed95c 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -869,16 +869,6 @@ impl MimeFactory { "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), )); - } else if let Loaded::Message { msg, .. } = &self.loaded - && msg.param.get_cmd() == SystemMessage::SecurejoinMessage - { - let step = msg.param.get(Param::Arg).unwrap_or_default(); - if step != "vg-request" && step != "vc-request" { - headers.push(( - "Auto-Submitted", - mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), - )); - } } if let Loaded::Message { msg, chat } = &self.loaded @@ -947,6 +937,22 @@ impl MimeFactory { )); } + if self.pre_message_mode == PreMessageMode::Post { + headers.push(( + "Chat-Is-Post-Message", + mail_builder::headers::raw::Raw::new("1").into(), + )); + } else if let PreMessageMode::Pre { + post_msg_rfc724_mid, + } = &self.pre_message_mode + { + headers.push(( + "Chat-Post-Message-ID", + mail_builder::headers::message_id::MessageId::new(post_msg_rfc724_mid.clone()) + .into(), + )); + } + let is_encrypted = self.will_be_encrypted(); // Add ephemeral timer for non-MDN messages. @@ -993,189 +999,29 @@ impl MimeFactory { Loaded::Mdn { .. } => self.render_mdn()?, }; - // Split headers based on header confidentiality policy. - - // Headers that must go into IMF header section. - // - // These are standard headers such as Date, In-Reply-To, References, which cannot be placed - // anywhere else according to the standard. Placing headers here also allows them to be fetched - // individually over IMAP without downloading the message body. This is why Chat-Version is - // placed here. - let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // Headers that MUST NOT (only) go into IMF header section: - // - Large headers which may hit the header section size limit on the server, such as - // Chat-User-Avatar with a base64-encoded image inside. - // - Headers duplicated here that servers mess up with in the IMF header section, like - // Message-ID. - // - Nonstandard headers that should be DKIM-protected because e.g. OpenDKIM only signs - // known headers. - // - // The header should be hidden from MTA - // by moving it either into protected part - // in case of encrypted mails - // or unprotected MIME preamble in case of unencrypted mails. - let mut hidden_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // Opportunistically protected headers. - // - // These headers are placed into encrypted part *if* the message is encrypted. Place headers - // which are not needed before decryption (e.g. Chat-Group-Name) or are not interesting if the - // message cannot be decrypted (e.g. Chat-Disposition-Notification-To) here. - // - // If the message is not encrypted, these headers are placed into IMF header section, so make - // sure that the message will be encrypted if you place any sensitive information here. - let mut protected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // MIME header . - unprotected_headers.push(( - "MIME-Version", - mail_builder::headers::raw::Raw::new("1.0").into(), - )); - - if self.pre_message_mode == PreMessageMode::Post { - unprotected_headers.push(( - "Chat-Is-Post-Message", - mail_builder::headers::raw::Raw::new("1").into(), - )); - } else if let PreMessageMode::Pre { - post_msg_rfc724_mid, - } = &self.pre_message_mode - { - protected_headers.push(( - "Chat-Post-Message-ID", - mail_builder::headers::message_id::MessageId::new(post_msg_rfc724_mid.clone()) - .into(), - )); - } - - for header @ (original_header_name, _header_value) in &headers { - let header_name = original_header_name.to_lowercase(); - if header_name == "message-id" { - unprotected_headers.push(header.clone()); - hidden_headers.push(header.clone()); - } else if is_hidden(&header_name) { - hidden_headers.push(header.clone()); - } else if header_name == "from" { - // Unencrypted securejoin messages should _not_ include the display name: - if is_encrypted || !is_securejoin_message { - protected_headers.push(header.clone()); - } - - unprotected_headers.push(( - original_header_name, - Address::new_address(None::<&'static str>, self.from_addr.clone()).into(), - )); - } else if header_name == "to" { - protected_headers.push(header.clone()); - if is_encrypted { - unprotected_headers.push(("To", hidden_recipients().into())); - } else { - unprotected_headers.push(header.clone()); - } - } else if header_name == "chat-broadcast-secret" { - if is_encrypted { - protected_headers.push(header.clone()); - } else { - bail!("Message is unecrypted, cannot include broadcast secret"); - } - } else if is_encrypted && header_name == "date" { - protected_headers.push(header.clone()); - - // Randomized date goes to unprotected header. - // - // We cannot just send "Thu, 01 Jan 1970 00:00:00 +0000" - // or omit the header because GMX then fails with - // - // host mx00.emig.gmx.net[212.227.15.9] said: - // 554-Transaction failed - // 554-Reject due to policy restrictions. - // 554 For explanation visit https://postmaster.gmx.net/en/case?... - // (in reply to end of DATA command) - // - // and the explanation page says - // "The time information deviates too much from the actual time". - // - // We also limit the range to 6 days (518400 seconds) - // because with a larger range we got - // error "500 Date header far in the past/future" - // which apparently originates from Symantec Messaging Gateway - // and means the message has a Date that is more - // than 7 days in the past: - // - let timestamp_offset = rand::random_range(0..518400); - let protected_timestamp = self.timestamp.saturating_sub(timestamp_offset); - let unprotected_date = - chrono::DateTime::::from_timestamp(protected_timestamp, 0) - .unwrap() - .to_rfc2822(); - unprotected_headers.push(( - "Date", - mail_builder::headers::raw::Raw::new(unprotected_date).into(), - )); - } else if is_encrypted { - protected_headers.push(header.clone()); - - match header_name.as_str() { - "subject" => { - unprotected_headers.push(( - "Subject", - mail_builder::headers::raw::Raw::new("[...]").into(), - )); - } - "in-reply-to" - | "references" - | "auto-submitted" - | "chat-version" - | "autocrypt-setup-message" => { - unprotected_headers.push(header.clone()); - } - _ => { - // Other headers are removed from unprotected part. - } - } - } else { - // Copy the header to the protected headers - // in case of signed-only message. - // If the message is not signed, this value will not be used. - protected_headers.push(header.clone()); - unprotected_headers.push(header.clone()) - } - } + let HeadersByConfidentiality { + mut unprotected_headers, + hidden_headers, + protected_headers, + } = group_headers_by_confidentiality( + headers, + &self.from_addr, + self.timestamp, + is_encrypted, + is_securejoin_message, + ); let use_std_header_protection = context .get_config_bool(Config::StdHeaderProtectionComposing) .await?; let outer_message = if let Some(encryption_pubkeys) = self.encryption_pubkeys { - // Store protected headers in the inner message. - let message = protected_headers - .into_iter() - .fold(message, |message, (header, value)| { - message.header(header, value) - }); - - // Add hidden headers to encrypted payload. - let mut message: MimePart<'static> = hidden_headers - .into_iter() - .fold(message, |message, (header, value)| { - message.header(header, value) - }); - - if use_std_header_protection { - message = unprotected_headers - .iter() - // Structural headers shouldn't be added as "HP-Outer". They are defined in - // . - .filter(|(name, _)| { - !(name.eq_ignore_ascii_case("mime-version") - || name.eq_ignore_ascii_case("content-type") - || name.eq_ignore_ascii_case("content-transfer-encoding") - || name.eq_ignore_ascii_case("content-disposition")) - }) - .fold(message, |message, (name, value)| { - message.header(format!("HP-Outer: {name}"), value.clone()) - }); - } + let mut message = add_headers_to_encrypted_part( + message, + &unprotected_headers, + hidden_headers, + protected_headers, + use_std_header_protection, + ); // Add gossip headers in chats with multiple recipients let multiple_recipients = @@ -1266,21 +1112,6 @@ impl MimeFactory { } } - // Set the appropriate Content-Type for the inner message. - for (h, v) in &mut message.headers { - if h == "Content-Type" - && let mail_builder::headers::HeaderType::ContentType(ct) = v - { - let mut ct_new = ct.clone(); - ct_new = ct_new.attribute("protected-headers", "v1"); - if use_std_header_protection { - ct_new = ct_new.attribute("hp", "cipher"); - } - *ct = ct_new; - break; - } - } - // Disable compression for SecureJoin to ensure // there are no compression side channels // leaking information about the tokens. @@ -1328,8 +1159,9 @@ impl MimeFactory { } let encrypted = if let Some(shared_secret) = shared_secret { + let sign = true; encrypt_helper - .encrypt_symmetrically(context, &shared_secret, message, compress) + .encrypt_symmetrically(context, &shared_secret, message, compress, sign) .await? } else { // Asymmetric encryption @@ -1363,35 +1195,7 @@ impl MimeFactory { .await? }; - // XXX: additional newline is needed - // to pass filtermail at - // : - let encrypted = encrypted + "\n"; - - // Set the appropriate Content-Type for the outer message - MimePart::new( - "multipart/encrypted; protocol=\"application/pgp-encrypted\"", - vec![ - // Autocrypt part 1 - MimePart::new("application/pgp-encrypted", "Version: 1\r\n").header( - "Content-Description", - mail_builder::headers::raw::Raw::new("PGP/MIME version identification"), - ), - // Autocrypt part 2 - MimePart::new( - "application/octet-stream; name=\"encrypted.asc\"", - encrypted, - ) - .header( - "Content-Description", - mail_builder::headers::raw::Raw::new("OpenPGP encrypted message"), - ) - .header( - "Content-Disposition", - mail_builder::headers::raw::Raw::new("inline; filename=\"encrypted.asc\";"), - ), - ], - ) + wrap_encrypted_part(encrypted) } else if matches!(self.loaded, Loaded::Mdn { .. }) { // Never add outer multipart/mixed wrapper to MDN // as multipart/report Content-Type is used to recognize MDNs @@ -1458,22 +1262,12 @@ impl MimeFactory { } }; - // Store the unprotected headers on the outer message. - let outer_message = unprotected_headers - .into_iter() - .fold(outer_message, |message, (header, value)| { - message.header(header, value) - }); - let MimeFactory { last_added_location_id, .. } = self; - let mut buffer = Vec::new(); - let cursor = Cursor::new(&mut buffer); - outer_message.clone().write_part(cursor).ok(); - let message = String::from_utf8_lossy(&buffer).to_string(); + let message = render_outer_message(unprotected_headers, outer_message); Ok(RenderedEmail { message, @@ -2134,6 +1928,263 @@ impl MimeFactory { } } +/// Stores the unprotected headers on the outer message, and renders it. +fn render_outer_message( + unprotected_headers: Vec<(&'static str, HeaderType<'static>)>, + outer_message: MimePart<'static>, +) -> String { + let outer_message = unprotected_headers + .into_iter() + .fold(outer_message, |message, (header, value)| { + message.header(header, value) + }); + + let mut buffer = Vec::new(); + let cursor = Cursor::new(&mut buffer); + outer_message.clone().write_part(cursor).ok(); + String::from_utf8_lossy(&buffer).to_string() +} + +/// Takes the encrypted part, wraps it in a MimePart, +/// and sets the appropriate Content-Type for the outer message +fn wrap_encrypted_part(encrypted: String) -> MimePart<'static> { + // XXX: additional newline is needed + // to pass filtermail at + // : + let encrypted = encrypted + "\n"; + + MimePart::new( + "multipart/encrypted; protocol=\"application/pgp-encrypted\"", + vec![ + // Autocrypt part 1 + MimePart::new("application/pgp-encrypted", "Version: 1\r\n").header( + "Content-Description", + mail_builder::headers::raw::Raw::new("PGP/MIME version identification"), + ), + // Autocrypt part 2 + MimePart::new( + "application/octet-stream; name=\"encrypted.asc\"", + encrypted, + ) + .header( + "Content-Description", + mail_builder::headers::raw::Raw::new("OpenPGP encrypted message"), + ) + .header( + "Content-Disposition", + mail_builder::headers::raw::Raw::new("inline; filename=\"encrypted.asc\";"), + ), + ], + ) +} + +fn add_headers_to_encrypted_part( + message: MimePart<'static>, + unprotected_headers: &[(&'static str, HeaderType<'static>)], + hidden_headers: Vec<(&'static str, HeaderType<'static>)>, + protected_headers: Vec<(&'static str, HeaderType<'static>)>, + use_std_header_protection: bool, +) -> MimePart<'static> { + // Store protected headers in the inner message. + let message = protected_headers + .into_iter() + .fold(message, |message, (header, value)| { + message.header(header, value) + }); + + // Add hidden headers to encrypted payload. + let mut message: MimePart<'static> = hidden_headers + .into_iter() + .fold(message, |message, (header, value)| { + message.header(header, value) + }); + + if use_std_header_protection { + message = unprotected_headers + .iter() + // Structural headers shouldn't be added as "HP-Outer". They are defined in + // . + .filter(|(name, _)| { + !(name.eq_ignore_ascii_case("mime-version") + || name.eq_ignore_ascii_case("content-type") + || name.eq_ignore_ascii_case("content-transfer-encoding") + || name.eq_ignore_ascii_case("content-disposition")) + }) + .fold(message, |message, (name, value)| { + message.header(format!("HP-Outer: {name}"), value.clone()) + }); + } + + // Set the appropriate Content-Type for the inner message + for (h, v) in &mut message.headers { + if h == "Content-Type" + && let mail_builder::headers::HeaderType::ContentType(ct) = v + { + let mut ct_new = ct.clone(); + ct_new = ct_new.attribute("protected-headers", "v1"); + if use_std_header_protection { + ct_new = ct_new.attribute("hp", "cipher"); + } + *ct = ct_new; + break; + } + } + + message +} + +struct HeadersByConfidentiality { + /// Headers that must go into IMF header section. + /// + /// These are standard headers such as Date, In-Reply-To, References, which cannot be placed + /// anywhere else according to the standard. Placing headers here also allows them to be fetched + /// individually over IMAP without downloading the message body. This is why Chat-Version is + /// placed here. + unprotected_headers: Vec<(&'static str, HeaderType<'static>)>, + + /// Headers that MUST NOT (only) go into IMF header section: + /// - Large headers which may hit the header section size limit on the server, such as + /// Chat-User-Avatar with a base64-encoded image inside. + /// - Headers duplicated here that servers mess up with in the IMF header section, like + /// Message-ID. + /// - Nonstandard headers that should be DKIM-protected because e.g. OpenDKIM only signs + /// known headers. + /// + /// The header should be hidden from MTA + /// by moving it either into protected part + /// in case of encrypted mails + /// or unprotected MIME preamble in case of unencrypted mails. + hidden_headers: Vec<(&'static str, HeaderType<'static>)>, + + /// Opportunistically protected headers. + /// + /// These headers are placed into encrypted part *if* the message is encrypted. Place headers + /// which are not needed before decryption (e.g. Chat-Group-Name) or are not interesting if the + /// message cannot be decrypted (e.g. Chat-Disposition-Notification-To) here. + /// + /// If the message is not encrypted, these headers are placed into IMF header section, so make + /// sure that the message will be encrypted if you place any sensitive information here. + protected_headers: Vec<(&'static str, HeaderType<'static>)>, +} + +/// Split headers based on header confidentiality policy. +/// See [`HeadersByConfidentiality`] for more info. +fn group_headers_by_confidentiality( + headers: Vec<(&'static str, HeaderType<'static>)>, + from_addr: &str, + timestamp: i64, + is_encrypted: bool, + is_securejoin_message: bool, +) -> HeadersByConfidentiality { + let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + let mut hidden_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + let mut protected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + + // MIME header . + unprotected_headers.push(( + "MIME-Version", + mail_builder::headers::raw::Raw::new("1.0").into(), + )); + + for header @ (original_header_name, _header_value) in &headers { + let header_name = original_header_name.to_lowercase(); + if header_name == "message-id" { + unprotected_headers.push(header.clone()); + hidden_headers.push(header.clone()); + } else if is_hidden(&header_name) { + hidden_headers.push(header.clone()); + } else if header_name == "from" { + // Unencrypted securejoin messages should _not_ include the display name: + if is_encrypted || !is_securejoin_message { + protected_headers.push(header.clone()); + } + + unprotected_headers.push(( + original_header_name, + Address::new_address(None::<&'static str>, from_addr.to_string()).into(), + )); + } else if header_name == "to" { + protected_headers.push(header.clone()); + if is_encrypted { + unprotected_headers.push(("To", hidden_recipients().into())); + } else { + unprotected_headers.push(header.clone()); + } + } else if header_name == "chat-broadcast-secret" { + if is_encrypted { + protected_headers.push(header.clone()); + } + } else if is_encrypted && header_name == "date" { + protected_headers.push(header.clone()); + + // Randomized date goes to unprotected header. + // + // We cannot just send "Thu, 01 Jan 1970 00:00:00 +0000" + // or omit the header because GMX then fails with + // + // host mx00.emig.gmx.net[212.227.15.9] said: + // 554-Transaction failed + // 554-Reject due to policy restrictions. + // 554 For explanation visit https://postmaster.gmx.net/en/case?... + // (in reply to end of DATA command) + // + // and the explanation page says + // "The time information deviates too much from the actual time". + // + // We also limit the range to 6 days (518400 seconds) + // because with a larger range we got + // error "500 Date header far in the past/future" + // which apparently originates from Symantec Messaging Gateway + // and means the message has a Date that is more + // than 7 days in the past: + // + let timestamp_offset = rand::random_range(0..518400); + let protected_timestamp = timestamp.saturating_sub(timestamp_offset); + let unprotected_date = + chrono::DateTime::::from_timestamp(protected_timestamp, 0) + .unwrap() + .to_rfc2822(); + unprotected_headers.push(( + "Date", + mail_builder::headers::raw::Raw::new(unprotected_date).into(), + )); + } else if is_encrypted { + protected_headers.push(header.clone()); + + match header_name.as_str() { + "subject" => { + unprotected_headers.push(( + "Subject", + mail_builder::headers::raw::Raw::new("[...]").into(), + )); + } + "in-reply-to" + | "references" + | "auto-submitted" + | "chat-version" + | "autocrypt-setup-message" + | "chat-is-post-message" => { + unprotected_headers.push(header.clone()); + } + _ => { + // Other headers are removed from unprotected part. + } + } + } else { + // Copy the header to the protected headers + // in case of signed-only message. + // If the message is not signed, this value will not be used. + protected_headers.push(header.clone()); + unprotected_headers.push(header.clone()) + } + } + HeadersByConfidentiality { + unprotected_headers, + hidden_headers, + protected_headers, + } +} + fn hidden_recipients() -> Address<'static> { Address::new_group(Some("hidden-recipients".to_string()), Vec::new()) } @@ -2241,5 +2292,114 @@ fn b_encode(value: &str) -> String { ) } +pub(crate) async fn render_symm_encrypted_securejoin_message( + context: &Context, + step: &str, + rfc724_mid: &str, + attach_self_pubkey: bool, + auth: &str, +) -> Result { + info!(context, "Sending secure-join message {step:?}."); + + let mut headers = Vec::<(&'static str, HeaderType<'static>)>::new(); + + let from_addr = context.get_primary_self_addr().await?; + let from = new_address_with_name("", from_addr.to_string()); + headers.push(("From", from.into())); + + let to: Vec> = vec![hidden_recipients()]; + headers.push(( + "To", + mail_builder::headers::address::Address::new_list(to.clone()).into(), + )); + + headers.push(( + "Subject", + mail_builder::headers::text::Text::new("Secure-Join".to_string()).into(), + )); + + let timestamp = create_smeared_timestamp(context); + let date = chrono::DateTime::::from_timestamp(timestamp, 0) + .unwrap() + .to_rfc2822(); + headers.push(("Date", mail_builder::headers::raw::Raw::new(date).into())); + + headers.push(( + "Message-ID", + mail_builder::headers::message_id::MessageId::new(rfc724_mid.to_string()).into(), + )); + + // Automatic Response headers + if context.get_config_bool(Config::Bot).await? { + headers.push(( + "Auto-Submitted", + mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), + )); + } + + let encrypt_helper = EncryptHelper::new(context).await?; + + if attach_self_pubkey { + let aheader = encrypt_helper.get_aheader().to_string(); + headers.push(( + "Autocrypt", + mail_builder::headers::raw::Raw::new(aheader).into(), + )); + } + + headers.push(( + "Secure-Join", + mail_builder::headers::raw::Raw::new(step.to_string()).into(), + )); + + headers.push(( + "Secure-Join-Auth", + mail_builder::headers::text::Text::new(auth.to_string()).into(), + )); + + let message: MimePart<'static> = MimePart::new("text/plain", "Secure-Join"); + + let is_encrypted = true; + let is_securejoin_message = true; + let HeadersByConfidentiality { + unprotected_headers, + hidden_headers, + protected_headers, + } = group_headers_by_confidentiality( + headers, + &from_addr, + timestamp, + is_encrypted, + is_securejoin_message, + ); + + let outer_message = { + let use_std_header_protection = true; + let message = add_headers_to_encrypted_part( + message, + &unprotected_headers, + hidden_headers, + protected_headers, + use_std_header_protection, + ); + + // Disable compression for SecureJoin to ensure + // there are no compression side channels + // leaking information about the tokens. + let compress = false; + // Only sign the message if we attach the pubkey. + let sign = attach_self_pubkey; + let encrypted = encrypt_helper + .encrypt_symmetrically(context, auth, message, compress, sign) + .await?; + + wrap_encrypted_part(encrypted) + }; + + let message = render_outer_message(unprotected_headers, outer_message); + + Ok(message) +} + #[cfg(test)] mod mimefactory_tests; diff --git a/src/mimeparser.rs b/src/mimeparser.rs index a04b5621fc..de23bb257a 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -18,10 +18,9 @@ use crate::authres::handle_authres; use crate::blob::BlobObject; use crate::chat::ChatId; use crate::config::Config; -use crate::constants; use crate::contact::ContactId; use crate::context::Context; -use crate::decrypt::{try_decrypt, validate_detached_signature}; +use crate::decrypt::{get_encrypted_pgp_message, validate_detached_signature}; use crate::dehtml::dehtml; use crate::download::PostMsgMetadata; use crate::events::EventType; @@ -36,6 +35,7 @@ use crate::tools::{ get_filemeta, parse_receive_headers, smeared_time, time, truncate_msg_text, validate_id, }; use crate::{chatlist_events, location, tools}; +use crate::{constants, token}; /// Public key extracted from `Autocrypt-Gossip` /// header with associated information. @@ -358,7 +358,7 @@ impl MimeMessage { // Remove headers that are allowed _only_ in the encrypted+signed part. It's ok to leave // them in signed-only emails, but has no value currently. - Self::remove_secured_headers(&mut headers, &mut headers_removed); + Self::remove_secured_headers(&mut headers, &mut headers_removed, false); let mut from = from.context("No from in message")?; let private_keyring = load_self_secret_keyring(context).await?; @@ -383,59 +383,65 @@ impl MimeMessage { PreMessageMode::None }; - let mail_raw; // Memory location for a possible decrypted message. - let decrypted_msg; // Decrypted signed OpenPGP message. - let secrets: Vec = context - .sql - .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { - let secret: String = row.get(0)?; - Ok(secret) - }) - .await?; + let encrypted_pgp_message = get_encrypted_pgp_message(&mail)?; - let (mail, is_encrypted) = - match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) { - Ok(Some(mut msg)) => { - mail_raw = msg.as_data_vec().unwrap_or_default(); + let secrets: Vec; + if let Some(e) = &encrypted_pgp_message + && crate::pgp::check_symmetric_encryption(e).is_ok() + { + secrets = load_shared_secrets(context).await?; + } else { + // No need to load all the secrets if the message isn't symmetrically encrypted + secrets = vec![]; + } - let decrypted_mail = mailparse::parse_mail(&mail_raw)?; - if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { - info!( - context, - "decrypted message mime-body:\n{}", - String::from_utf8_lossy(&mail_raw), - ); - } + let mail_raw; // Memory location for a possible decrypted message. + let decrypted_msg; // Decrypted signed OpenPGP message. - decrypted_msg = Some(msg); + let (mail, is_encrypted) = match tokio::task::block_in_place(|| { + encrypted_pgp_message.map(|e| crate::pgp::decrypt(e, &private_keyring, &secrets)) + }) { + Some(Ok(mut msg)) => { + mail_raw = msg.as_data_vec().unwrap_or_default(); - timestamp_sent = Self::get_timestamp_sent( - &decrypted_mail.headers, - timestamp_sent, - timestamp_rcvd, + let decrypted_mail = mailparse::parse_mail(&mail_raw)?; + if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { + info!( + context, + "decrypted message mime-body:\n{}", + String::from_utf8_lossy(&mail_raw), ); + } - let protected_aheader_values = decrypted_mail - .headers - .get_all_values(HeaderDef::Autocrypt.into()); - if !protected_aheader_values.is_empty() { - aheader_values = protected_aheader_values; - } + decrypted_msg = Some(msg); - (Ok(decrypted_mail), true) - } - Ok(None) => { - mail_raw = Vec::new(); - decrypted_msg = None; - (Ok(mail), false) - } - Err(err) => { - mail_raw = Vec::new(); - decrypted_msg = None; - warn!(context, "decryption failed: {:#}", err); - (Err(err), false) + timestamp_sent = Self::get_timestamp_sent( + &decrypted_mail.headers, + timestamp_sent, + timestamp_rcvd, + ); + + let protected_aheader_values = decrypted_mail + .headers + .get_all_values(HeaderDef::Autocrypt.into()); + if !protected_aheader_values.is_empty() { + aheader_values = protected_aheader_values; } - }; + + (Ok(decrypted_mail), true) + } + None => { + mail_raw = Vec::new(); + decrypted_msg = None; + (Ok(mail), false) + } + Some(Err(err)) => { + mail_raw = Vec::new(); + decrypted_msg = None; + warn!(context, "decryption failed: {:#}", err); + (Err(err), false) + } + }; let mut autocrypt_header = None; if incoming { @@ -608,7 +614,7 @@ impl MimeMessage { } } if signatures.is_empty() { - Self::remove_secured_headers(&mut headers, &mut headers_removed); + Self::remove_secured_headers(&mut headers, &mut headers_removed, is_encrypted); } if !is_encrypted { signatures.clear(); @@ -1718,20 +1724,34 @@ impl MimeMessage { .and_then(|msgid| parse_message_id(msgid).ok()) } + /// Remove headers that are only allowed to be present in an encrypted-and-signed message fn remove_secured_headers( headers: &mut HashMap, removed: &mut HashSet, + // Whether the message was encrypted (but not signed): + encrypted: bool, ) { remove_header(headers, "secure-join-fingerprint", removed); - remove_header(headers, "secure-join-auth", removed); remove_header(headers, "chat-verified", removed); remove_header(headers, "autocrypt-gossip", removed); - // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". - if let Some(secure_join) = remove_header(headers, "secure-join", removed) - && (secure_join == "vc-request" || secure_join == "vg-request") - { - headers.insert("secure-join".to_string(), secure_join); + if headers.get("secure-join") == Some(&"vc-request-pubkey".to_string()) && encrypted { + // vc-request-pubkey message is encrypted, but unsigned, + // and contains a Secure-Join-Auth header. + // + // It is unsigned in order not to leak Bob's identity to a server operator + // that scraped the AUTH token somewhere from the web, + // and because Alice anyways couldn't verify his signature at this step, + // because she doesn't know his public key yet. + } else { + remove_header(headers, "secure-join-auth", removed); + + // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". + if let Some(secure_join) = remove_header(headers, "secure-join", removed) + && (secure_join == "vc-request" || secure_join == "vg-request") + { + headers.insert("secure-join".to_string(), secure_join); + } } } @@ -2082,6 +2102,36 @@ impl MimeMessage { } } +/// Loads all the shared secrets +/// that will be tried to decrypt a symmetrically-encrypted message +async fn load_shared_secrets(context: &Context) -> Result> { + // First, try decrypting using the bobstate, + // because usually there will only be 1 or 2 of it, + // so, it should be fast + let mut secrets: Vec = context + .sql + .query_map_vec("SELECT id, invite FROM bobstate", (), |row| { + let invite: crate::securejoin::QrInvite = row.get(1)?; + Ok(invite.authcode().to_string()) + }) + .await?; + // Then, try decrypting using broadcast secrets + // Usually, the user won't be in more than ~10 broadcast channels + secrets.extend( + context + .sql + .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { + let secret: String = row.get(0)?; + Ok(secret) + }) + .await?, + ); + // Finally, try decrypting using AUTH tokens + // There can be a lot of AUTH tokens, because a new one is generated every time a QR code is shown + secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); + Ok(secrets) +} + fn rm_legacy_display_elements(text: &str) -> String { let mut res = None; for l in text.lines() { diff --git a/src/pgp.rs b/src/pgp.rs index c50bb96f08..2172e48b35 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -327,13 +327,10 @@ pub fn pk_calc_signature( /// /// Returns the decrypted and decompressed message. pub fn decrypt( - ctext: Vec, + msg: Message<'static>, private_keys_for_decryption: &[SignedSecretKey], mut shared_secrets: &[String], ) -> Result> { - let cursor = Cursor::new(ctext); - let (msg, _headers) = Message::from_armor(cursor)?; - let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption.iter().collect(); let empty_pw = Password::empty(); @@ -389,7 +386,9 @@ pub fn decrypt( /// with all of the known shared secrets. /// In order to prevent this, we do not try to symmetrically decrypt messages /// that use a string2key algorithm other than 'Salted'. -fn check_symmetric_encryption(msg: &Message<'_>) -> std::result::Result<(), &'static str> { +pub(crate) fn check_symmetric_encryption( + msg: &Message<'_>, +) -> std::result::Result<(), &'static str> { let Message::Encrypted { esk, .. } = msg else { return Err("not encrypted"); }; @@ -480,7 +479,7 @@ pub async fn symm_encrypt_autocrypt_setup(passphrase: &str, plain: Vec) -> R /// `shared secret` is the secret that will be used for symmetric encryption. pub async fn symm_encrypt_message( plain: Vec, - private_key_for_signing: SignedSecretKey, + private_key_for_signing: Option, shared_secret: &str, compress: bool, ) -> Result { @@ -503,8 +502,10 @@ pub async fn symm_encrypt_message( ); msg.encrypt_with_password(&mut rng, s2k, &shared_secret)?; - let hash_algorithm = private_key_for_signing.hash_alg(); - msg.sign(&*private_key_for_signing, Password::empty(), hash_algorithm); + if let Some(private_key_for_signing) = private_key_for_signing.as_deref() { + let hash_algorithm = private_key_for_signing.hash_alg(); + msg.sign(private_key_for_signing, Password::empty(), hash_algorithm); + } if compress { msg.compression(CompressionAlgorithm::ZLIB); } @@ -546,6 +547,16 @@ mod tests { use pgp::composed::Esk; use pgp::packet::PublicKeyEncryptedSessionKey; + fn decrypt_bytes( + bytes: Vec, + private_keys_for_decryption: &[SignedSecretKey], + shared_secrets: &[String], + ) -> Result> { + let cursor = Cursor::new(bytes); + let (msg, _headers) = Message::from_armor(cursor).unwrap(); + decrypt(msg, private_keys_for_decryption, shared_secrets) + } + #[expect(clippy::type_complexity)] fn pk_decrypt_and_validate<'a>( ctext: &'a [u8], @@ -556,7 +567,7 @@ mod tests { HashMap>, Vec, )> { - let mut msg = decrypt(ctext.to_vec(), private_keys_for_decryption, &[])?; + let mut msg = decrypt_bytes(ctext.to_vec(), private_keys_for_decryption, &[])?; let content = msg.as_data_vec()?; let ret_signature_fingerprints = valid_signature_fingerprints(&msg, public_keys_for_validation); @@ -737,14 +748,14 @@ mod tests { let shared_secret = "shared secret"; let ctext = symm_encrypt_message( plain.clone(), - load_self_secret_key(alice).await?, + Some(load_self_secret_key(alice).await?), shared_secret, true, ) .await?; let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; - let mut decrypted = decrypt( + let mut decrypted = decrypt_bytes( ctext.into(), &bob_private_keyring, &[shared_secret.to_string()], @@ -787,7 +798,7 @@ mod tests { // Trying to decrypt it should fail with a helpful error message: let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; - let error = decrypt( + let error = decrypt_bytes( ctext.into(), &bob_private_keyring, &[shared_secret.to_string()], @@ -824,7 +835,7 @@ mod tests { // Trying to decrypt it should fail with an OK error message: let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; - let error = decrypt(ctext.into(), &bob_private_keyring, &[]).unwrap_err(); + let error = decrypt_bytes(ctext.into(), &bob_private_keyring, &[]).unwrap_err(); assert_eq!( error.to_string(), diff --git a/src/qr.rs b/src/qr.rs index a289d9aafa..b5adbc39ec 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -61,6 +61,9 @@ pub enum Qr { /// Authentication code. authcode: String, + + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask the user whether to join the group. @@ -82,6 +85,9 @@ pub enum Qr { /// Authentication code. authcode: String, + + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask whether to join the broadcast channel. @@ -106,6 +112,9 @@ pub enum Qr { invitenumber: String, /// Authentication code. authcode: String, + + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Contact fingerprint is verified. @@ -483,7 +492,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { let name = decode_name(¶m, "n")?.unwrap_or_default(); - let invitenumber = param + let mut invitenumber = param .get("i") // For historic reansons, broadcasts currently use j instead of i for the invitenumber: .or_else(|| param.get("j")) @@ -501,6 +510,16 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { let grpname = decode_name(¶m, "g")?; let broadcast_name = decode_name(¶m, "b")?; + let mut is_v3 = param.get("v") == Some(&"3"); + + if authcode.is_some() && invitenumber.is_none() { + // Securejoin v3 doesn't need an invitenumber. + // We want to remove the invitenumber and the `v=3` parameter eventually; + // therefore, we accept v3 QR codes without an invitenumber. + is_v3 = true; + invitenumber = Some("".to_string()); + } + if let (Some(addr), Some(invitenumber), Some(authcode)) = (&addr, invitenumber, authcode) { let addr = ContactAddress::new(addr)?; let (contact_id, _) = Contact::add_or_lookup_ex( @@ -519,7 +538,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .await .with_context(|| format!("can't check if address {addr:?} is our address"))? { - if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { + if token::exists(context, token::Namespace::Auth, &authcode).await? { Ok(Qr::WithdrawVerifyGroup { grpname, grpid, @@ -546,6 +565,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fingerprint, invitenumber, authcode, + is_v3, }) } } else if let (Some(grpid), Some(name)) = (grpid, broadcast_name) { @@ -554,7 +574,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .await .with_context(|| format!("Can't check if {addr:?} is our address"))? { - if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { + if token::exists(context, token::Namespace::Auth, &authcode).await? { Ok(Qr::WithdrawJoinBroadcast { name, grpid, @@ -581,10 +601,11 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fingerprint, invitenumber, authcode, + is_v3, }) } } else if context.is_self_addr(&addr).await? { - if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { + if token::exists(context, token::Namespace::Auth, &authcode).await? { Ok(Qr::WithdrawVerifyContact { contact_id, fingerprint, @@ -605,6 +626,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fingerprint, invitenumber, authcode, + is_v3, }) } } else if let Some(addr) = addr { diff --git a/src/qr/qr_tests.rs b/src/qr/qr_tests.rs index d6dc88f726..43516c419c 100644 --- a/src/qr/qr_tests.rs +++ b/src/qr/qr_tests.rs @@ -1,3 +1,5 @@ +use regex::Regex; + use super::*; use crate::chat::{Chat, create_broadcast, create_group, get_chat_contacts}; use crate::config::Config; @@ -445,9 +447,28 @@ async fn test_decode_openpgp_without_addr() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_withdraw_verifycontact() -> Result<()> { +async fn test_withdraw_verifycontact_basic() -> Result<()> { + test_withdraw_verifycontact(false).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_withdraw_verifycontact_without_invite() -> Result<()> { + test_withdraw_verifycontact(true).await +} + +async fn test_withdraw_verifycontact(remove_invite: bool) -> Result<()> { let alice = TestContext::new_alice().await; - let qr = get_securejoin_qr(&alice, None).await?; + let mut qr = get_securejoin_qr(&alice, None).await?; + + if remove_invite { + // Remove the INVITENUBMER. It's not needed in Securejoin v3, + // but still included for backwards compatibility reasons. + // We want to be able to remove it in the future, + // therefore we test that things work without it. + let new_qr = Regex::new("&i=.*?&").unwrap().replace(&qr, "&"); + assert!(new_qr != *qr); + qr = new_qr.to_string(); + } // scanning own verify-contact code offers withdrawing assert!(matches!( @@ -466,6 +487,11 @@ async fn test_withdraw_verifycontact() -> Result<()> { check_qr(&alice, &qr).await?, Qr::WithdrawVerifyContact { .. } )); + // Test that removing the INVITENUMBER doesn't result in saving empty token: + assert_eq!( + token::exists(&alice, token::Namespace::InviteNumber, "").await?, + false + ); // someone else always scans as ask-verify-contact let bob = TestContext::new_bob().await; diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 906489c9b1..46e1be863c 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -180,22 +180,6 @@ pub(crate) async fn receive_imf_from_inbox( receive_imf_inner(context, rfc724_mid, imf_raw, seen).await } -/// Inserts a tombstone into `msgs` table -/// to prevent downloading the same message in the future. -/// -/// Returns tombstone database row ID. -async fn insert_tombstone(context: &Context, rfc724_mid: &str) -> Result { - let row_id = context - .sql - .insert( - "INSERT INTO msgs(rfc724_mid, chat_id) VALUES (?,?)", - (rfc724_mid, DC_CHAT_ID_TRASH), - ) - .await?; - let msg_id = MsgId::new(u32::try_from(row_id)?); - Ok(msg_id) -} - async fn get_to_and_past_contact_ids( context: &Context, mime_parser: &MimeMessage, @@ -496,7 +480,7 @@ pub(crate) async fn receive_imf_inner( } let trash = || async { - let msg_ids = vec![insert_tombstone(context, rfc724_mid).await?]; + let msg_ids = vec![message::insert_tombstone(context, rfc724_mid).await?]; Ok(Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, state: MessageState::Undefined, @@ -693,7 +677,7 @@ pub(crate) async fn receive_imf_inner( match res { securejoin::HandshakeMessage::Done | securejoin::HandshakeMessage::Ignore => { - let msg_id = insert_tombstone(context, rfc724_mid).await?; + let msg_id = message::insert_tombstone(context, rfc724_mid).await?; received_msg = Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, state: MessageState::InSeen, diff --git a/src/securejoin.rs b/src/securejoin.rs index 8bf532d692..59e9659f51 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -20,14 +20,14 @@ use crate::headerdef::HeaderDef; use crate::key::{DcKey, Fingerprint, load_self_public_key}; use crate::log::LogExt as _; use crate::log::warn; -use crate::message::{Message, Viewtype}; +use crate::message::{self, Message, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::Param; use crate::qr::check_qr; use crate::securejoin::bob::JoinerProgress; use crate::sync::Sync::*; -use crate::tools::{create_id, time}; -use crate::{SecurejoinSource, stats}; +use crate::tools::{create_id, create_outgoing_rfc724_mid, time}; +use crate::{SecurejoinSource, mimefactory, stats}; use crate::{SecurejoinUiPath, token}; mod bob; @@ -127,9 +127,6 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul None => None, }; let grpid = chat.as_ref().map(|c| c.grpid.as_str()); - let sync_token = token::lookup(context, Namespace::InviteNumber, grpid) - .await? - .is_none(); // Invite number is used to request the inviter key. let invitenumber = token::lookup_or_new(context, Namespace::InviteNumber, grpid).await?; @@ -156,12 +153,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul .unwrap_or_default(); let qr = if let Some(chat) = chat { - if sync_token { - context - .sync_qr_code_tokens(Some(chat.grpid.as_str())) - .await?; - context.scheduler.interrupt_smtp().await; - } + context + .sync_qr_code_tokens(Some(chat.grpid.as_str())) + .await?; + context.scheduler.interrupt_smtp().await; let chat_name = chat.get_name(); let chat_name_shortened = shorten_name(chat_name, 25); @@ -178,11 +173,11 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul if chat.typ == Chattype::OutBroadcast { // For historic reansons, broadcasts currently use j instead of i for the invitenumber. format!( - "https://i.delta.chat/#{fingerprint}&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&b={chat_name_urlencoded}", + "https://i.delta.chat/#{fingerprint}&v=3&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&b={chat_name_urlencoded}", ) } else { format!( - "https://i.delta.chat/#{fingerprint}&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&g={chat_name_urlencoded}", + "https://i.delta.chat/#{fingerprint}&v=3&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&g={chat_name_urlencoded}", ) } } else { @@ -190,12 +185,12 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul let self_name_urlencoded = utf8_percent_encode(&self_name_shortened, DISALLOWED_CHARACTERS) .to_string() .replace("%20", "+"); - if sync_token { - context.sync_qr_code_tokens(None).await?; - context.scheduler.interrupt_smtp().await; - } + + context.sync_qr_code_tokens(None).await?; + context.scheduler.interrupt_smtp().await; + format!( - "https://i.delta.chat/#{fingerprint}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}", + "https://i.delta.chat/#{fingerprint}&v=3&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}", ) }; @@ -346,12 +341,18 @@ pub(crate) enum HandshakeMessage { /// Step of Secure-Join protocol. #[derive(Debug, Display, PartialEq, Eq)] pub(crate) enum SecureJoinStep { - /// vc-request or vg-request + /// vc-request or vg-request; only used in legacy securejoin Request { invitenumber: String }, - /// vc-auth-required or vg-auth-required + /// vc-auth-required or vg-auth-required; only used in legacy securejoin AuthRequired, + /// vc-request-pubkey; only used in securejoin v3 + RequestPubkey, + + /// vc-pubkey; only used in securejoin v3 + Pubkey, + /// vc-request-with-auth or vg-request-with-auth RequestWithAuth, @@ -381,6 +382,8 @@ pub(crate) fn get_secure_join_step(mime_message: &MimeMessage) -> Option Some(SecureJoinStep::RequestPubkey), + "vc-pubkey" => Some(SecureJoinStep::Pubkey), "vg-auth-required" | "vc-auth-required" => Some(SecureJoinStep::AuthRequired), "vg-request-with-auth" | "vc-request-with-auth" => { Some(SecureJoinStep::RequestWithAuth) @@ -438,7 +441,10 @@ pub(crate) async fn handle_securejoin_handshake( // will improve security (completely unrelated to the securejoin protocol) // and is something we want to do in the future: // https://www.rfc-editor.org/rfc/rfc9580.html#name-surreptitious-forwarding - if !matches!(step, SecureJoinStep::Request { .. }) { + if !matches!( + step, + SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey | SecureJoinStep::Pubkey + ) { let mut self_found = false; let self_fingerprint = load_self_public_key(context).await?.dc_fingerprint(); for (addr, key) in &mime_message.gossiped_keys { @@ -504,7 +510,54 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob - the joiner's side ===== ==== Step 4 in "Setup verified contact" protocol ===== ========================================================*/ - bob::handle_auth_required(context, mime_message).await + bob::handle_auth_required_or_pubkey(context, mime_message).await + } + SecureJoinStep::RequestPubkey => { + /*======================================================== + ==== Alice - the inviter's side ===== + ==== Bob requests our public key (Securejoin v3) ===== + ========================================================*/ + + if mime_message.signature.is_some() { + warn!(context, "RequestPubkey is not supposed to be signed"); + return Ok(HandshakeMessage::Ignore); + } + let Some(auth) = mime_message.get_header(HeaderDef::SecureJoinAuth) else { + warn!( + context, + "Ignoring {step} message because of missing auth code." + ); + return Ok(HandshakeMessage::Ignore); + }; + if !token::exists(context, token::Namespace::Auth, auth).await? { + warn!(context, "Secure-join denied (bad auth)."); + return Ok(HandshakeMessage::Ignore); + } + + let rfc724_mid = create_outgoing_rfc724_mid(); + let addr = ContactAddress::new(&mime_message.from.addr)?; + let attach_self_pubkey = true; + let rendered_message = mimefactory::render_symm_encrypted_securejoin_message( + context, + "vc-pubkey", + &rfc724_mid, + attach_self_pubkey, + auth, + ) + .await?; + + let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; + insert_into_smtp(context, &rfc724_mid, &addr, rendered_message, msg_id).await?; + context.scheduler.interrupt_smtp().await; + + Ok(HandshakeMessage::Done) + } + SecureJoinStep::Pubkey => { + /*======================================================== + ==== Bob - the joiner's side ===== + ==== Alice sent us her pubkey (Securejoin v3) ===== + ========================================================*/ + bob::handle_auth_required_or_pubkey(context, mime_message).await } SecureJoinStep::RequestWithAuth => { /*========================================================== @@ -665,6 +718,24 @@ pub(crate) async fn handle_securejoin_handshake( } } +async fn insert_into_smtp( + context: &Context, + rfc724_mid: &str, + recipient: &str, + rendered_message: String, + msg_id: MsgId, +) -> Result<(), Error> { + context + .sql + .execute( + "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) + VALUES (?1, ?2, ?3, ?4)", + (&rfc724_mid, &recipient, &rendered_message, msg_id), + ) + .await?; + Ok(()) +} + /// Observe self-sent Securejoin message. /// /// In a multi-device-setup, there may be other devices that "see" the handshake messages. @@ -696,6 +767,8 @@ pub(crate) async fn observe_securejoin_on_other_device( match step { SecureJoinStep::Request { .. } | SecureJoinStep::AuthRequired + | SecureJoinStep::RequestPubkey + | SecureJoinStep::Pubkey | SecureJoinStep::Deprecated | SecureJoinStep::Unknown { .. } => { return Ok(HandshakeMessage::Ignore); diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index fd92985625..e676b274af 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -5,20 +5,22 @@ use anyhow::{Context as _, Result}; use super::HandshakeMessage; use super::qrinvite::QrInvite; use crate::chat::{self, ChatId, is_contact_in_chat}; -use crate::chatlist_events; use crate::constants::{Blocked, Chattype}; -use crate::contact::Origin; +use crate::contact::{Contact, Origin}; use crate::context::Context; use crate::events::EventType; use crate::key::self_fingerprint; use crate::log::LogExt; -use crate::message::{Message, MsgId, Viewtype}; +use crate::message::{self, Message, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::{Param, Params}; -use crate::securejoin::{ContactId, encrypted_and_signed, verify_sender_by_fingerprint}; +use crate::securejoin::{ + ContactId, encrypted_and_signed, insert_into_smtp, verify_sender_by_fingerprint, +}; use crate::stock_str; use crate::sync::Sync::*; -use crate::tools::{smeared_time, time}; +use crate::tools::{create_outgoing_rfc724_mid, smeared_time, time}; +use crate::{chatlist_events, mimefactory}; /// Starts the securejoin protocol with the QR `invite`. /// @@ -213,11 +215,11 @@ LIMIT 1 Ok(()) } -/// Handles `vc-auth-required` and `vg-auth-required` handshake messages. +/// Handles `vc-auth-required`, `vg-auth-required`, and `vc-pubkey` handshake messages. /// /// # Bob - the joiner's side /// ## Step 4 in the "Setup Contact protocol" -pub(super) async fn handle_auth_required( +pub(super) async fn handle_auth_required_or_pubkey( context: &Context, message: &MimeMessage, ) -> Result { @@ -299,47 +301,68 @@ pub(crate) async fn send_handshake_message( chat_id: ChatId, step: BobHandshakeMsg, ) -> Result<()> { - let mut msg = Message { - viewtype: Viewtype::Text, - text: step.body_text(invite), - hidden: true, - ..Default::default() - }; - msg.param.set_cmd(SystemMessage::SecurejoinMessage); + if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { + // Send a minimal symmetrically-encrypted vc-request-pubkey message + let rfc724_mid = create_outgoing_rfc724_mid(); + let contact = Contact::get_by_id(context, invite.contact_id()).await?; + let recipient = contact.get_addr(); + let attach_self_pubkey = false; + let rendered_message = mimefactory::render_symm_encrypted_securejoin_message( + context, + "vc-request-pubkey", + &rfc724_mid, + attach_self_pubkey, + invite.authcode(), + ) + .await?; - // Sends the step in Secure-Join header. - msg.param.set(Param::Arg, step.securejoin_header(invite)); + let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; + insert_into_smtp(context, &rfc724_mid, recipient, rendered_message, msg_id).await?; + context.scheduler.interrupt_smtp().await; + } else { + let mut msg = Message { + viewtype: Viewtype::Text, + text: step.body_text(invite), + hidden: true, + ..Default::default() + }; - match step { - BobHandshakeMsg::Request => { - // Sends the Secure-Join-Invitenumber header in mimefactory.rs. - msg.param.set(Param::Arg2, invite.invitenumber()); - msg.force_plaintext(); - } - BobHandshakeMsg::RequestWithAuth => { - // Sends the Secure-Join-Auth header in mimefactory.rs. - msg.param.set(Param::Arg2, invite.authcode()); - msg.param.set_int(Param::GuaranteeE2ee, 1); - - // Sends our own fingerprint in the Secure-Join-Fingerprint header. - let bob_fp = self_fingerprint(context).await?; - msg.param.set(Param::Arg3, bob_fp); - - // Sends the grpid in the Secure-Join-Group header. - // - // `Secure-Join-Group` header is deprecated, - // but old Delta Chat core requires that Alice receives it. - // - // Previous Delta Chat core also sent `Secure-Join-Group` header - // in `vg-request` messages, - // but it was not used on the receiver. - if let QrInvite::Group { grpid, .. } = invite { - msg.param.set(Param::Arg4, grpid); + msg.param.set_cmd(SystemMessage::SecurejoinMessage); + + // Sends the step in Secure-Join header. + msg.param.set(Param::Arg, step.securejoin_header(invite)); + + match step { + BobHandshakeMsg::Request => { + // Sends the Secure-Join-Invitenumber header in mimefactory.rs. + msg.param.set(Param::Arg2, invite.invitenumber()); + msg.force_plaintext(); } - } - }; + BobHandshakeMsg::RequestWithAuth => { + // Sends the Secure-Join-Auth header in mimefactory.rs. + msg.param.set(Param::Arg2, invite.authcode()); + msg.param.set_int(Param::GuaranteeE2ee, 1); + + // Sends our own fingerprint in the Secure-Join-Fingerprint header. + let bob_fp = self_fingerprint(context).await?; + msg.param.set(Param::Arg3, bob_fp); + + // Sends the grpid in the Secure-Join-Group header. + // + // `Secure-Join-Group` header is deprecated, + // but old Delta Chat core requires that Alice receives it. + // + // Previous Delta Chat core also sent `Secure-Join-Group` header + // in `vg-request` messages, + // but it was not used on the receiver. + if let QrInvite::Group { grpid, .. } = invite { + msg.param.set(Param::Arg4, grpid); + } + } + }; - chat::send_msg(context, chat_id, &mut msg).await?; + chat::send_msg(context, chat_id, &mut msg).await?; + } Ok(()) } diff --git a/src/securejoin/qrinvite.rs b/src/securejoin/qrinvite.rs index 4bb3b71e11..ecef2b2726 100644 --- a/src/securejoin/qrinvite.rs +++ b/src/securejoin/qrinvite.rs @@ -12,7 +12,7 @@ use crate::qr::Qr; /// Represents the data from a QR-code scan. /// -/// There are methods to conveniently access fields present in both variants. +/// There are methods to conveniently access fields present in all three variants. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub enum QrInvite { Contact { @@ -20,6 +20,7 @@ pub enum QrInvite { fingerprint: Fingerprint, invitenumber: String, authcode: String, + is_v3: bool, }, Group { contact_id: ContactId, @@ -28,6 +29,7 @@ pub enum QrInvite { grpid: String, invitenumber: String, authcode: String, + is_v3: bool, }, Broadcast { contact_id: ContactId, @@ -36,6 +38,7 @@ pub enum QrInvite { grpid: String, invitenumber: String, authcode: String, + is_v3: bool, }, } @@ -78,6 +81,14 @@ impl QrInvite { | Self::Broadcast { authcode, .. } => authcode, } } + + pub fn is_v3(&self) -> bool { + match *self { + QrInvite::Contact { is_v3, .. } => is_v3, + QrInvite::Group { is_v3, .. } => is_v3, + QrInvite::Broadcast { is_v3, .. } => is_v3, + } + } } impl TryFrom for QrInvite { @@ -90,11 +101,13 @@ impl TryFrom for QrInvite { fingerprint, invitenumber, authcode, + is_v3, } => Ok(QrInvite::Contact { contact_id, fingerprint, invitenumber, authcode, + is_v3, }), Qr::AskVerifyGroup { grpname, @@ -103,6 +116,7 @@ impl TryFrom for QrInvite { fingerprint, invitenumber, authcode, + is_v3, } => Ok(QrInvite::Group { contact_id, fingerprint, @@ -110,6 +124,7 @@ impl TryFrom for QrInvite { grpid, invitenumber, authcode, + is_v3, }), Qr::AskJoinBroadcast { name, @@ -118,6 +133,7 @@ impl TryFrom for QrInvite { fingerprint, authcode, invitenumber, + is_v3, } => Ok(QrInvite::Broadcast { name, grpid, @@ -125,6 +141,7 @@ impl TryFrom for QrInvite { fingerprint, authcode, invitenumber, + is_v3, }), _ => bail!("Unsupported QR type"), } diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 6c309089e9..a429bff4cb 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -1,6 +1,7 @@ use std::time::Duration; use deltachat_contact_tools::EmailAddress; +use regex::Regex; use super::*; use crate::chat::{CantSendReason, add_contact_to_chat, remove_contact_from_chat}; @@ -14,7 +15,7 @@ use crate::receive_imf::receive_imf; use crate::stock_str::{self, messages_e2e_encrypted}; use crate::test_utils::{ AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager, - TimeShiftFalsePositiveNote, get_chat_msg, + TimeShiftFalsePositiveNote, get_chat_msg, sync, }; use crate::tools::SystemTime; @@ -27,7 +28,7 @@ enum SetupContactCase { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_setup_contact() { +async fn test_setup_contact_basic() { test_setup_contact_ex(SetupContactCase::Normal).await } @@ -62,13 +63,13 @@ async fn test_setup_contact_ex(case: SetupContactCase) { bob.set_config(Config::Displayname, Some("Bob Examplenet")) .await .unwrap(); - let alice_auto_submitted_hdr; + let alice_auto_submitted_hdr: bool; match case { SetupContactCase::AliceIsBot => { alice.set_config_bool(Config::Bot, true).await.unwrap(); - alice_auto_submitted_hdr = "Auto-Submitted: auto-generated"; + alice_auto_submitted_hdr = true; } - _ => alice_auto_submitted_hdr = "Auto-Submitted: auto-replied", + _ => alice_auto_submitted_hdr = false, }; assert_eq!( @@ -118,12 +119,15 @@ async fn test_setup_contact_ex(case: SetupContactCase) { assert!(!sent.payload.contains("Bob Examplenet")); assert_eq!(sent.recipient(), EmailAddress::new(alice_addr).unwrap()); let msg = alice.parse_msg(&sent).await; - assert!(!msg.was_encrypted()); - assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-request"); - assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); + assert!(msg.signature.is_none()); + assert_eq!( + msg.get_header(HeaderDef::SecureJoin).unwrap(), + "vc-request-pubkey" + ); + assert!(msg.get_header(HeaderDef::SecureJoinAuth).is_some()); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); - tcm.section("Step 3: Alice receives vc-request, sends vc-auth-required"); + tcm.section("Step 3: Alice receives vc-request-pubkey, sends vc-pubkey"); alice.recv_msg_trash(&sent).await; assert_eq!( Chatlist::try_load(&alice, 0, None, None) @@ -134,14 +138,14 @@ async fn test_setup_contact_ex(case: SetupContactCase) { ); let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains(alice_auto_submitted_hdr)); + assert_eq!( + sent.payload.contains("Auto-Submitted: auto-generated"), + alice_auto_submitted_hdr + ); assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); - assert_eq!( - msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vc-auth-required" - ); + assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-pubkey"); let bob_chat = bob.get_chat(&alice).await; assert_eq!(bob_chat.can_send(&bob).await.unwrap(), true); @@ -170,7 +174,6 @@ async fn test_setup_contact_ex(case: SetupContactCase) { // Check Bob sent the right message. let sent = bob.pop_sent_msg().await; - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); assert!(!sent.payload.contains("Bob Examplenet")); let mut msg = alice.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -261,7 +264,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) { // Check Alice sent the right message to Bob. let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains(alice_auto_submitted_hdr)); + assert_eq!( + sent.payload.contains("Auto-Submitted: auto-generated"), + alice_auto_submitted_hdr + ); assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -421,18 +427,31 @@ async fn test_setup_contact_concurrent_calls() -> Result<()> { assert!(!alice_id.is_special()); assert_eq!(chat.typ, Chattype::Single); assert_ne!(claire_id, alice_id); - assert!( + assert_eq!( bob.pop_sent_msg() .await .payload() - .contains("alice@example.org") + .contains("alice@example.org"), + false // Alice's address must not be sent in cleartext ); Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_secure_join() -> Result<()> { +async fn test_secure_join_group_legacy() -> Result<()> { + test_secure_join_group_ex(false, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_group_v3() -> Result<()> { + test_secure_join_group_ex(true, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_group_v3_without_invite() -> Result<()> { + test_secure_join_group_ex(true, true).await +} + +async fn test_secure_join_group_ex(v3: bool, remove_invite: bool) -> Result<()> { let mut tcm = TestContextManager::new(); let alice = tcm.alice().await; let bob = tcm.bob().await; @@ -444,7 +463,8 @@ async fn test_secure_join() -> Result<()> { let alice_chatid = chat::create_group(&alice, "the chat").await?; tcm.section("Step 1: Generate QR-code, secure-join implied by chatid"); - let qr = get_securejoin_qr(&alice, Some(alice_chatid)).await.unwrap(); + let mut qr = get_securejoin_qr(&alice, Some(alice_chatid)).await.unwrap(); + manipulate_qr(v3, remove_invite, &mut qr); tcm.section("Step 2: Bob scans QR-code, sends vg-request"); let bob_chatid = join_securejoin(&bob, &qr).await?; @@ -456,9 +476,20 @@ async fn test_secure_join() -> Result<()> { EmailAddress::new("alice@example.org").unwrap() ); let msg = alice.parse_msg(&sent).await; - assert!(!msg.was_encrypted()); - assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vg-request"); - assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); + assert!(msg.signature.is_none()); + assert_eq!( + msg.get_header(HeaderDef::SecureJoin).unwrap(), + if v3 { + "vc-request-pubkey" + } else { + "vg-request" + } + ); + assert_eq!(msg.get_header(HeaderDef::SecureJoinAuth).is_some(), v3); + assert_eq!( + msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some(), + !v3 + ); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); // Old Delta Chat core sent `Secure-Join-Group` header in `vg-request`, @@ -469,19 +500,18 @@ async fn test_secure_join() -> Result<()> { // is only sent in `vg-request-with-auth` for compatibility. assert!(!msg.header_exists(HeaderDef::SecureJoinGroup)); - tcm.section("Step 3: Alice receives vg-request, sends vg-auth-required"); + tcm.section("Step 3: Alice receives vc-request-pubkey and sends vc-pubkey, or receives vg-request and sends vg-auth-required"); alice.recv_msg_trash(&sent).await; let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vg-auth-required" + if v3 { "vc-pubkey" } else { "vg-auth-required" } ); - tcm.section("Step 4: Bob receives vg-auth-required, sends vg-request-with-auth"); + tcm.section("Step 4: Bob receives vc-pubkey or vg-auth-required, sends v*-request-with-auth"); bob.recv_msg_trash(&sent).await; let sent = bob.pop_sent_msg().await; @@ -511,7 +541,6 @@ async fn test_secure_join() -> Result<()> { } // Check Bob sent the right handshake message. - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); let msg = alice.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( @@ -575,13 +604,21 @@ async fn test_secure_join() -> Result<()> { { // Now Alice's chat with Bob should still be hidden, the verified message should // appear in the group chat. + if v3 { + assert!( + ChatIdBlocked::lookup_by_contact(&alice, contact_bob.id) + .await? + .is_none() + ); + } else { + let chat = alice.get_chat(&bob).await; + assert_eq!( + chat.blocked, + Blocked::Yes, + "Alice's 1:1 chat with Bob is not hidden" + ); + } - let chat = alice.get_chat(&bob).await; - assert_eq!( - chat.blocked, - Blocked::Yes, - "Alice's 1:1 chat with Bob is not hidden" - ); // There should be 2 messages in the chat: // - The ChatProtectionEnabled message // - You added member bob@example.net @@ -640,6 +677,97 @@ async fn test_secure_join() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_broadcast_legacy() -> Result<()> { + test_secure_join_broadcast_ex(false, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_broadcast_v3() -> Result<()> { + test_secure_join_broadcast_ex(true, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_broadcast_v3_without_invite() -> Result<()> { + test_secure_join_broadcast_ex(true, true).await +} + +async fn test_secure_join_broadcast_ex(v3: bool, remove_invite: bool) -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let alice_chat_id = chat::create_broadcast(alice, "Channel".to_string()).await?; + let mut qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + manipulate_qr(v3, remove_invite, &mut qr); + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + + let sent = alice.send_text(alice_chat_id, "Hi channel").await; + assert!(sent.recipients.contains("bob@example.net")); + let rcvd = bob.recv_msg(&sent).await; + assert_eq!(rcvd.chat_id, bob_chat_id); + assert_eq!(rcvd.text, "Hi channel"); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_setup_contact_compatibility_legacy() -> Result<()> { + test_setup_contact_compatibility_ex(false, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_setup_contact_compatibility_v3() -> Result<()> { + test_setup_contact_compatibility_ex(true, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_setup_contact_compatibility_v3_without_invite() -> Result<()> { + test_setup_contact_compatibility_ex(true, true).await +} + +async fn test_setup_contact_compatibility_ex(v3: bool, remove_invite: bool) -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + alice.set_config(Config::Displayname, Some("Alice")).await?; + + let mut qr = get_securejoin_qr(alice, None).await?; + manipulate_qr(v3, remove_invite, &mut qr); + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + + let bob_chat = Chat::load_from_db(bob, bob_chat_id).await?; + assert_eq!(bob_chat.name, "Alice"); + assert!(bob_chat.can_send(bob).await?); + assert_eq!(bob_chat.typ, Chattype::Single); + assert_eq!(bob_chat.id, bob.get_chat(alice).await.id); + + let alice_chat = alice.get_chat(bob).await; + assert_eq!(alice_chat.name, "bob@example.net"); + assert!(alice_chat.can_send(alice).await?); + assert_eq!(alice_chat.typ, Chattype::Single); + + Ok(()) +} + +fn manipulate_qr(v3: bool, remove_invite: bool, qr: &mut String) { + if remove_invite { + // Remove the INVITENUBMER. It's not needed in Securejoin v3, + // but still included for backwards compatibility reasons. + // We want to be able to remove it in the future, + // therefore we test that things work without it. + let new_qr = Regex::new("&i=.*?&").unwrap().replace(qr, "&"); + // Broadcast channels use `j` for the INVITENUMBER + let new_qr = Regex::new("&j=.*?&").unwrap().replace(&new_qr, "&"); + assert!(new_qr != *qr); + *qr = new_qr.to_string(); + } + // If `!v3`, force legacy securejoin to run by removing the &v=3 parameter. + // If `remove_invite`, we can also remove the v=3 parameter, + // because a QR with AUTH but no INVITE is obviously v3 QR code. + if !v3 || remove_invite { + let new_qr = Regex::new("&v=3").unwrap().replace(qr, ""); + assert!(new_qr != *qr); + *qr = new_qr.to_string(); + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_adhoc_group_no_qr() -> Result<()> { let alice = TestContext::new_alice().await; @@ -1370,3 +1498,68 @@ gU6dGXsFMe/RpRHrIAkMAaM5xkxMDRuRJDxiUdS/X+Y8 Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_auth_token_is_synchronized() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice1 = &tcm.alice().await; + let alice2 = &tcm.alice().await; + let bob = &tcm.bob().await; + bob.set_config(Config::Displayname, Some("Bob")).await?; + + alice1.set_config_bool(Config::SyncMsgs, true).await?; + alice2.set_config_bool(Config::SyncMsgs, true).await?; + + // This creates first auth token: + let qr1 = get_securejoin_qr(alice1, None).await?; + + // This creates another auth token; both of them need to be synchronized + let qr2 = get_securejoin_qr(alice1, None).await?; + sync(alice1, alice2).await; + + // Note that Bob will throw away the AUTH token after sending `vc-request-with-auth`. + // Therefore, he will fail to decrypt the answer from Alice's second device, + // which leads to a "decryption failed: missing key" message in the logs. + // This is fine. + tcm.exec_securejoin_qr_multi_device(bob, &[alice1, alice2], &qr2) + .await; + + let contacts = Contact::get_all(alice2, 0, Some("Bob")).await?; + assert_eq!(contacts[0], alice2.add_or_lookup_contact_id(bob).await); + assert_eq!(contacts.len(), 1); + + let chatlist = Chatlist::try_load(alice2, 0, Some("Bob"), None).await?; + assert_eq!(chatlist.get_chat_id(0)?, alice2.get_chat(bob).await.id); + assert_eq!(chatlist.len(), 1); + + for qr in [qr1, qr2] { + let qr = check_qr(bob, &qr).await?; + let qr = QrInvite::try_from(qr)?; + assert!(token::exists(alice2, Namespace::InviteNumber, qr.invitenumber()).await?); + assert!(token::exists(alice2, Namespace::Auth, qr.authcode()).await?); + } + + // Check that alice2 only saves the invite number once: + let invite_count: u32 = alice2 + .sql + .query_get_value( + "SELECT COUNT(*) FROM tokens WHERE namespc=?;", + (Namespace::InviteNumber,), + ) + .await? + .unwrap(); + assert_eq!(invite_count, 1); + + // ...but knows two AUTH tokens: + let auth_count: u32 = alice2 + .sql + .query_get_value( + "SELECT COUNT(*) FROM tokens WHERE namespc=?;", + (Namespace::Auth,), + ) + .await? + .unwrap(); + assert_eq!(auth_count, 2); + + Ok(()) +} diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 11a2fb48b8..b8889dd387 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -1567,6 +1567,27 @@ ALTER TABLE contacts ADD COLUMN name_normalized TEXT; .await?; } + // Add UNIQUE bound to token, in order to avoid saving the same token multiple times + inc_and_check(&mut migration_version, 148)?; + if dbversion < migration_version { + sql.execute_migration( + "CREATE TABLE tokens_new ( + id INTEGER PRIMARY KEY, + namespc INTEGER DEFAULT 0, + foreign_key TEXT DEFAULT '', + token TEXT DEFAULT '' UNIQUE, + timestamp INTEGER DEFAULT 0, + foreign_id INTEGER NOT NULL DEFAULT 0 + ) STRICT; + INSERT OR IGNORE INTO tokens_new + SELECT id, namespc, foreign_key, token, timestamp, foreign_id FROM tokens; + DROP TABLE tokens; + ALTER TABLE tokens_new RENAME TO tokens;", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await? diff --git a/src/token.rs b/src/token.rs index 1a189a151f..2569e7a0ac 100644 --- a/src/token.rs +++ b/src/token.rs @@ -30,10 +30,14 @@ pub async fn save( token: &str, timestamp: i64, ) -> Result<()> { + if token.is_empty() { + info!(context, "Not saving empty {namespace} token"); + return Ok(()); + } context .sql .execute( - "INSERT INTO tokens (namespc, foreign_key, token, timestamp) VALUES (?, ?, ?, ?)", + "INSERT OR IGNORE INTO tokens (namespc, foreign_key, token, timestamp) VALUES (?, ?, ?, ?)", (namespace, foreign_key.unwrap_or(""), token, timestamp), ) .await?; @@ -56,12 +60,25 @@ pub async fn lookup( context .sql .query_get_value( - "SELECT token FROM tokens WHERE namespc=? AND foreign_key=? ORDER BY timestamp DESC LIMIT 1", + "SELECT token FROM tokens WHERE namespc=? AND foreign_key=? ORDER BY id DESC LIMIT 1", (namespace, foreign_key.unwrap_or("")), ) .await } +pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result> { + context + .sql + .query_map_vec( + // `ORDER BY id DESC` in order to try the most-recently saved tokens first. + // This improves performance when Bob scans a QR code that was just created. + "SELECT token FROM tokens WHERE namespc=? ORDER BY id DESC", + (namespace,), + |row| Ok(row.get(0)?), + ) + .await +} + pub async fn lookup_or_new( context: &Context, namespace: Namespace, diff --git a/test-data/golden/test_broadcast_joining_golden_alice b/test-data/golden/test_broadcast_joining_golden_alice index 260726c625..2aaf2fdbc8 100644 --- a/test-data/golden/test_broadcast_joining_golden_alice +++ b/test-data/golden/test_broadcast_joining_golden_alice @@ -2,5 +2,5 @@ OutBroadcast#Chat#1001: My Channel [1 member(s)] Icon: e9b6c7a78aa2e4f415644f55a -------------------------------------------------------------------------------- Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#1002🔒: Me (Contact#Contact#Self): You changed the group image. [INFO] √ -Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1005🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_private_chat b/test-data/golden/test_broadcast_joining_golden_private_chat index d4a1739d41..a77ef169a8 100644 --- a/test-data/golden/test_broadcast_joining_golden_private_chat +++ b/test-data/golden/test_broadcast_joining_golden_private_chat @@ -1,4 +1,4 @@ Single#Chat#1002: bob@example.net [KEY bob@example.net] -------------------------------------------------------------------------------- -Msg#1003: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#1007: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice1 b/test-data/golden/test_sync_broadcast_alice1 index d0d42cc228..7d450fb717 100644 --- a/test-data/golden/test_sync_broadcast_alice1 +++ b/test-data/golden/test_sync_broadcast_alice1 @@ -1,7 +1,7 @@ OutBroadcast#Chat#1001: Channel [0 member(s)] -------------------------------------------------------------------------------- Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#1007🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#1009🔒: Me (Contact#Contact#Self): hi √ -Msg#1010🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1008🔒: Me (Contact#Contact#Self): hi √ +Msg#1009🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice2 b/test-data/golden/test_sync_broadcast_alice2 index ef143e0d8d..6672cd38df 100644 --- a/test-data/golden/test_sync_broadcast_alice2 +++ b/test-data/golden/test_sync_broadcast_alice2 @@ -1,7 +1,7 @@ OutBroadcast#Chat#1001: Channel [0 member(s)] -------------------------------------------------------------------------------- Msg#1002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#1007🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#1009🔒: Me (Contact#Contact#Self): hi √ -Msg#1010🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1008🔒: Me (Contact#Contact#Self): hi √ +Msg#1009🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ --------------------------------------------------------------------------------