v4 of repairing Mixed-up mangled MIME messages

v4 of repairing Mixed-up mangled MIME messages

[Daniel Kahn Gillmor]

This is the fourth revision of the "Mixed up Mangling" series.
Previous versions:

 * v1 starts at id:20190528225452.17550-1-dkg@fifthhorseman.net
 * v2 starts at id:20190530172707.10378-1-dkg@fifthhorseman.net
 * v3 starts at id:20190531074842.16789-1-dkg@fifthhorseman.net

This is a simple rebase of v3 on top of the current notmuch master
branch.

Comments welcome, as always!

         --dkg

[PATCH v4 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.23.0

[PATCH v4 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 | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
 util/repair.h | 10 +++++++
 2 files changed, 89 insertions(+)

diff --git a/util/repair.c b/util/repair.c
index 629e6f23..829b166a 100644
--- a/util/repair.c
+++ b/util/repair.c
@@ -120,3 +120,82 @@ _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload)
 	return payload;
     }
 }
+
+/* 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 9974d693..492f5a20 100644
--- a/util/repair.h
+++ b/util/repair.h
@@ -25,9 +25,19 @@ extern "C" {
  * returned object will only be released when the original part is
  * disposed of.
  */
+
 GMimeObject *
 _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload);
 
+/* 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.23.0

[PATCH v4 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 e2db2ef5..a7d91d67 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -127,6 +127,12 @@ of its normal activity.
     found in that message, since it was able to index the built-in
     protected headers directly.
 
+    ``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 1301d78a..158ba5cf 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -387,11 +387,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);
@@ -444,7 +453,7 @@ _index_mime_part (notmuch_message_t *message,
 	    }
 	    _index_mime_part (message, indexopts, toindex, msg_crypto);
 	}
-	return;
+	goto DONE;
     }
 
     if (GMIME_IS_MESSAGE_PART (part)) {
@@ -454,14 +463,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);
@@ -475,7 +484,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 ();
@@ -521,6 +530,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.23.0

[PATCH v4 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                   | 22 ++++++++++++++++++++++
 test/T351-pgpmime-mangling.sh |  2 --
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 599d3b65..d4996a33 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,21 @@ _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)
 {
@@ -298,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.23.0

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

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> +/* 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;

Can g_mime_object_get_content_type plausibly fail (and return NULL) here?

> +    if (! GMIME_IS_MULTIPART (part))
> +	return false;

I guess this happens if the mime structure does not match the content
type declaration? Not sure if this needs a comment or if it's clear
enough.

> +    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);

there's a slight cognitive dissonance for me between the zero and one
based indexing schemes here. part0, part1, and part2? or maybe an
GMimeObject *part[3]
 
> +    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);

It might make sense to use the EMPTY_STRING macro here, although
currently it's only accesible via notmuch-private.h

> +    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 9974d693..492f5a20 100644
> --- a/util/repair.h
> +++ b/util/repair.h
> @@ -25,9 +25,19 @@ extern "C" {
>   * returned object will only be released when the original part is
>   * disposed of.
>   */
> +
>  GMimeObject *
>  _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload);
>  
> +/* 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.23.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: v4 of repairing Mixed-up mangled MIME messages

[Jameson Graef Rollins]

On Sun, Sep 08 2019, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> This is the fourth revision of the "Mixed up Mangling" series.
> Previous versions:
>
>  * v1 starts at id:20190528225452.17550-1-dkg@fifthhorseman.net
>  * v2 starts at id:20190530172707.10378-1-dkg@fifthhorseman.net
>  * v3 starts at id:20190531074842.16789-1-dkg@fifthhorseman.net
>
> This is a simple rebase of v3 on top of the current notmuch master
> branch.
>
> Comments welcome, as always!

Can we have notmuch auto-apply a tag, like the "encrypted" and "signed"
tags, that indicates mail has been mangled in this way?  I'm feeling
somewhat morally opposed to just silently fixing mail that's been broken
by bad/irresponsible actors on the net.  We need to keep pushing on MS
to fix this issue globally, so I for one would like to be reminded if
I'm still being affected by this.

jamie.

Re: v4 of repairing Mixed-up mangled MIME messages

[David Bremner]

Jameson Graef Rollins <jrollins@caltech.edu> writes:

> Can we have notmuch auto-apply a tag, like the "encrypted" and "signed"
> tags, that indicates mail has been mangled in this way?  I'm feeling
> somewhat morally opposed to just silently fixing mail that's been broken
> by bad/irresponsible actors on the net.  We need to keep pushing on MS
> to fix this issue globally, so I for one would like to be reminded if
> I'm still being affected by this.

It's side point, but it should rather be a property than a tag if we do
something like that. In hindsight I think "auto tags" were probably a
design mistake, since they are (easily) mutable by users.

d

Re: v4 of repairing Mixed-up mangled MIME messages

[Jameson Graef Rollins]

On Sat, Sep 14 2019, David Bremner <david@tethera.net> wrote:
> Jameson Graef Rollins <jrollins@caltech.edu> writes:
>
>> Can we have notmuch auto-apply a tag, like the "encrypted" and "signed"
>> tags, that indicates mail has been mangled in this way?  I'm feeling
>> somewhat morally opposed to just silently fixing mail that's been broken
>> by bad/irresponsible actors on the net.  We need to keep pushing on MS
>> to fix this issue globally, so I for one would like to be reminded if
>> I'm still being affected by this.
>
> It's side point, but it should rather be a property than a tag if we do
> something like that. In hindsight I think "auto tags" were probably a
> design mistake, since they are (easily) mutable by users.

Right, sorry, yes it should be a property.  I agree.

Any reason not to move "encrypted" and "signed" to be properties as
well?

jamie.

Re: v4 of repairing Mixed-up mangled MIME messages

[David Bremner]

Jameson Graef Rollins <jrollins@caltech.edu> writes:

> On Sat, Sep 14 2019, David Bremner <david@tethera.net> wrote:
>> Jameson Graef Rollins <jrollins@caltech.edu> writes:
>>
>>> Can we have notmuch auto-apply a tag, like the "encrypted" and "signed"
>>> tags, that indicates mail has been mangled in this way?  I'm feeling
>>> somewhat morally opposed to just silently fixing mail that's been broken
>>> by bad/irresponsible actors on the net.  We need to keep pushing on MS
>>> to fix this issue globally, so I for one would like to be reminded if
>>> I'm still being affected by this.
>>
>> It's side point, but it should rather be a property than a tag if we do
>> something like that. In hindsight I think "auto tags" were probably a
>> design mistake, since they are (easily) mutable by users.
>
> Right, sorry, yes it should be a property.  I agree.
>
> Any reason not to move "encrypted" and "signed" to be properties as
> well?

I guess it would require some client code to be adapted. I'm not sure
what really relies on those tags other than ad hoc searches.

d

Re: v4 of repairing Mixed-up mangled MIME messages

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On Sat 2019-09-14 00:29:40 -0700, Jameson Graef Rollins wrote:
> Can we have notmuch auto-apply a tag, like the "encrypted" and "signed"
> tags, that indicates mail has been mangled in this way?

i don't believe tags are appropriate for this use -- the growing
convention is to use properties for automated notes like this, and you
can see from the series that this is the case:

+	notmuch_message_add_property (message, "index.repaired", "mixedup");

> I'm feeling somewhat morally opposed to just silently fixing mail
> that's been broken by bad/irresponsible actors on the net.  We need to
> keep pushing on MS to fix this issue globally, so I for one would like
> to be reminded if I'm still being affected by this.

you should be able to do that with:

     notmuch search property:index.repaired=mixedup

does that satisfy your concerns?

     --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: v4 of repairing Mixed-up mangled MIME messages

[Jameson Graef Rollins]

On Sat, Sep 14 2019, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> On Sat 2019-09-14 00:29:40 -0700, Jameson Graef Rollins wrote:
>> Can we have notmuch auto-apply a tag, like the "encrypted" and "signed"
>> tags, that indicates mail has been mangled in this way?
>
> i don't believe tags are appropriate for this use -- the growing
> convention is to use properties for automated notes like this, and you
> can see from the series that this is the case:
>
> +	notmuch_message_add_property (message, "index.repaired", "mixedup");
>
>> I'm feeling somewhat morally opposed to just silently fixing mail
>> that's been broken by bad/irresponsible actors on the net.  We need to
>> keep pushing on MS to fix this issue globally, so I for one would like
>> to be reminded if I'm still being affected by this.
>
> you should be able to do that with:
>
>      notmuch search property:index.repaired=mixedup
>
> does that satisfy your concerns?

Oh, so sorry, I should have read the patches more closely.  Yes, that
property definitely satisfies.  Thank you for your completeness in the
series, dkg.

jamie.

Re: v4 of repairing Mixed-up mangled MIME messages

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On Sat 2019-09-14 14:33:56 -0300, David Bremner wrote:
> Jameson Graef Rollins <jrollins@caltech.edu> writes:
>> Any reason not to move "encrypted" and "signed" to be properties as
>> well?
>
> I guess it would require some client code to be adapted. I'm not sure
> what really relies on those tags other than ad hoc searches.

I think adding a property for "encrypted" and "signed" is a good idea,
but i agree with Bremner that it's unclear how to manage a smooth
transition from tags to properties for these annotations.

A simple patch that proposes the change -- both to code and to the
appropriate parts of notmuch documentation -- might be a good way to get
the conversation started properly.

    --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 3001 bytes --]

Thanks for the review, David.

On Fri 2019-09-13 22:58:27 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +/* 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;
>
> Can g_mime_object_get_content_type plausibly fail (and return NULL) here?

hm, yes, it can -- if it's passed NULL, for example.  It's not an
external function, but I can program this more defensively.  i'll do
that in the next revision.

>> +    if (! GMIME_IS_MULTIPART (part))
>> +	return false;
>
> I guess this happens if the mime structure does not match the content
> type declaration? Not sure if this needs a comment or if it's clear
> enough.

yeah, this is unlikely, maybe even impossible given the current gmime
implementation, but it's a defensive step.

>> +    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);
>
> there's a slight cognitive dissonance for me between the zero and one
> based indexing schemes here. part0, part1, and part2? or maybe an
> GMimeObject *part[3]

sure, i'll make it *parts[3] to avoid the dissonance. ("part" is already
taken -- it's the argument for the function.

>> +    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);
>
> It might make sense to use the EMPTY_STRING macro here, although
> currently it's only accesible via notmuch-private.h

hm, notmuch-private.h is in lib/, which seems inappropriate for use
outside the library.  however, i agree that this seems like a
superfluous call to strcmp -- i'll change it for a simple test.

I've updated this 2/4 patch on my mixed-up-mangling branch (visible at
https://gitlab.com/dkg/notmuch), and i'll send it to this thread
shortly.

            --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[PATCH v5 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 | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
 util/repair.h | 10 ++++++
 2 files changed, 94 insertions(+)

diff --git a/util/repair.c b/util/repair.c
index 629e6f23..9fba97b7 100644
--- a/util/repair.c
+++ b/util/repair.c
@@ -120,3 +120,87 @@ _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload)
 	return payload;
     }
 }
+
+/* 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 *parts[3] = {NULL, NULL, NULL};
+    GMimeContentType *type = NULL;
+    char *prelude_string = NULL;
+    bool prelude_is_empty;
+
+    if (part == NULL)
+	return false;
+    type = g_mime_object_get_content_type (part);
+    if (type == NULL)
+	return false;
+    if (! g_mime_content_type_is_type (type, "multipart", "mixed"))
+	return false;
+    if (! GMIME_IS_MULTIPART (part)) /* probably impossible */
+	return false;
+    mpart = GMIME_MULTIPART (part);
+    if (mpart == NULL)
+	return false;
+    if (g_mime_multipart_get_count (mpart) != 3)
+	return false;
+    parts[0] = g_mime_multipart_get_part (mpart, 0);
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (parts[0]),
+				       "text", "plain"))
+	return false;
+    if (! GMIME_IS_TEXT_PART (parts[0]))
+	return false;
+    parts[1] = g_mime_multipart_get_part (mpart, 1);
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (parts[1]),
+				       "application", "pgp-encrypted"))
+	return false;
+    parts[2] = g_mime_multipart_get_part (mpart, 2);
+    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (parts[2]),
+				       "application", "octet-stream"))
+	return false;
+
+    /* Is parts[0] length 0? */
+    prelude_string = g_mime_text_part_get_text (GMIME_TEXT_PART (parts[0]));
+    prelude_is_empty = (prelude_string[0] == '\0');
+    g_free (prelude_string);
+    if (! prelude_is_empty)
+	return false;
+
+    /* FIXME: after decoding and stripping whitespace, is parts[1]
+     * subpart just "Version: 1" ? */
+
+    /* FIXME: can we determine that parts[2] 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 9974d693..492f5a20 100644
--- a/util/repair.h
+++ b/util/repair.h
@@ -25,9 +25,19 @@ extern "C" {
  * returned object will only be released when the original part is
  * disposed of.
  */
+
 GMimeObject *
 _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload);
 
+/* 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.23.0

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

[Tomi Ollila]

On Sun, Sep 15 2019, Daniel Kahn Gillmor wrote:

> This patch implements a functional identification and repair process

If there is going to be more versions, then the above could be changed
to either 

1) This commit implements...

or just

2) Implement a functional ...

> 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.

If there is going to be more versions, then the above line could be removed...

(as committed changes are not "patches" ...)

Tomi

>
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
> ---
>  util/repair.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/repair.h | 10 ++++++
>  2 files changed, 94 insertions(+)
>
> diff --git a/util/repair.c b/util/repair.c
> index 629e6f23..9fba97b7 100644
> --- a/util/repair.c
> +++ b/util/repair.c
> @@ -120,3 +120,87 @@ _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload)
>  	return payload;
>      }
>  }
> +
> +/* 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 *parts[3] = {NULL, NULL, NULL};
> +    GMimeContentType *type = NULL;
> +    char *prelude_string = NULL;
> +    bool prelude_is_empty;
> +
> +    if (part == NULL)
> +	return false;
> +    type = g_mime_object_get_content_type (part);
> +    if (type == NULL)
> +	return false;
> +    if (! g_mime_content_type_is_type (type, "multipart", "mixed"))
> +	return false;
> +    if (! GMIME_IS_MULTIPART (part)) /* probably impossible */
> +	return false;
> +    mpart = GMIME_MULTIPART (part);
> +    if (mpart == NULL)
> +	return false;
> +    if (g_mime_multipart_get_count (mpart) != 3)
> +	return false;
> +    parts[0] = g_mime_multipart_get_part (mpart, 0);
> +    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (parts[0]),
> +				       "text", "plain"))
> +	return false;
> +    if (! GMIME_IS_TEXT_PART (parts[0]))
> +	return false;
> +    parts[1] = g_mime_multipart_get_part (mpart, 1);
> +    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (parts[1]),
> +				       "application", "pgp-encrypted"))
> +	return false;
> +    parts[2] = g_mime_multipart_get_part (mpart, 2);
> +    if (! g_mime_content_type_is_type (g_mime_object_get_content_type (parts[2]),
> +				       "application", "octet-stream"))
> +	return false;
> +
> +    /* Is parts[0] length 0? */
> +    prelude_string = g_mime_text_part_get_text (GMIME_TEXT_PART (parts[0]));
> +    prelude_is_empty = (prelude_string[0] == '\0');
> +    g_free (prelude_string);
> +    if (! prelude_is_empty)
> +	return false;
> +
> +    /* FIXME: after decoding and stripping whitespace, is parts[1]
> +     * subpart just "Version: 1" ? */
> +
> +    /* FIXME: can we determine that parts[2] 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 9974d693..492f5a20 100644
> --- a/util/repair.h
> +++ b/util/repair.h
> @@ -25,9 +25,19 @@ extern "C" {
>   * returned object will only be released when the original part is
>   * disposed of.
>   */
> +
>  GMimeObject *
>  _notmuch_repair_crypto_payload_skip_legacy_display (GMimeObject *payload);
>  
> +/* 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.23.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]

On Sun 2019-09-15 23:26:59 +0300, Tomi Ollila wrote:
> On Sun, Sep 15 2019, Daniel Kahn Gillmor wrote:
>
>> This patch implements a functional identification and repair process
>
> If there is going to be more versions, then the above could be changed
> to either 
>
> 1) This commit implements...
>
> or just
>
> 2) Implement a functional ...
>
>> 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.
>
> If there is going to be more versions, then the above line could be removed...
>
> (as committed changes are not "patches" ...)

I concur with (and appreciate) your terminological pedantry.  the
revision on my gitlab branch has adopted your suggestions, but i don't
want to burden the list with another revision right now.  hopefully this
change to the commit message won't block adoption -- anyone merging the
series should feel free to adjust the commit message as Tomi recommends.

     --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

[David Bremner]

David Bremner <david@tethera.net> writes:

> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> 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.
>
> the series LGTM; do you want me to merge from gitlab or ?
>
> d

I meant to send that to the list. I think it's just the commit message
changes for the one commit, right?

d\

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

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

On Mon 2019-09-16 07:49:21 -0300, David Bremner wrote:
> David Bremner <david@tethera.net> writes:
>
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>
>>> 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.
>>
>> the series LGTM; do you want me to merge from gitlab or ?
>
> I meant to send that to the list. I think it's just the commit message
> changes for the one commit, right?

That's correct.

Merging from gitlab would be fine with me.  i'm also fine if you decide
to just merge from the list, and hand-tweak the commit message to
include Tomi's cleanups :)

Thanks for reviewing!

       --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> On Mon 2019-09-16 07:49:21 -0300, David Bremner wrote:
>> David Bremner <david@tethera.net> writes:
>>
>>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>>
>>>> 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.
>>>
>>> the series LGTM; do you want me to merge from gitlab or ?
>>
>> I meant to send that to the list. I think it's just the commit message
>> changes for the one commit, right?
>
> That's correct.
>
> Merging from gitlab would be fine with me.  i'm also fine if you decide
> to just merge from the list, and hand-tweak the commit message to
> include Tomi's cleanups :)

I have done merged 23bcd003637f091c88f7d0a601d5fee82bc8e936 from
https://gitlab.com/dkg/notmuch.git. I also checked it against the tree
produced by the patches on the list.

d