Skip to content

feat: remove hidden headers #8543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
link2xt wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −148
Open

feat: remove hidden headers #8543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 6 additions & 129 deletions src/mimefactory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::key::{DcKey, SignedPublicKey, SignedSecretKey, load_self_public_key,
use crate::location;
use crate::log::warn;
use crate::message::{Message, MsgId, Viewtype};
use crate::mimeparser::{SystemMessage, is_hidden};
use crate::mimeparser::SystemMessage;
use crate::param::Param;
use crate::peer_channels::{create_iroh_header, get_iroh_topic_for_msg};
use crate::pgp::{SeipdVersion, addresses_from_public_key, pubkey_supports_seipdv2};
Expand Down Expand Up @@ -1535,52 +1535,20 @@ impl MimeFactory {
let is_mdn = matches!(self.loaded, Loaded::Mdn { .. });
let should_sign = true;

let HeadersByConfidentiality {
mut unprotected_headers,
hidden_headers,
protected_headers,
} = group_headers_by_confidentiality(headers);

let message = if self.encryption_pubkeys.is_some() {
add_headers_to_encrypted_part(message, hidden_headers, protected_headers)
add_headers_to_encrypted_part(message, headers)
} else if is_mdn {
// Never add outer multipart/mixed wrapper to MDN
// as multipart/report Content-Type is used to recognize MDNs
// by Delta Chat receiver and Chatmail servers
// allowing them to be unencrypted and not contain Autocrypt header
// without resetting Autocrypt encryption or triggering Chatmail filter
// that normally only allows encrypted mails.

// Hidden headers are dropped.
message
} else {
let message = hidden_headers
.into_iter()
.fold(message, |message, (header, value)| {
message.header(header, value)
});
let message = message.header(
"Message-ID",
mail_builder::headers::message_id::MessageId::new(rfc724_mid.clone()),
);
let message = MimePart::new("multipart/mixed", vec![message]);
let message = protected_headers
.iter()
.fold(message, |message, (header, value)| {
message.header(*header, value.clone())
});

// Deduplicate unprotected headers that also are in the protected headers:
let protected: HashSet<&str> =
HashSet::from_iter(protected_headers.iter().map(|(header, _value)| *header));
unprotected_headers.retain(|(header, _value)| !protected.contains(header));

unprotected_headers
.iter()
.cloned()
.fold(message, |message, (header, value)| {
message.header(header, value)
})
headers.iter().fold(message, |message, (header, value)| {
message.header(*header, value.clone())
})
};
let raw_message = part_to_bytes(message);

Expand Down Expand Up @@ -2278,14 +2246,11 @@ pub(crate) fn wrap_encrypted_part(encrypted: String) -> MimePart<'static> {

fn add_headers_to_encrypted_part(
message: MimePart<'static>,
hidden_headers: Vec<(&'static str, HeaderType<'static>)>,
protected_headers: Vec<(&'static str, HeaderType<'static>)>,
) -> MimePart<'static> {
// Store protected headers in the inner message.
// Add hidden headers to encrypted payload.
let mut message: MimePart<'static> = protected_headers
.into_iter()
.chain(hidden_headers)
.fold(message, |message, (header, value)| {
message.header(header, value)
});
Expand All @@ -2306,88 +2271,6 @@ fn add_headers_to_encrypted_part(
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>)>,
) -> 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();

for header @ (original_header_name, _header_value) in &headers {
let header_name = original_header_name.to_lowercase();
debug_assert_ne!(header_name, "from");
debug_assert_ne!(header_name, "message-id");
debug_assert_ne!(header_name, "autocrypt");

if is_hidden(&header_name) {
hidden_headers.push(header.clone());
} else if header_name == "to" {
protected_headers.push(header.clone());
unprotected_headers.push(("To", hidden_recipients().into()));
} else if header_name == "chat-broadcast-secret" {
protected_headers.push(header.clone());
} else {
protected_headers.push(header.clone());

match header_name.as_str() {
"subject" => {
unprotected_headers.push((
"Subject",
mail_builder::headers::raw::Raw::new("[...]").into(),
));
}
"chat-version" | "autocrypt-setup-message" | "chat-is-post-message" => {
unprotected_headers.push(header.clone());
}
_ => {
// Other headers are removed from unprotected part.
}
}
}
}
HeadersByConfidentiality {
unprotected_headers,
hidden_headers,
protected_headers,
}
}

fn hidden_recipients() -> Address<'static> {
Address::new_group(Some("hidden-recipients".to_string()), Vec::new())
}
Expand Down Expand Up @@ -2544,13 +2427,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message(
mail_builder::headers::text::Text::new(auth.to_string()).into(),
));

let HeadersByConfidentiality {
hidden_headers,
protected_headers,
..
} = group_headers_by_confidentiality(headers);

let message = add_headers_to_encrypted_part(message, hidden_headers, protected_headers);
let message = add_headers_to_encrypted_part(message, headers);

// Disable compression for SecureJoin to ensure
// there are no compression side channels
Expand Down
30 changes: 13 additions & 17 deletions src/mimefactory/mimefactory_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,11 @@ async fn test_render_reply() {
.unwrap();
}

/// Tests that avatar is not sent in unencrypted messages.
///
/// Avatars for address-contacts are not displayed anyway.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_selfavatar_unencrypted() -> anyhow::Result<()> {
async fn test_no_unencrypted_selfavatar() -> anyhow::Result<()> {
// create chat with bob, set selfavatar
let t = TestContext::new_alice().await;
t.allow_unencrypted().await?;
Expand All @@ -681,24 +684,17 @@ async fn test_selfavatar_unencrypted() -> anyhow::Result<()> {
let mut msg = Message::new_text("this is the text!".to_string());

let sent_msg = t.send_msg(chat.id, &mut msg).await;
let mut payload = sent_msg.payload().splitn(3, "\r\n\r\n");

let outer = payload.next().unwrap();
let inner = payload.next().unwrap();
let body = payload.next().unwrap();

assert_eq!(outer.match_indices("multipart/mixed").count(), 1);
assert_eq!(outer.match_indices("Message-ID:").count(), 1);
assert_eq!(outer.match_indices("Subject:").count(), 1);
assert_eq!(outer.match_indices("Autocrypt:").count(), 1);
assert_eq!(outer.match_indices("Chat-User-Avatar:").count(), 0);
let payload = sent_msg.payload();

assert_eq!(inner.match_indices("text/plain").count(), 1);
assert_eq!(inner.match_indices("Message-ID:").count(), 1);
assert_eq!(inner.match_indices("Chat-User-Avatar:").count(), 0);
assert_eq!(inner.match_indices("Subject:").count(), 0);
assert_eq!(payload.match_indices("multipart/mixed").count(), 0);
assert_eq!(payload.match_indices("Message-ID:").count(), 1);
assert_eq!(payload.match_indices("Subject:").count(), 1);
assert_eq!(payload.match_indices("Autocrypt:").count(), 1);
assert_eq!(payload.match_indices("Chat-User-Avatar:").count(), 0);
assert_eq!(payload.match_indices("text/plain").count(), 1);
assert_eq!(payload.match_indices("Message-ID:").count(), 1);

assert_eq!(body.match_indices("this is the text!").count(), 1);
assert_eq!(payload.match_indices("this is the text!").count(), 1);

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions src/receive_imf/receive_imf_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,10 +920,10 @@ async fn test_classic_mailing_list() -> Result<()> {
assert!(mime.contains("Chat-Version: 1.0\r\n"));
assert!(mime.contains("To: <[email protected]>\r\n"));
assert!(mime.contains("From: <[email protected]>\r\n"));
assert!(mime.contains(
assert!(mime.ends_with(
"\r\n\
\r\n\
Hello mailinglist!\r\n"
Hello mailinglist!"
));

receive_imf(&t.ctx, DC_MAILINGLIST2, false).await?;
Expand Down