[bug] don't extract openpgp header if valid attached key

The key extract should check first for attached keys and if this fails
then should try the OpenPGP header.

- Resolves: #7480

3 files changed, 72 insertions, 18 deletions

diff --git a/src/leap/mail/incoming/service.py b/src/leap/mail/incoming/service.py
index ec85eed..4ae4a40 100644
--- a/src/leap/mail/incoming/service.py
+++ b/src/leap/mail/incoming/service.py
@@ -304,7 +304,7 @@ class IncomingMail(Service):
                 logger.debug("skipping msg with decrypting errors...")
             elif self._is_msg(keys):
                 d = self._decrypt_doc(doc)
-                d.addCallback(self._extract_keys)
+                d.addCallback(self._maybe_extract_keys)
                 d.addCallbacks(self._add_message_locally, self._errback)
                 deferreds.append(d)
         d = defer.gatherResults(deferreds, consumeErrors=True)
@@ -594,7 +594,8 @@ class IncomingMail(Service):
         else:
             return failure
 
-    def _extract_keys(self, msgtuple):
+    @defer.inlineCallbacks
+    def _maybe_extract_keys(self, msgtuple):
         """
         Retrieve attached keys to the mesage and parse message headers for an
         *OpenPGP* header as described on the `IETF draft
@@ -621,20 +622,19 @@ class IncomingMail(Service):
         msg = self._parser.parsestr(data)
         _, fromAddress = parseaddr(msg['from'])
 
-        header = msg.get(OpenPGP_HEADER, None)
-        dh = defer.succeed(None)
-        if header is not None:
-            dh = self._extract_openpgp_header(header, fromAddress)
-
-        da = defer.succeed(None)
+        valid_attachment = False
         if msg.is_multipart():
-            da = self._extract_attached_key(msg.get_payload(), fromAddress)
+            valid_attachment = yield self._maybe_extract_attached_key(
+                msg.get_payload(), fromAddress)
 
-        d = defer.gatherResults([dh, da])
-        d.addCallback(lambda _: msgtuple)
-        return d
+        if not valid_attachment:
+            header = msg.get(OpenPGP_HEADER, None)
+            if header is not None:
+                yield self._maybe_extract_openpgp_header(header, fromAddress)
 
-    def _extract_openpgp_header(self, header, address):
+        defer.returnValue(msgtuple)
+
+    def _maybe_extract_openpgp_header(self, header, address):
         """
         Import keys from the OpenPGP header
 
@@ -679,7 +679,7 @@ class IncomingMail(Service):
                          % (header,))
         return d
 
-    def _extract_attached_key(self, attachments, address):
+    def _maybe_extract_attached_key(self, attachments, address):
         """
         Import keys from the attachments
 
@@ -689,6 +689,8 @@ class IncomingMail(Service):
         :type address: str
 
         :return: A Deferred that will be fired when all the keys are stored
+                 with a boolean True if there was a  valid key attached or
+                 False in other case
         :rtype: Deferred
         """
         MIME_KEY = "application/pgp-keys"
@@ -696,6 +698,7 @@ class IncomingMail(Service):
         def failed_put_key(failure):
             logger.info("An error has ocurred adding attached key for %s: %s"
                         % (address, failure.getErrorMessage()))
+            return False
 
         deferreds = []
         for attachment in attachments:
@@ -705,9 +708,11 @@ class IncomingMail(Service):
                     attachment.get_payload(),
                     OpenPGPKey,
                     address=address)
-                d.addErrback(failed_put_key)
+                d.addCallbacks(lambda _: True, failed_put_key)
                 deferreds.append(d)
-        return defer.gatherResults(deferreds)
+        d = defer.gatherResults(deferreds)
+        d.addCallback(lambda result: any(result))
+        return d
 
     def _add_message_locally(self, msgtuple):
         """
diff --git a/src/leap/mail/incoming/tests/test_incoming_mail.py b/src/leap/mail/incoming/tests/test_incoming_mail.py
index 589ddad..964c8fd 100644
--- a/src/leap/mail/incoming/tests/test_incoming_mail.py
+++ b/src/leap/mail/incoming/tests/test_incoming_mail.py
@@ -164,7 +164,6 @@ subject: independence of cyberspace
         message.attach(key)
         self.fetcher._keymanager.put_raw_key = Mock(
             return_value=defer.succeed(None))
-        self.fetcher._keymanager.fetch_key = Mock()
 
         def put_raw_key_called(_):
             self.fetcher._keymanager.put_raw_key.assert_called_once_with(
@@ -184,11 +183,61 @@ subject: independence of cyberspace
         message.attach(key)
         self.fetcher._keymanager.put_raw_key = Mock(
             return_value=defer.fail(KeyAddressMismatch()))
+
+        def put_raw_key_called(_):
+            self.fetcher._keymanager.put_raw_key.assert_called_once_with(
+                KEY, OpenPGPKey, address=ADDRESS_2)
+
+        d = self._do_fetch(message.as_string())
+        d.addCallback(put_raw_key_called)
+        return d
+
+    def testExtractAttachedKeyAndNotOpenPGPHeader(self):
+        KEY = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n..."
+        KEYURL = "https://leap.se/key.txt"
+        OpenPGP = "id=12345678; url=\"%s\"; preference=signencrypt" % (KEYURL,)
+
+        message = MIMEMultipart()
+        message.add_header("from", ADDRESS_2)
+        message.add_header("OpenPGP", OpenPGP)
+        key = MIMEApplication("", "pgp-keys")
+        key.set_payload(KEY)
+        message.attach(key)
+
+        self.fetcher._keymanager.put_raw_key = Mock(
+            return_value=defer.succeed(None))
         self.fetcher._keymanager.fetch_key = Mock()
 
         def put_raw_key_called(_):
             self.fetcher._keymanager.put_raw_key.assert_called_once_with(
                 KEY, OpenPGPKey, address=ADDRESS_2)
+            self.assertFalse(self.fetcher._keymanager.fetch_key.called)
+
+        d = self._do_fetch(message.as_string())
+        d.addCallback(put_raw_key_called)
+        return d
+
+    def testExtractOpenPGPHeaderIfInvalidAttachedKey(self):
+        KEY = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n..."
+        KEYURL = "https://leap.se/key.txt"
+        OpenPGP = "id=12345678; url=\"%s\"; preference=signencrypt" % (KEYURL,)
+
+        message = MIMEMultipart()
+        message.add_header("from", ADDRESS_2)
+        message.add_header("OpenPGP", OpenPGP)
+        key = MIMEApplication("", "pgp-keys")
+        key.set_payload(KEY)
+        message.attach(key)
+
+        self.fetcher._keymanager.put_raw_key = Mock(
+            return_value=defer.fail(KeyAddressMismatch()))
+        self.fetcher._keymanager.fetch_key = Mock()
+
+        def put_raw_key_called(_):
+            self.fetcher._keymanager.put_raw_key.assert_called_once_with(
+                KEY, OpenPGPKey, address=ADDRESS_2)
+            self.fetcher._keymanager.fetch_key.assert_called_once_with(
+                ADDRESS_2, KEYURL, OpenPGPKey)
 
         d = self._do_fetch(message.as_string())
         d.addCallback(put_raw_key_called)
diff --git a/src/leap/mail/tests/__init__.py b/src/leap/mail/tests/__init__.py
index de0088f..71452d2 100644
--- a/src/leap/mail/tests/__init__.py
+++ b/src/leap/mail/tests/__init__.py
@@ -91,7 +91,7 @@ class TestCaseWithKeyManager(unittest.TestCase, BaseLeapTest):
 
         nickserver_url = ''  # the url of the nickserver
         self._km = KeyManager(address, nickserver_url, self._soledad,
-                              ca_cert_path='', gpgbinary=self.GPG_BINARY_PATH)
+                              gpgbinary=self.GPG_BINARY_PATH)
         self._km._fetcher.put = Mock()
         self._km._fetcher.get = Mock(return_value=Response())