From 04ca103494badaab9fa7ccfd45f7b6aa5f253c7b Mon Sep 17 00:00:00 2001 From: "Piotr F. Mieszkowski" Date: Fri, 1 Mar 2024 20:13:16 +0100 Subject: [PATCH 1/2] Fix unencrypted delivery in case of message generation failure When we fail to produce byte representation of the email message being processed, we may end up bouncing a message. An example of such case would be a message with a Message-Id header that Python's email parser library cannot process. In such cases, just take whatever original content we have received and pass it to the destination without touching it to minimise any chances of breaking the overall flow. --- lacre/config.py | 5 +++++ lacre/daemon.py | 16 +++++++--------- test/modules/test_contracts.py | 23 ++++++++++++++++++++++- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/lacre/config.py b/lacre/config.py index f31b6e2..6d85115 100644 --- a/lacre/config.py +++ b/lacre/config.py @@ -143,6 +143,11 @@ def strict_mode(): return ("default" in cfg and cfg["default"]["enc_keymap_only"] == "yes") +def should_log_headers() -> bool: + """Check if Lacre should log message headers.""" + return flag_enabled('daemon', 'log_headers') + + class FromStrMixin: """Additional operations for configuration enums.""" diff --git a/lacre/daemon.py b/lacre/daemon.py index 093731d..4c93edd 100644 --- a/lacre/daemon.py +++ b/lacre/daemon.py @@ -42,9 +42,6 @@ class MailEncryptionProxy: if message.defects: LOG.warning("Issues found: %d; %s", len(message.defects), repr(message.defects)) - if conf.flag_enabled('daemon', 'log_headers'): - LOG.info('Message headers: %s', self._extract_headers(message)) - send = xport.SendFrom(envelope.mail_from) for operation in gate.delivery_plan(envelope.rcpt_tos, message, keys): LOG.debug(f"Sending mail via {operation!r}") @@ -55,21 +52,22 @@ class MailEncryptionProxy: # If the message can't be encrypted, deliver cleartext. LOG.error('Unable to encrypt message, delivering in cleartext: %s', e) if not isinstance(operation, KeepIntact): - self._send_unencrypted(operation, message, envelope, send) + self._send_unencrypted(operation, envelope, send) else: LOG.exception('Cannot perform: %s', operation) raise except: LOG.exception('Unexpected exception caught, bouncing message') - return xport.RESULT_ERROR + if conf.should_log_headers(): + LOG.info('Message headers: %s', self._extract_headers(message)) + return xport.RESULT_ERRORR return xport.RESULT_OK - def _send_unencrypted(self, operation, message, envelope, send: xport.SendFrom): - keep = KeepIntact(operation.recipients()) - new_message = keep.perform(message) - send(new_message, operation.recipients()) + def _send_unencrypted(self, operation, envelope, send: xport.SendFrom): + # Do not parse and re-generate the message, just send it as it is. + send(envelope.original_content, operation.recipients()) def _beginning(self, e: Envelope) -> bytes: double_eol_pos = e.original_content.find(DOUBLE_EOL_BYTES) diff --git a/test/modules/test_contracts.py b/test/modules/test_contracts.py index ac14e1a..1823faf 100644 --- a/test/modules/test_contracts.py +++ b/test/modules/test_contracts.py @@ -27,7 +27,8 @@ documentation. import email import email.mime.multipart from email.message import EmailMessage -from email.policy import SMTP +from email.policy import SMTP, SMTPUTF8 +from email.errors import HeaderParseError import unittest from configparser import RawConfigParser @@ -165,6 +166,26 @@ class EmailParsingTest(unittest.TestCase): self.assertIsInstance(payload, str) self.assertTrue(message_boundary in payload) + def test_fail_if_message_id_parsing_is_fixed(self): + # Unfortunately, Microsoft sends messages with Message-Id header values + # that email parser can't process. + # + # Bug: https://github.com/python/cpython/issues/105802 + # Fix: https://github.com/python/cpython/pull/108133 + + rawmsg = b"From: alice@lacre.io\r\n" \ + + b"To: bob@lacre.io\r\n" \ + + b"Subject: Test message\r\n" \ + + b"Content-Type: text/plain\r\n" \ + + b"Content-Transfer-Encoding: base64\r\n" \ + + b"Message-Id: <[yada-yada-yada@microsoft.com]>\r\n" \ + + b"\r\n" \ + + b"SGVsbG8sIFdvcmxkIQo=\r\n" + + msg = email.message_from_bytes(rawmsg, policy=SMTPUTF8) + self.assertEqual(len(msg.defects), 0) + self.assertRaises(IndexError, lambda: msg['Message-Id']) + class EmailTest(unittest.TestCase): def test_boundary_generated_after_as_string_call(self): -- 2.30.2 From 7806d8c32af119559819d9f83307aa9ab289db7e Mon Sep 17 00:00:00 2001 From: "Piotr F. Mieszkowski" Date: Fri, 1 Mar 2024 20:28:51 +0100 Subject: [PATCH 2/2] Log message headers on a hard error When we know we need to bounce a message and [daemon]log_headers is enabled, we record up to 2.5kB of message headers at ERROR level. This could help diagnosing issues later. Also: no longer record MIME Type, Charset and Content-Transfer-Encoding, as the issues related to these properties no longer occur. --- lacre/daemon.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lacre/daemon.py b/lacre/daemon.py index 4c93edd..09af75f 100644 --- a/lacre/daemon.py +++ b/lacre/daemon.py @@ -59,8 +59,10 @@ class MailEncryptionProxy: except: LOG.exception('Unexpected exception caught, bouncing message') + if conf.should_log_headers(): - LOG.info('Message headers: %s', self._extract_headers(message)) + LOG.error('Erroneous message headers: %s', self._beginning(envelope)) + return xport.RESULT_ERRORR return xport.RESULT_OK @@ -78,13 +80,6 @@ class MailEncryptionProxy: end = min(limit, 2560) return e.original_content[0:end] - def _extract_headers(self, message: email.message.Message): - return { - 'mime' : message.get_content_type(), - 'charsets' : message.get_charsets(), - 'cte' : message['Content-Transfer-Encoding'] - } - def _seconds_between(self, start_ms, end_ms) -> float: return (end_ms - start_ms) * 1000 -- 2.30.2