[PATCH 7/7] index: avoid indexing legacy-display parts

[Daniel Kahn Gillmor]

When we notice a legacy-display part during indexing, it makes more
sense to avoid indexing it as part of the message body.

Given that the protected subject will already be indexed, there is no
need to index this part at all, so we skip over it.

If this happens during indexing, we set a property on the message:
index.repaired=skip-protected-headers-legacy-display

Signed-off-by: Daniel Kahn Gillmor 
---
 doc/man7/notmuch-properties.rst |  6 ++
 lib/index.cc| 20 
 test/T356-protected-headers.sh  |  2 --
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 2e610683..e2db2ef5 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=skip-protected-headers-legacy-display`` indicates
+that when indexing the cleartext of an encrypted message, notmuch
+skipped over a "legacy-display" text/rfc822-headers part that it
+found in that message, since it was able to index the built-in
+protected headers directly.
+
 SEE ALSO
 
 
diff --git a/lib/index.cc b/lib/index.cc
index 8a3e2e09..1301d78a 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -435,8 +435,14 @@ _index_mime_part (notmuch_message_t *message,
continue;
}
child = g_mime_multipart_get_part (multipart, i);
-   _notmuch_message_crypto_potential_payload (msg_crypto, child, part, 
i);
-   _index_mime_part (message, indexopts, child, msg_crypto);
+   GMimeObject *toindex = child;
+   if (_notmuch_message_crypto_potential_payload (msg_crypto, child, 
part, i) &&
+   msg_crypto->decryption_status == 
NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   toindex = _notmuch_repair_crypto_payload_skip_legacy_display 
(child);
+   if (toindex != child)
+   notmuch_message_add_property (message, "index.repaired", 
"skip-protected-headers-legacy-display");
+   }
+   _index_mime_part (message, indexopts, toindex, msg_crypto);
}
return;
 }
@@ -573,8 +579,14 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT 
(encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
-_index_mime_part (message, indexopts, clear, msg_crypto);
+GMimeObject *toindex = clear;
+if (_notmuch_message_crypto_potential_payload (msg_crypto, clear, 
GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT) &&
+   msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   toindex = _notmuch_repair_crypto_payload_skip_legacy_display (clear);
+   if (toindex != clear)
+   notmuch_message_add_property (message, "index.repaired", 
"skip-protected-headers-legacy-display");
+}
+_index_mime_part (message, indexopts, toindex, msg_crypto);
 g_object_unref (clear);
 
 status = notmuch_message_add_property (message, "index.decryption", 
"success");
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 867b8722..925805df 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -148,12 +148,10 @@ test_json_nodes <<<"$output" \
 
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
 
 test_begin_subtest "do not treat legacy-display part as body when indexing"
-test_subtest_known_broken
 output=$(notmuch search --output=messages body:interrupting)
 test_expect_equal "$output" ''
 
 test_begin_subtest "identify message that had a legacy display part skipped 
during indexing"
-test_subtest_known_broken
 output=$(notmuch search --output=messages 
property:index.repaired=skip-protected-headers-legacy-display)
 test_expect_equal "$output" 
id:protected-with-legacy-disp...@crypto.notmuchmail.org
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 5/7] util/repair: add _notmuch_repair_crypto_payload_skip_legacy_display

[Daniel Kahn Gillmor]

This is a utility function designed to make it easier to
"fast-forward" past a legacy-display part associated with a
cryptographic envelope, and show the user the intended message body.

The bulk of the ugliness in here is in the test function
_notmuch_crypto_payload_has_legacy_display, which tests all of the
things we'd expect to be true in a a cryptographic payload that
contains a legacy display part.

Signed-off-by: Daniel Kahn Gillmor 
---
 util/repair.c | 98 +++
 util/repair.h | 17 +
 2 files changed, 115 insertions(+)

diff --git a/util/repair.c b/util/repair.c
index f91c1244..0809f7b4 100644
--- a/util/repair.c
+++ b/util/repair.c
@@ -18,4 +18,102 @@
  * Authors: Daniel Kahn Gillmor 
  */
 
+#include 
 #include "repair.h"
+
+
+static bool
+_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;
+
+if (! g_mime_content_type_is_type (g_mime_object_get_content_type 
(payload),
+  "multipart", "mixed"))
+   return false;
+protected_header_parameter = g_mime_object_get_content_type_parameter 
(payload, "protected-headers");
+if ((! protected_header_parameter) || strcmp (protected_header_parameter, 
"v1"))
+   return false;
+if (! GMIME_IS_MULTIPART (payload))
+   return false;
+mpayload = GMIME_MULTIPART (payload);
+if (mpayload == NULL)
+   return false;
+if (g_mime_multipart_get_count (mpayload) != 2)
+   return false;
+first = g_mime_multipart_get_part (mpayload, 0);
+if (! g_mime_content_type_is_type (g_mime_object_get_content_type (first),
+  "text", "rfc822-headers"))
+   return false;
+protected_header_parameter = g_mime_object_get_content_type_parameter 
(first, "protected-headers");
+if ((! protected_header_parameter) || strcmp (protected_header_parameter, 
"v1"))
+   return false;
+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;
+   break;
+   }
+   GMimeHeader *ph = g_mime_header_list_get_header (protected_headers, 
g_mime_header_get_name (dh));
+   if (ph == NULL) {
+   ret = false;
+   break;
+   }
+   if (strcmp (g_mime_header_get_value (dh), g_mime_header_get_value 
(ph))) {
+   ret = false;
+   break;
+   }
+   }
+}
+
+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;
+}
+
+GMimeObject *
+_notmuch_repair_crypto_payload_skip_legacy_display (GMim

[PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

[Daniel Kahn Gillmor]

Our _notmuch_message_crypto_potential_payload implementation could
only return a failure if bad arguments were passed to it.  It is an
internal function, so if that happens it's an entirely internal bug
for notmuch.

It will be more useful for this function to return whether or not the
part is in fact a cryptographic payload, so we dispense with the
status return.

If some future change suggests adding a status return back, there are
only a handful of call sites, and no pressure to retain a stable API,
so it could be changed easily. But for now, go with the simpler
function.

Signed-off-by: Daniel Kahn Gillmor 
---
 lib/index.cc  |  9 ++---
 mime-node.c   |  6 +-
 util/crypto.c | 27 ---
 util/crypto.h |  7 +--
 4 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index db3dd568..8a3e2e09 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -407,7 +407,6 @@ _index_mime_part (notmuch_message_t *message,
_notmuch_message_add_term (message, "tag", "encrypted");
 
for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
-   notmuch_status_t status;
GMimeObject *child;
if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
/* Don't index the signature, but index its content type. */
@@ -436,11 +435,7 @@ _index_mime_part (notmuch_message_t *message,
continue;
}
child = g_mime_multipart_get_part (multipart, i);
-   status = _notmuch_message_crypto_potential_payload (msg_crypto, 
child, part, i);
-   if (status)
-   _notmuch_database_log (notmuch_message_get_database (message),
-  "Warning: failed to mark the potential 
cryptographic payload (%s).\n",
-  notmuch_status_to_string (status));
+   _notmuch_message_crypto_potential_payload (msg_crypto, child, part, 
i);
_index_mime_part (message, indexopts, child, msg_crypto);
}
return;
@@ -578,7 +573,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, 
GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
+_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT 
(encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
 _index_mime_part (message, indexopts, clear, msg_crypto);
 g_object_unref (clear);
 
diff --git a/mime-node.c b/mime-node.c
index d2125f90..f2bab64e 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -293,8 +293,6 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
 static bool
 _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild)
 {
-notmuch_status_t status;
-
 /* Deal with the different types of parts */
 if (GMIME_IS_PART (part)) {
node->part = part;
@@ -335,9 +333,7 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject 
*part, int numchild)
node_verify (node, part);
}
 } else {
-   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, 
numchild);
-   if (status)
-   fprintf (stderr, "Warning: failed to record potential crypto 
payload (%s).\n", notmuch_status_to_string (status));
+   _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, 
node->parent ? node->parent->part : NULL, numchild);
 }
 
 return true;
diff --git a/util/crypto.c b/util/crypto.c
index 225f537a..31cf056b 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -135,19 +135,16 @@ _notmuch_message_crypto_potential_sig_list 
(_notmuch_message_crypto_t *msg_crypt
 }
 
 
-notmuch_status_t
-_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t 
*msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum)
+bool
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t 
*msg_crypto, GMimeObject *part, GMimeObject *parent, int childnum)
 {
 const char *protected_headers = NULL;
 const char *forwarded = NULL;
 const char *subject = NULL;
 
-if (! msg_crypto || ! payload)
-   return NOTMUCH_STATUS_NULL_POINTER;
-
 /* only fire on the first payload part encountered */
 if (msg_crypto->payload_encountered)
-   return NOTMUCH_STATUS_SUCCESS;
+   return false;
 
 /* the first child of multipart/encrypted that matches the
  * encryption protocol should be "control information" metadata,
@@ -155,11 +152,11 @@ _notmuch_message_crypto_potential_payload 
(_notmuch_message_crypto_t *msg_crypto
  * https://tools.ietf.org/html/rfc1847#page-8) */
 if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum == 
GMIME_MULTIP

v3 of legacy-display cleanup

[Daniel Kahn Gillmor]

This is the third revision of the series that cleans up legacy-display
protected headers parts so that notmuch users only have to look at one
subject line.

version 2 can be found at id:20190531075907.17035-1-...@fifthhorseman.net
version 1 can be found at id:20190531042825.27774-1-...@fifthhorseman.net

--
Now that notmuch can handle and interpret protected subject lines, it
should also avoid forcing the user to look at "legacy display" parts
that some MUAs (notably enigmail) copies of the protected headers that
are intended to be rendered only by legacy clients -- clients capable
of decryption but which don't understand how to handle protected
headers.
--

The main differences from version 2 are:

 * i've directly included the two-patch "repair" series (originally
   found at id:20190531074027.16337-1-...@fifthhorseman.net), and

 * i've rebased against the uncrustified master, as well as applying
   the uncrustify rules to my changes here.

If we can get this merged, i'll send a subsequent revision of the
series that repairs "mixed-up MIME" mangled messages.

I would appreciate any feedback!

  --dkg


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 6/7] cli/{show,reply}: skip over legacy-display parts

[Daniel Kahn Gillmor]

Make use of the previous changes to fast-forward past any
legacy-display parts during "notmuch show" and "notmuch reply".

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c| 11 ++-
 test/T356-protected-headers.sh |  2 --
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index f2bab64e..599d3b65 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -333,7 +333,16 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject 
*part, int numchild)
node_verify (node, part);
}
 } else {
-   _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, 
node->parent ? node->parent->part : NULL, numchild);
+   if (_notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, 
part, node->parent ? node->parent->part : NULL, numchild) &&
+   node->ctx->msg_crypto->decryption_status == 
NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   GMimeObject *clean_payload = 
_notmuch_repair_crypto_payload_skip_legacy_display (part);
+   if (clean_payload != part) {
+   /* only one layer of recursion is possible here
+* because there can be only a single cryptographic
+* payload: */
+   return _mime_node_set_up_part (node, clean_payload, numchild);
+   }
+   }
 }
 
 return true;
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 43dfffe6..867b8722 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -137,14 +137,12 @@ id:protected-hea...@crypto.notmuchmail.org
 id:subjectless-protected-hea...@crypto.notmuchmail.org'
 
 test_begin_subtest "when rendering protected headers, avoid rendering 
legacy-display part"
-test_subtest_known_broken
 output=$(notmuch show --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
 'subject:[0][0][0]["headers"]["Subject"]="Interrupting Cow"' \
 
'no_legacy_display:[0][0][0]["body"][0]["content"][1]["content-type"]="text/plain"'
 
 test_begin_subtest "when replying, avoid rendering legacy-display part"
-test_subtest_known_broken
 output=$(notmuch reply --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
 
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: add keywords to notmuch-emacs-mua.desktop

[Daniel Kahn Gillmor]

Ping! If there is a reason that this trivial patch has languished, i'd
be happy to receive critical feedback.  Or maybe we can just merge it?

   --dkg

On Mon 2019-06-10 04:44:39 +0300, Daniel Kahn Gillmor wrote:
> Debian's lintian has an informational alert
> desktop-entry-lacks-keywords-entry, which recommends including
> Keywords= in a .desktop file.
>
> I dug around a bit in /usr/share/applications/*.desktop to make sure
> that we covered the range of keywords other e-mail applications are
> using.  If anyone has other suggestions for keywords, they can add
> them to this list.
>
> Signed-off-by: Daniel Kahn Gillmor 
> ---
>  emacs/notmuch-emacs-mua.desktop | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/emacs/notmuch-emacs-mua.desktop b/emacs/notmuch-emacs-mua.desktop
> index 0d9af2a4..752a1d7b 100644
> --- a/emacs/notmuch-emacs-mua.desktop
> +++ b/emacs/notmuch-emacs-mua.desktop
> @@ -8,3 +8,4 @@ Icon=emblem-mail
>  Terminal=false
>  Type=Application
>  Categories=Network;Email;
> +Keywords=Mail;E-mail;Email;
> -- 
> 2.20.1
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/7] repair: set up codebase for repair functionality

[Daniel Kahn Gillmor]

This adds no functionality directly, but is a useful starting point
for adding new repair functionality.

Signed-off-by: Daniel Kahn Gillmor 
---
 doc/man7/notmuch-properties.rst | 12 
 lib/notmuch-private.h   |  1 +
 notmuch-client.h|  1 +
 util/Makefile.local |  1 +
 util/repair.c   | 21 +
 util/repair.h   | 17 +
 6 files changed, 53 insertions(+)
 create mode 100644 util/repair.c
 create mode 100644 util/repair.h

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 802e6763..2e610683 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -109,6 +109,18 @@ of its normal activity.
 example, an AES-128 key might be stashed in a notmuch property as:
 ``session-key=7:14B16AF65536C28AF209828DFE34C9E0``.
 
+**index.repaired**
+
+Some messages arrive in forms that are confusing to view; they can
+be mangled by mail transport agents, or the sending mail user
+agent may structure them in a way that is confusing.  If notmuch
+knows how to both detect and repair such a problematic message, it
+will do so during indexing.
+
+If it applies a message repair during indexing, it will use the
+``index.repaired`` property to note the type of repair(s) it
+performed.
+
 SEE ALSO
 
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 9bdb68ab..28ced3a2 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -53,6 +53,7 @@ NOTMUCH_BEGIN_DECLS
 #include "error_util.h"
 #include "string-util.h"
 #include "crypto.h"
+#include "repair.h"
 
 #ifdef DEBUG
 # define DEBUG_DATABASE_SANITY 1
diff --git a/notmuch-client.h b/notmuch-client.h
index d1b78017..74690054 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -52,6 +52,7 @@
 
 #include "talloc-extra.h"
 #include "crypto.h"
+#include "repair.h"
 
 #define unused(x) x ## _unused __attribute__ ((unused))
 
diff --git a/util/Makefile.local b/util/Makefile.local
index 46f8af3a..f5d72f79 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -6,6 +6,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c 
$(dir)/hex-escape.c \
  $(dir)/string-util.c $(dir)/talloc-extra.c 
$(dir)/zlib-extra.c \
$(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c \
+   $(dir)/repair.c \
$(dir)/unicode-util.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
diff --git a/util/repair.c b/util/repair.c
new file mode 100644
index ..f91c1244
--- /dev/null
+++ b/util/repair.c
@@ -0,0 +1,21 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2019 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/ .
+ *
+ * Authors: Daniel Kahn Gillmor 
+ */
+
+#include "repair.h"
diff --git a/util/repair.h b/util/repair.h
new file mode 100644
index ..70e2b7bc
--- /dev/null
+++ b/util/repair.h
@@ -0,0 +1,17 @@
+#ifndef _REPAIR_H
+#define _REPAIR_H
+
+#include "gmime-extra.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* This is a collection of message structure and message format repair
+ * techniques that are designed to improve the user experience of
+ * notmuch */
+
+#ifdef __cplusplus
+}
+#endif
+#endif
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/7] test: avoid showing legacy-display parts

[Daniel Kahn Gillmor]

Enigmail generates a "legacy-display" part when it sends encrypted
mail with a protected Subject: header.  This part is intended to
display the Subject for mail user agents that are capable of
decryption, but do not know how to deal with embedded protected
headers.

This part is the first child of a two-part multipart/mixed
cryptographic payload within a cryptographic envelope that includes
encryption (that is, it is not just a cleartext signed message).  It
uses Content-Type: text/rfc822-headers.

That is:

A └┬╴multipart/encrypted
B  ├─╴application/pgp-encrypted
C  └┬╴application/octet-stream
*   ╤ 
D   └┬╴multipart/mixed; protected-headers=v1 (cryptographic payload)
E├─╴text/rfc822-headers; protected-headers=v1 (legacy-display part)
F└─╴… (actual message body)

In discussions with jrollins, i've come to the conclusion that a
legacy-display part should be stripped entirely from "notmuch show"
and "notmuch reply" now that these tools can understand and interpret
protected headers.

You can tell when a message part is a protected header part this way:

 * is the payload (D) multipart/mixed with exactly two children?
 * is its first child (E) Content-Type: text/rfc822-headers?
 * does the first child (E) have the property protected-headers=v1?
 * do all the headers in the body of the first child (E) match
   the protected headers in the payload part (D) itself?

If this is the case, and we already know how to deal with the
protected header, then there is no reason to try to render the
legacy-display part itself for the user.

Furthermore, when indexing, if we are indexing properly, we should
avoid indexing the text in E as part of the message body.

'notmuch reply' is an interesting case: the standard use of 'notmuch
reply' will end up omitting all mention of protected Subject:.

The right fix is for the replying MUA to be able to protect its
headers, and for it to set them appropriately based on headers found
in the original message.

If a replying MUA is unable to protect headers, but still wants the
user to be able to see the original header, a replying MUA that
notices that the original message's subject differs from the proposed
reply subject may choose to include the original's subject in the
quoted/attributed text. (this would be a stopgap measure; it's not
even clear that there is user demand for it)

This test suite change indicates what we want to happen for this case
(the tests are currently broken), and includes three additional TODO
suggestions of subtle cases for anyone who wants to flesh out the test
suite even further.  (i believe all these cases should be already
fixed by the rest of this series, but haven't had time to write the
tests for the unusual cases)

Signed-off-by: Daniel Kahn Gillmor 
---
 test/T356-protected-headers.sh| 33 +++
 .../protected-with-legacy-display.eml | 40 +++
 2 files changed, 73 insertions(+)
 create mode 100644 
test/corpora/protected-headers/protected-with-legacy-display.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 4af018f3..43dfffe6 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -136,4 +136,37 @@ id:nested-rfc822-mess...@crypto.notmuchmail.org
 id:protected-hea...@crypto.notmuchmail.org
 id:subjectless-protected-hea...@crypto.notmuchmail.org'
 
+test_begin_subtest "when rendering protected headers, avoid rendering 
legacy-display part"
+test_subtest_known_broken
+output=$(notmuch show --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+'subject:[0][0][0]["headers"]["Subject"]="Interrupting Cow"' \
+
'no_legacy_display:[0][0][0]["body"][0]["content"][1]["content-type"]="text/plain"'
+
+test_begin_subtest "when replying, avoid rendering legacy-display part"
+test_subtest_known_broken
+output=$(notmuch reply --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
+
+test_begin_subtest "do not treat legacy-display part as body when indexing"
+test_subtest_known_broken
+output=$(notmuch search --output=messages body:interrupting)
+test_expect_equal "$output" ''
+
+test_begin_subtest "identify message that had a legacy display part skipped 
during indexing"
+test_subtest_known_broken
+output=$(notmuch search --output=messages 
property:index.repaired=skip-protected-headers-legacy-display)
+test_expect_equal "$output" 
id:protected-with-legacy-disp...@crypto.notmuchmail.org
+
+# TODO: test that a part that looks like a legacy-display in
+# multipart/s

[PATCH 1/7] mime-node: split out _mime_node_set_up_part

[Daniel Kahn Gillmor]

This is a code reorganization that should have no functional effect,
but will make future changes simpler, because a future commit will
reuse the _mime_node_set_up_part functionality without touching
_mime_node_create.

In the course of splitting out this function, I noticed a comment in
the codebase that referred to an older name of _mime_node_create
(message_part_create), where this functionality originally resided.
I've fixed that comment to refer to the new function instead.

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 3133ca44..d2125f90 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -264,14 +264,15 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject 
*part)
g_error_free (err);
 }
 
+static bool
+_mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild);
+
 static mime_node_t *
 _mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
 {
 mime_node_t *node = talloc_zero (parent, mime_node_t);
-notmuch_status_t status;
 
 /* Set basic node properties */
-node->part = part;
 node->ctx = parent->ctx;
 if (! talloc_reference (node, node->ctx)) {
fprintf (stderr, "Out of memory.\n");
@@ -282,10 +283,24 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
*part, int numchild)
 node->part_num = node->next_part_num = -1;
 node->next_child = 0;
 
+if (_mime_node_set_up_part (node, part, numchild))
+   return node;
+talloc_free (node);
+return NULL;
+}
+
+/* associate a MIME part with a node. */
+static bool
+_mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild)
+{
+notmuch_status_t status;
+
 /* Deal with the different types of parts */
 if (GMIME_IS_PART (part)) {
+   node->part = part;
node->nchildren = 0;
 } else if (GMIME_IS_MULTIPART (part)) {
+   node->part = part;
node->nchildren = g_mime_multipart_get_count (GMIME_MULTIPART (part));
 } else if (GMIME_IS_MESSAGE_PART (part)) {
/* Promote part to an envelope and open it */
@@ -297,11 +312,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
*part, int numchild)
 } else {
fprintf (stderr, "Warning: Unknown mime part type: %s.\n",
 g_type_name (G_OBJECT_TYPE (part)));
-   talloc_free (node);
-   return NULL;
+   return false;
 }
 
-/* Handle PGP/MIME parts */
+/* Handle PGP/MIME parts (by definition not cryptographic payload parts) */
 if (GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt != 
NOTMUCH_DECRYPT_FALSE)) {
if (node->nchildren != 2) {
/* this violates RFC 3156 section 4, so we won't bother with it. */
@@ -321,12 +335,12 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
*part, int numchild)
node_verify (node, part);
}
 } else {
-   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);
+   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, 
numchild);
if (status)
fprintf (stderr, "Warning: failed to record potential crypto 
payload (%s).\n", notmuch_status_to_string (status));
 }
 
-return node;
+return true;
 }
 
 mime_node_t *
@@ -347,7 +361,7 @@ mime_node_child (mime_node_t *parent, int child)
 } else if (GMIME_IS_MESSAGE (parent->part)) {
sub = g_mime_message_get_mime_part (GMIME_MESSAGE (parent->part));
 } else {
-   /* This should have been caught by message_part_create */
+   /* This should have been caught by _mime_node_set_up_part */
INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
g_type_name (G_OBJECT_TYPE (parent->part)));
 }
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] uncrustify: clean up whitespace in notmuch-show.c

[Daniel Kahn Gillmor]

Signed-off-by: Daniel Kahn Gillmor 
---
 notmuch-show.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 9779cfa5..21792a57 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -492,7 +492,7 @@ format_part_text (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
 /* The disposition and content-type metadata are associated with
  * the envelope for message parts */
 GMimeObject *meta = node->envelope_part ? (
-   GMIME_OBJECT (node->envelope_part) ) : node->part ;
+   GMIME_OBJECT (node->envelope_part) ) : node->part;
 GMimeContentType *content_type = g_mime_object_get_content_type (meta);
 const bool leaf = GMIME_IS_PART (node->part);
 GMimeStream *stream = params->out_stream;
@@ -515,7 +515,7 @@ format_part_text (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
const char *disposition = _get_disposition (meta);
const char *cid = g_mime_object_get_content_id (meta);
const char *filename = leaf ? (
-   g_mime_part_get_filename (GMIME_PART (node->part)) ) : NULL ;
+   g_mime_part_get_filename (GMIME_PART (node->part)) ) : NULL;
 
if (disposition &&
strcasecmp (disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
@@ -688,7 +688,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
 /* The disposition and content-type metadata are associated with
  * the envelope for message parts */
 GMimeObject *meta = node->envelope_part ? (
-   GMIME_OBJECT (node->envelope_part) ): node->part;
+   GMIME_OBJECT (node->envelope_part) ) : node->part;
 GMimeContentType *content_type = g_mime_object_get_content_type (meta);
 char *content_string;
 const char *disposition = _get_disposition (meta);
@@ -965,7 +965,7 @@ show_message (void *ctx,
}
}
 }
-DONE :
+  DONE:
 talloc_free (local);
 return status;
 }
@@ -1320,7 +1320,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, 
char *argv[])
ret = do_show (config, query, formatter, sprinter, );
 }
 
-DONE :
+  DONE:
 g_mime_stream_flush (params.out_stream);
 g_object_unref (params.out_stream);
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] test: replace use of gdb with LD_PRELOAD shims in T070-insert.sh

[Daniel Kahn Gillmor]

On Mon 2019-06-24 21:44:13 +0300, Tomi Ollila wrote:
> If this suite speedup is merged, I'd suggest using original David's patch
> 2/2 due to consistency reasons -- $gen_test_filename is used likewise in
> other test_expect_code cases. and, unless $TEST_DIRECTORY contains '"'s, 
> '$'s or '`'s it work (in our current cases $gen_test_name does not contain
> the above characters (gen_test_filename=$TEST_DIRECTORY/$gen_test_name))
>
> I've looked this quite a lot lately (mostly for fun). I'll send email in
> near future some suggestions (ten (10) or so) how we could improve the
> situation here, which then could be applied everywhere...

If you insist, i'm ok to wait on your cleanup for pathnames that have
unusual characters in them, though i'd also be happy to see the cleanup
applied to make *inconsistent* things consistent, rather than enforcing
a broken-yet-uniform consistency and then applying the cleanup.

But we should not use Bremner's patch 2/2 directly, because it moves
shim generation below gen_insert_msg, as mentioned in
id:8736kipe9f@fifthhorseman.net.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] test: replace use of gdb with LD_PRELOAD shims in T070-insert.sh

[Daniel Kahn Gillmor]

On Fri 2019-06-14 08:16:14 -0300, David Bremner wrote:
> Ralph Seichter  writes:
>
>> * Daniel Kahn Gillmor:
>>
>>> Perhaps Ralph Seichter (explicitly cc'ed above) could comment on how
>>> it'll affect homebrew?
>>
>> MacPorts, actually. ;-) I have not yet been able to look into this patch
>> series, but I hope to be able to do so soonish.
>>
>> -Ralph
>
> Supposedly DYLD_INSERT_LIBRARIES does (did?) the same job as LD_PRELOAD,
> but there seems to some macOS specific complications.

Do you think we could go ahead and apply these patches on master now
anyway, and fix them up subsequently to make sure they apply to MacOS?
I don't know what the "MacOS specific complications" are.  Can they be
spelled out in more detail?  I note that
https://stackoverflow.com/questions/34114587/dyld-library-path-dyld-insert-libraries-not-working
suggests that DYLD_INSERT_LIBRARIES won't work with signed binaries, but
i don't think the binaries tested during the test suite are signed
binaries, are they?

Alternately if DYLD_INSERT_LIBRARIES doesn't work, should we just skip
this test on MacOS?  or have it fall back to gdb?

I'd really like to see this test suite speedup merged.

--dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: locales and notmuch

[Daniel Kahn Gillmor]

(sorry for the late reply to this thread)

On Thu 2019-02-21 15:11:48 -0400, David Bremner wrote:
> to be unique case-insensitively, so I decided to convert them to lower
> case on input. This turns out to be "fun", if we try to handle things
> other than ASCII.  So one option is to just insist prefixes are ASCII.
>
> Otherwise we could insist they are UTF-8, ignoring the locale. The
> fullest generality (I think) is to first convert from the users locale
> to utf8, as in the attached sample program.

I don't think this discussion fully covers just how "fun" this
conversion is.

Even if we assume UTF-8 in the database (which i think we should),
making something all lower-case is locale-dependent.  The classic
example, iirc, is that in most UTF-8 locales, U+0049 LATIN CAPITAL
LETTER I downcases to U+0069 LATIN SMALL LETTER I, but in tr_TR
(Turkish), it downcases to U+0131 LATIN SMALL LETTER DOTLESS I.  (and
upper-casing U+0069 LATIN SMALL LETTER I in tr_TR yields U+0130 LATIN
CAPITAL LETTER I WITH DOT ABOVE)

Similarly, if there's anything that the DB cares about collation for,
that also varies dramatically across UTF-8 locales.

sigh.

I have no problem with asserting that all character strings in the
notmuch database are UTF-8.  That's just the only sane thing to do in
2019.  But if we build any feature into notmuch that makes assumptions
or requirements about upper-casing, lower-casing, or collating strings,
and that feature interacts between the currently-running locale and
whatever locale was used to store data in the the database in the past,
and those locales can differ, we may be inflicting some subtle pain on
users.

(note that i'm assuming in this discussion that we're *just* talking
about metadata -- notmuch configuration options, explicit xapian terms,
etc, but *not* the indexed text of the messages, which is an entirely
different kettle of fish)

I see two protective approaches for handling this simply yet being clear
about our concerns.  Both methods introduce a clear dependency on some
UTF-8 locale, in the way that we also have clear dependencies on GMime
or Xapian.

 a) assert that all text strings in the notmuch db's metadata are
C.UTF-8, and enforce this explicitly in the codebase.

or,

 b) upon database initialization, select a UTF-8 locale (probably based
on the user's locale during "notmuch setup") and store it in the
database (perhaps reporting and displaying it via a "notmuch config"
value).  If any locale-dependent function is used against
in-database metadata while a *different* locale is active in the
environment, warn that this mismatch is happening, and prefer the
locale stored in the db.

I don't have the capacity to work on this kind of safeguard right now,
but someone who wants to learn more about locales and notmuch could try
to implement it and we could see what happens.  Being explicit about the
concern like this might help to raise the profile of the specific risky
codepaths, which in turn could prompt someone to make a more
sophisticated and useful fix than either of the guardrails described
above.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/8] CLI: replace some constructs with more uncrustify friendly ones

[Daniel Kahn Gillmor]

On Mon 2019-06-17 01:01:39 -0400, David Bremner wrote:
> Yes, the C11 standard seems pretty clear here, 6.3.1.{1,2}

Thanks for the reference.  I'm fine with accepting these changes, and
chalking this up as one more piece of programming arcana that i've
learned from working on the notmuch project :)

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

On Sun 2019-06-16 02:52:52 +0300, Daniel Kahn Gillmor wrote:
> On Fri 2019-06-14 22:34:16 +0200, VA wrote:
>> As a general rule, an application MUST prefix their own property names 
>> with "x--". It is recommended to report an application's 
>> properties on the notmuch wiki, to open collaboration with other 
>> projects having common use cases, ultimately opening to standardization 
>> outside a project's namespace.
>
> I like this text.

I note that in id:874l92gcy2@tethera.net, Bremner points out:

>>> BTW, notmuch is currently using . as a namespace separator, so this
>>> would look more like "x-project."

(as opposed to "x-project-' , that is. note the difference in
punctuation)

--dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 2/2] test: replace use of gdb with LD_PRELOAD shims in T070-insert.sh

[Daniel Kahn Gillmor]

On Sun 2019-06-16 14:35:53 +0300, Tomi Ollila wrote:
> On Mon, Jun 10 2019, Daniel Kahn Gillmor wrote:
>> +test_expect_code 1 "$(printf "notmuch_with_shim shim-%q insert < %q" 
>> "$code" "$gen_msg_filename")"
>
> does   test_expect_code 1 'notmuch_with_shim shim-$code insert < 
> "$gen_msg_filename"'

hm, i think your proposal would do the right thing, but if someone was
to "clobber those variables in the call path" as you put it -- or if it
ended up getting evaluated by a subshell that didn't have those
variables exported, it would fail.

Furthermore, when test_expect_code fails, at least one of the failure
paths prints out the literal string that it received as the "$2"
argument, so it's nice to have the literal string fully-expanded before
it gets passed to test_expect_code.

So for both of those reasons (fragility of variables in the callpath;
and clearer test failure reporting) i prefer the way i've done it even
if it is a bit harder to read.

I won't fight too hard about this though, i've got other things on my
plate with a higher priority.  So if you want to offer a third variant
of bremner's patches with your preferred approach, i'll probably be ok
with it.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/8] CLI: replace some constructs with more uncrustify friendly ones

[Daniel Kahn Gillmor]

On Thu 2019-06-13 08:08:32 -0300, David Bremner wrote:
>- add parens in some ternery operators

itym "ternary"

> @@ -120,13 +120,13 @@ _process_string_arg (const notmuch_opt_desc_t 
> *arg_desc, char next, const char *
>  static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
>  {
>  return
> - !!opt_desc->opt_inherit +
> - !!opt_desc->opt_bool +
> - !!opt_desc->opt_int +
> - !!opt_desc->opt_keyword +
> - !!opt_desc->opt_flags +
> - !!opt_desc->opt_string +
> - !!opt_desc->opt_position;
> + (bool) opt_desc->opt_inherit +
> + (bool) opt_desc->opt_bool +
> + (bool) opt_desc->opt_int +
> + (bool) opt_desc->opt_keyword +
> + (bool) opt_desc->opt_flags +
> + (bool) opt_desc->opt_string +
> + (bool) opt_desc->opt_position;
>  }

i find this is deeply weird.  It looks like it is coercing various types
into bools, and then summing a list of bools.

While the spec might well say that the sum of two bools should be an int
(i haven't checked), it's not at all obvious to me that the infix +
operator should assume that type.  (float + float is a float, not an
int, for example)

in some sense, the !! operator works better here because i know that its
output is likely to be an int, so summing makes sense.

I can live with this if we need it for making uncrustify nicer, but i
just wanted to register that it looks to me like a regression in terms
of readability.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] configure: fix mktemp call for macOS

[Daniel Kahn Gillmor]

On Fri 2019-06-14 20:57:07 +0300, Tomi Ollila wrote:

> Yes. $TMPDIR could be problematic if it makes the full (unix domain
> socket?) path be too long.

Modern, maintained version of GnuPG will only place sockets directly in
the $GNUPGHOME if /run/user/$(id -u) (i.e., $XDG_RUNTIME_DIR) is not
available.

I've just sent a message to gnupg-devel asking for help from upstream
there, this shouldn't be this complicated.  see
id:87blyxd2qu@fifthhorseman.net on gnupg-de...@gnupg.org ("safe
ephemeral GnuPG homdirs").

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

On Fri 2019-06-14 22:34:16 +0200, VA wrote:
> The wiki would serve to advertise each projects interests, and if some 
> other project has a common interest, they could get together to 
> standardize it in the interest of both projects?

Makes sense to me.  Each one then gets to deal with the legacy of having
the old "x--foo" property and new standard form "foo", which is
kind of annoying bookkeeping work, but maybe not too hard to do.  I'm
sure someone can write a converter script pretty simply once
"x--foo" is fully deprecated in such a transition.

> IMHO, libnotmuch should stay focused on the core: indexing and tagging, 
> avoid becoming bloated by staying minimal, doing one thing well. Else, 
> it would not deserve the "notmuch" name anymore!
>
> However, maybe this could be in some extras, maybe a separate 
> notmuch-extensions library.

Hm, i'm not so worried about keeping the semantics of the "notmuch" name
:P

If some useful feature can happen most efficiently at indexing time, and
the index is built by the library, i think the ecosystem is best served
by making sure that libnotmuch can just do it directly.

> For messages with a plain text part, I'm taking the first 100 chars. If 
> there's no plain text part but an HTML part, I'm using some random 
> html2text library (https://github.com/Alir3z4/html2text) and take the 
> first 100 chars.

makes sense, thanks for the simple and straightforward description.  I
assume if the message has neither a text/plain nor a text/html part,
then no property is added.  And for messages with multiple text/plain
parts, you just take the first text/plain part encountered in a
depth-first traversal of the MIME tree?

> Here's what we could add:
>
> As a general rule, an application MUST prefix their own property names 
> with "x--". It is recommended to report an application's 
> properties on the notmuch wiki, to open collaboration with other 
> projects having common use cases, ultimately opening to standardization 
> outside a project's namespace.

I like this text.

As a minor nit-pick, I'd change the MUST to a SHOULD if we're using
RFC-2119-style requirements keywords here, since i can imagine an
application developer talking here on the notmuch list and coming to
consensus in some particular use case that a given property should not
be project-specific.  iow, they need to know *why* they're not following
the recommendation.

Thanks for writing this up!

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] debian: bump Standards-Version to 4.3.0 (no changes needed)

[Daniel Kahn Gillmor]

On Mon 2019-06-10 04:22:50 +0300, Daniel Kahn Gillmor wrote:
> /usr/share/doc/debian-policy/upgrading-checklist.txt.gz suggests that
> notmuch is already compliant with debian-policy 4.3.0.
>
> Signed-off-by: Daniel Kahn Gillmor 

just wanted to note that i screwed up my own e-mail address in the
Signed-off-by: line here (sigh).  If you could fix that when merging,
i'd appreciate it.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

On Mon 2019-06-10 14:55:48 +0200, VA wrote:
> Le 09/06/2019 à 21:58, Daniel Kahn Gillmor a écrit :
>> We do expose this functionality in the library, so it's not the end of
>> the world to expose it in the python bindings, but i do worry a little
>> bit about encouraging people to fiddle with markers set by the indexer.
>
> Conflicts could be easily avoided by recommending a prefix like 
> "x--" to property names (see link below).

That makes sense -- we'd also want to encourage projects to note their
project name someplace central; having a namespace registry helps to
avoid collisions, and also helps to advertise the projects that derive
from notmuch :)

maybe update the wiki for that?

That said, if a given property is concretely useful, i don't want to
have it languish permanently in the x-- namespace, or to force
users to adopt that particular MUA to use it -- i'd like to make it
available to all the notmuch-derived mail user agents.  Maybe that
happens only when the feature gets added to libnotmuch itself?  How do
we incentivize projects into getting that kind of widely-useful feature
into the libnotmuch mainstream instead of having it as a differentiating
factor for their specific MUA?

how should we think about managing these parts of the ecosystem?

> I'm writing a MUA, and am using properties to store an excerpt of each 
> message, so the MUA can show a preview of each message (like the Gmail 
> web app, or Apple mail client).

cool, this sounds super useful.  if you have a chance to write up how
you're generating/selecting this extract, i'd love to read more.

> Some pleople recently suggested sync info or other MUA metadata:
> https://notmuchmail.org/pipermail/notmuch/2019/027403.html

Thanks for the pointer, i'd missed that thread back in February.

I agree that there's sufficient interest to take this use case more
seriously.

> I took the documentation from notmuch.h but only edited to be consistent 
> with Python. Those warnings should also be added in notmuch.h, right?

Yeah, that's probably a good idea.  Is it asking too much to ask you to
update notmuch.h as well when you resubmit this series?

Thanks for helping make notmuch better!

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH V2] test: aggregate-results.sh: consistent style. zero forks.

[Daniel Kahn Gillmor]

On Mon 2019-06-10 21:39:23 +0300, Tomi Ollila wrote:
> - all variables in $((...)) without leading $
> - all comparisons use -gt, -eq or -ne
> - no -a nor -o inside [ ... ] expressions
> - all indentation levels using one tab
>
> Dropped unnecessary empty string check when reading results files.
>
> Replaced pluralize() which was executed in subshell with
> pluralize_s(). pluralize_s sets $s to 's' or '' based on value of
> $1. Calls to pluralize_s are done in context of current shell, so
> no forks to subshells executed.
> ---
>
> V2: added quotes all "$variable" references where empty values or
> IFS characters could make a difference. Not in this script, but
> servers better example as a usage style elsewhere (where it could
> matter).

LGTM.  Thanks, Tomi!

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: STYLE and uncrustify

[Daniel Kahn Gillmor]

Hi Bremner--

Thanks for doing this kind of cleanup work.  Long-term consistency is
worth the short-term pain.  The main short-term pain comes from dealing
with changes that are in-flight.  As someone with a couple of series
that are in flight, of course i'd prefer that you merge my changes
first, before you apply this cleanup so i don't have to rebase.  :P But
i care more that we get the cleanup done, and i'm also fine with
rebasing if you do that. (rebasing against a consistent codebase is
easy, compared to planning and implementing features and fixes
correctly)

rip the band-aid off!

On Fri 2019-06-07 07:58:24 -0300, David Bremner wrote:
> I'm pondering running uncrustify on all/most of the notmuch codebase,
> but I noticed a few things that uncrustify does are either not
> documented in STYLE, or maybe contradicted.
>
> 1) Should block comments start with '*' ? Uncrustify thinks yes, STYLE
>is silent, the codebase says mostly yes.  I think update STYLE to
>match uncrustify here.
>
> 2) Should there be a space after '!'? Uncrustify says yes, STYLE is
>silent, the codebase is inconsistent. Updating STYLE would be the
>easy thing here, but I remember previous discussions being
>inconclusive.

I'm fine going with uncrustify's decision for these two points.

> 3) Similar for space between '++' and '--' and operand

for whatever reason, i find myself preferring these attached without a
space between ++ and -- and the operand, whether as a prefix or postfix
operator.  If it's possible (i don't really know uncrustify) i'd like to
have the codebase say both "foo++" and "++foo".  But if it's hard to
convince uncrustify, or if anyone else has a preference for "foo ++"
i'll cope.

> 4) uncrustify wants to move 'const char* foo' to 'const char *foo'.

Yes, please.  I prefer keeping the * next to the variable.

 --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] fix misspelling

[Daniel Kahn Gillmor]

Signed-off-by: Daniel Kahn Gillmor 
---
 lib/messages.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/messages.c b/lib/messages.c
index 04fa19f8..7ddfaf26 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -117,7 +117,7 @@ _notmuch_messages_has_next (notmuch_messages_t *messages)
return false;
 
 if (! messages->is_of_list_type)
-   INTERNAL_ERROR("_notmuch_messages_has_next not implimented for msets");
+   INTERNAL_ERROR("_notmuch_messages_has_next not implemented for msets");
 
 return (messages->iterator->next != NULL);
 }
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

Hi VA--

On Sat 2019-06-08 17:37:10 +0200, VA wrote:
> These methods were simply missing from the Python bindings.
> From 47dcf1659377f1ec8a237fbe474a5412123d0aa1 Mon Sep 17 00:00:00 2001
> From: hydrargyrum 
> Date: Sun, 27 Jan 2019 09:43:57 +0100
> Subject: [PATCH] python: bind add_property/remove_property and related methods
>
> Methods for manipulating properties were not bound in Python.

Thanks for offering this patch.

I think we've thus far deliberately avoided exposing property
manipulation from python because properties have (by convention i think)
tended to be things that are set or cleared only by the notmuch indexer
itself.  (see notmuch-properties(7) for description of those properties)

We do expose this functionality in the library, so it's not the end of
the world to expose it in the python bindings, but i do worry a little
bit about encouraging people to fiddle with markers set by the indexer.

as nomtuch-properties(7) says:

   Extensions  to  notmuch  which make use of properties are encouraged to
   report the specific properties used to the upstream notmuch project, as
   a way of avoiding collisions in the property namespace.

So: no objections from me to the idea of the functionality here -- if
someone thinks this is useful for some project, i hope they'll speak up
for how they plan to use it.

That said, i'd love it if at least the docstrings for functions which
modify properties included some concise, thoughtful suggestion that
encourages collaboration with the community and urges caution when
potentially tampering with properties used by other parts of the
ecosystem.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/2] debian: bump Standards-Version to 4.3.0 (no changes needed)

[Daniel Kahn Gillmor]

/usr/share/doc/debian-policy/upgrading-checklist.txt.gz suggests that
notmuch is already compliant with debian-policy 4.3.0.

Signed-off-by: Daniel Kahn Gillmor 
---
 debian/control | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index 31d6471c..56849500 100644
--- a/debian/control
+++ b/debian/control
@@ -29,7 +29,7 @@ Build-Depends:
  gnupg ,
  bash-completion (>=1.9.0~),
  texinfo
-Standards-Version: 4.1.3
+Standards-Version: 4.3.0
 Homepage: https://notmuchmail.org/
 Vcs-Git: https://git.notmuchmail.org/git/notmuch -b release
 Vcs-Browser: https://git.notmuchmail.org/git/notmuch
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] append _unused to the expression defined using unused() macro

[Daniel Kahn Gillmor]

On Thu 2019-05-30 22:56:14 +0300, Tomi Ollila wrote:
> This way if variables defined using unused() macro are actually
> used then code will not compile...
>
> - removed unused usage around one argc and one argv since those
>   were used
>
> - changed one unused (char *argv[]) to unused (char **argv) to
>   work with modified unused() macro definition

lgtm.  i've tested it against the master branch, and it works fine.  I
like the belt-and-suspenders approach.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 2/2] test: replace use of gdb with LD_PRELOAD shims in T070-insert.sh

[Daniel Kahn Gillmor]

From: David Bremner 

This removes the dependency of this test script on gdb, and
considerably speeds up the running of the tests.

---

dkg cleaned up the placement of gen_insert_msg, and used printf's %q
to handle the perverse case where $gen_msg_filename happens to have a
U+0022 QUOTATION MARK ('"') in it.

Signed-off-by: Daniel Kahn Gillmor 
---
 test/T070-insert.sh | 48 -
 1 file changed, 17 insertions(+), 31 deletions(-)

diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 48165caa..017124fc 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -2,8 +2,6 @@
 test_description='"notmuch insert"'
 . $(dirname "$0")/test-lib.sh || exit 1
 
-test_require_external_prereq gdb
-
 # subtests about file permissions assume that we're working with umask
 # 022 by default.
 umask 022
@@ -246,19 +244,19 @@ test_expect_code 1 "notmuch insert $gen_msg_filename 2>&1"
 notmuch config set new.tags $OLDCONFIG
 
 # DUPLICATE_MESSAGE_ID is not tested here, because it should actually pass.
-
-for code in OUT_OF_MEMORY XAPIAN_EXCEPTION FILE_NOT_EMAIL \
-READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
-cat < index-file-$code.gdb
-set breakpoint pending on
-set logging file index-file-$code.log
-set logging on
-break notmuch_database_index_file
-commands
-return NOTMUCH_STATUS_$code
-continue
-end
-run
+# pregenerate all of the test shims
+for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR 
OUT_OF_MEMORY XAPIAN_EXCEPTION; do
+make_shim shim-$code <
+#include 
+notmuch_status_t
+notmuch_database_index_file (notmuch_database_t *notmuch,
+ const char *filename,
+ notmuch_indexopts_t *indexopts,
+ notmuch_message_t **message_ret)
+{
+  return NOTMUCH_STATUS_$code;
+}
 EOF
 done
 
@@ -266,30 +264,18 @@ gen_insert_msg
 
 for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
 test_begin_subtest "EXIT_FAILURE when index_file returns $code"
-test_expect_code 1 \
- "${TEST_GDB} --batch-silent --return-child-result \
--ex 'set args insert < $gen_msg_filename' \
--x index-file-$code.gdb notmuch"
+test_expect_code 1 "$(printf "notmuch_with_shim shim-%q insert < %q" 
"$code" "$gen_msg_filename")"
 
 test_begin_subtest "success exit with --keep when index_file returns $code"
-test_expect_code 0 \
- "${TEST_GDB} --batch-silent --return-child-result \
--ex 'set args insert --keep < $gen_msg_filename' \
--x index-file-$code.gdb notmuch"
+test_expect_code 0 "$(printf "notmuch_with_shim shim-%q insert --keep < 
%q" "$code" "$gen_msg_filename")"
 done
 
 for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
 test_begin_subtest "EX_TEMPFAIL when index_file returns $code"
-test_expect_code 75 \
- "${TEST_GDB} --batch-silent --return-child-result \
--ex 'set args insert < $gen_msg_filename' \
--x index-file-$code.gdb notmuch"
+test_expect_code 75 "$(printf "notmuch_with_shim shim-%q insert < %q" 
"$code" "$gen_msg_filename")"
 
 test_begin_subtest "success exit with --keep when index_file returns $code"
-test_expect_code 0 \
- "${TEST_GDB} --batch-silent --return-child-result \
--ex 'set args insert --keep < $gen_msg_filename' \
--x index-file-$code.gdb notmuch"
+test_expect_code 0 "$(printf "notmuch_with_shim shim-%q insert --keep < 
%q" "$code" "$gen_msg_filename")"
 done
 
 test_done
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: notmuch vim failing to show rfc822 attachments

[Daniel Kahn Gillmor]

Hi Kay--

On Tue 2019-05-07 01:51:50 +0200, Kay wrote:

> I've recently switched to using notmuch for mail management and I like
> it so far. The only thing stopping me from full joy is that neither alot
> nor the notmuch vim frontend can display message/rfc822 attachments.

that sounds frustrating!

As i think you can see from the silence on the list, notmuch vim doesn't
get a lot of developer attention right now.  If you can demonstrate the
problem with notmuch 0.29, or from a local git development copy, and you
have a fix you'd like to propose, folks would probably look at it.

If you want help with alot, i recommend using the issue tracker at
https://github.com/pazz/alot

sorry to not have a better answer for you!

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] test: replace use of gdb with LD_PRELOAD shims in T070-insert.sh

[Daniel Kahn Gillmor]

On Sun 2019-05-26 10:08:54 -0300, David Bremner wrote:
> This removes the dependency of this test script on gdb, and
> considerably speeds up the running of the tests.

This series looks good to me.  I've tested it with moreutils parallel
installed, and it reduces total CPU time for the parallel test suite
from ~76 seconds of CPU to ~56 seconds, a > %25 reduction in CPU cost
for the whole test suite, despite touching only one test.  This is
awesome work.

I also like the elegance of the proposed change.  It would be great to
see it extended to the rest of our test suite's dependency on GDB if we
can do it. (T050, T060, and T380 i think)

Details on my profiling:

On the current master (bc396c967c7cd8e7a109858e428d7bf97173f7a7),
without these changes applied, the whole test suite consumes this much
CPU on my 4-core intel i5-2540M (2.60GHz):

real0m33.106s
user1m1.150s
sys 0m14.998s

On the same machine, with the pair of patches applied, the whole test
suite consumes this:

real0m27.557s
user0m44.172s
sys 0m12.018s

Caveat: i don't know how well LD_PRELOAD works on non-GNU/Linux
platforms, and we're not using LD_PRELOAD elsewhere in the test suite
yet.  Perhaps Ralph Seichter (explicitly cc'ed above) could comment on
how it'll affect homebrew?  Do we need to consider a variant for
platforms that can't do LD_PRELOAD?

One nit-pick below:

> diff --git a/test/T070-insert.sh b/test/T070-insert.sh
> index 48165caa..ab26ecd4 100755
> --- a/test/T070-insert.sh
> +++ b/test/T070-insert.sh
> @@ -2,8 +2,6 @@
>  test_description='"notmuch insert"'
>  . $(dirname "$0")/test-lib.sh || exit 1
>  
> -test_require_external_prereq gdb
> -
>  # subtests about file permissions assume that we're working with umask
>  # 022 by default.
>  umask 022
> @@ -246,50 +244,38 @@ test_expect_code 1 "notmuch insert $gen_msg_filename 
> 2>&1"
>  notmuch config set new.tags $OLDCONFIG
>  
>  # DUPLICATE_MESSAGE_ID is not tested here, because it should actually pass.
> +gen_insert_msg
>  
> -for code in OUT_OF_MEMORY XAPIAN_EXCEPTION FILE_NOT_EMAIL \
> -READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
> -cat < index-file-$code.gdb
> -set breakpoint pending on
> -set logging file index-file-$code.log
> -set logging on
> -break notmuch_database_index_file
> -commands
> -return NOTMUCH_STATUS_$code
> -continue
> -end
> -run
> +# pregenerate all of the test shims
> +for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR 
> OUT_OF_MEMORY XAPIAN_EXCEPTION; do
> +make_shim shim-$code < +#include 
> +#include 
> +notmuch_status_t
> +notmuch_database_index_file (notmuch_database_t *notmuch,
> + const char *filename,
> + notmuch_indexopts_t *indexopts,
> + notmuch_message_t **message_ret)
> +{
> +  return NOTMUCH_STATUS_$code;
> +}
>  EOF
>  done
>  
> -gen_insert_msg
> -

Why put the shim generation below gen_insert_msg?  If it were above, it
would make the comment about DUPLICATE_MESSAGE_ID less confusing, and
the changeset could be shorter.

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: aggregate-results.sh: consistent style. zero forks.

[Daniel Kahn Gillmor]

On Tue 2019-06-04 22:46:24 +0300, Tomi Ollila wrote:
> - all variables in $((...)) without leading $
> - all comparisons use -gt, -eq or -ne
> - no -a nor -o inside [ ... ] expressions
> - all indentation levels using one tab
>
> Dropped unnecessary empty string check when reading results files.
>
> Replaced pluralize() which was executed in subshell with
> pluralize_s(). pluralize_s sets $s to 's' or '' based on value of
> $1. Calls to pluralize_s are done in context of current shell, so
> no forks to subshells executed.

I'm fine with all these changes, but:

> -if [ "$fixed" = "0" ] && [ "$failed" = "0" ]; then
> +if [ $fixed -eq 0 ] && [ $failed -eq 0 ]; then

I think we've set $fixed and $failed above to a non-empty string value,
so this is technically correct.  But the shell programmer nit-picker in
me gets nervous seeing any variable used inside a test without proper
wrapping, and i'm going to have trouble adopting $foo instead of "$foo"
in other shell scripts.  it seems to require a lot of global reasoning
about the state of a given variable to use it without quotes safely, and
it introduces some subtle requirements (like, no unsetting these
variables and no setting them to the empty string).

So anyway, i don't see the harm in using "$foo" instead in this case,
and it seems like it actually reduces the cognitive burden of
maintiaining the code.  i'm happy to listen to any compelling story
yo've got for why this is an important change, but i'd prefer to avoid
it.  (i don't at all mind unwrapping "0" to 0 though)

thanks for doing this cleanup!

 --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [WIP PATCH] test: make test-serially to run test serially

[Daniel Kahn Gillmor]

On Wed 2019-05-08 19:46:25 +0300, Tomi Ollila wrote:
> This is easier and less error prone than mistyping NOTMUCH_TEST_SERIALIZE
> manually from command line (mistype make test-serially and it just doesn't
> work)
> ---
>
> quick first version. this works, but someone(tm) w/ native english experience
> could say how the naming sounds like...

This is an unusual use of make in my experience (i'm not used to seeing
variables exported in what looks like a rule dependency line), but i'm
not particularly fluent in make in the first place, so take that with a
grain of salt.

As a native en_US speaker, i have no problem with "make test-serially"
(though my usual idiom is to invoke the test suite with "make check" for
some reason).

But if this is useful for even one person who actively cares about the
notmuch codebase (Tomi), i think it should be merged.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Cycle-expand all org-style in show-mode and search all

[Daniel Kahn Gillmor]

Hi Pierre--

sorry for the delay in responding here, i'd missed this proposal when it
came in!

I *think* what you're trying to do here is, when reading a thread in
emacs' notmuch-show mode, you want to use "C-s" (I-search?) to find
whatever you're searching for in the folded messages as well as the
expanded ones.  Is that right?  do you always want to search in the
folded messages, or only sometimes?

In my own use of notmuch-emacs, i confess i've appreciated only
searching in expanded messages sometimes, and resented having to use
M-RET to expand-all before searching other times.  I don't know whether
i'd prefer one or the other in any given situation, and i guess i'd like
to be able to switch between them, but i'm not sure how to do that in a
user-friendly way.

On Wed 2019-04-10 12:33:23 +0200, Pierre Neidhardt wrote:
> The attached patch seems to work.
> What do you think?

I don't think i understand well enough what this change is trying to do,
even from reading the thread.  And the commit message itself is
*definitely* not enough to understand what the intent of the patch is.
It certainly doesn't seem to be related to the thread's subject line
"cycle-expand all org-style in show-mode and search all".

We generally try to make notmuch commit messages contain enough
motivation that when you go back and read it a year later you can tell
what you were trying to do here. (some of us are very forgetful!)

You don't need to copy the entire upstream emacs documentation for
invisible text or anything, but it'd be good to have the commit message
explain the problem encountered, and at least point at the mechanism the
patch is using to try to fix it.  Ideally, it should put a little bit of
thought into similar scenarios that it is *not* trying to change, so
that we can tell that it's scoped correctly.

One way to legitimately cut down on the length of the commit message
(and to ensure that your fix doesn't itself get broken later) is to
include a change to the test suite in your commit.  A good addition to
the test suite shows a plausible series of actions, and documents what
the correct output *should* be.  (even better if the use case is broken
before your patch, and fixed afterward!)

So anyway, could you take a stab at regenerate the 

(as an aside, i note that you signed your e-mail to the list, but the
patch appears to be outside the cryptographic signature.  Is seems
suboptimal -- you'd surely like us to evaluate your signature over the
message *including* the patch.  how was this message generated?  if it
was in notmuch-emacs, can you help me to recreate the steps you took so
that maybe we can fix it up or at least document it as a dangerous
path?)

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] debian: enable build hardening features

[Daniel Kahn Gillmor]

Debian's build hardening toolchain options produce binary artifacts
that are more resistant to compromise.  The most visible change for
notmuch today is likely to be the addition of the "bindnow" linker
flag, which contributes to making the "Global Offset Table" fully
read-only.

See https://wiki.debian.org/Hardening for more details.

Signed-off-by: Daniel Kahn Gillmor 
---
 debian/rules | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/debian/rules b/debian/rules
index d056edb6..ebd10481 100755
--- a/debian/rules
+++ b/debian/rules
@@ -2,6 +2,8 @@
 
 python3_all = py3versions -s | xargs -n1 | xargs -t -I {} env {}
 
+export DEB_BUILD_MAINT_OPTIONS = hardening=+all
+
 %:
dh $@ --with python2,python3,elpa
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] test: replace use of gdb with LD_PRELOAD shims in T070-insert.sh

[Daniel Kahn Gillmor]

One more nit-pick:

On Sun 2019-05-26 10:08:54 -0300, David Bremner wrote:
> +test_expect_code 0 "notmuch_with_shim shim-$code insert --keep < 
> \"$gen_msg_filename\""

This kind of business breaks obscurely if $gen_msg_filename happens to
have U+0022 QUOTATION MARK in it.  That's a pretty perverse situation,
but i'd generally prefer to use bash's builtin printf's %q for
robustness.

A revised patch will follow shortly that fixes both of my nitpicks.

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/2] debian: Add appropriate substitution variables to debian/control

[Daniel Kahn Gillmor]

Without this change, dh_gencontrol emits:

dpkg-gencontrol: warning: package python-notmuch: substitution variable 
${python:Provides} unused, but is defined
dpkg-gencontrol: warning: package python-notmuch: substitution variable 
${python:Versions} unused, but is defined
dpkg-gencontrol: warning: package notmuch-mutt: substitution variable 
${perl:Depends} unused, but is defined

Signed-off-by: Daniel Kahn Gillmor 
---
 debian/control | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index 56849500..ff646c6b 100644
--- a/debian/control
+++ b/debian/control
@@ -77,6 +77,8 @@ Package: python-notmuch
 Architecture: all
 Section: python
 Depends: ${misc:Depends}, ${python:Depends}, libnotmuch5 (>= ${source:Version})
+Provides: ${python:Provides}
+XB-Python-Version: ${python:Versions}
 Description: Python interface to the notmuch mail search and index library
  Notmuch is a system for indexing, searching, reading, and tagging
  large collections of email messages in maildir or mh format. It uses
@@ -152,7 +154,8 @@ Depends:
  notmuch (>= 0.4),
  libmail-box-perl, libmailtools-perl,
  libstring-shellquote-perl, libterm-readline-gnu-perl,
- ${misc:Depends}
+ ${misc:Depends},
+ ${perl:Depends},
 Recommends: mutt
 Enhances: notmuch, mutt
 Description: thread-based email index, search and tagging (Mutt interface)
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: add keywords to notmuch-emacs-mua.desktop

[Daniel Kahn Gillmor]

Debian's lintian has an informational alert
desktop-entry-lacks-keywords-entry, which recommends including
Keywords= in a .desktop file.

I dug around a bit in /usr/share/applications/*.desktop to make sure
that we covered the range of keywords other e-mail applications are
using.  If anyone has other suggestions for keywords, they can add
them to this list.

Signed-off-by: Daniel Kahn Gillmor 
---
 emacs/notmuch-emacs-mua.desktop | 1 +
 1 file changed, 1 insertion(+)

diff --git a/emacs/notmuch-emacs-mua.desktop b/emacs/notmuch-emacs-mua.desktop
index 0d9af2a4..752a1d7b 100644
--- a/emacs/notmuch-emacs-mua.desktop
+++ b/emacs/notmuch-emacs-mua.desktop
@@ -8,3 +8,4 @@ Icon=emblem-mail
 Terminal=false
 Type=Application
 Categories=Network;Email;
+Keywords=Mail;E-mail;Email;
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [BUG] emacs: notmuch-mua-attachment-check finds triggering string inside forwarded messages

[Daniel Kahn Gillmor]

On Wed 2019-05-08 19:19:18 +0200, Örjan Ekeberg wrote:

> I have found what seems to be a bug, or at least a misbehaviour of the
> "missing attachment warning" implemented by the otherwise so nice
> notmuch-mua-attachment-check.
>
> It works fine to detect the regexp for attachments in simple messages.
> The problem is that it also triggers the warning if a matching string is
> present inside a forwarded message.  This is particularly annoying when
> forwarding messages originating from MS-Exchange since those seem to
> always include a hidden header "X-MS-Has-Attach" where the word "Attach"
> constantly leads to false missing-attachment warnings.

I've tagged this with notmuch::bug so that it shows up at
https://nmbug.notmuchmail.org/status/, and i'm directly cc'ing dme,
since i think he's the author of notmuch-mua-attachment-check.  

> Would it be possible to somehow restrict the regexp search to the part
> of the message actually being authored?  Would it be too simplistic to
> end the search at the first occurrence of "\n\n<#" ?

that's pretty simplistic, but does seem better than the status quo.  do
you have a proposed patch?

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [emacs] Auto-rotate pictures

[Daniel Kahn Gillmor]

Hi Pierre--

On Wed 2019-04-03 11:10:54 +0200, Pierre Neidhardt wrote:

> Some pictures embed "exif" metadata with autorotate information.
> I know that mu4e can autorotate inlined pictures.
> I guess it wouldn't be too hard to implement this for Emacs Notmuch
> either.

If you have pointers to how mu4e does the autorotation, that might
encourage one of the elisp hackers who reads this list see how to get
notmuch-emacs to do the same thing.

i am not one of the elisp hackers, sadly :(

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: feature request: caching message arrival time

[Daniel Kahn Gillmor]

On Mon 2019-06-03 18:02:53 +0200, Örjan Ekeberg wrote:
> As far as I understand the autocrypt protocol (i.e. not much;-) ), the
> vulnerability is that an incoming message with a later time-stamp than
> the locally saved autocrypt status can update the stored state
> (e.g. turn off encryption).  Manipulating the time-stamp to make the
> message appear to be *older* than it really is should only mean that it is
> less likely to update the saved state?
>
> If this is correct, using the oldest of all the time-stamps seen in the
> Date-header and any of the Received-headers should be the most
> defensive.

It's the most defensive against one form of attack: forging e-mails
intended to update the user's Autocrypt state about a given peer.

But another form of attack is also possible: convincing the user to
*not* update their Autocrypt state about a given peer, while leaving the
original message otherwise plausible and intact, thereby raising no
suspicions about delivery problems.

I'd like notmuch's Autocrypt implementation to try to defend against
either attack where possible.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: feature request: caching message arrival time

[Daniel Kahn Gillmor]

On Mon 2019-06-03 16:02:48 +0200, Ralph Seichter wrote:
> Not meaning to complicate things, but Notmuch does not receive messages
> at all. ;-) One needs to rely on some software to populate the Maildir
> tree (Dovecot LMTP in my case, Postfix or some other MTA for local
> delivery in other cases). Any software transporting the raw messages
> can, and sometimes must, manipulate the header data, and the order in
> which files within the Maildir tree are created is also not determined
> by Notmuch.
>
> As an example: My nightly backup script disables local delivery for the
> duration of the backup process. Once reactivated, delivery of queued
> messages resumes, but it is not guaranteed to happen in the order of
> arrival. So even the local MTA, although trusted, might induce issues in
> terms of delivery time.

I agree with you!  the e-mail system, like any other store-and-forward
ecosystem, offers no guarantees of message delivery.

fwiw, i'm not claiming that the time notmuch receives the message is
guaranteed to be close to the time that the message was sent.

but i can guarantee two things:

 * notmuch cannot receive the message *before* it was sent :)

 * if the local system clock is correct, notmuch can place a plausible
   upper bound on the Date: header that is included in the message.

This alone is useful data.

 --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: feature request: caching message arrival time

[Daniel Kahn Gillmor]

On Mon 2019-06-03 10:57:15 +0200, Örjan Ekeberg wrote:
> Daniel Kahn Gillmor  writes:
>
>> So Autocrypt defines the "effective date" of a message as the *earliest*
>> of two dates: the date that the message is first seen, and the Date:
>> header itself.  So we want our augmented Autocrypt header ingestion
>> routine to search for all other messages we know about from the sender
>> that have both a later firstseen= property *and* a later Date: header.
>
> Would it be possible to use the earliest date seen in any of the
> Received: headers as a safeguard against future-dated messages?

Sure, assuming that you trust the closest MTA in the chain of MTAs that
handed the message off to you, since an adversarial proximal MTA could
manipulate all the existing Received: headers as well.

But I'm a bit uncomfortable with it: this sort of protection actually
opens up a new attack vector that didn't exist before -- any MTA in the
chain can now make the message seem like it was actually from the
*past*, just by setting its own Received: header.

Technically, of course, any MTA could munge the actual Date: header as
well to perform this kind of attack, but that munging would at least
have the potential to be detected by anyone who cares to verify DKIM
headers; but Received: headers are impossible to cover with DKIM.

If there was no expense to the indexing and storage, i'd say it would be
good to just go ahead and index the earliest Received: header as well,
to have that data trivially available as a data point in evaluating
incoming messages.  But since it sounds like there's a cost (in
performance and storage) that would need to be profiled, i don't know
that i can say it's worth the tradeoff.

Since notmuch actually knows when it recieved the message, it seems like
it would be simplest (and less vulnerable to manipulation) to just
record that timestamp directly.

 --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: cleanup commented archive services

[Daniel Kahn Gillmor]

MarkMail and Nabble both support https.  I can no longer get any DNS
resolution for opensubscriber.com.

Signed-off-by: Daniel Kahn Gillmor 
---
 emacs/notmuch-show.el | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e13ca3d7..cf759918 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -188,9 +188,8 @@ each attachment handler is logged in buffers with names 
beginning
 ("Mail Archive, The" . "https://mid.mail-archive.com/;)
 ("LKML" . "https://lkml.kernel.org/r/;)
 ;; FIXME: can these services be searched by `Message-Id' ?
-;; ("MarkMail" . "http://markmail.org/;)
-;; ("Nabble" . "http://nabble.com/;)
-;; ("opensubscriber" . "http://opensubscriber.com/;)
+;; ("MarkMail" . "https://markmail.org/;)
+;; ("Nabble" . "https://nabble.com/;)
 )
   "List of Mailing List Archives to use when stashing links.
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: feature request: caching message arrival time

[Daniel Kahn Gillmor]

On Sat 2019-06-01 16:19:19 +0200, Ralph Seichter wrote:
> I'm interested. Right now I frankly don't know what knowing when a
> message was first seen by Notmuch might be useful for. That makes it
> a bit difficult for me to contemplate your questions.

Sure, thanks for asking!

As i went to write this down, it became a lot longer than i'd expected.
sorry about that!  On the positive side, i may have convinced myself in
the process that the threat this mechanism would defend against is small
enough that it may not be worth the additional implementation (though if
the implementation were there, we'd certainly want to use it).

So, this is a story about Autocrypt state, out-of-order delivery, and
e-mails with suspicious date stamps ("from the future"). (if you're
reading this message haven't been following Autocrypt closely, you can
read up at https://www.autocrypt.org/)

--

When receiving an e-mail sent From: the peer f...@example.org, an
Autocrypt-capable client needs to update the Autocrypt state for that
peer's e-mail address ("f...@example.org").  This is the case for
messages that have an Autocrypt: header *and* for messages that *don't*
have one.

Both kinds of messages update the Autocrypt peer state, because if you
start receiving Autocrypt-free messages from someone who used Autocrypt
in the past, your client needs to make a note of that and consider it
when it makes its recommendation for new outbound messages to that peer.

Additionally, sometimes we receive e-mail messages out of order.
sometimes this is because we're suddenly running across a cache of old
messages, sometimes it's because we've just popped online after a day
off, and sometimes it's because SMTP had a hiccup (there are probably
many other reasons).

We also probably don't want to store state about everyone who has ever
sent us mail *without* using Autocrypt.  At the moment, at least, that's
probably most senders, and it's both a waste of space and a potential
privacy concern to record a lot of empty state that just indicates that
you got mail from someone at some point in the past.  So if we've never
seen an Autocrypt header from a given peer, there's no state to update.

So now consider the following set of e-mail messages all from the same
sender; mails with a * have an Autocrypt header, and the times
following the message indicates its Date: header in an abstract way
(higher numbers are later than lower numbers).

 A: (time 1)
 B*: (time 2)
 C: (time 3)

Let's assume that i update Autocrypt state about the peer upon receipt
of each message, regardless of what order the messages were sent.  We
want the Autocrypt state to be immutable, independent of the order of
delivery.

If i receive them at times 4, 5, and 6 in order (A, B, C) then i'll
think that the Autocrypt state for the peer is "we had an Autocrypt
header earlier (from B), but a more recent delivery (C) suggests that
they might not be using Autocrypt reliably" (depending on the actual
difference in time between the Date:s of B and C, the peer might end up
with an Autocrypt recommendation called "discourage").  This is the
correct state for us to end up in.

But now imagine that at times 4, 5, and 6 i receive the messages in the
order A, C, B.  If i don't store Autocrypt state for the peer at times 4
and 5, because i've never seen an Autocrypt header for the peer before,
and there is none in messages A and C.  Then my end result is that i'll
think that the Autocrypt state for the peer is just the Autocrypt header
from B.

But that's it's different from what we ended up with when we received
the messages in order.

Now, we can improve on this with the following extra technique: when a
peer goes from no Autocrypt state to having an Autocrypt state, we can
search the existing index for messages from that peer with a later Date:
header.  If we find such a message, then we should include it in our
calculations.  If we do that, then we end up with the correct state,
regardless of the order of delivery.  good!

So far, we haven't needed the firstseen= property yet.  There's one
final wrinkle that introduces the need for it: message Date: headers can
be wrong.  They can even be grossly wrong -- they can be from the
future.  This can happen when the sender's clock is bad, mainly, but it
can also happen through malice (someone wanting to forge a message to
mess with the receipient's state about a given peer, for example).

So Autocrypt defines the "effective date" of a message as the *earliest*
of two dates: the date that the message is first seen, and the Date:
header itself.  So we want our augmented Autocrypt header ingestion
routine to search for all other messages we know about from the sender
that have both a later firstseen= property *and* a later Date: header.

Otherwise, one poorly formed e-mail without an Autocrypt header with the
Date: set to the year 3000 (the "bogus future message") would make it so
that the peer's recommendation would be set to "discourage" when 

Re: forwarding multiple messages from notmuch emacs

[Daniel Kahn Gillmor]

On Sun 2016-04-10 15:26:35 +0100, David Edmondson wrote:
> On Tue, Apr 23 2013, Daniel Kahn Gillmor wrote:
>> hi notmuch folks--
>>
>> i'd like to be able to forward several messages from a given thread (up
>> to and including the whole thread) to someone else.  I use
>> notmuch-emacs.
>>
>> I don't think it's possible to do this at the moment;
>> notmuch-show-forward-message just forwards the message where the point
>> is located, even if the current region covers more than one message.
>
> I believe that this is fixed in commit
> a982773dfb6e8efe1bcee90e888f2560ad006fb5. Could you test and confirm,
> please?

Many years later, i follow up on this message -- yes, it does work!  in
particular, 'F' invokes notmuch-show-forward-open-messages, which is
exactly what i want it to do.  Thank you, dme!

sorry for the lag in following up.

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

feature request: caching message arrival time

[Daniel Kahn Gillmor]

Hi Notmuch folks--

I'm working on Autocrypt integration for notmuch right now, and it
occurs to me that it might be useful to know the time that any given
message was first seen by notmuch.

I'm trying to not get distracted by implementing such a feature, but I
wanted to log this as a feature request, along with a few thoughts about
it.

My idea is that the first time notmuch indexes a message, it would add a
property to the message like firstseen=2019-05-31T23:15:24Z.

Some nuances spring to mind:

 * This should *not* be cleared and reset on reindexing, so it doesn't
   belong in the index.* property namespace.

 * What happens when you delete a message?  Maybe we should keep that
   value around for "ghosts" too -- can ghost documents have properties?
   Or is it bad to remember that we've seen the message if someone
   deletes it?

 * When even the ghost goes away (e.g. full thread deletion), presumably
   this property would go away.  So If you deleted the message from your
   message store, notmuch would forget about it, and then the next time
   you ingest it it would get a later "firstseen=" property.  I'm ok
   with this.

 * i don't think we have a way to search properties by range (e.g. the
   way that we can search date ranges).  i don't need that feature for
   my use case, but maybe someone will come up with a use case that
   wants it?  is there a way to store the datestamp in a way that it can
   be scanned the way that "date" can?  or do we already have this and
   i'm just unaware?

 * What is the cost in terms of database size?  It doesn't look like it
   would be expensive to me, but i haven't profiled it.

 * if we make such a change, how should we deal with already-indexed
   messages?

Anyone have any thoughts, suggestions, or objections to this?  I'm happy
to explain more about my use case if people are interested too.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: viewing duplicate messages

[Daniel Kahn Gillmor]

On Fri 2019-05-31 12:45:46 -0300, Jorge P. de Morais Neto wrote:
> I have searched and found no copy.  That's because I physically delete
> these Dell notification messages after reading and acting on them.  This
> way there is always 0 or 1 such message, so Notmuch does not get
> confused.  But I will save the next ones and give you.  In fact, in an
> attempt to temporarily increase the frequency of notifications, I have
> just subscribed to notifications about everything related to my notebook
> series, even MS-Windows drivers.

thanks!  i don't mean to interrupt your workflow, which makes total
sense to me.  feel free to send them to me as they come in.

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 4/5] cli/{show,reply}: skip over legacy-display parts

[Daniel Kahn Gillmor]

Make use of the previous changes to fast-forward past any
legacy-display parts during "notmuch show" and "notmuch reply".

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c| 11 ++-
 test/T356-protected-headers.sh |  2 --
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 33c51156..41cc7581 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -332,7 +332,16 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject 
*part, int numchild) {
node_verify (node, part);
}
 } else {
-   _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, 
node->parent ? node->parent->part : NULL, numchild);
+   if (_notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, 
part, node->parent ? node->parent->part : NULL, numchild) &&
+   node->ctx->msg_crypto->decryption_status == 
NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   GMimeObject *clean_payload = 
_notmuch_repair_crypto_payload_skip_legacy_display (part);
+   if (clean_payload != part) {
+   /* only one layer of recursion is possible here
+* because there can be only a single cryptographic
+* payload: */
+   return _mime_node_set_up_part (node, clean_payload, numchild);
+   }
+   }
 }
 
 return true;
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 43dfffe6..867b8722 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -137,14 +137,12 @@ id:protected-hea...@crypto.notmuchmail.org
 id:subjectless-protected-hea...@crypto.notmuchmail.org'
 
 test_begin_subtest "when rendering protected headers, avoid rendering 
legacy-display part"
-test_subtest_known_broken
 output=$(notmuch show --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
 'subject:[0][0][0]["headers"]["Subject"]="Interrupting Cow"' \
 
'no_legacy_display:[0][0][0]["body"][0]["content"][1]["content-type"]="text/plain"'
 
 test_begin_subtest "when replying, avoid rendering legacy-display part"
-test_subtest_known_broken
 output=$(notmuch reply --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
 
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 3/5] util/repair: add _notmuch_repair_crypto_payload_skip_legacy_display

[Daniel Kahn Gillmor]

This is a utility function designed to make it easier to
"fast-forward" past a legacy-display part associated with a
cryptographic envelope, and show the user the intended message body.

The bulk of the ugliness in here is in the test function
_notmuch_crypto_payload_has_legacy_display, which tests all of the
things we'd expect to be true in a a cryptographic payload that
contains a legacy display part.

Signed-off-by: Daniel Kahn Gillmor 
---
 util/repair.c | 98 +++
 util/repair.h | 17 +
 2 files changed, 115 insertions(+)

diff --git a/util/repair.c b/util/repair.c
index f91c1244..040f2c02 100644
--- a/util/repair.c
+++ b/util/repair.c
@@ -18,4 +18,102 @@
  * Authors: Daniel Kahn Gillmor 
  */
 
+#include 
 #include "repair.h"
+
+
+static bool
+_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;
+
+if (! g_mime_content_type_is_type (g_mime_object_get_content_type 
(payload),
+  "multipart", "mixed"))
+   return false;
+protected_header_parameter = g_mime_object_get_content_type_parameter 
(payload, "protected-headers");
+if ((! protected_header_parameter) || strcmp (protected_header_parameter, 
"v1"))
+   return false;
+if (! GMIME_IS_MULTIPART (payload))
+   return false;
+mpayload = GMIME_MULTIPART (payload);
+if (mpayload == NULL)
+   return false;
+if (g_mime_multipart_get_count (mpayload) != 2)
+   return false;
+first = g_mime_multipart_get_part (mpayload, 0);
+if (! g_mime_content_type_is_type (g_mime_object_get_content_type (first),
+  "text", "rfc822-headers"))
+   return false;
+protected_header_parameter = g_mime_object_get_content_type_parameter 
(first, "protected-headers");
+if ((! protected_header_parameter) || strcmp (protected_header_parameter, 
"v1"))
+   return false;
+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;
+   break;
+   }
+   GMimeHeader *ph = g_mime_header_list_get_header (protected_headers, 
g_mime_header_get_name (dh));
+   if (ph == NULL) {
+   ret = false;
+   break;
+   }
+   if (strcmp (g_mime_header_get_value (dh), g_mime_header_get_value 
(ph))) {
+   ret = false;
+   break;
+   }
+   }
+}
+
+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;
+}
+
+GMimeObject *
+_notmuch_repair_crypto_payload_skip_legacy_display (GMim

[PATCH v2 1/5] test: avoid showing legacy-display parts

[Daniel Kahn Gillmor]

Enigmail generates a "legacy-display" part when it sends encrypted
mail with a protected Subject: header.  This part is intended to
display the Subject for mail user agents that are capable of
decryption, but do not know how to deal with embedded protected
headers.

This part is the first child of a two-part multipart/mixed
cryptographic payload within a cryptographic envelope that includes
encryption (that is, it is not just a cleartext signed message).  It
uses Content-Type: text/rfc822-headers.

That is:

A └┬╴multipart/encrypted
B  ├─╴application/pgp-encrypted
C  └┬╴application/octet-stream
*   ╤ 
D   └┬╴multipart/mixed; protected-headers=v1 (cryptographic payload)
E├─╴text/rfc822-headers; protected-headers=v1 (legacy-display part)
F└─╴… (actual message body)

In discussions with jrollins, i've come to the conclusion that a
legacy-display part should be stripped entirely from "notmuch show"
and "notmuch reply" now that these tools can understand and interpret
protected headers.

You can tell when a message part is a protected header part this way:

 * is the payload (D) multipart/mixed with exactly two children?
 * is its first child (E) Content-Type: text/rfc822-headers?
 * does the first child (E) have the property protected-headers=v1?
 * do all the headers in the body of the first child (E) match
   the protected headers in the payload part (D) itself?

If this is the case, and we already know how to deal with the
protected header, then there is no reason to try to render the
legacy-display part itself for the user.

Furthermore, when indexing, if we are indexing properly, we should
avoid indexing the text in E as part of the message body.

'notmuch reply' is an interesting case: the standard use of 'notmuch
reply' will end up omitting all mention of protected Subject:.

The right fix is for the replying MUA to be able to protect its
headers, and for it to set them appropriately based on headers found
in the original message.

If a replying MUA is unable to protect headers, but still wants the
user to be able to see the original header, a replying MUA that
notices that the original message's subject differs from the proposed
reply subject may choose to include the original's subject in the
quoted/attributed text. (this would be a stopgap measure; it's not
even clear that there is user demand for it)

This test suite change indicates what we want to happen for this case
(the tests are currently broken), and includes three additional TODO
suggestions of subtle cases for anyone who wants to flesh out the test
suite even further.  (i believe all these cases should be already
fixed by the rest of this series, but haven't had time to write the
tests for the unusual cases)

Signed-off-by: Daniel Kahn Gillmor 
---
 test/T356-protected-headers.sh| 33 +++
 .../protected-with-legacy-display.eml | 40 +++
 2 files changed, 73 insertions(+)
 create mode 100644 
test/corpora/protected-headers/protected-with-legacy-display.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 4af018f3..43dfffe6 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -136,4 +136,37 @@ id:nested-rfc822-mess...@crypto.notmuchmail.org
 id:protected-hea...@crypto.notmuchmail.org
 id:subjectless-protected-hea...@crypto.notmuchmail.org'
 
+test_begin_subtest "when rendering protected headers, avoid rendering 
legacy-display part"
+test_subtest_known_broken
+output=$(notmuch show --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+'subject:[0][0][0]["headers"]["Subject"]="Interrupting Cow"' \
+
'no_legacy_display:[0][0][0]["body"][0]["content"][1]["content-type"]="text/plain"'
+
+test_begin_subtest "when replying, avoid rendering legacy-display part"
+test_subtest_known_broken
+output=$(notmuch reply --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
+
+test_begin_subtest "do not treat legacy-display part as body when indexing"
+test_subtest_known_broken
+output=$(notmuch search --output=messages body:interrupting)
+test_expect_equal "$output" ''
+
+test_begin_subtest "identify message that had a legacy display part skipped 
during indexing"
+test_subtest_known_broken
+output=$(notmuch search --output=messages 
property:index.repaired=skip-protected-headers-legacy-display)
+test_expect_equal "$output" 
id:protected-with-legacy-disp...@crypto.notmuchmail.org
+
+# TODO: test that a part that looks like a legacy-display in
+# multipart/s

Re: [PATCH 4/6] mime-node: split out _mime_node_set_up_part

[Daniel Kahn Gillmor]

On Fri 2019-05-31 00:41:54 -0400, Daniel Kahn Gillmor wrote:
> This particular re-organization has a slight conflict with the patch
> proposed in id:20190530172707.10378-5-...@fifthhorseman.net (patch 4/4
> from the "mixed-up mangling repair" series).

looking in more detail, i found some subtle differences that made
merging them trickier than expected.

please instead consider v3 of "mixed-up messages" and v2 of this "skip
protected header legacy display parts" series, both of which depend on
the "setup for message repair" series (found at
id:20190531074027.16337-1-...@fifthhorseman.net).

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 2/5] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

[Daniel Kahn Gillmor]

Our _notmuch_message_crypto_potential_payload implementation could
only return a failure if bad arguments were passed to it.  It is an
internal function, so if that happens it's an entirely internal bug
for notmuch.

It will be more useful for this function to return whether or not the
part is in fact a cryptographic payload, so we dispense with the
status return.

If some future change suggests adding a status return back, there are
only a handful of call sites, and no pressure to retain a stable API,
so it could be changed easily. But for now, go with the simpler
function.

Signed-off-by: Daniel Kahn Gillmor 
---
 lib/index.cc  |  9 ++---
 mime-node.c   |  6 +-
 util/crypto.c | 27 ---
 util/crypto.h |  7 +--
 4 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 1fd9e67e..deb76f6f 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -405,7 +405,6 @@ _index_mime_part (notmuch_message_t *message,
  _notmuch_message_add_term (message, "tag", "encrypted");
 
for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
-   notmuch_status_t status;
GMimeObject *child;
if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
/* Don't index the signature, but index its content type. */
@@ -434,11 +433,7 @@ _index_mime_part (notmuch_message_t *message,
continue;
}
child = g_mime_multipart_get_part (multipart, i);
-   status = _notmuch_message_crypto_potential_payload (msg_crypto, 
child, part, i);
-   if (status)
-   _notmuch_database_log (notmuch_message_get_database (message),
-  "Warning: failed to mark the potential 
cryptographic payload (%s).\n",
-  notmuch_status_to_string (status));
+   _notmuch_message_crypto_potential_payload (msg_crypto, child, part, 
i);
_index_mime_part (message, indexopts, child, msg_crypto);
}
return;
@@ -577,7 +572,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, 
GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
+_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT 
(encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
 _index_mime_part (message, indexopts, clear, msg_crypto);
 g_object_unref (clear);
 
diff --git a/mime-node.c b/mime-node.c
index 0be03de7..33c51156 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -292,8 +292,6 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
 /* associate a MIME part with a node. */
 static bool
 _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild) {
-notmuch_status_t status;
-
 /* Deal with the different types of parts */
 if (GMIME_IS_PART (part)) {
node->part = part;
@@ -334,9 +332,7 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject 
*part, int numchild) {
node_verify (node, part);
}
 } else {
-   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, 
numchild);
-   if (status)
-   fprintf (stderr, "Warning: failed to record potential crypto 
payload (%s).\n", notmuch_status_to_string (status));
+   _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, 
node->parent ? node->parent->part : NULL, numchild);
 }
 
 return true;
diff --git a/util/crypto.c b/util/crypto.c
index 9e185e03..743c751a 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -132,19 +132,16 @@ _notmuch_message_crypto_potential_sig_list 
(_notmuch_message_crypto_t *msg_crypt
 }
 
 
-notmuch_status_t
-_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t 
*msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum)
+bool
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t 
*msg_crypto, GMimeObject *part, GMimeObject *parent, int childnum)
 {
 const char *protected_headers = NULL;
 const char *forwarded = NULL;
 const char *subject = NULL;
 
-if (!msg_crypto || !payload)
-   return NOTMUCH_STATUS_NULL_POINTER;
-
 /* only fire on the first payload part encountered */
 if (msg_crypto->payload_encountered)
-   return NOTMUCH_STATUS_SUCCESS;
+   return false;
 
 /* the first child of multipart/encrypted that matches the
  * encryption protocol should be "control information" metadata,
@@ -152,11 +149,11 @@ _notmuch_message_crypto_potential_payload 
(_notmuch_message_crypto_t *msg_crypto
  * https://tools.ietf.org/html/rfc1847#page-8) */
 if (parent && GMIME_IS_MULTIPART_ENC

[PATCH v2 5/5] index: avoid indexing legacy-display parts

[Daniel Kahn Gillmor]

When we notice a legacy-display part during indexing, it makes more
sense to avoid indexing it as part of the message body.

Given that the protected subject will already be indexed, there is no
need to index this part at all, so we skip over it.

If this happens during indexing, we set a property on the message:
index.repaired=skip-protected-headers-legacy-display

Signed-off-by: Daniel Kahn Gillmor 
---
 doc/man7/notmuch-properties.rst |  6 ++
 lib/index.cc| 20 
 test/T356-protected-headers.sh  |  2 --
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 2e610683..e2db2ef5 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=skip-protected-headers-legacy-display`` indicates
+that when indexing the cleartext of an encrypted message, notmuch
+skipped over a "legacy-display" text/rfc822-headers part that it
+found in that message, since it was able to index the built-in
+protected headers directly.
+
 SEE ALSO
 
 
diff --git a/lib/index.cc b/lib/index.cc
index deb76f6f..769e29a9 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -433,8 +433,14 @@ _index_mime_part (notmuch_message_t *message,
continue;
}
child = g_mime_multipart_get_part (multipart, i);
-   _notmuch_message_crypto_potential_payload (msg_crypto, child, part, 
i);
-   _index_mime_part (message, indexopts, child, msg_crypto);
+   GMimeObject *toindex = child;
+   if (_notmuch_message_crypto_potential_payload (msg_crypto, child, 
part, i) &&
+   msg_crypto->decryption_status == 
NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   toindex = _notmuch_repair_crypto_payload_skip_legacy_display 
(child);
+   if (toindex != child)
+   notmuch_message_add_property (message, "index.repaired", 
"skip-protected-headers-legacy-display");
+   }
+   _index_mime_part (message, indexopts, toindex, msg_crypto);
}
return;
 }
@@ -572,8 +578,14 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT 
(encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
-_index_mime_part (message, indexopts, clear, msg_crypto);
+GMimeObject *toindex = clear;
+if (_notmuch_message_crypto_potential_payload (msg_crypto, clear, 
GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT) &&
+   msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   toindex = _notmuch_repair_crypto_payload_skip_legacy_display (clear);
+   if (toindex != clear)
+   notmuch_message_add_property (message, "index.repaired", 
"skip-protected-headers-legacy-display");
+}
+_index_mime_part (message, indexopts, toindex, msg_crypto);
 g_object_unref (clear);
 
 status = notmuch_message_add_property (message, "index.decryption", 
"success");
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 867b8722..925805df 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -148,12 +148,10 @@ test_json_nodes <<<"$output" \
 
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
 
 test_begin_subtest "do not treat legacy-display part as body when indexing"
-test_subtest_known_broken
 output=$(notmuch search --output=messages body:interrupting)
 test_expect_equal "$output" ''
 
 test_begin_subtest "identify message that had a legacy display part skipped 
during indexing"
-test_subtest_known_broken
 output=$(notmuch search --output=messages 
property:index.repaired=skip-protected-headers-legacy-display)
 test_expect_equal "$output" 
id:protected-with-legacy-disp...@crypto.notmuchmail.org
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

v2 of skipping over legacy-display protected header parts

[Daniel Kahn Gillmor]

This is the second revision of the series that skips over
"legacy-display" protected header parts.

v1 can be found at id:20190531042825.27774-1-...@fifthhorseman.net

--
Now that notmuch can handle and interpret protected subject lines, it
should also avoid forcing the user to look at "legacy display" parts
that some MUAs (notably enigmail) copies of the protected headers that
are intended to be rendered only by legacy clients -- clients capable
of decryption but which don't understand how to handle protected
headers.
--

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

Comments welcome!

 --dkg


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[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 
---
 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 ..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...@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...@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...@mangling.notmuchmail.org
+
+test_begin_subtest "index cleartext of 'Mixed-Up' mangled PGP/MIME message"
+test_expect_success 'notmuch reindex --decrypt=true 
id:mixed...@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...@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 ..a09f6191
--- /dev/null
+++ b/test/corpora/mangling/mixed-up.eml
@@ -0,0 +1,33 @@
+From: test_su...@notmuchmail.org
+To: test_su...@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +
+Message-ID: 
+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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[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 
---
 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...@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...@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...@mangling.notmuchmail.org
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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-...@fifthhorseman.net.  Version 2
can be found at id:20190530172707.10378-1-...@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-...@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


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[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 
---
 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 
  */
 
+#include 
 #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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[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 
---
 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...@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...@mangling.notmuchmail.org)
 test_json_nodes <<<"$output" \
 'body:["original"]'"$bodytext"
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/2] mime-node: split out _mime_node_set_up_part

[Daniel Kahn Gillmor]

This is a code reorganization that should have no functional effect,
but will make future changes simpler, because a future commit will
reuse the _mime_node_set_up_part functionality without touching
_mime_node_create.

In the course of splitting out this function, I noticed a comment in
the codebase that referred to an older name of _mime_node_create
(message_part_create), where this functionality originally resided.
I've fixed that comment to refer to the new function instead.

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 3492bcd0..0be03de7 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -264,14 +264,15 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject 
*part)
g_error_free (err);
 }
 
+static bool
+_mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild);
+
 static mime_node_t *
 _mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
 {
 mime_node_t *node = talloc_zero (parent, mime_node_t);
-notmuch_status_t status;
 
 /* Set basic node properties */
-node->part = part;
 node->ctx = parent->ctx;
 if (!talloc_reference (node, node->ctx)) {
fprintf (stderr, "Out of memory.\n");
@@ -282,10 +283,23 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
*part, int numchild)
 node->part_num = node->next_part_num = -1;
 node->next_child = 0;
 
+if (_mime_node_set_up_part (node, part, numchild))
+   return node;
+talloc_free (node);
+return NULL;
+}
+
+/* associate a MIME part with a node. */
+static bool
+_mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild) {
+notmuch_status_t status;
+
 /* Deal with the different types of parts */
 if (GMIME_IS_PART (part)) {
+   node->part = part;
node->nchildren = 0;
 } else if (GMIME_IS_MULTIPART (part)) {
+   node->part = part;
node->nchildren = g_mime_multipart_get_count (GMIME_MULTIPART (part));
 } else if (GMIME_IS_MESSAGE_PART (part)) {
/* Promote part to an envelope and open it */
@@ -297,11 +311,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
*part, int numchild)
 } else {
fprintf (stderr, "Warning: Unknown mime part type: %s.\n",
 g_type_name (G_OBJECT_TYPE (part)));
-   talloc_free (node);
-   return NULL;
+   return false;
 }
 
-/* Handle PGP/MIME parts */
+/* Handle PGP/MIME parts (by definition not cryptographic payload parts) */
 if (GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt != 
NOTMUCH_DECRYPT_FALSE)) {
if (node->nchildren != 2) {
/* this violates RFC 3156 section 4, so we won't bother with it. */
@@ -321,12 +334,12 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
*part, int numchild)
node_verify (node, part);
}
 } else {
-   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);
+   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, 
numchild);
if (status)
fprintf (stderr, "Warning: failed to record potential crypto 
payload (%s).\n", notmuch_status_to_string (status));
 }
 
-return node;
+return true;
 }
 
 mime_node_t *
@@ -347,7 +360,7 @@ mime_node_child (mime_node_t *parent, int child)
 } else if (GMIME_IS_MESSAGE (parent->part)) {
sub = g_mime_message_get_mime_part (GMIME_MESSAGE (parent->part));
 } else {
-   /* This should have been caught by message_part_create */
+   /* This should have been caught by _mime_node_set_up_part */
INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
g_type_name (G_OBJECT_TYPE (parent->part)));
 }
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Setup for message repair

[Daniel Kahn Gillmor]

This series of two commits offers no functional change, but it is a
useful basis for two outstanding series that i'd like to get merged:

 * "mixed up" message mangling
 * skipping legacy-display protected headers

Both of these series include code that touches lightly on notmuch's
MIME tree-crawling code, both for rendering ("notmuch show" and
"notmuch reply") and during indexing.

I'm hoping that these changes will make it somewhat less painful to
think about merging both series.

 --dkg


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/2] repair: set up codebase for repair functionality

[Daniel Kahn Gillmor]

This adds no functionality directly, but is a useful starting point
for adding new repair functionality.

Signed-off-by: Daniel Kahn Gillmor 
---
 doc/man7/notmuch-properties.rst | 12 
 lib/notmuch-private.h   |  1 +
 notmuch-client.h|  1 +
 util/Makefile.local |  1 +
 util/repair.c   | 21 +
 util/repair.h   | 17 +
 6 files changed, 53 insertions(+)
 create mode 100644 util/repair.c
 create mode 100644 util/repair.h

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 802e6763..2e610683 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -109,6 +109,18 @@ of its normal activity.
 example, an AES-128 key might be stashed in a notmuch property as:
 ``session-key=7:14B16AF65536C28AF209828DFE34C9E0``.
 
+**index.repaired**
+
+Some messages arrive in forms that are confusing to view; they can
+be mangled by mail transport agents, or the sending mail user
+agent may structure them in a way that is confusing.  If notmuch
+knows how to both detect and repair such a problematic message, it
+will do so during indexing.
+
+If it applies a message repair during indexing, it will use the
+``index.repaired`` property to note the type of repair(s) it
+performed.
+
 SEE ALSO
 
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 6fc5b366..ff3c6bbe 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -53,6 +53,7 @@ NOTMUCH_BEGIN_DECLS
 #include "error_util.h"
 #include "string-util.h"
 #include "crypto.h"
+#include "repair.h"
 
 #ifdef DEBUG
 # define DEBUG_DATABASE_SANITY 1
diff --git a/notmuch-client.h b/notmuch-client.h
index 39e26f2e..16fd4d55 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -52,6 +52,7 @@
 
 #include "talloc-extra.h"
 #include "crypto.h"
+#include "repair.h"
 
 #define unused(x) x __attribute__ ((unused))
 
diff --git a/util/Makefile.local b/util/Makefile.local
index 46f8af3a..f5d72f79 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -6,6 +6,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c 
$(dir)/hex-escape.c \
  $(dir)/string-util.c $(dir)/talloc-extra.c 
$(dir)/zlib-extra.c \
$(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c \
+   $(dir)/repair.c \
$(dir)/unicode-util.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
diff --git a/util/repair.c b/util/repair.c
new file mode 100644
index ..f91c1244
--- /dev/null
+++ b/util/repair.c
@@ -0,0 +1,21 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2019 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/ .
+ *
+ * Authors: Daniel Kahn Gillmor 
+ */
+
+#include "repair.h"
diff --git a/util/repair.h b/util/repair.h
new file mode 100644
index ..70e2b7bc
--- /dev/null
+++ b/util/repair.h
@@ -0,0 +1,17 @@
+#ifndef _REPAIR_H
+#define _REPAIR_H
+
+#include "gmime-extra.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* This is a collection of message structure and message format repair
+ * techniques that are designed to improve the user experience of
+ * notmuch */
+
+#ifdef __cplusplus
+}
+#endif
+#endif
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 4/6] mime-node: split out _mime_node_set_up_part

[Daniel Kahn Gillmor]

On Fri 2019-05-31 00:28:23 -0400, Daniel Kahn Gillmor wrote:
> This is a code reorganization that should have no functional effect,
> but will make future changes simpler, because a future commit will
> reuse the _mime_node_set_up_part functionality.

This particular re-organization has a slight conflict with the patch
proposed in id:20190530172707.10378-5-...@fifthhorseman.net (patch 4/4
from the "mixed-up mangling repair" series).

in particular that, patch inserts some code when handling
GMIME_IS_MULTIPART in _mime_node_create, but if this patch is applied
first, that same hunk should amend _mime_node_set_up_part instead.

Whichever series you decide to apply series first, it should be
straightforward for me to re-base the other one to avoid merge
conflicts.  Let me know!

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 6/6] index: avoid indexing legacy-display parts

[Daniel Kahn Gillmor]

When we notice a legacy-display part during indexing, it makes more
sense to avoid indexing it as part of the message body.

Given that the protected subject will already be indexed, there is no
need to index this part at all, so we skip over it.

Signed-off-by: Daniel Kahn Gillmor 
---
 lib/index.cc   | 14 ++
 test/T356-protected-headers.sh |  1 -
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index deb76f6f..d3f21f91 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -433,8 +433,11 @@ _index_mime_part (notmuch_message_t *message,
continue;
}
child = g_mime_multipart_get_part (multipart, i);
-   _notmuch_message_crypto_potential_payload (msg_crypto, child, part, 
i);
-   _index_mime_part (message, indexopts, child, msg_crypto);
+   GMimeObject *toindex = child;
+   if (_notmuch_message_crypto_potential_payload (msg_crypto, child, 
part, i) &&
+   msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL)
+   toindex = _notmuch_crypto_payload_skip_legacy_display (child);
+   _index_mime_part (message, indexopts, toindex, msg_crypto);
}
return;
 }
@@ -572,8 +575,11 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT 
(encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
-_index_mime_part (message, indexopts, clear, msg_crypto);
+GMimeObject *toindex = clear;
+if (_notmuch_message_crypto_potential_payload (msg_crypto, clear, 
GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT) &&
+   msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL)
+   toindex = _notmuch_crypto_payload_skip_legacy_display (clear);
+_index_mime_part (message, indexopts, toindex, msg_crypto);
 g_object_unref (clear);
 
 status = notmuch_message_add_property (message, "index.decryption", 
"success");
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index af0b686b..295e3750 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -148,7 +148,6 @@ test_json_nodes <<<"$output" \
 
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
 
 test_begin_subtest "do not treat legacy-display part as body when indexing"
-test_subtest_known_broken
 output=$(notmuch search --output=messages body:interrupting)
 test_expect_equal "$output" ''
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/6] util/crypto: add _notmuch_crypto_payload_skip_legacy_display

[Daniel Kahn Gillmor]

This is a utility function designed to make it easier to
"fast-forward" past a legacy-display part associated with a
cryptographic envelope, and show the user the intended message body.

The bulk of the ugliness in here is in the test function
_notmuch_crypto_payload_has_legacy_display, which tests all of the
things we'd expect to be true in a a cryptographic payload that
contains a legacy display part.

Signed-off-by: Daniel Kahn Gillmor 
---
 util/crypto.c | 96 +++
 util/crypto.h | 17 +
 2 files changed, 113 insertions(+)

diff --git a/util/crypto.c b/util/crypto.c
index 04dd91d4..6c533f8d 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -209,3 +209,99 @@ _notmuch_message_crypto_successful_decryption 
(_notmuch_message_crypto_t *msg_cr
 
 return NOTMUCH_STATUS_SUCCESS;
 }
+
+static bool
+_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;
+
+if (! g_mime_content_type_is_type (g_mime_object_get_content_type 
(payload),
+  "multipart", "mixed"))
+   return false;
+protected_header_parameter = g_mime_object_get_content_type_parameter 
(payload, "protected-headers");
+if ((! protected_header_parameter) || strcmp (protected_header_parameter, 
"v1"))
+   return false;
+if (! GMIME_IS_MULTIPART (payload))
+   return false;
+mpayload = GMIME_MULTIPART (payload);
+if (mpayload == NULL)
+   return false;
+if (g_mime_multipart_get_count (mpayload) != 2)
+   return false;
+first = g_mime_multipart_get_part (mpayload, 0);
+if (! g_mime_content_type_is_type (g_mime_object_get_content_type (first),
+  "text", "rfc822-headers"))
+   return false;
+protected_header_parameter = g_mime_object_get_content_type_parameter 
(first, "protected-headers");
+if ((! protected_header_parameter) || strcmp (protected_header_parameter, 
"v1"))
+   return false;
+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;
+   break;
+   }
+   GMimeHeader *ph = g_mime_header_list_get_header (protected_headers, 
g_mime_header_get_name (dh));
+   if (ph == NULL) {
+   ret = false;
+   break;
+   }
+   if (strcmp (g_mime_header_get_value (dh), g_mime_header_get_value 
(ph))) {
+   ret = false;
+   break;
+   }
+   }
+}
+
+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;
+}
+
+GMimeObject *
+_notmuch_crypto

[PATCH 4/6] mime-node: split out _mime_node_set_up_part

[Daniel Kahn Gillmor]

This is a code reorganization that should have no functional effect,
but will make future changes simpler, because a future commit will
reuse the _mime_node_set_up_part functionality.

In the course of splitting out this function, I noticed a comment in
the codebase that referred to the original name of _mime_node_create,
where this functionality originally resided.  I've fixed that comment
to refer to the new function instead.

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c | 43 ++-
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index a48a2119..b8143b27 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -264,13 +264,36 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject 
*part)
g_error_free (err);
 }
 
+static bool
+_mime_node_set_up_part (mime_node_t *node, GMimeObject *part) {
+/* Deal with the different types of parts */
+if (GMIME_IS_PART (part)) {
+   node->part = part;
+   node->nchildren = 0;
+} else if (GMIME_IS_MULTIPART (part)) {
+   node->part = part;
+   node->nchildren = g_mime_multipart_get_count (GMIME_MULTIPART (part));
+} else if (GMIME_IS_MESSAGE_PART (part)) {
+   /* Promote part to an envelope and open it */
+   GMimeMessagePart *message_part = GMIME_MESSAGE_PART (part);
+   GMimeMessage *message = g_mime_message_part_get_message (message_part);
+   node->envelope_part = message_part;
+   node->part = GMIME_OBJECT (message);
+   node->nchildren = 1;
+} else {
+   fprintf (stderr, "Warning: Unknown mime part type: %s.\n",
+g_type_name (G_OBJECT_TYPE (part)));
+   return false;
+}
+return true;
+}
+
 static mime_node_t *
 _mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
 {
 mime_node_t *node = talloc_zero (parent, mime_node_t);
 
 /* Set basic node properties */
-node->part = part;
 node->ctx = parent->ctx;
 if (!talloc_reference (node, node->ctx)) {
fprintf (stderr, "Out of memory.\n");
@@ -281,21 +304,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
 node->part_num = node->next_part_num = -1;
 node->next_child = 0;
 
-/* Deal with the different types of parts */
-if (GMIME_IS_PART (part)) {
-   node->nchildren = 0;
-} else if (GMIME_IS_MULTIPART (part)) {
-   node->nchildren = g_mime_multipart_get_count (GMIME_MULTIPART (part));
-} else if (GMIME_IS_MESSAGE_PART (part)) {
-   /* Promote part to an envelope and open it */
-   GMimeMessagePart *message_part = GMIME_MESSAGE_PART (part);
-   GMimeMessage *message = g_mime_message_part_get_message (message_part);
-   node->envelope_part = message_part;
-   node->part = GMIME_OBJECT (message);
-   node->nchildren = 1;
-} else {
-   fprintf (stderr, "Warning: Unknown mime part type: %s.\n",
-g_type_name (G_OBJECT_TYPE (part)));
+if (! _mime_node_set_up_part (node, part)) {
talloc_free (node);
return NULL;
 }
@@ -344,7 +353,7 @@ mime_node_child (mime_node_t *parent, int child)
 } else if (GMIME_IS_MESSAGE (parent->part)) {
sub = g_mime_message_get_mime_part (GMIME_MESSAGE (parent->part));
 } else {
-   /* This should have been caught by message_part_create */
+   /* This should have been caught by _mime_node_set_up_part */
INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
g_type_name (G_OBJECT_TYPE (parent->part)));
 }
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/6] test: avoid showing legacy-display parts

[Daniel Kahn Gillmor]

Enigmail generates a "legacy-display" part when it sends encrypted
mail with a protected Subject: header.  This part is intended to
display the Subject for mail user agents that are capable of
decryption, but do not know how to deal with embedded protected
headers.

This part is the first child of a two-part multipart/mixed
cryptographic payload within a cryptographic envelope that includes
encryption (that is, it is not just a cleartext signed message).  It
uses Content-Type: text/rfc822-headers.

That is:

A └┬╴multipart/encrypted
B  ├─╴application/pgp-encrypted
C  └┬╴application/octet-stream
*   ╤ 
D   └┬╴multipart/mixed; protected-headers=v1 (cryptographic payload)
E├─╴text/rfc822-headers; protected-headers=v1 (legacy-display part)
F└─╴… (actual message body)

In discussions with jrollins, i've come to the conclusion that a
legacy-display part should be stripped entirely from "notmuch show"
and "notmuch reply" now that these tools can understand and interpret
protected headers.

You can tell when a message part is a protected header part this way:

 * is the payload (D) multipart/mixed with exactly two children?
 * is its first child (E) Content-Type: text/rfc822-headers?
 * does the first child (E) have the property protected-headers=v1?
 * do all the headers in the body of the first child (E) match
   the protected headers in the payload part (D) itself?

If this is the case, and we already know how to deal with the
protected header, then there is no reason to try to render the
legacy-display part itself for the user.

Furthermore, when indexing, if we are indexing properly, we should
avoid indexing the text in E as part of the message body.

'notmuch reply' is an interesting case: the standard use of 'notmuch
reply' will end up omitting all mention of protected Subject:.

The right fix is for the replying MUA to be able to protect its
headers, and for it to set them appropriately based on headers found
in the original message.

If a replying MUA is unable to protect headers, but still wants the
user to be able to see the original header, a replying MUA that
notices that the original message's subject differs from the proposed
reply subject may choose to include the original's subject in the
quoted/attributed text. (this would be a stopgap measure; it's not
even clear that there is user demand for it)

This test suite change indicates what we want to happen for this case
(the tests are currently broken), and includes three additional TODO
suggestions of subtle cases for anyone who wants to flesh out the test
suite even further.  (i believe all these cases should be already
fixed by the rest of this series, but haven't had time to write the
tests for the unusual cases)

Signed-off-by: Daniel Kahn Gillmor 
---
 test/T356-protected-headers.sh| 28 +
 .../protected-with-legacy-display.eml | 40 +++
 2 files changed, 68 insertions(+)
 create mode 100644 
test/corpora/protected-headers/protected-with-legacy-display.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 4af018f3..aeee3493 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -136,4 +136,32 @@ id:nested-rfc822-mess...@crypto.notmuchmail.org
 id:protected-hea...@crypto.notmuchmail.org
 id:subjectless-protected-hea...@crypto.notmuchmail.org'
 
+test_begin_subtest "When rendering protected headers, avoid rendering 
legacy-display part"
+test_subtest_known_broken
+output=$(notmuch show --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+'subject:[0][0][0]["headers"]["Subject"]="Interrupting Cow"' \
+
'no_legacy_display:[0][0][0]["body"][0]["content"][1]["content-type"]="text/plain"'
+
+test_begin_subtest "When replying, avoid rendering legacy-display part"
+test_subtest_known_broken
+output=$(notmuch reply --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
+
+test_begin_subtest "do not treat legacy-display part as body when indexing"
+test_subtest_known_broken
+output=$(notmuch search --output=messages body:interrupting)
+test_expect_equal "$output" ''
+
+# TODO: test that a part that looks like a legacy-display in
+# multipart/signed, but not encrypted, is indexed and not stripped.
+
+# TODO: test that a legacy-display in a decrypted subpart (not in the
+# cryptographic payload) is indexed and not stripped.
+
+# TODO: test that a legacy-display inside multiple MIME layers that
+# include an encryption layer (e.g. multipart/encrypted around
+# multipart/sig

[PATCH 2/6] util/crypto: make _n_m_crypto_potential_payload whether part is a payload

[Daniel Kahn Gillmor]

_notmuch_message_crypto_potential_payload could only return a failure
if bad arguments were passed to it.  It is an internal function, so if
that happens it's an entirely internal bug for notmuch.

It will be more useful for this function to return whether or not the
part is in fact a cryptographic payload, so we dispense with the
status return.

If future changes to that function suggest adding a status return
back, there are only a handful of call sites, and no pressure to
retain a stable API, so it could be changed easily in that case.

Signed-off-by: Daniel Kahn Gillmor 
---
 lib/index.cc  |  9 ++---
 mime-node.c   |  5 +
 util/crypto.c | 13 +
 util/crypto.h |  5 -
 4 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 1fd9e67e..deb76f6f 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -405,7 +405,6 @@ _index_mime_part (notmuch_message_t *message,
  _notmuch_message_add_term (message, "tag", "encrypted");
 
for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
-   notmuch_status_t status;
GMimeObject *child;
if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
/* Don't index the signature, but index its content type. */
@@ -434,11 +433,7 @@ _index_mime_part (notmuch_message_t *message,
continue;
}
child = g_mime_multipart_get_part (multipart, i);
-   status = _notmuch_message_crypto_potential_payload (msg_crypto, 
child, part, i);
-   if (status)
-   _notmuch_database_log (notmuch_message_get_database (message),
-  "Warning: failed to mark the potential 
cryptographic payload (%s).\n",
-  notmuch_status_to_string (status));
+   _notmuch_message_crypto_potential_payload (msg_crypto, child, part, 
i);
_index_mime_part (message, indexopts, child, msg_crypto);
}
return;
@@ -577,7 +572,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, 
GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
+_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT 
(encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
 _index_mime_part (message, indexopts, clear, msg_crypto);
 g_object_unref (clear);
 
diff --git a/mime-node.c b/mime-node.c
index 3492bcd0..a48a2119 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -268,7 +268,6 @@ static mime_node_t *
 _mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
 {
 mime_node_t *node = talloc_zero (parent, mime_node_t);
-notmuch_status_t status;
 
 /* Set basic node properties */
 node->part = part;
@@ -321,9 +320,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
node_verify (node, part);
}
 } else {
-   status = _notmuch_message_crypto_potential_payload 
(node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);
-   if (status)
-   fprintf (stderr, "Warning: failed to record potential crypto 
payload (%s).\n", notmuch_status_to_string (status));
+   _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, 
parent ? parent->part : NULL, numchild);
 }
 
 return node;
diff --git a/util/crypto.c b/util/crypto.c
index 9e185e03..04dd91d4 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -132,19 +132,16 @@ _notmuch_message_crypto_potential_sig_list 
(_notmuch_message_crypto_t *msg_crypt
 }
 
 
-notmuch_status_t
+bool
 _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t 
*msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum)
 {
 const char *protected_headers = NULL;
 const char *forwarded = NULL;
 const char *subject = NULL;
 
-if (!msg_crypto || !payload)
-   return NOTMUCH_STATUS_NULL_POINTER;
-
 /* only fire on the first payload part encountered */
 if (msg_crypto->payload_encountered)
-   return NOTMUCH_STATUS_SUCCESS;
+   return false;
 
 /* the first child of multipart/encrypted that matches the
  * encryption protocol should be "control information" metadata,
@@ -156,7 +153,7 @@ _notmuch_message_crypto_potential_payload 
(_notmuch_message_crypto_t *msg_crypto
if (ct && enc_type) {
const char *part_type = g_mime_content_type_get_mime_type (ct);
if (part_type && strcmp (part_type, enc_type) == 0)
-   return NOTMUCH_STATUS_SUCCESS;
+   return false;
}
 }
 
@@ -166,7 +163,7 @@ _notmuch_message_crypto_potential_payload 
(_notmuch_message_crypto_t *msg_crypto
  * envelope: */
 if ((msg_crypto->decryption_sta

[PATCH 5/6] cli/{show,reply}: skip over legacy-display parts

[Daniel Kahn Gillmor]

Make use of the previous changes to fast-forward past any
legacy-display parts during "notmuch show" and "notmuch reply".

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c| 9 -
 test/T356-protected-headers.sh | 2 --
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index b8143b27..a1a9db29 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -329,7 +329,14 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
node_verify (node, part);
}
 } else {
-   _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, 
parent ? parent->part : NULL, numchild);
+   if (_notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, 
part, parent ? parent->part : NULL, numchild) &&
+   node->ctx->msg_crypto->decryption_status == 
NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+   GMimeObject *clean_payload = 
_notmuch_crypto_payload_skip_legacy_display (part);
+   if (clean_payload != part) {
+   if (! _mime_node_set_up_part (node, clean_payload))
+   fprintf (stderr, "Error: failed to skip legacy display 
part\n");
+   }
+   }
 }
 
 return node;
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index aeee3493..af0b686b 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -137,14 +137,12 @@ id:protected-hea...@crypto.notmuchmail.org
 id:subjectless-protected-hea...@crypto.notmuchmail.org'
 
 test_begin_subtest "When rendering protected headers, avoid rendering 
legacy-display part"
-test_subtest_known_broken
 output=$(notmuch show --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
 'subject:[0][0][0]["headers"]["Subject"]="Interrupting Cow"' \
 
'no_legacy_display:[0][0][0]["body"][0]["content"][1]["content-type"]="text/plain"'
 
 test_begin_subtest "When replying, avoid rendering legacy-display part"
-test_subtest_known_broken
 output=$(notmuch reply --format=json 
id:protected-with-legacy-disp...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
 
'no_legacy_display:["original"]["body"][0]["content"][1]["content-type"]="text/plain"'
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Hiding legacy-display parts

[Daniel Kahn Gillmor]

Now that notmuch can handle and interpret protected subject lines, it
should also avoid forcing the user to look at "legacy display" parts
that some MUAs (notably enigmail) copies of the protected headers that
are intended to be rendered only by legacy clients -- clients capable
of decryption but which don't understand how to handle protected
headers.

This series starts off with known-broken tests and ends by fixing the
problem by "fast-forwarding" through those MIME parts that are
reliably detectable as legacy-display parts.

Comments and feedback welcome!

 --dkg


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] mime-node: be clearer about decryption

[Daniel Kahn Gillmor]

Part 0 of a multipart/encrypted object is
GMIME_MULTIPART_ENCRYPTED_VERSION; part 1 is
GMIME_MULTIPART_ENCRYPTED_CONTENT.  Using the name for what we want
describes our intent more clearly than using a magic number in the
code.

Signed-off-by: Daniel Kahn Gillmor 
---
 mime-node.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index 3492bcd0..a93cbb31 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -339,7 +339,7 @@ mime_node_child (mime_node_t *parent, int child)
return NULL;
 
 if (GMIME_IS_MULTIPART (parent->part)) {
-   if (child == 1 && parent->decrypted_child)
+   if (child == GMIME_MULTIPART_ENCRYPTED_CONTENT && 
parent->decrypted_child)
sub = parent->decrypted_child;
else
sub = g_mime_multipart_get_part
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: viewing duplicate messages

[Daniel Kahn Gillmor]

On Thu 2019-05-30 19:11:22 -0300, Jorge P. de Morais Neto wrote:
> I have a Dell laptop and I subscribed to email notifications about
> firmware updates.  It turns out that Dell sends all such notifications
> (which obviously have different content) with the same message-id.

Have you reported this to Dell?  This sounds like something that anyone
at Dell who understands the technical issues would want to fix.  If
you've reported it publicly, i'd appreciate a pointer to the discussion.

If you'd care to forward me a zipfile containing examples of at least
two distinct messages with the same message ID, i'd love to look through
a copy of this sort of confusion.  It's good to understand the different
ways these sort of things go wrong.

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [FEATURE] indexing arbitrary headers

[Daniel Kahn Gillmor]

On Sat 2017-06-03 13:28:46 -0300, David Bremner wrote:
> Łukasz Stelmach  writes:
>
>> I'd like to ask for a new feature: indexing of arbitrary headers. Not
>> all headers but a few selected by users.
>>
>> For example, I get a lot of mails from a Gerrit system. I'd like to keep
>> them for a while and remove them when they are old enough. Although
>> these messages are distinguishable in my inbox, they've got subjects
>> like "Change in ..." and are from "nobody". But these may happen in
>> other e-mails too. The only 100% sure way to distringuish those e-mails
>> is to look for headers Gerrit adds.
>>
>
> Thanks for writing this up. Similar requests have been around for a
> while, but it's good to have a note in nmbug.
>
> There are two main blockers I'm aware of (other than someone writing and
> reviewing the code).
>
> 1) We need to index all copies of a message (e.g. using
>something like the series at  id:20170507124012.30188-2-da...@tethera.net)
>
> 2) we need to decide what happens when someone changes the list of
>indexed headers.

I just wanted to point out that this specific request has now been added
to notmuch, thanks to David Bremner!  It will hopefully be part of the
forthcoming 0.29 release:

NEWS currently says:

Add support for indexing user specified headers (e.g. List-Id). See
notmuch-config(1) for details. This requires reindexing after changing
the set of headers to be indexed.

Bremner, i'm not sure how/when you want to clear the notmuch::wishlist
tag on the OP here, but it's nice to be able to reduce the stack listed
at https://nmbug.notmuchmail.org/status/#Wish-list

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Safe and useful handling of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

On Thu 2019-05-30 02:21:02 +, Rollins, Jameson wrote:
> The way he handles the repair seems reasonable to me (modulo
> a couple minor comments in reply).

I've responded to jamie's review here with v2 of this series, which can
be found at id:20190530172707.10378-1-...@fifthhorseman.net.

If anyone else wants to review, please review that revision.

Regards,

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 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 
---
 doc/man7/notmuch-properties.rst | 13 +
 lib/index.cc| 22 +-
 lib/notmuch-private.h   |  1 +
 test/T351-pgpmime-mangling.sh   |  2 --
 4 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 802e6763..31de576e 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -109,6 +109,19 @@ of its normal activity.
 example, an AES-128 key might be stashed in a notmuch property as:
 ``session-key=7:14B16AF65536C28AF209828DFE34C9E0``.
 
+**index.repaired**
+
+Some mail transport agents mangle messages in transit in ways that
+are both detectable and reversible.  If notmuch encounters such a
+mangling during indexing, it will try to index the repaired form
+of the message (while still leaving the message on disk
+untouched).  If successful, it will use the ``index.repaired``
+property to note the kind of mangling that was repaired.
+Currently, only one form of repairable mangling is detected and
+repaired, which is denoted with ``index.repaired=mixedup``.  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/lib/notmuch-private.h b/lib/notmuch-private.h
index 6fc5b366..ff3c6bbe 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -53,6 +53,7 @@ NOTMUCH_BEGIN_DECLS
 #include "error_util.h"
 #include "string-util.h"
 #include "crypto.h"
+#include "repair.h"
 
 #ifdef DEBUG
 # define DEBUG_DATABASE_SANITY 1
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=mixed

[PATCH v2 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 
---
 util/Makefile.local |   1 +
 util/repair.c   | 101 
 util/repair.h   |  22 ++
 3 files changed, 124 insertions(+)
 create mode 100644 util/repair.c
 create mode 100644 util/repair.h

diff --git a/util/Makefile.local b/util/Makefile.local
index 46f8af3a..f5d72f79 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -6,6 +6,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c 
$(dir)/hex-escape.c \
  $(dir)/string-util.c $(dir)/talloc-extra.c 
$(dir)/zlib-extra.c \
$(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c \
+   $(dir)/repair.c \
$(dir)/unicode-util.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
diff --git a/util/repair.c b/util/repair.c
new file mode 100644
index ..56d6c5b5
--- /dev/null
+++ b/util/repair.c
@@ -0,0 +1,101 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2019 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/ .
+ *
+ * Authors: Daniel Kahn Gillmor 
+ */
+
+#include 
+#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 (

[PATCH v2 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 
---
 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 ..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...@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...@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...@mangling.notmuchmail.org
+
+test_begin_subtest "index cleartext of 'Mixed-Up' mangled PGP/MIME message"
+test_expect_success 'notmuch reindex --decrypt=true 
id:mixed...@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...@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 ..a09f6191
--- /dev/null
+++ b/test/corpora/mangling/mixed-up.eml
@@ -0,0 +1,33 @@
+From: test_su...@notmuchmail.org
+To: test_su...@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +
+Message-ID: 
+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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

v2 of Safe and useful handling of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

This is the second revision of the series initially posted at
id:20190528225452.17550-1-...@fifthhorseman.net.

The changes in this series from v1 are in response to the helpful
review by jrollins.  In particular:

 * test to ensure that 'notmuch reply' is fixed as well
 
 * the repair functionality moves from util/crypto.[ch] into its own
   source file at util/repair.[ch]

I welcome further review, comments, or suggestions.

Regards,

--dkg




___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 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 
---
 mime-node.c   | 22 ++
 notmuch-client.h  |  1 +
 test/T351-pgpmime-mangling.sh |  2 --
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 3492bcd0..cafe1537 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)
 {
@@ -286,6 +300,14 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
 if (GMIME_IS_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)) {
/* Promote part to an envelope and open it */
diff --git a/notmuch-client.h b/notmuch-client.h
index 39e26f2e..16fd4d55 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -52,6 +52,7 @@
 
 #include "talloc-extra.h"
 #include "crypto.h"
+#include "repair.h"
 
 #define unused(x) x __attribute__ ((unused))
 
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...@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...@mangling.notmuchmail.org)
 test_json_nodes <<<"$output" \
 'body:["original"]'"$bodytext"
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 4/4] cli/show: show repaired form of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

On Thu 2019-05-30 02:09:47 +, Rollins, Jameson wrote:
> On Wed, May 29 2019, Jameson Graef Rollins  wrote:
>> On Tue, May 28 2019, Daniel Kahn Gillmor  wrote:
>>> When showing a message that has been mangled in transit by an MTA in
>>> the "Mixed up" way, "notmuch show" should instead show the repaired
>>> form of the message.
>>
>> Given that this fix is in mime-node.c it will apply to reply as well,
>> not just show, yes?
>
> Not that we really need to bother changing the commit log for that...
> just checking.

Yes, it does fix notmuch reply as well.  Do you think that we should
augment the test suite to include 'notmuch reply' too?

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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

[Daniel Kahn Gillmor]

On Thu 2019-05-30 02:18:57 +, Rollins, Jameson wrote:
> I understand that this fix is for multipart/encrypted messages, but I'm
> not sure I would call the repair function itself a "crypto function".
> Given that I can imagine more repair functions in the future, would it
> make sense to break them out into their own library?

I agree that this technically isn't a "crypto function", and as such
might not belong where i've put it.  What would you think about
util/repair.{c,h} or util/demangling.{c,h} ?

  --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] test: signature verification during decryption (session keys)

[Daniel Kahn Gillmor]

When the user knows the signer's key, we want "notmuch show" to be
able to verify the signature of an encrypted and signed message
regardless of whether we are using a stashed session key or not.

I wrote this test because I was surprised to see signature
verification failing when viewing some encrypted messages after
upgrading to GPGME 1.13.0-1 in debian experimental.

The added tests here all pass with GPGME 1.12.0, but the final test
fails with 1.13.0, due to some buggy updates to GPGME upstream: see
https://dev.gnupg.org/T3464 for more details.

While the bug needs to be fixed in GPGME, notmuch's test suite needs
to make sure that GMime is doing what we expect it to do; i was a bit
surprised that it hadn't caught the problem, hence this patch.

I've fixed this bug in debian experimental with gpgme 1.13.0-2, so the
tests should pass on any debian system.  I've also fixed it in the
gpgme packages (1.13.0-2~ppa1) in the ubuntu xenial PPA
(ppa:notmuch/notmuch) that notmuch uses for Travis CI.

Signed-off-by: Daniel Kahn Gillmor 
---
 test/T357-index-decryption.sh| 19 +
 test/corpora/crypto/encrypted-signed.eml | 35 
 2 files changed, 54 insertions(+)
 create mode 100644 test/corpora/crypto/encrypted-signed.eml

diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 8a2d4c02..1ac2836a 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -226,6 +226,7 @@ output=$(notmuch dump | LC_ALL=C sort)
 expected='#= simple-encryp...@crypto.notmuchmail.org index.decryption=failure
 #notmuch-dump batch-tag:3 config,properties,tags
 +encrypted +inbox +unread -- id:basic-encryp...@crypto.notmuchmail.org
++encrypted +inbox +unread -- id:encrypted-sig...@crypto.notmuchmail.org
 +encrypted +inbox +unread -- id:simple-encryp...@crypto.notmuchmail.org'
 test_expect_equal \
 "$output" \
@@ -288,6 +289,24 @@ test_expect_equal \
 "$output" \
 "$expected"
 
+goodsig='good_sig:[0][0][0]["crypto"]["signed"]["status"][0]["status"]="good"'
+nosig='no_sig:[0][0][0]["crypto"]!"signed"'
+
+test_begin_subtest "verify signature without a session key stashed when 
--decrypt=true"
+output=$(notmuch show --format=json --decrypt=true 
id:encrypted-sig...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" "$goodsig"
+
+test_begin_subtest "do not verify sig without a session key stashed if 
--decrypt=auto"
+output=$(notmuch show --format=json id:encrypted-sig...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" "$nosig"
+
+test_begin_subtest "verify signature when --decrypt=stash"
+output=$(notmuch show --format=json --decrypt=stash 
id:encrypted-sig...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" "$goodsig"
+
+test_begin_subtest "verify signature with stashed session key"
+output=$(notmuch show --format=json id:encrypted-sig...@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" "$goodsig"
 
 # TODO: test removal of a message from the message store between
 # indexing and reindexing.
diff --git a/test/corpora/crypto/encrypted-signed.eml 
b/test/corpora/crypto/encrypted-signed.eml
new file mode 100644
index ..0345e3e9
--- /dev/null
+++ b/test/corpora/crypto/encrypted-signed.eml
@@ -0,0 +1,35 @@
+From: test_su...@notmuchmail.org
+To: test_su...@notmuchmail.org
+Subject: Lyrics
+Date: Wed 29 May 2019 06:09:22 PM EDT
+Message-ID: 
+MIME-Version: 1.0
+Content-Type: multipart/encrypted; boundary="=-=-=";
+   protocol="application/pgp-encrypted"
+
+--=-=-=
+Content-Type: application/pgp-encrypted
+
+Version: 1
+
+--=-=-=
+Content-Type: application/octet-stream
+
+-BEGIN PGP MESSAGE-
+
+hIwDxE023q1UqxYBBAC9z781zV7QAInGMKHX6TKU5Xw/OkoWXahpDL88F6Ocm5R9
+7M9z2ocvlyrbgRhqE+nvFeGH/K7rVkBBT6TAcdIe/C8Qzbd3stPPcx1PlunGROj7
+H/WAcmDksK3HkXpHwmInUtzNw1pkhOoLy/sFSbPvtyg8GCUzXbafHAIIo0rB2tLB
+DwGWD3l4WdcyQWuYD9QJKuDIqdWo8E3TTcKkiOAt/6liwPNZ0jGzDeCuSTnWFj6Z
+AiXGeNtD3I1tCN/8T3NjEKOCQ+bdT5Y06dDaL61FpQ23eIuSUgksVxjnkEAb6iPe
+07gjzcyNuGP3WPI/0qu0wtZwpAQxvaNygDsQj/OjR5kn9luBd/VqodM3TWWS8miV
+m0z1tYbqYAQWW6TS7fXlsyXoOxTLW5MCfe3D36VSErL/NJItETklVKzNfKjMmRKx
+CI2ZUzugxPWSLQzOp5yl7iICk8e+vS9TkQw2j0nXAQYLYgmqZMhf4av5GlFv3tQu
+heO4XLT6NBDTHMFTDbgW42kE0N4MDPc29AqVFGImcTHvflF4Vp0qIbSJdIcHwKkU
+5LKqvicAa0lsIoJbsW3lHrzowyjov2vLH/VGd/wIX+MS3KT7cySdyp8HVMcwwyZu
+Y9nrTN/7G1FwKWlcGa4uJNcFFkYlcEymZj1EX2cyrdezPtX7K5vhwBYddptFD+Bn
+IVkghRut3UDeXe83F8OutWiZfK5EVYABq/aP3//hIbQl2o4Dkd3z9m+8LobrIV5s
+NXjAjU5WQOjRLoHBebG2HkMpFsWhXD/Fb/Bb58VOpdI=
+=x12v
+-END PGP MESSAGE-
+--=-=-=--
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Safe and useful handling of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

On Tue 2019-05-28 18:58:22 -0400, Daniel Kahn Gillmor wrote:
> I forgot to mention: this test case makes use of the test_json_nodes
> functionality introduced in 03/17 of the protected header series.
>
> So please only consider this after that patch has been merged.

It has been merged, so this series is good to go.  Please review!

I'm running this series today, and it is definitely useful.  It'd be
great to get it into 0.29 if we can.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] NEWS: add a note about protected headers

[Daniel Kahn Gillmor]

Signed-off-by: Daniel Kahn Gillmor 
---
 NEWS | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 8f250ab9..c08c564e 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,10 @@ mboxes); e.g. `gzip -9 $MAIL/archive/giant-message && 
notmuch new`
 should work. Note that maildir flag syncing for gzipped messages is
 currently untested.
 
+Notmuch is now capable of indexing, searching and rendering
+cryptographically-protected Subject: headers of the form produced by
+Enigmail and K-9 mail in encrypted messages.
+
 Command Line Interface
 --
 
@@ -30,7 +34,8 @@ Fix several performance problems with `notmuch reindex`.
 
 `notmuch show` and `notmuch reply` now emit per-message cryptographic
 status in their json and sexp output formats.  See devel/schemata for
-more details about what is included there.
+more details about what is included there.  This status includes
+information about cryptographic protections for the Subject header.
 
 Emacs
 -
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Protected Headers (2nd major revision, more testing!)

[Daniel Kahn Gillmor]

On Wed 2019-05-29 08:44:00 -0300, David Bremner wrote:
> It's in.

Thanks!

> I did bodge things up slightly due to the threading of patch 14, but I
> added the trivial patch with the one line change I missed.

that's not a bodge at all compared to the several different bodges i did
on that threading.  i've verified that the result
(2c1e5c186ee36fb215d3f312f9801884f4720d8f) is as i expected.  Thanks for
the merge!

--dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Safe and useful handling of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

On Tue 2019-05-28 18:54:48 -0400, Daniel Kahn Gillmor wrote:
> The test case included in this series should be sufficient to show the
> problem specifically

I forgot to mention: this test case makes use of the test_json_nodes
functionality introduced in 03/17 of the protected header series.

So please only consider this after that patch has been merged.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Safe and useful handling of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

I've documented an unfortunate MTA habit over in
https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1
which i've named "Mixed Up" mangling.  In particular, popular versions
of Microsoft Exchange take a multipart/encrypted e-mail and transform
it unaccountably to multipart/mixed.

While the right thing to do long term is to get a fix for those MTAs
(which i'm also working on, elsewhere), even if we succeed in doing
that, messages that have already been mangled by them will live on in
our mailboxes forever.

What follows is a series of patches that lets a notmuch user deal with
any message mangled in this particular form in a sensible way.  In
particular, they should be able to decrypt the message correctly
despite its nonstandard structure.  This represents a deviation from
standard MIME handling procedures, but i believe it is a safe
deviation, and a useful one.

The test case included in this series should be sufficient to show the
problem specifically, but if anyone wants to receive encrypted e-mail
from me that demonstrates the problem to their personal key, i can
provide.  Please let me know by replying off-list and making sure i
know your OpenPGP key fingerprint.

If anyone has examples of other common, detectable, and repairable
message manglings, please let me know.  I'd be happy to add them to my
documentation of this sort of thing at least, and if we can fix them
up for notmuch users, even better.

All the best,

--dkg


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 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 
---
 test/T351-pgpmime-mangling.sh  | 28 +
 test/corpora/mangling/mixed-up.eml | 33 ++
 2 files changed, 61 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 ..17f94a31
--- /dev/null
+++ b/test/T351-pgpmime-mangling.sh
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+
+test_description='PGP/MIME message mangling'
+. $(dirname "$0")/test-lib.sh || exit 1
+
+add_gnupg_home
+add_email_corpus mangling
+
+test_begin_subtest "handle 'Mixed-Up' mangled PGP/MIME message"
+test_subtest_known_broken
+output=$(notmuch show --format=json --decrypt=true 
id:mixed...@mangling.notmuchmail.org)
+test_json_nodes <<<"$output" \
+'body:[0][0][0]["body"][0]["content"][1]["content"]="The 
password is \"abcd1234!\", please do not tell anyone.\n"'
+
+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...@mangling.notmuchmail.org
+
+test_begin_subtest "reindex 'Mixed-Up' mangled PGP/MIME message"
+test_expect_success 'notmuch reindex --decrypt=true 
id:mixed...@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...@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 ..a09f6191
--- /dev/null
+++ b/test/corpora/mangling/mixed-up.eml
@@ -0,0 +1,33 @@
+From: test_su...@notmuchmail.org
+To: test_su...@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +
+Message-ID: 
+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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 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 
---
 doc/man7/notmuch-properties.rst | 13 +
 lib/index.cc| 22 +-
 test/T351-pgpmime-mangling.sh   |  2 --
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 802e6763..31de576e 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -109,6 +109,19 @@ of its normal activity.
 example, an AES-128 key might be stashed in a notmuch property as:
 ``session-key=7:14B16AF65536C28AF209828DFE34C9E0``.
 
+**index.repaired**
+
+Some mail transport agents mangle messages in transit in ways that
+are both detectable and reversible.  If notmuch encounters such a
+mangling during indexing, it will try to index the repaired form
+of the message (while still leaving the message on disk
+untouched).  If successful, it will use the ``index.repaired``
+property to note the kind of mangling that was repaired.
+Currently, only one form of repairable mangling is detected and
+repaired, which is denoted with ``index.repaired=mixedup``.  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 17f94a31..a20066e4 100755
--- a/test/T351-pgpmime-mangling.sh
+++ b/test/T351-pgpmime-mangling.sh
@@ -13,7 +13,6 @@ test_json_nodes <<<"$output" \
 'body:[0][0][0]["body"][0]["content"][1]["content"]="The 
password is \"abcd1234!\", please do not tell anyone.\n"'
 
 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...@mangling.notmuchmail.org
 
@@ -21,7 +20,6 @@ test_begin_subtest "reindex 'Mixed-Up' mangled PGP/MIME 
message"
 test_expec

Re: [PATCH v4 06/17] cli/show: add information about which headers were protected

[Daniel Kahn Gillmor]

On Tue 2019-05-28 08:10:35 -0300, David Bremner wrote:
> Daniel Kahn Gillmor  writes:
>>  decrypted?: {
>>status: msgdecstatus,
>> +  # map encrypted headers that differed from the outside 
>> headers.
>> +  # the value of each item in the map is what that field 
>> showed externally
>> +  # (maybe null if it was not present in the external 
>> headers).
>> +  header-mask:  { header_name: string|null,*}
>>  }
>
> Apologies for not catching this before (and for fussing so much about
> the schemata file), but this notation for repeated key-value pairs
> doesn't seem ideal to me.

Don't apologize for your fussing -- we have good documentation in
notmuch in large part because of this kind of fussiness.

> I would say either
>
> header-mask: { (header_name: string|null)* }
> header-mask: { header_name*: string|null }
>
> Either would need a brief explanation above, as this the first map
> defined with an arbitrary number of members.

I agree with you that both of your proposed notations are better than
the one i'd picked initially.  I'm not convinced that the explanatory
text above needs to be expanded, because it is an arbitrary header_name
→ string map and i think the text captures that idea fairly succinctly,
but if you want to add additional text about "what does the kleene star
mean here" i wouldn't object to such an explanation.

So, I'm fine with either proposed notation, and i think you and i have
the same mental model of what this is supposed to be, so i trust you to
choose one of them and to write any additional text.

Is it ok if i punt that decision to you?

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 4/4] cli/show: show repaired form of "Mixed Up" mangled messages

[Daniel Kahn Gillmor]

When showing a message that has been mangled in transit by an MTA in
the "Mixed up" way, "notmuch show" should instead show 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 
---
 mime-node.c   | 22 ++
 test/T351-pgpmime-mangling.sh |  1 -
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index 3492bcd0..cafe1537 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)
 {
@@ -286,6 +300,14 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, 
int numchild)
 if (GMIME_IS_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)) {
/* Promote part to an envelope and open it */
diff --git a/test/T351-pgpmime-mangling.sh b/test/T351-pgpmime-mangling.sh
index a20066e4..a613fe3e 100755
--- a/test/T351-pgpmime-mangling.sh
+++ b/test/T351-pgpmime-mangling.sh
@@ -7,7 +7,6 @@ add_gnupg_home
 add_email_corpus mangling
 
 test_begin_subtest "handle 'Mixed-Up' mangled PGP/MIME message"
-test_subtest_known_broken
 output=$(notmuch show --format=json --decrypt=true 
id:mixed...@mangling.notmuchmail.org)
 test_json_nodes <<<"$output" \
 'body:[0][0][0]["body"][0]["content"][1]["content"]="The 
password is \"abcd1234!\", please do not tell anyone.\n"'
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/4] util/crypto: 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 
---
 util/crypto.c | 80 +++
 util/crypto.h |  9 ++
 2 files changed, 89 insertions(+)

diff --git a/util/crypto.c b/util/crypto.c
index 9e185e03..c8c79847 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -28,6 +28,86 @@ void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t 
*crypto))
 {
 }
 
+/* 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;
+}
+
+
 GMimeObject *
 _notmuch_crypto_decrypt (bool *attempted,
 notmuch_decryption_policy_t decrypt,
diff --git a/util/crypto.h b/util/crypto.h
index fdbb5da5..09c0ebcb 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -22,6 +22,15 @@ _notmuch_crypto_decrypt (bool *attempted,
 GMimeDecryptResult **decrypt_result,
 GError **err);
 
+/* 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_unr

[PATCH 1/2] NEWS: include information about per-message cryptographic status

[Daniel Kahn Gillmor]

---
 NEWS | 4 
 1 file changed, 4 insertions(+)

diff --git a/NEWS b/NEWS
index d8aa272f..999affc7 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,10 @@ Command Line Interface
 `notmuch show` now supports --body=false and --include-html with
 --format=text
 
+`notmuch show` and `notmuch reply` now emit per-message cryptographic
+status in their json and sexp output formats.  See devel/schemata for
+more details about what is included there.
+
 Emacs
 -
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/2] NEWS: note parallel test suite

[Daniel Kahn Gillmor]

---
 NEWS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/NEWS b/NEWS
index 999affc7..60a69936 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,13 @@ The minimum supported major version of Emacs is now 24.
 Support for GNU Emacs older than 25.1 is deprecated with this release,
 and may be removed in a future release.
 
+Test Suite
+--
+
+If either GNU parallel or moreutils parallel is installed, the tests
+in the test suite will now be run in parallel (one per available
+core).  This can be disabled with NOTMUCH_TEST_SERIALIZE=1.
+
 Notmuch 0.28.4 (2019-05-05)
 ===
 
-- 
2.20.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] NEWS: News for my changes for 0.29

[Daniel Kahn Gillmor]

On Mon 2019-05-27 07:46:55 -0300, David Bremner wrote:
> +Dependencies
> +
> +
> +Support for GMime 2.6 is removed.
> +

I'd add here:

The minimum supported version of GMime is now 3.0.3.  GMime also needs
to have been compiled with cryptographic support.

   --dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Protected Headers (2nd major revision, more testing!)

[Daniel Kahn Gillmor]

On Sun 2019-05-26 18:15:53 -0400, Daniel Kahn Gillmor wrote:
> this series delivers a concrete improvement: users of notmuch can now
> read, index, and search for the subject lines of encrypted messages
> sent from MUAs like Enigmail and K-9 mail.

Many thanks to jrollins and bremner for their reviews of this series.

Apologies for my clumsy handling of the revisions for those changes, but
i believe that the latest versions of these patches in this thread are
now ready for merging.  I've tagged the obsoleted versions appropriately
with nmbug, so after "nmbug pull", this query should identify 17
messages with patches for this sequence (choose X for however your
own installation identifies this series):

tag:notmuch::patch and not tag:notmuch::obsolete and thread:XX

i've left v4 of 06/17 and 10/17 tagged notmuch::needs-review because
they represent changes under discussion and i don't want to claim that
my resolution to the discussion matches Bremner's.

This series is also applied on the "protected-headers" branch at
https://gitlab.com/dkg/notmuch.git if anyone wants to compare what
they've merged from the mailing list with how i've interpreted it.

Please let me know if you have any other reviews.

all the best,

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 10/17] indexing: record protected subject when indexing cleartext

[Daniel Kahn Gillmor]

When indexing the cleartext of an encrypted message, record any
protected subject in the database, which should make it findable and
visible in search.

Signed-off-by: Daniel Kahn Gillmor 
---
 lib/index.cc   | 42 ++
 lib/message.cc |  8 +++
 lib/notmuch-private.h  |  4 
 test/T356-protected-headers.sh | 16 +
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index f216ae5d..1fd9e67e 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -367,13 +367,15 @@ _index_content_type (notmuch_message_t *message, 
GMimeObject *part)
 
 static void
 _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t 
*indexopts,
-   GMimeMultipartEncrypted *part);
+   GMimeMultipartEncrypted *part,
+   _notmuch_message_crypto_t *msg_crypto);
 
 /* Callback to generate terms for each mime part of a message. */
 static void
 _index_mime_part (notmuch_message_t *message,
  notmuch_indexopts_t *indexopts,
- GMimeObject *part)
+ GMimeObject *part,
+ _notmuch_message_crypto_t *msg_crypto)
 {
 GMimeStream *stream, *filter;
 GMimeFilter *discard_non_term_filter;
@@ -403,6 +405,8 @@ _index_mime_part (notmuch_message_t *message,
  _notmuch_message_add_term (message, "tag", "encrypted");
 
for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
+   notmuch_status_t status;
+   GMimeObject *child;
if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
/* Don't index the signature, but index its content type. */
if (i == GMIME_MULTIPART_SIGNED_SIGNATURE) {
@@ -419,7 +423,8 @@ _index_mime_part (notmuch_message_t *message,
 g_mime_multipart_get_part (multipart, i));
if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) {
_index_encrypted_mime_part(message, indexopts,
-  GMIME_MULTIPART_ENCRYPTED 
(part));
+  GMIME_MULTIPART_ENCRYPTED (part),
+  msg_crypto);
} else {
if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) {
_notmuch_database_log (notmuch_message_get_database 
(message),
@@ -428,8 +433,13 @@ _index_mime_part (notmuch_message_t *message,
}
continue;
}
-   _index_mime_part (message, indexopts,
- g_mime_multipart_get_part (multipart, i));
+   child = g_mime_multipart_get_part (multipart, i);
+   status = _notmuch_message_crypto_potential_payload (msg_crypto, 
child, part, i);
+   if (status)
+   _notmuch_database_log (notmuch_message_get_database (message),
+  "Warning: failed to mark the potential 
cryptographic payload (%s).\n",
+  notmuch_status_to_string (status));
+   _index_mime_part (message, indexopts, child, msg_crypto);
}
return;
 }
@@ -439,7 +449,7 @@ _index_mime_part (notmuch_message_t *message,
 
mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART 
(part));
 
-   _index_mime_part (message, indexopts, g_mime_message_get_mime_part 
(mime_message));
+   _index_mime_part (message, indexopts, g_mime_message_get_mime_part 
(mime_message), msg_crypto);
 
return;
 }
@@ -516,7 +526,8 @@ _index_mime_part (notmuch_message_t *message,
 static void
 _index_encrypted_mime_part (notmuch_message_t *message,
notmuch_indexopts_t *indexopts,
-   GMimeMultipartEncrypted *encrypted_data)
+   GMimeMultipartEncrypted *encrypted_data,
+   _notmuch_message_crypto_t *msg_crypto)
 {
 notmuch_status_t status;
 GError *err = NULL;
@@ -553,6 +564,10 @@ _index_encrypted_mime_part (notmuch_message_t *message,
return;
 }
 if (decrypt_result) {
+   status = _notmuch_message_crypto_successful_decryption (msg_crypto);
+   if (status)
+   _notmuch_database_log_append (notmuch, "failed to mark the message 
as decrypted (%s)\n",
+ notmuch_status_to_string (status));
if (get_sk) {
status = notmuch_message_add_property (message, "session-key",
   
g_mime_decrypt_result_get_session_key (decrypt_result));
@@ -562,7 +577,8 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
g_object_unref (decrypt_result);
 }
-_index_mime_part (message, indexopts, clear);
+status = _notmuc

Re: [PATCH v3 10/17] indexing: record protected subject when indexing cleartext

[Daniel Kahn Gillmor]

On Mon 2019-05-27 17:17:20 -0400, Daniel Kahn Gillmor wrote:
> When indexing the cleartext of an encrypted message, record any
> protected subject in the database, which should make it findable and
> visible in search.

ugh, please ignore v3 of this patch (10/17) as well.  v4 should be
coming shortly.

   --dkg, juggling too many branches


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 06/17] cli/show: add information about which headers were protected

[Daniel Kahn Gillmor]

On Mon 2019-05-27 16:40:59 -0400, Daniel Kahn Gillmor wrote:
> This allows a clever UI frontend to mark whether a header was
> protected (or not), and if it was protected, to show the details to
> an interested user.
>
> As before, we only handle Subject for now, but we might be able to
> handle other headers in the future.

bah, please excuse this version (v3), i'm juggling too many variant
trees locally, and got confused.  v4 should be the correct one, as it
uses "header-mask", and not "masked-headers".  I explained in
id:87d0k34oik@fifthhorseman.net why i prefer "header-mask", and that
preference still holds.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 06/17] cli/show: add information about which headers were protected

[Daniel Kahn Gillmor]

The header-mask member of the per-message crypto object allows a
clever UI frontend to mark whether a header was protected (or not).
And if it was protected, it contains enough information to show useful
detail to an interested user.  For example, an MUA could offer a "show
what this message's Subject looked like on the wire" feature in expert
mode.

As before, we only handle Subject for now, but we might be able to
handle other headers in the future.

Signed-off-by: Daniel Kahn Gillmor 
---
 devel/schemata |  9 +
 notmuch-show.c | 21 +
 test/T356-protected-headers.sh |  4 ++--
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 72feb7b7..c0f69e7e 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -48,6 +48,9 @@ threadid = string
 # Message ID, sans "id:"
 messageid = string
 
+# E-mail header name, sans trailing colon, like "Subject" or "In-Reply-To"
+header_name = string
+
 notmuch show schema
 ---
 
@@ -88,9 +91,15 @@ crypto = {
   status:  sigstatus,
   # was the set of signatures described under encrypted cover?
   encrypted:   bool,
+  # which of the headers is covered by sigstatus?
+  headers: [header_name*]
 },
 decrypted?: {
   status: msgdecstatus,
+  # map encrypted headers that differed from the outside 
headers.
+  # the value of each item in the map is what that field 
showed externally
+  # (maybe null if it was not present in the external headers).
+  header-mask:  { header_name: string|null,*}
 }
 }
 
diff --git a/notmuch-show.c b/notmuch-show.c
index b1f6a4bb..4dfe9c1d 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -645,6 +645,12 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
sp->map_key (sp, "encrypted");
sp->boolean (sp, msg_crypto->signature_encrypted);
}
+   if (msg_crypto->payload_subject) {
+   sp->map_key (sp, "headers");
+   sp->begin_list (sp);
+   sp->string (sp, "Subject");
+   sp->end (sp);
+   }
sp->end (sp);
}
if (msg_crypto->decryption_status != 
NOTMUCH_MESSAGE_DECRYPTED_NONE) {
@@ -652,6 +658,21 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
sp->begin_map (sp);
sp->map_key (sp, "status");
sp->string (sp, msg_crypto->decryption_status == 
NOTMUCH_MESSAGE_DECRYPTED_FULL ? "full" : "partial");
+
+   if (msg_crypto->payload_subject) {
+   const char *subject = g_mime_message_get_subject 
GMIME_MESSAGE (node->part);
+   if (subject == NULL || strcmp (subject, 
msg_crypto->payload_subject)) {
+   /* protected subject differs from the external 
header */
+   sp->map_key (sp, "header-mask");
+   sp->begin_map (sp);
+   sp->map_key (sp, "Subject");
+   if (subject == NULL)
+   sp->null (sp);
+   else
+   sp->string (sp, subject);
+   sp->end (sp);
+   }
+   }
sp->end (sp);
}
}
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 8a8fef6a..68d431e9 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -22,7 +22,7 @@ test_json_nodes <<<"$output" \
 test_begin_subtest "verify protected header is visible with decryption"
 output=$(notmuch show --decrypt=true --format=json 
id:protected-hea...@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
-'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' 
\
+'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", 
"header-mask": {"Subject": "Subject Unavailable"}}}' \
 'subject:[0][0][0]["headers"]["Subject"]="This is a protected 
header"'
 
 test_begin_subtest "misplaced protected headers should not be made visible 
during decryption"
@@ -58,7 +58,7 @@ test_json_nodes <<<"$outpu