[PATCH 2/2] legacy-display: drop tests that try to match headers in a Legacy Display part

[Daniel Kahn Gillmor]

These tests were an attempt to establish that the content of the
"Legacy Display" part is the same as the actual protected headers of
the message.  But this is more conservative than we need to be.

https://www.ietf.org/id/draft-autocrypt-lamps-protected-headers-02.html
section 5.3 makes clear that the Legacy Display part is purely
decorative, and section 5.2.1 clarifies that the detection can be done
purely by MIME structure and Content-Type alone.

Furthermore, now that we're accepting text/plain Legacy Display parts,
it's not clear the lines in the Legacy Display part should be
interpreted as needing an exact string match (e.g. "real" headers are
likely to be RFC 2047 encoded, but the text/plain Legacy Display part
probably should not be).

The concerns that motivated this test in the past were twofold: that
we might accidentally hide some information from the reader of the
message that they should have available to them, or that we could
introduce a covert channel that would be invisible to other clients.

I no longer think these are significant concerns:

 a) There will be no accidental misidentification of a Legacy Display
    part.  The identification of the Legacy Display part is
    unambiguous due to MIME structure and Content-Type.  MIME
    structure MUST be the first child part of a two-part
    multipart/mixed Cryptographic Payload. And the
    protected-headers=v1 content-type parameter must be present on
    both the cryptographic payload and the legacy display part, so no
    one would accidentally generate this structure and have it be
    accidentally matched.

 b) As for creating a covert channel, many such channels already
    exist.  For example, non-standard e-mail headers, custom MIME
    types, unusual MIME structures, etc, all make it possible to ship
    some content in a message that will be visible in some MUAs but
    not in others.  This doesn't make the situation demonstrably
    worse.

Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
---
 util/repair.c | 60 ++-------------------------------------------------
 1 file changed, 2 insertions(+), 58 deletions(-)

diff --git a/util/repair.c b/util/repair.c
index 4385d16f..6c13601d 100644
--- a/util/repair.c
+++ b/util/repair.c
@@ -27,13 +27,7 @@ _notmuch_crypto_payload_has_legacy_display (GMimeObject *payload)
 {
     GMimeMultipart *mpayload;
     const char *protected_header_parameter;
-    GMimeTextPart *legacy_display;
-    char *legacy_display_header_text = NULL;
-    GMimeStream *stream = NULL;
-    GMimeParser *parser = NULL;
-    GMimeObject *legacy_header_object = NULL, *first;
-    GMimeHeaderList *legacy_display_headers = NULL, *protected_headers = NULL;
-    bool ret = false;
+    GMimeObject *first;
 
     if (! g_mime_content_type_is_type (g_mime_object_get_content_type (payload),
 				       "multipart", "mixed"))
@@ -60,57 +54,7 @@ _notmuch_crypto_payload_has_legacy_display (GMimeObject *payload)
     if (! GMIME_IS_TEXT_PART (first))
 	return false;
 
-    /* ensure that the headers in the first part all match the values
-     * found in the payload's own protected headers!  if they don't,
-     * we should not treat this as a valid "legacy-display" part.
-     *
-     * Crafting a GMimeHeaderList object from the content of the
-     * text/rfc822-headers part is pretty clumsy; we should probably
-     * push something into GMime that makes this a one-shot
-     * operation. */
-    if ((protected_headers = g_mime_object_get_header_list (payload), protected_headers) &&
-	(legacy_display = GMIME_TEXT_PART (first), legacy_display) &&
-	(legacy_display_header_text = g_mime_text_part_get_text (legacy_display), legacy_display_header_text) &&
-	(stream = g_mime_stream_mem_new_with_buffer (legacy_display_header_text, strlen (legacy_display_header_text)), stream) &&
-	(g_mime_stream_write (stream, "\r\n\r\n", 4) == 4) &&
-	(g_mime_stream_seek (stream, 0, GMIME_STREAM_SEEK_SET) == 0) &&
-	(parser = g_mime_parser_new_with_stream (stream), parser) &&
-	(legacy_header_object = g_mime_parser_construct_part (parser, NULL), legacy_header_object) &&
-	(legacy_display_headers = g_mime_object_get_header_list (legacy_header_object), legacy_display_headers)) {
-	/* walk through legacy_display_headers, comparing them against
-	 * their values in the protected_headers: */
-	ret = true;
-	for (int i = 0; i < g_mime_header_list_get_count (legacy_display_headers); i++) {
-	    GMimeHeader *dh = g_mime_header_list_get_header_at (legacy_display_headers, i);
-	    if (dh == NULL) {
-		ret = false;
-		goto DONE;
-	    }
-	    GMimeHeader *ph = g_mime_header_list_get_header (protected_headers, g_mime_header_get_name (dh));
-	    if (ph == NULL) {
-		ret = false;
-		goto DONE;
-	    }
-	    const char *dhv = g_mime_header_get_value (dh);
-	    const char *phv = g_mime_header_get_value (ph);
-	    if (dhv == NULL || phv == NULL || strcmp (dhv, phv)) {
-		ret = false;
-		goto DONE;
-	    }
-	}
-    }
-
- DONE:
-    if (legacy_display_header_text)
-	g_free (legacy_display_header_text);
-    if (stream)
-	g_object_unref (stream);
-    if (parser)
-	g_object_unref (parser);
-    if (legacy_header_object)
-	g_object_unref (legacy_header_object);
-
-    return ret;
+    return true;
 }
 
 GMimeObject *
-- 
2.24.0