From 6902250d6bae1bb75b24d6e6e4f700119498c4ae Mon Sep 17 00:00:00 2001
From: Alexander Krotov <ilabdsf@gmail.com>
Date: Mon, 13 Jul 2020 05:08:42 +0300
Subject: [PATCH] securejoin: do not check the signatures existance twice

Mimeparser.was_encrypted() checks if the message is an Autocrypt encrypted
message. It already means the message has a valid signature.

This commit documents a few functions to make it clear that signatures
stored in Mimeparser must be valid and must always come from encrypted
messages.

Also one unwrap() is eliminated in encrypted_and_signed(). It is possible
to further simplify encrypted_and_signed() by skipping the was_encrypted()
check, because the function only returns true if there is a matching
signature, but it is helpful for debugging to distinguish between
non-Autocrypt messages and messages whose fingerprint does not match.
---
 src/e2ee.rs       |  6 ++++++
 src/mimeparser.rs | 12 ++++++++++++
 src/securejoin.rs | 27 +++++++++++----------------
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/src/e2ee.rs b/src/e2ee.rs
index 7e6a7ce08..6faa0dd81 100644
--- a/src/e2ee.rs
+++ b/src/e2ee.rs
@@ -115,6 +115,12 @@ impl EncryptHelper {
     }
 }
 
+/// Tries to decrypt a message, but only if it is structured as an
+/// Autocrypt message, i.e.  encrypted and signed with a valid
+/// signature.
+///
+/// Returns decrypted body and a set of valid signature fingerprints
+/// if successful.
 pub async fn try_decrypt(
     context: &Context,
     mail: &ParsedMail<'_>,
diff --git a/src/mimeparser.rs b/src/mimeparser.rs
index 71657d5c3..f467538ac 100644
--- a/src/mimeparser.rs
+++ b/src/mimeparser.rs
@@ -46,7 +46,14 @@ pub struct MimeMessage {
     pub from: Vec<SingleInfo>,
     pub chat_disposition_notification_to: Option<SingleInfo>,
     pub decrypting_failed: bool,
+
+    /// Set of valid signature fingerprints if a message is an
+    /// Autocrypt encrypted and signed message.
+    ///
+    /// If a message is not encrypted or the signature is not valid,
+    /// this set is empty.
     pub signatures: HashSet<Fingerprint>,
+
     pub gossipped_addr: HashSet<String>,
     pub is_forwarded: bool,
     pub is_system_message: SystemMessage,
@@ -401,6 +408,11 @@ impl MimeMessage {
         }
     }
 
+    /// Returns true if the message was encrypted as defined in
+    /// Autocrypt standard.
+    ///
+    /// This means the message was both encrypted and signed with a
+    /// valid signature.
     pub fn was_encrypted(&self) -> bool {
         !self.signatures.is_empty()
     }
diff --git a/src/securejoin.rs b/src/securejoin.rs
index b0e4cba71..ef2901ef3 100644
--- a/src/securejoin.rs
+++ b/src/securejoin.rs
@@ -1008,24 +1008,19 @@ fn encrypted_and_signed(
     if !mimeparser.was_encrypted() {
         warn!(context, "Message not encrypted.",);
         false
-    } else if mimeparser.signatures.is_empty() {
-        warn!(context, "Message not signed.",);
-        false
-    } else if expected_fingerprint.is_none() {
+    } else if let Some(expected_fingerprint) = expected_fingerprint {
+        if !mimeparser.signatures.contains(expected_fingerprint) {
+            warn!(
+                context,
+                "Message does not match expected fingerprint {}.", expected_fingerprint,
+            );
+            false
+        } else {
+            true
+        }
+    } else {
         warn!(context, "Fingerprint for comparison missing.");
         false
-    } else if !mimeparser
-        .signatures
-        .contains(expected_fingerprint.unwrap())
-    {
-        warn!(
-            context,
-            "Message does not match expected fingerprint {}.",
-            expected_fingerprint.unwrap(),
-        );
-        false
-    } else {
-        true
     }
 }