v3 of repairing Mixed-up mangled MIME messages

v3 of repairing Mixed-up mangled MIME messages

[Daniel Kahn Gillmor]

This is the third revision of the "Mixed up Mangling" series.  Version
1 was at id:20190528225452.17550-1-dkg@fifthhorseman.net.  Version 2
can be found at id:20190530172707.10378-1-dkg@fifthhorseman.net.

The main difference here is that this series now depends on the
two-part series "Setup for message repair", found at
id:20190531074027.16337-1-dkg@fifthhorseman.net, and should be
somewhat easier to merge with v2 of the "skipping legacy display"
series.  (previous versions of the "legacy display" and "mixed up"
series could fail in subtle ways when merged in the wrong order)

Comments welcome, as always!

         --dkg

[PATCH v3 1/4] test: add test for "Mixed-Up Mime" message mangling

[Daniel Kahn Gillmor]

Some MTAs mangle e-mail messages in transit in ways that are
repairable.

Microsoft Exchange (in particular, the version running today on
Office365's mailservers) appears to mangle multipart/encrypted
messages in a way that makes them undecryptable by the recipient.

I've documented this in section 4.1 "Mixed-up encryption" of draft -00
of
https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling

Fortunately, it's possible to repair such a message, and notmuch can
do that so that a user who receives an encrypted message from a user
of office365.com can still decrypt the message.

Enigmail already knows about this particular kind of mangling.  It
describes it as "broken PGP email format probably caused by an old
Exchange server", and it tries to repair by directly changing the
message held by the user.  if this kind of repair goes wrong, the
repair process can cause data loss
(https://sourceforge.net/p/enigmail/bugs/987/, yikes).

The tests introduced here are currently broken.  In subsequent
patches, i'll introduce a non-destructive form of repair for notmuch
so that notmuch users can read mail that has been mangled in this way,
and the tests will succeed.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T351-pgpmime-mangling.sh      | 36 ++++++++++++++++++++++++++++++
 test/corpora/mangling/mixed-up.eml | 33 +++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100755 test/T351-pgpmime-mangling.sh
 create mode 100644 test/corpora/mangling/mixed-up.eml

diff --git a/test/T351-pgpmime-mangling.sh b/test/T351-pgpmime-mangling.sh
new file mode 100755
index 00000000..f65b8a24
--- /dev/null
+++ b/test/T351-pgpmime-mangling.sh
@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+
+test_description='PGP/MIME message mangling'
+. $(dirname "$0")/test-lib.sh || exit 1
+
+add_gnupg_home
+add_email_corpus mangling
+
+bodytext='["body"][0]["content"][1]["content"]="The password is \"abcd1234!\", please do not tell anyone.\n"'
+
+test_begin_subtest "show 'Mixed-Up' mangled PGP/MIME message correctly"
+test_subtest_known_broken
+output=$(notmuch show --format=json --decrypt=true id:mixed-up@mangling.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'body:[0][0][0]'"$bodytext"
+
+test_begin_subtest "reply to 'Mixed-Up' mangled PGP/MIME message correctly"
+test_subtest_known_broken
+output=$(notmuch reply --format=json --decrypt=true id:mixed-up@mangling.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'body:["original"]'"$bodytext"
+
+test_begin_subtest "repaired 'Mixed-up' messages can be found with index.repaired=mixedup"
+test_subtest_known_broken
+output=$(notmuch search --output=messages property:index.repaired=mixedup)
+test_expect_equal "$output" id:mixed-up@mangling.notmuchmail.org
+
+test_begin_subtest "index cleartext of 'Mixed-Up' mangled PGP/MIME message"
+test_expect_success 'notmuch reindex --decrypt=true id:mixed-up@mangling.notmuchmail.org'
+
+test_begin_subtest "search cleartext of 'Mixed-Up' mangled PGP/MIME message"
+test_subtest_known_broken
+output=$(notmuch search --output=messages body:password)
+test_expect_equal "$output" id:mixed-up@mangling.notmuchmail.org
+
+test_done
diff --git a/test/corpora/mangling/mixed-up.eml b/test/corpora/mangling/mixed-up.eml
new file mode 100644
index 00000000..a09f6191
--- /dev/null
+++ b/test/corpora/mangling/mixed-up.eml
@@ -0,0 +1,33 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <mixed-up@mangling.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="=-=-="
+
+--=-=-=
+Content-Type: text/plain; charset="us-ascii"
+Content-Transfer-Encoding: quoted-printable
+
+
+--=-=-=
+Content-Type: application/pgp-encrypted
+Content-Transfer-Encoding: base64
+
+VmVyc2lvbjogMQ0K
+
+--=-=-=
+Content-Type: application/octet-stream
+Content-Transfer-Encoding: base64
+
+LS0tLS1CRUdJTiBQR1AgTUVTU0FHRS0tLS0tDQoNCmhJd0R4RTAyM3ExVXF4WUJCQUNwNzBlN0tQ
+eTlPWWFoZUlya0x6bWhxMWxScW15NTFhTDFqQkwwSy9xTjdyZksNCkJaRUcxY1I4amVMalRGZFBL
+UExWS0pJODByN0ZnS0kweXd2V3ZsNlIxYUUxVHk1Qm5WWFQ5WHpDckVIN2ZxQ2wNClNLSzgyRXZv
+bFhUb2hBWkhVcmg2SzY2ZVFRVFRJQUMxbjdCMEE4aEVyemtnYU00K3NlTjNMbHZlelQ2VExOS00N
+CkFUcHFzRWJNMk1WckdndzBiM29Vc0dHQVBFdDJNbWpORVlzcmlLbnF3dDZkSkRaYy8vWHloamdN
+UWF5aUQ4ZGENCk4xZ1Qzb3FndS9nS0NwQlpEWXpIZjlPdFZpMlVubEZEV3k2cnJNWkxqV0RuSXY0
+dmU5UG4vcW9sd0hWanpkSjENClpmak5DNXQwejNYQURLR3JqTjl3dXRyNHFtN1NUVzFySEFYSFA2
+OFRRVHhJMHFnSktqUFhOS1dFdzZnPQ0KPXBKRzQNCi0tLS0tRU5EIFBHUCBNRVNTQUdFLS0tLS0N
+Cg==
+--=-=-=--
-- 
2.20.1

[PATCH v3 2/4] util/repair: identify and repair "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

This patch implements a functional identification and repair process
for "Mixed Up" MIME messages as described in
https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1

The detection test is not entirely complete, in that it does not
verify the contents of the latter two message subparts, but this is
probably safe to skip, because those two parts are unlikely to be
readable anyway, and the only part we are effectively omitting (the
first subpart) is guaranteed to be empty anyway, so its removal can be
reversed if you want to do so.  I've left FIXMEs in the code so that
anyone excited about adding these additional checks can see where to
put them in.

I'll use this functionality in the next two patches.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 util/repair.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 util/repair.h |  9 ++++++
 2 files changed, 89 insertions(+)

diff --git a/util/repair.c b/util/repair.c
index f91c1244..56d6c5b5 100644
--- a/util/repair.c
+++ b/util/repair.c
@@ -18,4 +18,84 @@
  * Authors: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
  */
 
+#include <stdbool.h>
 #include "repair.h"
+
+/* see
+ * https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1.1 */
+static bool
+_notmuch_is_mixed_up_mangled (GMimeObject *part)
+{
+    GMimeMultipart *mpart = NULL;
+    GMimeObject *first, *second, *third = NULL;
+    char *prelude_string = NULL;
+    bool prelude_is_empty;
+
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (part),
+				       "multipart", "mixed"))
+	return false;
+    if (! GMIME_IS_MULTIPART (part))
+	return false;
+    mpart = GMIME_MULTIPART (part);
+    if (mpart == NULL)
+	return false;
+    if (g_mime_multipart_get_count (mpart) != 3)
+	return false;
+    first = g_mime_multipart_get_part (mpart, 0);
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (first),
+				       "text", "plain"))
+	return false;
+    if (! GMIME_IS_TEXT_PART (first))
+	return false;
+    second = g_mime_multipart_get_part (mpart, 1);
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (second),
+				       "application", "pgp-encrypted"))
+	return false;
+    third = g_mime_multipart_get_part (mpart, 2);
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (third),
+				       "application", "octet-stream"))
+	return false;
+
+    /* Is first subpart length 0? */
+    prelude_string = g_mime_text_part_get_text (GMIME_TEXT_PART (first));
+    prelude_is_empty = ! (strcmp ("", prelude_string));
+    g_free (prelude_string);
+    if (! prelude_is_empty)
+	return false;
+
+    /* FIXME: after decoding and stripping whitespace, is second
+     * subpart just "Version: 1" ? */
+
+    /* FIXME: can we determine that third subpart is *only* PGP
+     * encrypted data?  I tried g_mime_part_get_openpgp_data () but
+     * found https://github.com/jstedfast/gmime/issues/60 */
+
+    return true;
+}
+
+
+/* see
+ * https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1.2 */
+GMimeObject*
+_notmuch_repair_mixed_up_mangled (GMimeObject *part)
+{
+    GMimeMultipart *mpart = NULL, *mpart_ret = NULL;
+    GMimeObject *ret = NULL;
+
+    if (! _notmuch_is_mixed_up_mangled (part))
+	return NULL;
+    mpart = GMIME_MULTIPART (part);
+    ret = GMIME_OBJECT (g_mime_multipart_encrypted_new ());
+    if (ret == NULL)
+	return NULL;
+    mpart_ret = GMIME_MULTIPART (ret);
+    if (mpart_ret == NULL) {
+	g_object_unref (ret);
+	return NULL;
+    }
+    g_mime_object_set_content_type_parameter (ret, "protocol", "application/pgp-encrypted");
+
+    g_mime_multipart_insert (mpart_ret, 0, g_mime_multipart_get_part (mpart, 1));
+    g_mime_multipart_insert (mpart_ret, 1, g_mime_multipart_get_part (mpart, 2));
+    return ret;
+}
diff --git a/util/repair.h b/util/repair.h
index 70e2b7bc..a7fc885a 100644
--- a/util/repair.h
+++ b/util/repair.h
@@ -11,6 +11,15 @@ extern "C" {
  * techniques that are designed to improve the user experience of
  * notmuch */
 
+/* Detecting and repairing "Mixed-Up MIME mangling". see
+ * https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1
+ * If this returns NULL, the message was probably not "Mixed up".  If
+ * it returns non-NULL, then there is a newly-allocated MIME part that
+ * represents the repaired version.  The caller is responsible for
+ * ensuring that any returned object is freed with g_object_unref. */
+GMimeObject*
+_notmuch_repair_mixed_up_mangled (GMimeObject *part);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.20.1

[PATCH v3 3/4] index: repair "Mixed Up" messages before indexing.

[Daniel Kahn Gillmor]

When encountering a message that has been mangled in the "mixed up"
way by an intermediate MTA, notmuch should instead repair it and index
the repaired form.

When it does this, it also associates the index.repaired=mixedup
property with the message.  If a problem is found with this repair
process, or an improved repair process is proposed later, this should
make it easy for people to reindex the relevant message.  The property
will also hopefully make it easier to diagnose this particular problem
in the future.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 doc/man7/notmuch-properties.rst |  6 ++++++
 lib/index.cc                    | 22 +++++++++++++++++-----
 test/T351-pgpmime-mangling.sh   |  2 --
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 2e610683..a6846e11 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -121,6 +121,12 @@ of its normal activity.
     ``index.repaired`` property to note the type of repair(s) it
     performed.
 
+    ``index.repaired=mixedup`` indicates the repair of a "Mixed Up"
+    encrypted PGP/MIME message, a mangling typically produced by
+    Microsoft's Exchange MTA.  See
+    https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling
+    for more information.
+
 SEE ALSO
 ========
 
diff --git a/lib/index.cc b/lib/index.cc
index 1fd9e67e..44a42deb 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -385,11 +385,20 @@ _index_mime_part (notmuch_message_t *message,
     GMimeContentType *content_type;
     char *body;
     const char *charset;
+    GMimeObject *repaired_part = NULL;
 
     if (! part) {
 	_notmuch_database_log (notmuch_message_get_database (message),
 			      "Warning: Not indexing empty mime part.\n");
-	return;
+	goto DONE;
+    }
+
+    repaired_part = _notmuch_repair_mixed_up_mangled (part);
+    if (repaired_part) {
+	/* This was likely "Mixed Up" in transit!  We will instead use
+	 * the more likely-to-be-correct variant. */
+	notmuch_message_add_property (message, "index.repaired", "mixedup");
+	part = repaired_part;
     }
 
     _index_content_type (message, part);
@@ -441,7 +450,7 @@ _index_mime_part (notmuch_message_t *message,
 				       notmuch_status_to_string (status));
 	    _index_mime_part (message, indexopts, child, msg_crypto);
 	}
-	return;
+	goto DONE;
     }
 
     if (GMIME_IS_MESSAGE_PART (part)) {
@@ -451,14 +460,14 @@ _index_mime_part (notmuch_message_t *message,
 
 	_index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message), msg_crypto);
 
-	return;
+	goto DONE;
     }
 
     if (! (GMIME_IS_PART (part))) {
 	_notmuch_database_log (notmuch_message_get_database (message),
 			      "Warning: Not indexing unknown mime part: %s.\n",
 			      g_type_name (G_OBJECT_TYPE (part)));
-	return;
+	goto DONE;
     }
 
     disposition = g_mime_object_get_content_disposition (part);
@@ -473,7 +482,7 @@ _index_mime_part (notmuch_message_t *message,
 
 	/* XXX: Would be nice to call out to something here to parse
 	 * the attachment into text and then index that. */
-	return;
+	goto DONE;
     }
 
     byte_array = g_byte_array_new ();
@@ -519,6 +528,9 @@ _index_mime_part (notmuch_message_t *message,
 
 	free (body);
     }
+ DONE:
+    if (repaired_part)
+	g_object_unref (repaired_part);
 }
 
 /* descend (if desired) into the cleartext part of an encrypted MIME
diff --git a/test/T351-pgpmime-mangling.sh b/test/T351-pgpmime-mangling.sh
index f65b8a24..4555f937 100755
--- a/test/T351-pgpmime-mangling.sh
+++ b/test/T351-pgpmime-mangling.sh
@@ -21,7 +21,6 @@ test_json_nodes <<<"$output" \
                 'body:["original"]'"$bodytext"
 
 test_begin_subtest "repaired 'Mixed-up' messages can be found with index.repaired=mixedup"
-test_subtest_known_broken
 output=$(notmuch search --output=messages property:index.repaired=mixedup)
 test_expect_equal "$output" id:mixed-up@mangling.notmuchmail.org
 
@@ -29,7 +28,6 @@ test_begin_subtest "index cleartext of 'Mixed-Up' mangled PGP/MIME message"
 test_expect_success 'notmuch reindex --decrypt=true id:mixed-up@mangling.notmuchmail.org'
 
 test_begin_subtest "search cleartext of 'Mixed-Up' mangled PGP/MIME message"
-test_subtest_known_broken
 output=$(notmuch search --output=messages body:password)
 test_expect_equal "$output" id:mixed-up@mangling.notmuchmail.org
 
-- 
2.20.1

[PATCH v3 4/4] cli/{show, reply}: use repaired form of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

When showing or replying to a message that has been mangled in transit
by an MTA in the "Mixed up" way, notmuch should instead use the
repaired form of the message.

Tracking the repaired GMimeObject for the lifetime of the mime_node so
that it is cleaned up properly is probably the trickiest part of this
patch, but the choices here are based on the idea that the
mime_node_context is the memory manager for the whole mime_node tree
in the first place, so new GMimeObject tree created on-the-fly during
message parsing should be disposed of in the same place.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 mime-node.c                   | 21 +++++++++++++++++++++
 test/T351-pgpmime-mangling.sh |  2 --
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 0be03de7..559028cc 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -36,6 +36,9 @@ typedef struct mime_node_context {
     GMimeMessage *mime_message;
     _notmuch_message_crypto_t *msg_crypto;
 
+    /* repaired/unmangled parts that will need to be cleaned up */
+    GSList *repaired_parts;
+
     /* Context provided by the caller. */
     _notmuch_crypto_t *crypto;
 } mime_node_context_t;
@@ -52,9 +55,20 @@ _mime_node_context_free (mime_node_context_t *res)
     if (res->stream)
 	g_object_unref (res->stream);
 
+    if (res->repaired_parts)
+	g_slist_free_full (res->repaired_parts, g_object_unref);
+
     return 0;
 }
 
+/* keep track of objects that need to be destroyed when the mime node
+   context goes away. */
+static void
+_mime_node_context_track_repaired_part (mime_node_context_t *ctx, GMimeObject *part) {
+    if (part)
+	ctx->repaired_parts = g_slist_prepend (ctx->repaired_parts, part);
+}
+
 const _notmuch_message_crypto_t*
 mime_node_get_message_crypto_status (mime_node_t *node)
 {
@@ -299,6 +313,13 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild) {
 	node->part = part;
 	node->nchildren = 0;
     } else if (GMIME_IS_MULTIPART (part)) {
+	GMimeObject *repaired_part = _notmuch_repair_mixed_up_mangled (part);
+	if (repaired_part) {
+	    /* This was likely "Mixed Up" in transit!  We replace it
+	     * with the more likely-to-be-correct variant. */
+	    _mime_node_context_track_repaired_part (node->ctx, repaired_part);
+	    part = repaired_part;
+	}
 	node->part = part;
 	node->nchildren = g_mime_multipart_get_count (GMIME_MULTIPART (part));
     } else if (GMIME_IS_MESSAGE_PART (part)) {
diff --git a/test/T351-pgpmime-mangling.sh b/test/T351-pgpmime-mangling.sh
index 4555f937..71a68c05 100755
--- a/test/T351-pgpmime-mangling.sh
+++ b/test/T351-pgpmime-mangling.sh
@@ -9,13 +9,11 @@ add_email_corpus mangling
 bodytext='["body"][0]["content"][1]["content"]="The password is \"abcd1234!\", please do not tell anyone.\n"'
 
 test_begin_subtest "show 'Mixed-Up' mangled PGP/MIME message correctly"
-test_subtest_known_broken
 output=$(notmuch show --format=json --decrypt=true id:mixed-up@mangling.notmuchmail.org)
 test_json_nodes <<<"$output" \
                 'body:[0][0][0]'"$bodytext"
 
 test_begin_subtest "reply to 'Mixed-Up' mangled PGP/MIME message correctly"
-test_subtest_known_broken
 output=$(notmuch reply --format=json --decrypt=true id:mixed-up@mangling.notmuchmail.org)
 test_json_nodes <<<"$output" \
                 'body:["original"]'"$bodytext"
-- 
2.20.1