diff --git a/CHANGELOG.md b/CHANGELOG.md index 359ab0d20f..616b11545b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - BREAKING: jsonrpc: - `get_chatlist_items_by_entries` now takes only chatids instead of `ChatListEntries` - `get_chatlist_entries` now returns `Vec` of chatids instead of `ChatListEntries` +- BREAKING: Remove Secure-Join-Fingerprint header from "vc-contact-confirm", "vg-member-added" messages. +- BREAKING: Remove "vc-contact-confirm-received", "vg-member-added-received" messages from Securejoin protocol. ## [1.114.0] - 2023-04-24 diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 24a22de0e6..d8a48e6cad 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -945,17 +945,6 @@ impl<'a> MimeFactory<'a> { "Secure-Join".to_string(), "vg-member-added".to_string(), )); - // FIXME: Old clients require Secure-Join-Fingerprint header. Remove this - // eventually. - let fingerprint = Peerstate::from_addr(context, email_to_add) - .await? - .context("No peerstate found in db")? - .public_key_fingerprint - .context("No public key fingerprint in db for the member to add")?; - headers.protected.push(Header::new( - "Secure-Join-Fingerprint".into(), - fingerprint.hex(), - )); } } SystemMessage::GroupNameChanged => { diff --git a/src/securejoin.rs b/src/securejoin.rs index f854b7d980..642d5c822e 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -9,7 +9,7 @@ use crate::aheader::EncryptPreference; use crate::chat::{self, Chat, ChatId, ChatIdBlocked}; use crate::config::Config; use crate::constants::Blocked; -use crate::contact::{Contact, ContactId, Origin, VerifiedStatus}; +use crate::contact::{Contact, ContactId, Origin}; use crate::context::Context; use crate::e2ee::ensure_secret_key_exists; use crate::events::EventType; @@ -465,14 +465,9 @@ pub(crate) async fn handle_securejoin_handshake( info_chat_id(context, contact_id).await?, ) .await?; - send_alice_handshake_msg( - context, - contact_id, - "vc-contact-confirm", - Some(fingerprint), - ) - .await - .context("failed sending vc-contact-confirm message")?; + send_alice_handshake_msg(context, contact_id, "vc-contact-confirm", None) + .await + .context("failed sending vc-contact-confirm message")?; inviter_progress!(context, contact_id, 1000); } @@ -494,33 +489,8 @@ pub(crate) async fn handle_securejoin_handshake( } } "vg-member-added-received" | "vc-contact-confirm-received" => { - /*========================================================== - ==== Alice - the inviter side ==== - ==== Step 8 in "Out-of-band verified groups" protocol ==== - ==========================================================*/ - - if let Ok(contact) = Contact::get_by_id(context, contact_id).await { - if contact.is_verified(context).await? == VerifiedStatus::Unverified { - warn!(context, "{} invalid.", step); - return Ok(HandshakeMessage::Ignore); - } - if join_vg { - let field_grpid = mime_message - .get_header(HeaderDef::SecureJoinGroup) - .map(|s| s.as_str()) - .unwrap_or_else(|| ""); - if let Err(err) = chat::get_chat_id_by_grpid(context, field_grpid).await { - warn!(context, "Failed to lookup chat_id from grpid: {}", err); - return Err( - err.context(format!("Chat for group {} not found", &field_grpid)) - ); - } - } - Ok(HandshakeMessage::Ignore) // "Done" deletes the message and breaks multi-device - } else { - warn!(context, "{} invalid.", step); - Ok(HandshakeMessage::Ignore) - } + // The peer isn't updated yet and still sends these messages. + Ok(HandshakeMessage::Ignore) } _ => { warn!(context, "invalid step: {}", step); @@ -540,12 +510,6 @@ pub(crate) async fn handle_securejoin_handshake( /// The inviting device has marked a peer as verified on vg-request-with-auth/vc-request-with-auth /// before sending vg-member-added/vc-contact-confirm - so, if we observe vg-member-added/vc-contact-confirm, /// we can mark the peer as verified as well. -/// -/// - if we see the self-sent-message vg-member-added-received -/// we know that we're an joiner-observer. -/// the joining device has marked the peer as verified on vg-member-added/vc-contact-confirm -/// before sending vg-member-added-received - so, if we observe vg-member-added-received, -/// we can mark the peer as verified as well. pub(crate) async fn observe_securejoin_on_other_device( context: &Context, mime_message: &MimeMessage, @@ -563,9 +527,7 @@ pub(crate) async fn observe_securejoin_on_other_device( "vg-request-with-auth" | "vc-request-with-auth" | "vg-member-added" - | "vc-contact-confirm" - | "vg-member-added-received" - | "vc-contact-confirm-received" => { + | "vc-contact-confirm" => { if !encrypted_and_signed( context, mime_message, @@ -631,32 +593,6 @@ pub(crate) async fn observe_securejoin_on_other_device( } peerstate.prefer_encrypt = EncryptPreference::Mutual; peerstate.save_to_db(&context.sql).await.unwrap_or_default(); - } else if let Some(fingerprint) = - mime_message.get_header(HeaderDef::SecureJoinFingerprint) - { - // FIXME: Old versions of DC send this header instead of gossips. Remove this - // eventually. - let fingerprint = fingerprint.parse()?; - if mark_peer_as_verified( - context, - fingerprint, - Contact::load_from_db(context, contact_id) - .await? - .get_addr() - .to_owned(), - ) - .await - .is_err() - { - could_not_establish_secure_connection( - context, - contact_id, - info_chat_id(context, contact_id).await?, - format!("Fingerprint mismatch on observing {step}.").as_ref(), - ) - .await?; - return Ok(HandshakeMessage::Ignore); - } } else { could_not_establish_secure_connection( context, @@ -960,7 +896,7 @@ mod tests { VerifiedStatus::Unverified ); - // Step 7: Bob receives vc-contact-confirm, sends vc-contact-confirm-received + // Step 7: Bob receives vc-contact-confirm bob.recv_msg(&sent).await; assert_eq!( contact_alice.is_verified(&bob.ctx).await.unwrap(), @@ -985,15 +921,6 @@ mod tests { let text = msg.get_text().unwrap(); assert!(text.contains("alice@example.org verified")); } - - // Check Bob sent the final message - let sent = bob.pop_sent_msg().await; - let msg = alice.parse_msg(&sent).await; - assert!(msg.was_encrypted()); - assert_eq!( - msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vc-contact-confirm-received" - ); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -1111,20 +1038,12 @@ mod tests { VerifiedStatus::Unverified ); - // Step 7: Bob receives vc-contact-confirm, sends vc-contact-confirm-received + // Step 7: Bob receives vc-contact-confirm bob.recv_msg(&sent).await; assert_eq!( contact_alice.is_verified(&bob.ctx).await?, VerifiedStatus::BidirectVerified ); - - let sent = bob.pop_sent_msg().await; - let msg = alice.parse_msg(&sent).await; - assert!(msg.was_encrypted()); - assert_eq!( - msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vc-contact-confirm-received" - ); Ok(()) } @@ -1308,7 +1227,7 @@ mod tests { VerifiedStatus::Unverified ); - // Step 7: Bob receives vg-member-added, sends vg-member-added-received + // Step 7: Bob receives vg-member-added bob.recv_msg(&sent).await; { // Bob has Alice verified, message shows up in the group chat. @@ -1355,14 +1274,6 @@ mod tests { } } - let sent = bob.pop_sent_msg().await; - let msg = alice.parse_msg(&sent).await; - assert!(msg.was_encrypted()); - assert_eq!( - msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vg-member-added-received" - ); - let bob_chat = Chat::load_from_db(&bob.ctx, bob_chatid).await?; assert!(bob_chat.is_protected()); assert!(bob_chat.typ == Chattype::Group); diff --git a/src/securejoin/bobstate.rs b/src/securejoin/bobstate.rs index 04b1edd333..dc9406d758 100644 --- a/src/securejoin/bobstate.rs +++ b/src/securejoin/bobstate.rs @@ -386,17 +386,6 @@ impl BobState { } } - self.send_handshake_message(context, BobHandshakeMsg::ContactConfirmReceived) - .await - .map_err(|_| { - warn!( - context, - "Failed to send vc-contact-confirm-received/vg-member-added-received" - ); - }) - // This is not an error affecting the protocol outcome. - .ok(); - self.update_next(&context.sql, SecureJoinStep::Completed) .await?; Ok(Some(BobHandshakeStage::Completed)) @@ -442,9 +431,6 @@ async fn send_handshake_message( msg.param.set(Param::Arg2, invite.authcode()); msg.param.set_int(Param::GuaranteeE2ee, 1); } - BobHandshakeMsg::ContactConfirmReceived => { - msg.param.set_int(Param::GuaranteeE2ee, 1); - } }; // Sends our own fingerprint in the Secure-Join-Fingerprint header. @@ -466,8 +452,6 @@ enum BobHandshakeMsg { Request, /// vc-request-with-auth or vg-request-with-auth RequestWithAuth, - /// vc-contact-confirm-received or vg-member-added-received - ContactConfirmReceived, } impl BobHandshakeMsg { @@ -495,10 +479,6 @@ impl BobHandshakeMsg { QrInvite::Contact { .. } => "vc-request-with-auth", QrInvite::Group { .. } => "vg-request-with-auth", }, - Self::ContactConfirmReceived => match invite { - QrInvite::Contact { .. } => "vc-contact-confirm-received", - QrInvite::Group { .. } => "vg-member-added-received", - }, } } }