From e2b3cc5c88dd8f66ec02fe644944218d58e996d8 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Tue, 22 Sep 2015 17:03:51 +0200 Subject: [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 --- src/leap/mail/incoming/service.py | 37 +++++++++------- src/leap/mail/incoming/tests/test_incoming_mail.py | 51 +++++++++++++++++++++- src/leap/mail/tests/__init__.py | 2 +- 3 files changed, 72 insertions(+), 18 deletions(-) (limited to 'src/leap/mail') 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()) -- cgit v1.2.3