Add a config option to sign all messages with Autocrypt header (#3986)

Although it does a little for security, it will help to protect from unwanted server-side
modifications and bugs. And now we have a time to test "multipart/signed" messages compatibility
with other MUAs.
This commit is contained in:
iequidoo
2023-02-18 23:08:10 -03:00
committed by iequidoo
parent 89696582ad
commit d1923d68a5
5 changed files with 154 additions and 2 deletions

View File

@@ -300,6 +300,9 @@ pub enum Config {
/// See `crate::authres::update_authservid_candidates`.
AuthservIdCandidates,
/// Make all outgoing messages with Autocrypt header "multipart/signed".
SignUnencrypted,
/// Let the core save all events to the database.
/// This value is used internally to remember the MsgId of the logging xdc
#[strum(props(default = "0"))]

View File

@@ -763,6 +763,12 @@ impl Context {
.await?
.unwrap_or_default(),
);
res.insert(
"sign_unencrypted",
self.get_config_int(Config::SignUnencrypted)
.await?
.to_string(),
);
res.insert(
"debug_logging",

View File

@@ -124,6 +124,19 @@ impl EncryptHelper {
Ok(ctext)
}
/// Signs the passed-in `mail` using the private key from `context`.
/// Returns the payload and the signature.
pub async fn sign(
self,
context: &Context,
mail: lettre_email::PartBuilder,
) -> Result<(lettre_email::MimeMessage, String)> {
let sign_key = SignedSecretKey::load_self(context).await?;
let mime_message = mail.build();
let signature = pgp::pk_calc_signature(mime_message.as_string().as_bytes(), &sign_key)?;
Ok((mime_message, signature))
}
}
/// Ensures a private key exists for the configured user.

View File

@@ -779,10 +779,36 @@ impl<'a> MimeFactory<'a> {
};
// Store protected headers in the outer message.
headers
let message = headers
.protected
.into_iter()
.fold(message, |message, header| message.header(header))
.fold(message, |message, header| message.header(header));
if self.should_skip_autocrypt()
|| !context.get_config_bool(Config::SignUnencrypted).await?
{
message
} else {
let (payload, signature) = encrypt_helper.sign(context, message).await?;
PartBuilder::new()
.header((
"Content-Type".to_string(),
"multipart/signed; protocol=\"application/pgp-signature\"".to_string(),
))
.child(payload)
.child(
PartBuilder::new()
.content_type(
&"application/pgp-signature; name=\"signature.asc\""
.parse::<mime::Mime>()
.unwrap(),
)
.header(("Content-Description", "OpenPGP digital signature"))
.header(("Content-Disposition", "attachment; filename=\"signature\";"))
.body(signature)
.build(),
)
}
};
// Store the unprotected headers on the outer message.
@@ -2140,6 +2166,96 @@ mod tests {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_selfavatar_unencrypted_signed() {
// create chat with bob, set selfavatar
let t = TestContext::new_alice().await;
t.set_config(Config::SignUnencrypted, Some("1"))
.await
.unwrap();
let chat = t.create_chat_with_contact("bob", "bob@example.org").await;
let file = t.dir.path().join("avatar.png");
let bytes = include_bytes!("../test-data/image/avatar64x64.png");
tokio::fs::write(&file, bytes).await.unwrap();
t.set_config(Config::Selfavatar, Some(file.to_str().unwrap()))
.await
.unwrap();
// send message to bob: that should get multipart/mixed because of the avatar moved to inner header;
// make sure, `Subject:` stays in the outer header (imf header)
let mut msg = Message::new(Viewtype::Text);
msg.set_text(Some("this is the text!".to_string()));
let sent_msg = t.send_msg(chat.id, &mut msg).await;
let mut payload = sent_msg.payload().splitn(4, "\r\n\r\n");
let part = payload.next().unwrap();
assert_eq!(part.match_indices("multipart/signed").count(), 1);
assert_eq!(part.match_indices("Subject:").count(), 0);
assert_eq!(part.match_indices("Autocrypt:").count(), 1);
assert_eq!(part.match_indices("Chat-User-Avatar:").count(), 0);
let part = payload.next().unwrap();
assert_eq!(part.match_indices("multipart/mixed").count(), 1);
assert_eq!(part.match_indices("Subject:").count(), 1);
assert_eq!(part.match_indices("Autocrypt:").count(), 0);
assert_eq!(part.match_indices("Chat-User-Avatar:").count(), 0);
let part = payload.next().unwrap();
assert_eq!(part.match_indices("text/plain").count(), 1);
assert_eq!(part.match_indices("Chat-User-Avatar:").count(), 1);
assert_eq!(part.match_indices("Subject:").count(), 0);
let body = payload.next().unwrap();
assert_eq!(body.match_indices("this is the text!").count(), 1);
let bob = TestContext::new_bob().await;
bob.recv_msg(&sent_msg).await;
let alice_id = Contact::lookup_id_by_addr(&bob.ctx, "alice@example.org", Origin::Unknown)
.await
.unwrap()
.unwrap();
let alice_contact = Contact::load_from_db(&bob.ctx, alice_id).await.unwrap();
assert!(alice_contact
.get_profile_image(&bob.ctx)
.await
.unwrap()
.is_some());
// if another message is sent, that one must not contain the avatar
// and no artificial multipart/mixed nesting
let sent_msg = t.send_msg(chat.id, &mut msg).await;
let mut payload = sent_msg.payload().splitn(3, "\r\n\r\n");
let part = payload.next().unwrap();
assert_eq!(part.match_indices("multipart/signed").count(), 1);
assert_eq!(part.match_indices("Subject:").count(), 0);
assert_eq!(part.match_indices("Autocrypt:").count(), 1);
assert_eq!(part.match_indices("Chat-User-Avatar:").count(), 0);
let part = payload.next().unwrap();
assert_eq!(part.match_indices("text/plain").count(), 1);
assert_eq!(part.match_indices("Subject:").count(), 1);
assert_eq!(part.match_indices("Autocrypt:").count(), 0);
assert_eq!(part.match_indices("multipart/mixed").count(), 0);
assert_eq!(part.match_indices("Chat-User-Avatar:").count(), 0);
let body = payload.next().unwrap();
assert_eq!(body.match_indices("this is the text!").count(), 1);
assert_eq!(body.match_indices("text/plain").count(), 0);
assert_eq!(body.match_indices("Chat-User-Avatar:").count(), 0);
assert_eq!(body.match_indices("Subject:").count(), 0);
bob.recv_msg(&sent_msg).await;
let alice_contact = Contact::load_from_db(&bob.ctx, alice_id).await.unwrap();
assert!(alice_contact
.get_profile_image(&bob.ctx)
.await
.unwrap()
.is_some());
}
/// Test that removed member address does not go into the `To:` field.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_remove_member_bcc() -> Result<()> {

View File

@@ -262,6 +262,20 @@ pub async fn pk_encrypt(
.await?
}
/// Signs `plain` text using `private_key_for_signing`.
pub fn pk_calc_signature(
plain: &[u8],
private_key_for_signing: &SignedSecretKey,
) -> Result<String> {
let msg = Message::new_literal_bytes("", plain).sign(
private_key_for_signing,
|| "".into(),
Default::default(),
)?;
let signature = msg.into_signature().to_armored_string(None)?;
Ok(signature)
}
/// Decrypts the message with keys from the private key keyring.
///
/// Receiver private keys are provided in