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

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

[Daniel Kahn Gillmor]

Hi all--

Way back in id:20180511055544.13676-1-dkg@fifthhorseman.net, i
proposed support for protected headers (in particular, for being able
to read and search for subject lines of encrypted messages which
protect the Subject).  Although that series was reviewed by Bremner, i
never managed to get it in shape for merging.

This is a revision of that series, applied against the current master,
having taken into account those reviews and the current state of the
notmuch codebase.  I'm hoping that we can get it into 0.29 before the
feature freeze.

The major change since the earlier version is that i've dropped the
proposed --protected-subject flag for "notmuch reply".  An MUA that
wants to reply to an encrypted message needs to keep a lot of state
active during message composition, including the fact that it was a
reply to an encrypted message, and so forth.  It needs to know that if
the user switches encryption off or on during message composition (for
whatever reason, like adding a Cc to someone for whom we don't have
keys, or discovering that some of the recipients keys are no longer
valid), it needs to think about whether the subject line is stripped
or not actively, and passing a simple --protected-subject flag to
"notmuch reply" during the initial setup of message composition is
insufficient for that purpose.  So this series doesn't pretend to
handle that case directly -- clients will need to consider it
themselves.

See the message in commit "cli/reply: ensure encrypted Subject: line
does not leak in the clear" for more thoughts about what a reasonable
replying MUA might do.

This series also (like its earlier incarnation) doesn't get all the
way to the point of generating encrypted or signed messages that
protect their Subject lines.  That might require some e-lisp hackery
that i haven't done; or it might be best solved by a "notmuch deliver"
outbound message handler (which is also work i haven't done). Or maybe
there's some other better solution that i haven't thought of yet.  I
welcome discussion and suggestions along those lines.

The other thing this series does not do is to expose information about
the protected headers through the library or the python bindings.  I
think the pieces are in place to make that happen, but I have not
considered the API deeply enough to take a concrete attempt.  Again,
suggestions (and patches) welcome!

However, despite the above-mentioned limitations, 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.

Also: please don't be scared of the length of this series.  Although
there are 17 patches, the distinct majority of them are extensions to
the test suite, to make sure that we cover weird corner cases between
the MIME spec and this now-common form of header protection.

As always, review, feedback, critique, and patches are welcome.

Happy Hacking,

   --dkg

[PATCH v2 01/17] cli/show: emit headers after emitting body

[Daniel Kahn Gillmor]

This paves the way for emitting protected headers after verification
and decryption, because it means that the headers will only be emitted
after the body has been parsed.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 notmuch-show.c    |  6 +++---
 test/T170-sexp.sh | 10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index c6a7a10a..0816a5e1 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -619,9 +619,6 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	sp->begin_map (sp);
 	format_message_sprinter (sp, node->envelope_file);
 
-	sp->map_key (sp, "headers");
-	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), false);
-
 	if (output_body) {
 	    sp->map_key (sp, "body");
 	    sp->begin_list (sp);
@@ -657,6 +654,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    sp->end (sp);
 	}
 
+	sp->map_key (sp, "headers");
+	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), false);
+
 	sp->end (sp);
 	return;
     }
diff --git a/test/T170-sexp.sh b/test/T170-sexp.sh
index fe7a9dff..24be8351 100755
--- a/test/T170-sexp.sh
+++ b/test/T170-sexp.sh
@@ -5,16 +5,16 @@ test_description="--format=sexp output"
 test_begin_subtest "Show message: sexp"
 add_message "[subject]=\"sexp-show-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[bcc]=\"test_suite+bcc@notmuchmail.org\"" "[reply-to]=\"test_suite+replyto@notmuchmail.org\"" "[body]=\"sexp-show-message\""
 output=$(notmuch show --format=sexp "sexp-show-message")
-test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :headers (:Subject \"sexp-show-subject\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Bcc \"test_suite+bcc@notmuchmail.org\" :Reply-To \"test_suite+replyto@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :body ((:id 1 :content-type \"text/plain\" :content \"sexp-show-message\n\")) :crypto ()) ())))"
+test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :body ((:id 1 :content-type \"text/plain\" :content \"sexp-show-message\n\")) :crypto () :headers (:Subject \"sexp-show-subject\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Bcc \"test_suite+bcc@notmuchmail.org\" :Reply-To \"test_suite+replyto@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\")) ())))"
 
 # This should be the same output as above.
 test_begin_subtest "Show message: sexp --body=true"
 output=$(notmuch show --format=sexp --body=true "sexp-show-message")
-test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :headers (:Subject \"sexp-show-subject\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Bcc \"test_suite+bcc@notmuchmail.org\" :Reply-To \"test_suite+replyto@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :body ((:id 1 :content-type \"text/plain\" :content \"sexp-show-message\n\")) :crypto ()) ())))"
+test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :body ((:id 1 :content-type \"text/plain\" :content \"sexp-show-message\n\")) :crypto () :headers (:Subject \"sexp-show-subject\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Bcc \"test_suite+bcc@notmuchmail.org\" :Reply-To \"test_suite+replyto@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\")) ())))"
 
 test_begin_subtest "Show message: sexp --body=false"
 output=$(notmuch show --format=sexp --body=false "sexp-show-message")
-test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :headers (:Subject \"sexp-show-subject\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Bcc \"test_suite+bcc@notmuchmail.org\" :Reply-To \"test_suite+replyto@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :crypto ()) ())))"
+test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :crypto () :headers (:Subject \"sexp-show-subject\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Bcc \"test_suite+bcc@notmuchmail.org\" :Reply-To \"test_suite+replyto@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\")) ())))"
 
 test_begin_subtest "Search message: sexp"
 add_message "[subject]=\"sexp-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"sexp-search-message\""
@@ -24,7 +24,7 @@ test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000
 test_begin_subtest "Show message: sexp, utf-8"
 add_message "[subject]=\"sexp-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
 output=$(notmuch show --format=sexp "jsön-show-méssage")
-test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :headers (:Subject \"sexp-show-utf8-body-sübjéct\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :body ((:id 1 :content-type \"text/plain\" :content \"jsön-show-méssage\n\")) :crypto ()) ())))"
+test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :filename (\"${gen_msg_filename}\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\" \"unread\") :body ((:id 1 :content-type \"text/plain\" :content \"jsön-show-méssage\n\")) :crypto () :headers (:Subject \"sexp-show-utf8-body-sübjéct\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\")) ())))"
 
 test_begin_subtest "Search message: sexp, utf-8"
 add_message "[subject]=\"sexp-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
@@ -44,6 +44,6 @@ output=$(notmuch show --format=sexp "id:$id")
 filename=$(notmuch search --output=files "id:$id")
 # Get length of README after base64-encoding, minus additional newline.
 attachment_length=$(( $(base64 $NOTMUCH_SRCDIR/test/README | wc -c) - 1 ))
-test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename (\"$filename\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\") :headers (:Subject \"sexp-show-inline-attachment-filename\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"test_suite@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :body ((:id 1 :content-type \"multipart/mixed\" :content ((:id 2 :content-type \"text/plain\" :content \"This is a test message with inline attachment with a filename\") (:id 3 :content-type \"application/octet-stream\" :content-disposition \"inline\" :filename \"README\" :content-transfer-encoding \"base64\" :content-length $attachment_length)))) :crypto ()) ())))"
+test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename (\"$filename\") :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\") :body ((:id 1 :content-type \"multipart/mixed\" :content ((:id 2 :content-type \"text/plain\" :content \"This is a test message with inline attachment with a filename\") (:id 3 :content-type \"application/octet-stream\" :content-disposition \"inline\" :filename \"README\" :content-transfer-encoding \"base64\" :content-length $attachment_length)))) :crypto () :headers (:Subject \"sexp-show-inline-attachment-filename\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"test_suite@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\")) ())))"
 
 test_done
-- 
2.20.1

[PATCH v2 02/17] util/crypto: add information about the payload part

[Daniel Kahn Gillmor]

When walking the MIME tree, if we discover that we are at the
cryptographic payload, then we would like to record at least the
Subject header of the current MIME part.

In the future, we might want to record many other headers as well, but
for now we will stick with just the Subject.

See
https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html#cryptographic-envelope
for more description of the Cryptographic Payload vs. the
Cryptographic Envelope.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 util/crypto.c | 37 +++++++++++++++++++++++++++++++++++++
 util/crypto.h |  5 +++++
 2 files changed, 42 insertions(+)

diff --git a/util/crypto.c b/util/crypto.c
index 3f8ac25a..9e185e03 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -90,6 +90,8 @@ _notmuch_message_crypto_destructor (_notmuch_message_crypto_t *msg_crypto)
 	return 0;
     if (msg_crypto->sig_list)
 	g_object_unref (msg_crypto->sig_list);
+    if (msg_crypto->payload_subject)
+	talloc_free (msg_crypto->payload_subject);
     return 0;
 }
 
@@ -133,6 +135,10 @@ _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)
 {
+    const char *protected_headers = NULL;
+    const char *forwarded = NULL;
+    const char *subject = NULL;
+
     if (!msg_crypto || !payload)
 	return NOTMUCH_STATUS_NULL_POINTER;
 
@@ -156,6 +162,37 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
 
     msg_crypto->payload_encountered = true;
 
+    /* don't bother recording anything if there is no cryptographic
+     * envelope: */
+    if ((msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_FULL) &&
+	(msg_crypto->sig_list == NULL))
+	return NOTMUCH_STATUS_SUCCESS;
+
+    /* Verify that this payload has headers that are intended to be
+     * exported to the larger message: */
+
+    /* Consider a payload that uses Alexei Melinkov's forwarded="no" for
+     * message/global or message/rfc822:
+     * https://tools.ietf.org/html/draft-melnikov-smime-header-signing-05#section-4 */
+    forwarded = g_mime_object_get_content_type_parameter (payload, "forwarded");
+    if (GMIME_IS_MESSAGE_PART (payload) && forwarded && strcmp (forwarded, "no") == 0) {
+	GMimeMessage *message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (payload));
+	subject = g_mime_message_get_subject (message);
+	/* FIXME: handle more than just Subject: at some point */
+    } else {
+	/* Consider "memoryhole"-style protected headers as practiced by Enigmail and K-9 */
+	protected_headers = g_mime_object_get_content_type_parameter (payload, "protected-headers");
+	if (protected_headers && strcasecmp("v1", protected_headers) == 0)
+	    subject = g_mime_object_get_header (payload, "Subject");
+	/* FIXME: handle more than just Subject: at some point */
+    }
+
+    if (subject) {
+	if (msg_crypto->payload_subject)
+	    talloc_free (msg_crypto->payload_subject);
+	msg_crypto->payload_subject = talloc_strdup (msg_crypto, subject);
+    }
+
     return NOTMUCH_STATUS_SUCCESS;
 }
 
diff --git a/util/crypto.h b/util/crypto.h
index c6fa7f4b..fdbb5da5 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -59,6 +59,11 @@ typedef struct _notmuch_message_crypto {
      * is not part of the cryptographic envelope) */
     bool payload_encountered;
 
+    /* the value of any "Subject:" header in the cryptographic payload
+     * (the top level part within the crypto envelope), converted to
+     * UTF-8 */
+    char * payload_subject;
+
     /* if both signed and encrypted, was the signature encrypted? */
     bool signature_encrypted;
 } _notmuch_message_crypto_t;
-- 
2.20.1

[PATCH v2 03/17] test: new test framework to compare json parts

[Daniel Kahn Gillmor]

From: Jameson Graef Rollins <jrollins@finestructure.net>

This makes it easier to write fairly compact, readable tests of json
output, without needing to sanitize away parts that we don't care
about.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/json_check_nodes.py | 113 +++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh         |  24 +++++++++
 2 files changed, 137 insertions(+)
 create mode 100755 test/json_check_nodes.py

diff --git a/test/json_check_nodes.py b/test/json_check_nodes.py
new file mode 100755
index 00000000..afc19f4b
--- /dev/null
+++ b/test/json_check_nodes.py
@@ -0,0 +1,113 @@
+#!/usr/bin/env python
+import re
+import sys
+import json
+
+
+EXPR_RE = re.compile('(?P<label>[a-zA-Z0-9_-]+):(?P<address>[^=!]+)(?:(?P<type>[=!])(?P<val>.*))?', re.DOTALL|re.MULTILINE)
+
+
+if len(sys.argv) < 2:
+    sys.exit('usage: '+ sys.argv[0] + """ EXPR [EXPR]
+
+Takes json data on stdin and evaluates test expressions specified in
+arguments.  Each test is evaluated, and output is printed only if the
+test fails.  If any test fails the return value of execution will be
+non-zero.
+
+EXPR can be one of following types:
+
+Value test: test that object in json data found at address is equal to specified value:
+
+  label:address|value
+
+Existence test: test that dict or list in json data found at address
+does *not* contain the specified key:
+
+  label:address!key
+
+Extract: extract object from json data found at address and print
+
+  label:address
+
+Results are printed to stdout prefixed by expression label.  In all
+cases the test will fail if object does not exist in data.
+
+Example:
+
+0 $ echo '["a", "b", {"c": 1}]' | python3 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"'
+second_d: value not equal: data[1] = 'b' != 'd'
+no_c: dict contains key: data[2]["c"] = "c"
+1 $
+
+""")
+
+
+# parse expressions from arguments
+exprs = []
+for expr in sys.argv[1:]:
+    m = re.match(EXPR_RE, expr)
+    if not m:
+        sys.exit("Invalid expression: {}".format(expr))
+    exprs.append(m)
+
+data = json.load(sys.stdin)
+
+fail = False
+
+for expr in exprs:
+    # print(expr.groups(),fail)
+
+    e = 'data{}'.format(expr.group('address'))
+    try:
+        val = eval(e)
+    except SyntaxError:
+        fail = True
+        print("{}: syntax error on evaluation of object: {}".format(
+            expr.group('label'), e))
+        continue
+    except:
+        fail = True
+        print("{}: object not found: data{}".format(
+            expr.group('label'), expr.group('address')))
+        continue
+
+    if expr.group('type') == '=':
+        try:
+            obj_val = json.loads(expr.group('val'))
+        except:
+            fail = True
+            print("{}: error evaluating value: {}".format(
+                expr.group('label'), expr.group('address')))
+            continue
+        if val != obj_val:
+            fail = True
+            print("{}: value not equal: data{} = {} != {}".format(
+                expr.group('label'), expr.group('address'), repr(val), repr(obj_val)))
+
+    elif expr.group('type') == '!':
+        if not isinstance(val, (dict, list)):
+            fail = True
+            print("{}: not a dict or a list: data{}".format(
+                expr.group('label'), expr.group('address')))
+            continue
+        try:
+            idx = json.loads(expr.group('val'))
+            if idx in val:
+                fail = True
+                print("{}: {} contains key: {}[{}] = {}".format(
+                    expr.group('label'), type(val), e, expr.group('val'), val[idx]))
+        except SyntaxError:
+            fail = True
+            print("{}: syntax error on evaluation of value: {}".format(
+                expr.group('label'), expr.group('val')))
+            continue
+
+
+    elif expr.group('type') is None:
+        print("{}: {}".format(expr.group('label'), val))
+
+
+if fail:
+    sys.exit(1)
+sys.exit(0)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index ff18fae6..616cb674 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -507,6 +507,30 @@ test_sort_json () {
         "import sys, json; json.dump(sorted(json.load(sys.stdin)),sys.stdout)"
 }
 
+# test for json objects:
+# read the source of test/json_check_nodes.py (or the output when
+# invoking it without arguments) for an explanation of the syntax.
+test_json_nodes () {
+        exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_json_eval without test_begin_subtest"
+	fi
+	inside_subtest=
+	test "$#" > 0 ||
+	    error "bug in the test script: test_json_nodes needs at least 1 parameter"
+
+	if ! test_skip "$test_subtest_name"
+	then
+	    output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON "$TEST_DIRECTORY"/json_check_nodes.py "$@")
+		if [ "$?" = 0 ]
+		then
+			test_ok_
+		else
+			test_failure_ "$output"
+		fi
+	fi
+}
+
 test_emacs_expect_t () {
 	test "$#" = 1 ||
 	error "bug in the test script: not 1 parameter to test_emacs_expect_t"
-- 
2.20.1

Re: [PATCH v2 03/17] test: new test framework to compare json parts

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
> +
> +Value test: test that object in json data found at address is equal to specified value:
> +
> +  label:address|value
> +

should this maybe be label:address=value ? At least looking that the
regex and the examples.

Re: [PATCH v2 03/17] test: new test framework to compare json parts

[Rollins, Jameson]

On Mon, May 27 2019, David Bremner <david@tethera.net> wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>> +
>> +Value test: test that object in json data found at address is equal to specified value:
>> +
>> +  label:address|value
>> +
>
> should this maybe be label:address=value ? At least looking that the
> regex and the examples.

This description is wrong, the comparison actually uses '=' in the code
(and in the example).  I'll fix the usage.

jamie.

[PATCH v3 03/17] test: new test framework to compare json parts

[Daniel Kahn Gillmor]

From: Jameson Graef Rollins <jrollins@finestructure.net>

This makes it easier to write fairly compact, readable tests of json
output, without needing to sanitize away parts that we don't care
about.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/json_check_nodes.py | 113 +++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh         |  24 +++++++++
 2 files changed, 137 insertions(+)
 create mode 100755 test/json_check_nodes.py

diff --git a/test/json_check_nodes.py b/test/json_check_nodes.py
new file mode 100755
index 00000000..a622969a
--- /dev/null
+++ b/test/json_check_nodes.py
@@ -0,0 +1,113 @@
+#!/usr/bin/env python
+import re
+import sys
+import json
+
+
+EXPR_RE = re.compile('(?P<label>[a-zA-Z0-9_-]+):(?P<address>[^=!]+)(?:(?P<type>[=!])(?P<val>.*))?', re.DOTALL|re.MULTILINE)
+
+
+if len(sys.argv) < 2:
+    sys.exit('usage: '+ sys.argv[0] + """ EXPR [EXPR]
+
+Takes json data on stdin and evaluates test expressions specified in
+arguments.  Each test is evaluated, and output is printed only if the
+test fails.  If any test fails the return value of execution will be
+non-zero.
+
+EXPR can be one of following types:
+
+Value test: test that object in json data found at address is equal to specified value:
+
+  label:address=value
+
+Existence test: test that dict or list in json data found at address
+does *not* contain the specified key:
+
+  label:address!key
+
+Extract: extract object from json data found at address and print
+
+  label:address
+
+Results are printed to stdout prefixed by expression label.  In all
+cases the test will fail if object does not exist in data.
+
+Example:
+
+0 $ echo '["a", "b", {"c": 1}]' | python3 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"'
+second_d: value not equal: data[1] = 'b' != 'd'
+no_c: dict contains key: data[2]["c"] = "c"
+1 $
+
+""")
+
+
+# parse expressions from arguments
+exprs = []
+for expr in sys.argv[1:]:
+    m = re.match(EXPR_RE, expr)
+    if not m:
+        sys.exit("Invalid expression: {}".format(expr))
+    exprs.append(m)
+
+data = json.load(sys.stdin)
+
+fail = False
+
+for expr in exprs:
+    # print(expr.groups(),fail)
+
+    e = 'data{}'.format(expr.group('address'))
+    try:
+        val = eval(e)
+    except SyntaxError:
+        fail = True
+        print("{}: syntax error on evaluation of object: {}".format(
+            expr.group('label'), e))
+        continue
+    except:
+        fail = True
+        print("{}: object not found: data{}".format(
+            expr.group('label'), expr.group('address')))
+        continue
+
+    if expr.group('type') == '=':
+        try:
+            obj_val = json.loads(expr.group('val'))
+        except:
+            fail = True
+            print("{}: error evaluating value: {}".format(
+                expr.group('label'), expr.group('address')))
+            continue
+        if val != obj_val:
+            fail = True
+            print("{}: value not equal: data{} = {} != {}".format(
+                expr.group('label'), expr.group('address'), repr(val), repr(obj_val)))
+
+    elif expr.group('type') == '!':
+        if not isinstance(val, (dict, list)):
+            fail = True
+            print("{}: not a dict or a list: data{}".format(
+                expr.group('label'), expr.group('address')))
+            continue
+        try:
+            idx = json.loads(expr.group('val'))
+            if idx in val:
+                fail = True
+                print("{}: {} contains key: {}[{}] = {}".format(
+                    expr.group('label'), type(val), e, expr.group('val'), val[idx]))
+        except SyntaxError:
+            fail = True
+            print("{}: syntax error on evaluation of value: {}".format(
+                expr.group('label'), expr.group('val')))
+            continue
+
+
+    elif expr.group('type') is None:
+        print("{}: {}".format(expr.group('label'), val))
+
+
+if fail:
+    sys.exit(1)
+sys.exit(0)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index ff18fae6..616cb674 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -507,6 +507,30 @@ test_sort_json () {
         "import sys, json; json.dump(sorted(json.load(sys.stdin)),sys.stdout)"
 }
 
+# test for json objects:
+# read the source of test/json_check_nodes.py (or the output when
+# invoking it without arguments) for an explanation of the syntax.
+test_json_nodes () {
+        exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_json_eval without test_begin_subtest"
+	fi
+	inside_subtest=
+	test "$#" > 0 ||
+	    error "bug in the test script: test_json_nodes needs at least 1 parameter"
+
+	if ! test_skip "$test_subtest_name"
+	then
+	    output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON "$TEST_DIRECTORY"/json_check_nodes.py "$@")
+		if [ "$?" = 0 ]
+		then
+			test_ok_
+		else
+			test_failure_ "$output"
+		fi
+	fi
+}
+
 test_emacs_expect_t () {
 	test "$#" = 1 ||
 	error "bug in the test script: not 1 parameter to test_emacs_expect_t"
-- 
2.20.1

Re: [PATCH v3 03/17] test: new test framework to compare json parts

[Daniel Kahn Gillmor]

On Mon 2019-05-27 16:34:27 -0400, Daniel Kahn Gillmor wrote:
> From: Jameson Graef Rollins <jrollins@finestructure.net>
>
> This makes it easier to write fairly compact, readable tests of json
> output, without needing to sanitize away parts that we don't care
> about.

woops, patches crossed in the ether!  feel free to prefer the version
directly from Jamie -- they should be the same thing :)

    --dkg

Re: [PATCH v3 03/17] test: new test framework to compare json parts

[Rollins, Jameson]

On Mon, May 27 2019, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> On Mon 2019-05-27 16:34:27 -0400, Daniel Kahn Gillmor wrote:
>> From: Jameson Graef Rollins <jrollins@finestructure.net>
>>
>> This makes it easier to write fairly compact, readable tests of json
>> output, without needing to sanitize away parts that we don't care
>> about.
>
> woops, patches crossed in the ether!  feel free to prefer the version
> directly from Jamie -- they should be the same thing :)

I actually fixed one other very minor thing in the script, so I would
prefer mine.

jamie.

[PATCH v3] test: new test framework to compare json parts

[Rollins, Jameson]

From: Jameson Graef Rollins <jrollins@finestructure.net>

This makes it easier to write fairly compact, readable tests of json
output, without needing to sanitize away parts that we don't care
about.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/json_check_nodes.py | 114 +++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh         |  24 +++++++++
 2 files changed, 138 insertions(+)
 create mode 100755 test/json_check_nodes.py

diff --git a/test/json_check_nodes.py b/test/json_check_nodes.py
new file mode 100755
index 00000000..17403c57
--- /dev/null
+++ b/test/json_check_nodes.py
@@ -0,0 +1,114 @@
+#!/usr/bin/env python
+import re
+import sys
+import json
+
+
+EXPR_RE = re.compile('(?P<label>[a-zA-Z0-9_-]+):(?P<address>[^=!]+)(?:(?P<type>[=!])(?P<val>.*))?', re.DOTALL|re.MULTILINE)
+
+
+if len(sys.argv) < 2:
+    sys.exit('usage: '+ sys.argv[0] + """ EXPR [EXPR]
+
+Takes json data on stdin and evaluates test expressions specified in
+arguments.  Each test is evaluated, and output is printed only if the
+test fails.  If any test fails the return value of execution will be
+non-zero.
+
+EXPR can be one of following types:
+
+Value test: test that object in json data found at address is equal to
+specified value:
+
+  label:address=value
+
+Existence test: test that dict or list in json data found at address
+does *not* contain the specified key:
+
+  label:address!key
+
+Extract: extract object from json data found at address and print
+
+  label:address
+
+Results are printed to stdout prefixed by expression label.  In all
+cases the test will fail if object does not exist in data.
+
+Example:
+
+0 $ echo '["a", "b", {"c": 1}]' | python3 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"'
+second_d: value not equal: data[1] = 'b' != 'd'
+no_c: dict contains key: data[2]["c"] = 1
+1 $
+
+""")
+
+
+# parse expressions from arguments
+exprs = []
+for expr in sys.argv[1:]:
+    m = re.match(EXPR_RE, expr)
+    if not m:
+        sys.exit("Invalid expression: {}".format(expr))
+    exprs.append(m)
+
+data = json.load(sys.stdin)
+
+fail = False
+
+for expr in exprs:
+    # print(expr.groups(),fail)
+
+    e = 'data{}'.format(expr.group('address'))
+    try:
+        val = eval(e)
+    except SyntaxError:
+        fail = True
+        print("{}: syntax error on evaluation of object: {}".format(
+            expr.group('label'), e))
+        continue
+    except:
+        fail = True
+        print("{}: object not found: data{}".format(
+            expr.group('label'), expr.group('address')))
+        continue
+
+    if expr.group('type') == '=':
+        try:
+            obj_val = json.loads(expr.group('val'))
+        except:
+            fail = True
+            print("{}: error evaluating value: {}".format(
+                expr.group('label'), expr.group('address')))
+            continue
+        if val != obj_val:
+            fail = True
+            print("{}: value not equal: data{} = {} != {}".format(
+                expr.group('label'), expr.group('address'), repr(val), repr(obj_val)))
+
+    elif expr.group('type') == '!':
+        if not isinstance(val, (dict, list)):
+            fail = True
+            print("{}: not a dict or a list: data{}".format(
+                expr.group('label'), expr.group('address')))
+            continue
+        try:
+            idx = json.loads(expr.group('val'))
+            if idx in val:
+                fail = True
+                print("{}: {} contains key: {}[{}] = {}".format(
+                    expr.group('label'), type(val).__name__, e, expr.group('val'), val[idx]))
+        except SyntaxError:
+            fail = True
+            print("{}: syntax error on evaluation of value: {}".format(
+                expr.group('label'), expr.group('val')))
+            continue
+
+
+    elif expr.group('type') is None:
+        print("{}: {}".format(expr.group('label'), val))
+
+
+if fail:
+    sys.exit(1)
+sys.exit(0)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index ff18fae6..616cb674 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -507,6 +507,30 @@ test_sort_json () {
         "import sys, json; json.dump(sorted(json.load(sys.stdin)),sys.stdout)"
 }
 
+# test for json objects:
+# read the source of test/json_check_nodes.py (or the output when
+# invoking it without arguments) for an explanation of the syntax.
+test_json_nodes () {
+        exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_json_eval without test_begin_subtest"
+	fi
+	inside_subtest=
+	test "$#" > 0 ||
+	    error "bug in the test script: test_json_nodes needs at least 1 parameter"
+
+	if ! test_skip "$test_subtest_name"
+	then
+	    output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON "$TEST_DIRECTORY"/json_check_nodes.py "$@")
+		if [ "$?" = 0 ]
+		then
+			test_ok_
+		else
+			test_failure_ "$output"
+		fi
+	fi
+}
+
 test_emacs_expect_t () {
 	test "$#" = 1 ||
 	error "bug in the test script: not 1 parameter to test_emacs_expect_t"
-- 
2.20.1

[PATCH v2 04/17] cli/show: add tests for viewing protected headers

[Daniel Kahn Gillmor]

Here we add several variant e-mail messages, some of which have
correctly-structured protected headers, and some of which do not.  The
goal of the tests is to ensure that the right protected subjects get
reported.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh                | 66 +++++++++++++++++++
 ...le-wrapped-with-phony-protected-header.eml | 38 +++++++++++
 .../misplaced-protected-header.eml            | 35 ++++++++++
 .../nested-rfc822-message.eml                 | 32 +++++++++
 .../no-protected-header-attribute.eml         | 29 ++++++++
 .../phony-protected-header-bad-encryption.eml | 30 +++++++++
 .../protected-headers/protected-header.eml    | 30 +++++++++
 .../wrapped-protected-header.eml              | 39 +++++++++++
 8 files changed, 299 insertions(+)
 create mode 100755 test/T356-protected-headers.sh
 create mode 100644 test/corpora/protected-headers/double-wrapped-with-phony-protected-header.eml
 create mode 100644 test/corpora/protected-headers/misplaced-protected-header.eml
 create mode 100644 test/corpora/protected-headers/nested-rfc822-message.eml
 create mode 100644 test/corpora/protected-headers/no-protected-header-attribute.eml
 create mode 100644 test/corpora/protected-headers/phony-protected-header-bad-encryption.eml
 create mode 100644 test/corpora/protected-headers/protected-header.eml
 create mode 100644 test/corpora/protected-headers/wrapped-protected-header.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
new file mode 100755
index 00000000..599ff1ed
--- /dev/null
+++ b/test/T356-protected-headers.sh
@@ -0,0 +1,66 @@
+#!/usr/bin/env bash
+
+# TODO:
+#  * check S/MIME as well as PGP/MIME
+#  * process headers protected by signature
+
+test_description='Message decryption with protected headers'
+. $(dirname "$0")/test-lib.sh || exit 1
+
+##################################################
+
+add_gnupg_home
+
+add_email_corpus protected-headers
+
+test_begin_subtest "verify protected header is not visible without decryption"
+output=$(notmuch show --format=json id:protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'no_crypto_info:[0][0][0]["crypto"]={}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Subject Unavailable"'
+
+test_begin_subtest "verify protected header is visible with decryption"
+output=$(notmuch show --decrypt=true --format=json id:protected-header@crypto.notmuchmail.org)
+test_subtest_known_broken
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"'
+
+test_begin_subtest "misplaced protected headers should not be made visible during decryption"
+output=$(notmuch show --decrypt=true --format=json id:misplaced-protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Subject Unavailable"'
+
+test_begin_subtest "verify double-wrapped phony protected header is not visible when inner decryption fails"
+output=$(notmuch show --decrypt=true --format=json id:double-wrapped-with-phony-protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Subject Unavailable"'
+
+test_begin_subtest "cleartext phony protected headers should not be made visible when decryption fails"
+output=$(notmuch show --decrypt=true --format=json id:phony-protected-header-bad-encryption@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'no_crypto_info:[0][0][0]["crypto"]={}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Subject Unavailable"'
+
+test_begin_subtest "wrapped protected headers should not be made visible during decryption"
+output=$(notmuch show --decrypt=true --format=json id:wrapped-protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "partial"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="[mailing-list] Subject Unavailable"'
+
+test_begin_subtest "internal headers without protected-header attribute should be skipped"
+output=$(notmuch show --decrypt=true --format=json id:no-protected-header-attribute@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Subject Unavailable"'
+
+test_begin_subtest "verify nested message/rfc822 protected header is visible"
+output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-message@crypto.notmuchmail.org)
+test_subtest_known_broken
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="This is a message using draft-melnikov-smime-header-signing"'
+
+test_done
diff --git a/test/corpora/protected-headers/double-wrapped-with-phony-protected-header.eml b/test/corpora/protected-headers/double-wrapped-with-phony-protected-header.eml
new file mode 100644
index 00000000..b05cb545
--- /dev/null
+++ b/test/corpora/protected-headers/double-wrapped-with-phony-protected-header.eml
@@ -0,0 +1,38 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <double-wrapped-with-phony-protected-header@crypto.notmuchmail.org>
+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-----
+
+hIwDxE023q1UqxYBBACkvfKZEkuRUQ2ujdel8U2ufplGxE2oNOK+CI5S1O8cS9vE
+DIkVIXAtpZcCc31pYBTRl0TwCrLKFT/siYfshbxyWjMZjX/Jc38Yjg9pDFTIZ312
+LoM5uH22f1X8O8020HgH+CQk9T4s9bBuvxTvJ6GQvK/ssnoYsGr9TGcjjh3uMdLp
+AXkkF76a2iimkq2163ee/8X0vgI+2fx6EjJJvlcSIlDcUvhYHIt8kjnlADSBMpho
+gaMa90baGlE1RAK9nSBC+4ty0fIlfsgcecRtFEifFRj6foYPFIFzkgwhRkXovouG
+FyXi8QrDVS8cz61I03PMVsFHo4FtJw9cAfvTh45QFGl+inW2pSvZyRnyu6uHDe61
+NqUTJOVN4B+dFPbKafUKuJ4YGXLsDoQoE8VF0lwznA7AOATmqPQpp+Anq40C/4Su
+Zf1hGaBTuYjlChSTMxX+wV22+PQwJmK3tl1NQRFGlR1pQZWdNcu6/6RGooiVZSg+
+VsmtZjgpZa8aaEEnrsIEVPfvbIZ4OQhmgNi4CYNB306UOjIh3/8m+8JmlkxPiGXW
+gnzNUTuwKytlZnIgT1o9a7PAkz+ZiHhMLmk5nPN+dlwsVN7Ff1FHqLIMbKaZbeKK
+txvhw7/NdaCALnjamqtDJTc4kL50F44DC0im0U9hcoy8X/HBrYkTGfHgRttCp5V/
+XisGT6/rzyUzTi2usZpRtl3WhHrE0Uj0w2Bm/Qqe64vNd3F8xwuJ5qMZ3QLVxoX0
+MPTajY1pLgfMViqLaLV8fR8hLmattxaO92sbVuxHiaba8er3jzO2HfmRLqesio7u
+8FXZQnBgeqBkoRlrHhvScuZLJVU1I4UHd9s3mcR+IY5VvjxdPMcnxTNqcRB/He4H
+MrrH26P0uSFe6WJYQVXEDt4OO73ROyFZE0+rSw1z+VnjmHVIzUVvvFqwJZo6Y/0v
+1+3ab4TGMPJSkfQYHY8/O1RF67BNlA==
+=gizc
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/misplaced-protected-header.eml b/test/corpora/protected-headers/misplaced-protected-header.eml
new file mode 100644
index 00000000..f1a72f0d
--- /dev/null
+++ b/test/corpora/protected-headers/misplaced-protected-header.eml
@@ -0,0 +1,35 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <misplaced-protected-header@crypto.notmuchmail.org>
+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-----
+
+hIwDxE023q1UqxYBBACwbgx3N72gYKIU63tNE6kf6UA5ed39VFXh3zdM6eDdA0bG
+DWt5yROckkCeCvMoFaRswK8MiX8aGG0GdH6VKhyn7HjT/Dm84QLwoB0ccZs3MnwU
+aJ9yTC9HbX3yfTVZYOu0w47NZho/LXX2Yd1pi8OUgrPg44fjgvx2kNRQ9EsNBdLA
+/AGMhwwcTPHjyWQ4XYZoL6WeVJfq2C0m3hQ3bxrKuAzW53HrSa4tPCXzX3G8KEz5
+sSk3ZOmajSvLde0LG8bxwexgAHC/Wd07e2HgHtZ/H+Cw9oYLgwcgVyXg7sGVrMrs
+IlwW0Njf93DJmJZuTD8P9XJc3h1VzKA+YhbtnofFZw4JexpHcC+R8Lcso16Mkp91
+7Ig0E8WTZ+K+judGS010b5ND2ETyc+TYY4/XJ2R90pbNrRLNTFG+P2HUob6PBCwE
+rXot6TeBSgm+k4bvl9aMKyrBSplKktQey4WsdblbJnJUxSl/rMpW6xwglkyIgrCU
+vbhffqgB8y1JLmK6Ow/A6Pzi3T6Zn95zu2GN8+yAOzDhGwlAfIV85TYnX6ybOkX/
+Amoh7qNS17pzc6ch/mif/RsSPYo+y2UQuVFhG+kOy9oGAQOOHeiCWZPa09o3R2Jn
+myMg1FPgoDgsjE6QpD0mx9ORdPGC2e8jwrifS/W9eHJ2QG+mNkcKlAr5b8WiUTkq
+hEZ+BaaVhbXN8EuHHTJT6YojusCIsXI0BMF1su1KupQw+dwQnys8wuy45Fr3H58x
+zqHoU9KzdQGLbeJTgA==
+=+EWE
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/nested-rfc822-message.eml b/test/corpora/protected-headers/nested-rfc822-message.eml
new file mode 100644
index 00000000..059783ce
--- /dev/null
+++ b/test/corpora/protected-headers/nested-rfc822-message.eml
@@ -0,0 +1,32 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <nested-rfc822-message@crypto.notmuchmail.org>
+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-----
+
+hIwDxE023q1UqxYBBADCWqobSSS78XdrxhBh5W01OZbUMdnrwrYJsiG9fQoVfFHN
+eALvOfviTcSBD97/jO2IRL2W8hyF7k1BVAYMwSuxe4qLbLdsxK1i4KBRIFRkm990
+ipBddgFXV16WNO2cTK7boEJ7Xfjp/zjoS2z2YUXsdGx3OSJciyHBVJki2UfkL9LA
+egHa7dsw6BxoNbAkrD+ijVbsFrKHeeJIlWkNbSYOk/YLmqLAEy1CYvSvC8ZSBtQT
+fVYc37fc3RB0vQC+Vu5k5d/I5Z1/Yz+McBJDMNvcn4yoFiXemY8YVFvj7iC0sbuq
+lwitvgMYaljhb8RUQAa3Dy08Jju09DIBcCgRsx32U+3aqZ0MhU6CRgt8kc9oK1g4
+yBVppqpX6hCXjtt9LUArY3DIchRb+IWTXsb+eDR700GXDyNMk1G5WUl0eLuw75uz
+EqU5Tjh36fP0ceMESjaxuxyhhw1jjE3ON7vqFQRVcs7UtazbxznWQH3Z73mDmY3G
+q9JGMOOqVnnFdnEq8vDFF7m+Cp3N1ieyXUXjn3aLtvSRMmVV20Q5QXSFg8nP6juT
+Yn1xZjqOodSeig1ITZZF58Whv+LHGtzDHwV8
+=cNYF
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/no-protected-header-attribute.eml b/test/corpora/protected-headers/no-protected-header-attribute.eml
new file mode 100644
index 00000000..880f60e3
--- /dev/null
+++ b/test/corpora/protected-headers/no-protected-header-attribute.eml
@@ -0,0 +1,29 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <no-protected-header-attribute@crypto.notmuchmail.org>
+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-----
+
+hIwDxE023q1UqxYBA/9GY8NN4NDwpNttr/hTXpS701Z8TDr3hC89obZNnNpYxSct
+p+YkS+FsPMLimIDfU1meG8R+YgtQOJIhmKPHW8CLQ1heBsX0Dcv2oLxXodqNGD7M
+/szVRR6duVnALPgmV66vkcBHKbsiuv8EO86C7G1hAnXfs0H47WoeUz9dQ6RaHdKw
+AVbxw7KWVbiP+S4SO1rvNsAL1xiRPA0FFmDRMyoFRC/618dGS6HitkD0UR708oVt
+PooD4Rk22c8b549wvZ88flGk+WBCLhyXAuWYPHwag1DLzLjWH5r+XmK2O7JoQZeq
+k7JM/M8QM+xetFaPmsWs52IynhXyWpXBBanm9NEsNEiIB59480D7tJ0oivo8T24d
+izSAMGATP26ReatoXltCl9x8uUfUSAjWt8iJ1+n/3ds=
+=hGDA
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/phony-protected-header-bad-encryption.eml b/test/corpora/protected-headers/phony-protected-header-bad-encryption.eml
new file mode 100644
index 00000000..15dc08ab
--- /dev/null
+++ b/test/corpora/protected-headers/phony-protected-header-bad-encryption.eml
@@ -0,0 +1,30 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <phony-protected-header-bad-encryption@crypto.notmuchmail.org>
+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
+Subject: this should not show up as a protected header
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBA/9ZaOuxGtLVWiA7KQfB+4td1AILd1uy039UDb+9YwlhmJTq
+mNqVJu+ZkFniZPMliM0z1QRBkBeL2Q7MrHAdYxYBKrDHKVja4O7jwqeKjy5BzQCW
+fnyT+sb2Mh+dz5P2voF3XJHgqzhFY1rtVEatXSZADwwIVU6oZqGZ8GOELNGSd9KX
+ASNElH7WGZB/TQ5X+MktzOLExx5QWaRK9skogI2RRoOquS7KpMcjzb2FWaJDjr1s
+hd8FCQVjWuUDrolMGH8cgeq9iUBlHMzfPY6/jeGHNrjk12wwhBNcq6O95uzXtIRS
+BM2xnwCYec6wYJ46fHukTgv+286nSQcV0XT6a+qM5GMgV5DMHW2vSyl6kTszJ3EP
+xvQBfPCItA==
+=Gkxz
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/protected-header.eml b/test/corpora/protected-headers/protected-header.eml
new file mode 100644
index 00000000..dec822c2
--- /dev/null
+++ b/test/corpora/protected-headers/protected-header.eml
@@ -0,0 +1,30 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <protected-header@crypto.notmuchmail.org>
+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
+Subject: this should not show up as a protected header
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBA/9ZaOuxGtLVWiA7KQfB+4td1AILd1uy039UDb+9YwlhmJTq
+mNqVJu+ZkFniZPMliM0z1QRBkBeL2Q7MrHAdYxYBKrDHKVja4O7jwqeKjy5BzQCW
+fnyT+sb2Mh+dz5P2voF3XJHgqzhFY1rtVEatXSZADwwIVU6oZqGZ8GOELNGSd9KX
+ASNElH7WGZB/TQ5X+MktzOLExx5QWaRK9skogI2RRoOquS7KpMcjzb2FWaJDjr1s
+RGboX7NG3xCvNUV2ByFTvLOeo7eO1GfUsabTUbMMvh3AE1UvHgCu8VJiRrMdmPln
+BM2xnwCYec6wYJ46fHukTgv+286nSQcV0XT6a+qM5GMgV5DMHW2vSyl6kTszJ3EP
+xvQBfPCItA==
+=Gkxz
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/wrapped-protected-header.eml b/test/corpora/protected-headers/wrapped-protected-header.eml
new file mode 100644
index 00000000..9a3c1384
--- /dev/null
+++ b/test/corpora/protected-headers/wrapped-protected-header.eml
@@ -0,0 +1,39 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: [mailing-list] Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <wrapped-protected-header@crypto.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="zzzz"
+
+--zzzz
+Content-Type: multipart/encrypted; boundary="=-=-=";
+	protocol="application/pgp-encrypted"
+
+--=-=-=
+Content-Type: application/pgp-encrypted
+
+Version: 1
+
+--=-=-=
+Content-Type: application/octet-stream
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBA/9ZaOuxGtLVWiA7KQfB+4td1AILd1uy039UDb+9YwlhmJTq
+mNqVJu+ZkFniZPMliM0z1QRBkBeL2Q7MrHAdYxYBKrDHKVja4O7jwqeKjy5BzQCW
+fnyT+sb2Mh+dz5P2voF3XJHgqzhFY1rtVEatXSZADwwIVU6oZqGZ8GOELNGSd9KX
+ASNElH7WGZB/TQ5X+MktzOLExx5QWaRK9skogI2RRoOquS7KpMcjzb2FWaJDjr1s
+RGboX7NG3xCvNUV2ByFTvLOeo7eO1GfUsabTUbMMvh3AE1UvHgCu8VJiRrMdmPln
+BM2xnwCYec6wYJ46fHukTgv+286nSQcV0XT6a+qM5GMgV5DMHW2vSyl6kTszJ3EP
+xvQBfPCItA==
+=Gkxz
+-----END PGP MESSAGE-----
+--=-=-=--
+
+--zzzz
+Content-Type: text/plain
+
+This message body was re-wrapped by a mailing list
+which is why the protected headers no longer work.
+--zzzz--
-- 
2.20.1

[PATCH v2 05/17] cli/show: emit payload subject instead of outside subject

[Daniel Kahn Gillmor]

Correctly fix the two outstanding tests so that the protected (hidden)
subject is properly reported.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 notmuch-client.h               |  2 +-
 notmuch-reply.c                |  4 +++-
 notmuch-show.c                 | 14 +++++++++-----
 test/T356-protected-headers.sh |  2 --
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index a82cb431..39e26f2e 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -232,7 +232,7 @@ format_part_sprinter (const void *ctx, struct sprinter *sp, mime_node_t *node,
 
 void
 format_headers_sprinter (struct sprinter *sp, GMimeMessage *message,
-			 bool reply);
+			 bool reply, const _notmuch_message_crypto_t *msg_crypto);
 
 typedef enum {
     NOTMUCH_SHOW_TEXT_PART_REPLY = 1 << 0,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7f284229..2689b247 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -663,7 +663,9 @@ static int do_reply(notmuch_config_t *config,
 
 	    /* The headers of the reply message we've created */
 	    sp->map_key (sp, "reply-headers");
-	    format_headers_sprinter (sp, reply, true);
+	    /* FIXME: send msg_crypto here to avoid killing the
+	     * subject line on reply to encrypted messages! */
+	    format_headers_sprinter (sp, reply, true, NULL);
 
 	    /* Start the original */
 	    sp->map_key (sp, "original");
diff --git a/notmuch-show.c b/notmuch-show.c
index 0816a5e1..b1f6a4bb 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -197,7 +197,7 @@ _is_from_line (const char *line)
 
 void
 format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
-			 bool reply)
+			 bool reply, const _notmuch_message_crypto_t *msg_crypto)
 {
     /* Any changes to the JSON or S-Expression format should be
      * reflected in the file devel/schemata. */
@@ -209,7 +209,10 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
     sp->begin_map (sp);
 
     sp->map_key (sp, "Subject");
-    sp->string (sp, g_mime_message_get_subject (message));
+    if (msg_crypto && msg_crypto->payload_subject) {
+	sp->string (sp, msg_crypto->payload_subject);
+    } else
+	sp->string (sp, g_mime_message_get_subject (message));
 
     sp->map_key (sp, "From");
     sp->string (sp, g_mime_message_get_from_string (message));
@@ -616,6 +619,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
      * reflected in the file devel/schemata. */
 
     if (node->envelope_file) {
+	const _notmuch_message_crypto_t *msg_crypto = NULL;
 	sp->begin_map (sp);
 	format_message_sprinter (sp, node->envelope_file);
 
@@ -626,8 +630,8 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    sp->end (sp);
 	}
 
+	msg_crypto = mime_node_get_message_crypto_status (node);
 	if (notmuch_format_version >= 4) {
-	    const _notmuch_message_crypto_t *msg_crypto = mime_node_get_message_crypto_status (node);
 	    sp->map_key (sp, "crypto");
 	    sp->begin_map (sp);
 	    if (msg_crypto->sig_list ||
@@ -655,7 +659,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	}
 
 	sp->map_key (sp, "headers");
-	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), false);
+	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), false, msg_crypto);
 
 	sp->end (sp);
 	return;
@@ -748,7 +752,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	sp->begin_map (sp);
 
 	sp->map_key (sp, "headers");
-	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), false);
+	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), false, NULL);
 
 	sp->map_key (sp, "body");
 	sp->begin_list (sp);
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 599ff1ed..8a8fef6a 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -21,7 +21,6 @@ test_json_nodes <<<"$output" \
 
 test_begin_subtest "verify protected header is visible with decryption"
 output=$(notmuch show --decrypt=true --format=json id:protected-header@crypto.notmuchmail.org)
-test_subtest_known_broken
 test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"'
@@ -58,7 +57,6 @@ test_json_nodes <<<"$output" \
 
 test_begin_subtest "verify nested message/rfc822 protected header is visible"
 output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-message@crypto.notmuchmail.org)
-test_subtest_known_broken
 test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="This is a message using draft-melnikov-smime-header-signing"'
-- 
2.20.1

[PATCH v2 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 <dkg@fifthhorseman.net>
---
 devel/schemata                 |  6 ++++++
 notmuch-show.c                 | 21 +++++++++++++++++++++
 test/T356-protected-headers.sh |  4 ++--
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 72feb7b7..9d3c8d30 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -88,9 +88,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-header@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 <<<"$output" \
 test_begin_subtest "verify nested message/rfc822 protected header is visible"
 output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-message@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 message using draft-melnikov-smime-header-signing"'
 
 test_done
-- 
2.20.1

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

[David Bremner]

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

> 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 <dkg@fifthhorseman.net>
> ---
>  devel/schemata                 |  6 ++++++
>  notmuch-show.c                 | 21 +++++++++++++++++++++
>  test/T356-protected-headers.sh |  4 ++--
>  3 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/devel/schemata b/devel/schemata
> index 72feb7b7..9d3c8d30 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -88,9 +88,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,*}
>                  }

I think you also need to add a definition for header_name to schemata
(in the same way that messageid is defined as a string).

The name "header-mask" is a bit generic, but I don't have my head in
this topic like you do. I was thinking of something like
"replaced-headers", but it's only a mild suggestion.

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

[Rollins, Jameson]

On Mon, May 27 2019, David Bremner <david@tethera.net> wrote:
> The name "header-mask" is a bit generic, but I don't have my head in
> this topic like you do. I was thinking of something like
> "replaced-headers", but it's only a mild suggestion.

I think the point is that the headers are more accurately "masked" than
"replaced", since you can look under the hood and recover what the
original header was.

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

[David Bremner]

"Rollins, Jameson" <jrollins@caltech.edu> writes:

> On Mon, May 27 2019, David Bremner <david@tethera.net> wrote:
>> The name "header-mask" is a bit generic, but I don't have my head in
>> this topic like you do. I was thinking of something like
>> "replaced-headers", but it's only a mild suggestion.
>
> I think the point is that the headers are more accurately "masked" than
> "replaced", since you can look under the hood and recover what the
> original header was.

Yes, I see what you mean.  It's just that for me a "mask" brings to mind
bitwise operations.  I guess I'd prefer "masked-headers" to
"header-mask", but if the two of you are convinced then I won't block
it.

d

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

[Daniel Kahn Gillmor]

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.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 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..072b8d39 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).
+                  masked-headers:  { header_name: string|null,*}
                 }
 }
 
diff --git a/notmuch-show.c b/notmuch-show.c
index b1f6a4bb..583b87f6 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, "masked-headers");
+			    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..359e245f 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-header@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
-                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"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 <<<"$output" \
 test_begin_subtest "verify nested message/rfc822 protected header is visible"
 output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-message@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
-                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"Subject": "Subject Unavailable"}}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="This is a message using draft-melnikov-smime-header-signing"'
 
 test_done
-- 
2.20.1

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

[Daniel Kahn Gillmor]

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

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.fsf@fifthhorseman.net why i prefer "header-mask", and that
preference still holds.

           --dkg

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

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

[Daniel Kahn Gillmor]

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

On Mon 2019-05-27 07:12:52 -0300, David Bremner wrote:
> I think you also need to add a definition for header_name to schemata
> (in the same way that messageid is defined as a string).

thanks, done in v3, which you should see shortly.

> The name "header-mask" is a bit generic, but I don't have my head in
> this topic like you do. I was thinking of something like
> "replaced-headers", but it's only a mild suggestion.

i went through several variations on this, and settled finally on
header-mask.  I considered "masked-headers" and "replaced-headers" but
ultimately discarded them because they were unclear as to whether the
thing they were showing was the thing that was masked/replaced, or the
thing that was doing the masking/replacing.

I think that "header-mask" is unambiguous in indicating that its
contents are the things that are doing the masking-replacing, so i'd
prefer to stick with it. (though i'm willing to entertain other
proposals that have the same lack of ambiguity)

          --dkg

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

[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 <dkg@fifthhorseman.net>
---
 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-header@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 <<<"$output" \
 test_begin_subtest "verify nested message/rfc822 protected header is visible"
 output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-message@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 message using draft-melnikov-smime-header-signing"'
 
 test_done
-- 
2.20.1

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

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> 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. 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.

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

[Daniel Kahn Gillmor]

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

On Tue 2019-05-28 08:10:35 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> 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

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

[PATCH v2 07/17] test: add test for missing external subject

[Daniel Kahn Gillmor]

Adding another test to ensure that we handle protected headers
gracefully when no external subject is present.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh                |  6 ++++
 .../subjectless-protected-header.eml          | 29 +++++++++++++++++++
 2 files changed, 35 insertions(+)
 create mode 100644 test/corpora/protected-headers/subjectless-protected-header.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 68d431e9..59ab58d7 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -25,6 +25,12 @@ test_json_nodes <<<"$output" \
                 '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 "when no external header is present, show masked subject as null"
+output=$(notmuch show --decrypt=true --format=json id:subjectless-protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "header-mask": {"Subject": null}}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"'
+
 test_begin_subtest "misplaced protected headers should not be made visible during decryption"
 output=$(notmuch show --decrypt=true --format=json id:misplaced-protected-header@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
diff --git a/test/corpora/protected-headers/subjectless-protected-header.eml b/test/corpora/protected-headers/subjectless-protected-header.eml
new file mode 100644
index 00000000..7163b9ae
--- /dev/null
+++ b/test/corpora/protected-headers/subjectless-protected-header.eml
@@ -0,0 +1,29 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <subjectless-protected-header@crypto.notmuchmail.org>
+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
+Subject: this should not show up as a protected header
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBA/9ZaOuxGtLVWiA7KQfB+4td1AILd1uy039UDb+9YwlhmJTq
+mNqVJu+ZkFniZPMliM0z1QRBkBeL2Q7MrHAdYxYBKrDHKVja4O7jwqeKjy5BzQCW
+fnyT+sb2Mh+dz5P2voF3XJHgqzhFY1rtVEatXSZADwwIVU6oZqGZ8GOELNGSd9KX
+ASNElH7WGZB/TQ5X+MktzOLExx5QWaRK9skogI2RRoOquS7KpMcjzb2FWaJDjr1s
+RGboX7NG3xCvNUV2ByFTvLOeo7eO1GfUsabTUbMMvh3AE1UvHgCu8VJiRrMdmPln
+BM2xnwCYec6wYJ46fHukTgv+286nSQcV0XT6a+qM5GMgV5DMHW2vSyl6kTszJ3EP
+xvQBfPCItA==
+=Gkxz
+-----END PGP MESSAGE-----
+--=-=-=--
-- 
2.20.1

[PATCH v2 08/17] test: show cryptographic envelope information for signed mails

[Daniel Kahn Gillmor]

Make sure that we emit the correct cryptographic envelope status for
cleartext signed messages.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh                | 11 ++++++-
 .../signed-protected-header.eml               | 29 +++++++++++++++++++
 .../protected-headers/simple-signed-mail.eml  | 28 ++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 test/corpora/protected-headers/signed-protected-header.eml
 create mode 100644 test/corpora/protected-headers/simple-signed-mail.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 59ab58d7..62d7e210 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -2,7 +2,6 @@
 
 # TODO:
 #  * check S/MIME as well as PGP/MIME
-#  * process headers protected by signature
 
 test_description='Message decryption with protected headers'
 . $(dirname "$0")/test-lib.sh || exit 1
@@ -67,4 +66,14 @@ test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "header-mask": {"Subject": "Subject Unavailable"}}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="This is a message using draft-melnikov-smime-header-signing"'
 
+test_begin_subtest "show cryptographic envelope on signed mail"
+output=$(notmuch show --verify --format=json id:simple-signed-mail@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525609971, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "status": "good"}]}}'
+
+test_begin_subtest "verify signed protected header"
+output=$(notmuch show --verify --format=json id:signed-protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525350527, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "status": "good"}], "headers": ["Subject"]}}'
+
 test_done
diff --git a/test/corpora/protected-headers/signed-protected-header.eml b/test/corpora/protected-headers/signed-protected-header.eml
new file mode 100644
index 00000000..c3a21b85
--- /dev/null
+++ b/test/corpora/protected-headers/signed-protected-header.eml
@@ -0,0 +1,29 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: This is a signed message
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <signed-protected-header@crypto.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/signed; boundary="=-=-=";
+ protocol="application/pgp-signature";
+ micalg=pgp-sha512
+
+--=-=-=
+Content-Type: text/plain; protected-headers="v1"
+Subject: This is a signed message
+
+Here is the signed message body.
+
+--=-=-=
+Content-Disposition: attachment; filename=signature.asc
+Content-Type: application/pgp-signature
+
+-----BEGIN PGP SIGNATURE-----
+
+iLMEAQEKAB0WIQRa6rEfXjPc6HXdt1ttkmEtlORjgQUCWusAfwAKCRBtkmEtlORj
+geIJA/0WcyxlwDfXRMbiGE/crLBYhLpXK6ZMzjEn6HQDntMIk3Kr61rAwL8edKGx
+gbxr1+XlMYRt+PJDhi8iI0odDI1YjiBjjc0bXUoDn60UcjL2MPGshI3426CA7cqB
+cMaoRHajfdxYjSzzfh8duVgi0vmUnsyoePBhANRbDIVmCQS11g==
+=c4cq
+-----END PGP SIGNATURE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/simple-signed-mail.eml b/test/corpora/protected-headers/simple-signed-mail.eml
new file mode 100644
index 00000000..ebf4b786
--- /dev/null
+++ b/test/corpora/protected-headers/simple-signed-mail.eml
@@ -0,0 +1,28 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: This is a signed message
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <simple-signed-mail@crypto.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/signed; boundary="=-=-=";
+ protocol="application/pgp-signature";
+ micalg=pgp-sha512
+
+--=-=-=
+Content-Type: text/plain
+
+Here is the signed message body.
+
+--=-=-=
+Content-Disposition: attachment; filename=signature.asc
+Content-Type: application/pgp-signature
+
+-----BEGIN PGP SIGNATURE-----
+
+iLMEAQEKAB0WIQRa6rEfXjPc6HXdt1ttkmEtlORjgQUCWu718wAKCRBtkmEtlORj
+gUXaA/4/m6CPRgC9JODRKRWo3Szi5D3zg7uf29DIJu9m2vVRw5o0ZeHcxLb26UPe
+qdjPq6GBclkXdeTH9Nv2TW5cToJmMA9UvESeRRzbe6ytvswNEYdSbiYAsv/k9t6K
+KQO2ZSbsbVlkh8xVYC3ORiUS775YrPxVT6QlPkMKAXw3l3Zwcg==
+=jnDO
+-----END PGP SIGNATURE-----
+--=-=-=--
-- 
2.20.1

[PATCH v2 09/17] cli/reply: ensure encrypted Subject: line does not leak in the clear

[Daniel Kahn Gillmor]

Now that we can decrypt headers, we want to make sure that clients
using "notmuch reply" to prepare a reply don't leak cleartext in their
subject lines.  In particular, the ["reply-headers"]["Subject"] should
by default show the external Subject.

A replying MUA that intends to protect the Subject line should show
the user the Subject from ["original"]["headers"]["Subject"] instead
of using ["reply-headers"]["Subject"].

This minor asymmetry with "notmuch show" is intentional.  While both
tools always render the cleartext subject line when they know it (in
["headers"]["Subject"] for "notmuch show" and in
["original"]["headers"]["Subject"] for "notmuch reply"), "notmuch
reply" should never leak something that should stay under encrypted
cover in "reply-headers".

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 62d7e210..ff37f6bd 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -76,4 +76,11 @@ output=$(notmuch show --verify --format=json id:signed-protected-header@crypto.n
 test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525350527, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "status": "good"}], "headers": ["Subject"]}}'
 
+test_begin_subtest "protected subject does not leak by default in replies"
+output=$(notmuch reply --decrypt=true --format=json id:protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:["original"]["crypto"]={"decrypted": {"status": "full", "header-mask": {"Subject": "Subject Unavailable"}}}' \
+                'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
+                'reply-subject:["reply-headers"]["Subject"]="Re: Subject Unavailable"'
+
 test_done
-- 
2.20.1

[PATCH v2 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 <dkg@fifthhorseman.net>
---
 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 = _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);
 
     status = notmuch_message_add_property (message, "index.decryption", "success");
@@ -606,6 +622,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
     InternetAddressList *addresses;
     const char *subject;
     notmuch_status_t status;
+    _notmuch_message_crypto_t *msg_crypto;
 
     status = _notmuch_message_file_get_mime_message (message_file,
 						     &mime_message);
@@ -628,7 +645,14 @@ _notmuch_message_index_file (notmuch_message_t *message,
 
     status = _notmuch_message_index_user_headers (message, mime_message);
 
-    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message));
+    msg_crypto = _notmuch_message_crypto_new (NULL);
+    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message), msg_crypto);
+    if (msg_crypto && msg_crypto->payload_subject) {
+	_notmuch_message_gen_terms (message, "subject", msg_crypto->payload_subject);
+	_notmuch_message_update_subject (message, msg_crypto->payload_subject);
+    }
+
+    talloc_free (msg_crypto);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
diff --git a/lib/message.cc b/lib/message.cc
index dc4a96ad..9e1005a3 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1238,6 +1238,14 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
     message->modified = true;
 }
 
+void
+_notmuch_message_update_subject (notmuch_message_t *message,
+				 const char *subject)
+{
+    message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
+    message->modified = true;
+}
+
 /* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
  * must call _notmuch_message_sync. */
 void
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index e46df9a8..6fc5b366 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -325,6 +325,10 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *from,
 				    const char *subject);
 
+void
+_notmuch_message_update_subject (notmuch_message_t *message,
+				 const char *subject);
+
 void
 _notmuch_message_upgrade_last_mod (notmuch_message_t *message);
 
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index ff37f6bd..fee3b043 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -83,4 +83,20 @@ test_json_nodes <<<"$output" \
                 'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
                 'reply-subject:["reply-headers"]["Subject"]="Re: Subject Unavailable"'
 
+test_begin_subtest "protected subject is not indexed by default"
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" ''
+
+test_begin_subtest "reindex message with protected header"
+test_expect_success 'notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "protected subject is indexed when cleartext is indexed"
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" 'id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "indexed protected subject is visible in search"
+output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.org')
+test_json_nodes <<<"$output" \
+                'subject:[0]["subject"]="This is a protected header"'
+
 test_done
-- 
2.20.1

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

[David Bremner]

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

> +    status = _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);

If you're going to ignore the return value here (not sure if that's a
good idea?)  please explicitly cast to void rather than putting in
status to ignore.

[PATCH v3 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 <dkg@fifthhorseman.net>
---
 lib/index.cc                   | 46 +++++++++++++++++++++++++++-------
 lib/message.cc                 |  8 ++++++
 lib/notmuch-private.h          |  4 +++
 test/T356-protected-headers.sh | 16 ++++++++++++
 4 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index f216ae5d..2e0bacb1 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,12 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 	}
 	g_object_unref (decrypt_result);
     }
-    _index_mime_part (message, indexopts, clear);
+    status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
+    if (status)
+	_notmuch_database_log_append (notmuch,
+				      "failed to mark the potential cryptographic payload (%s).\n",
+				      notmuch_status_to_string (status));
+    _index_mime_part (message, indexopts, clear, msg_crypto);
     g_object_unref (clear);
 
     status = notmuch_message_add_property (message, "index.decryption", "success");
@@ -606,6 +626,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
     InternetAddressList *addresses;
     const char *subject;
     notmuch_status_t status;
+    _notmuch_message_crypto_t *msg_crypto;
 
     status = _notmuch_message_file_get_mime_message (message_file,
 						     &mime_message);
@@ -628,7 +649,14 @@ _notmuch_message_index_file (notmuch_message_t *message,
 
     status = _notmuch_message_index_user_headers (message, mime_message);
 
-    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message));
+    msg_crypto = _notmuch_message_crypto_new (NULL);
+    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message), msg_crypto);
+    if (msg_crypto && msg_crypto->payload_subject) {
+	_notmuch_message_gen_terms (message, "subject", msg_crypto->payload_subject);
+	_notmuch_message_update_subject (message, msg_crypto->payload_subject);
+    }
+
+    talloc_free (msg_crypto);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
diff --git a/lib/message.cc b/lib/message.cc
index dc4a96ad..9e1005a3 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1238,6 +1238,14 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
     message->modified = true;
 }
 
+void
+_notmuch_message_update_subject (notmuch_message_t *message,
+				 const char *subject)
+{
+    message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
+    message->modified = true;
+}
+
 /* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
  * must call _notmuch_message_sync. */
 void
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index e46df9a8..6fc5b366 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -325,6 +325,10 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *from,
 				    const char *subject);
 
+void
+_notmuch_message_update_subject (notmuch_message_t *message,
+				 const char *subject);
+
 void
 _notmuch_message_upgrade_last_mod (notmuch_message_t *message);
 
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 29b0c42a..714c847c 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -90,4 +90,20 @@ test_json_nodes <<<"$output" \
                 'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
                 'reply-subject:["reply-headers"]["Subject"]="Re: This is a protected header"'
 
+test_begin_subtest "protected subject is not indexed by default"
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" ''
+
+test_begin_subtest "reindex message with protected header"
+test_expect_success 'notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "protected subject is indexed when cleartext is indexed"
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" 'id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "indexed protected subject is visible in search"
+output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.org')
+test_json_nodes <<<"$output" \
+                'subject:[0]["subject"]="This is a protected header"'
+
 test_done
-- 
2.20.1

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

[Daniel Kahn Gillmor]

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

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

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

_notmuch_database_log vs _notmuch_database_log_append [was: Re: [PATCH v2 10/17] indexing: record protected subject when indexing cleartext]

[Daniel Kahn Gillmor]

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

On Mon 2019-05-27 07:24:41 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +    status = _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);
>
> If you're going to ignore the return value here (not sure if that's a
> good idea?)  please explicitly cast to void rather than putting in
> status to ignore.

Good catch, thanks.  I've logged the error with
_notmuch_database_log_append in v3 of this patch, i hope that makes
sense to you!

i note that _index_encrypted_mime_part() itself uses an odd mixture of
_notmuch_database_log_append and _notmuch_database_log, which maybe is a
sign that more cleanup is due there.

I confess i always get a bit confused about when to use one vs. the
other, though.  _log resets the database's status_string, while
_log_append just appends to it.

On IRC, you wrote "I'd say it makes sense to append for warnings", which
is a plausible rule of thumb, but seems like it might not map to the
intent of how we expect the status_string to be used -- is it for the
caller of the library?  or something else?

For example, should every call into the library reset the status string,
and then there would only be one _database_log() function?  (i don't
know whether that's feasible, or what it would mean for internal code
that already calls exported functions directly).

It'd probably be worthwhile for someone to do an audit of those uses and
come up with some normalized way of handling this that we can clearly
explain, because i think it's a bit unwieldy now.  I'm writing this
report with the intent of tagging this e-mail with notmuch::bug, in the
hopes that someone interested in doing maintenance work will take this
on as a future project :)

       --dkg

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

[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 <dkg@fifthhorseman.net>
---
 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 = _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);
 
     status = notmuch_message_add_property (message, "index.decryption", "success");
@@ -606,6 +622,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
     InternetAddressList *addresses;
     const char *subject;
     notmuch_status_t status;
+    _notmuch_message_crypto_t *msg_crypto;
 
     status = _notmuch_message_file_get_mime_message (message_file,
 						     &mime_message);
@@ -628,7 +645,14 @@ _notmuch_message_index_file (notmuch_message_t *message,
 
     status = _notmuch_message_index_user_headers (message, mime_message);
 
-    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message));
+    msg_crypto = _notmuch_message_crypto_new (NULL);
+    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message), msg_crypto);
+    if (msg_crypto && msg_crypto->payload_subject) {
+	_notmuch_message_gen_terms (message, "subject", msg_crypto->payload_subject);
+	_notmuch_message_update_subject (message, msg_crypto->payload_subject);
+    }
+
+    talloc_free (msg_crypto);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
diff --git a/lib/message.cc b/lib/message.cc
index dc4a96ad..9e1005a3 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1238,6 +1238,14 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
     message->modified = true;
 }
 
+void
+_notmuch_message_update_subject (notmuch_message_t *message,
+				 const char *subject)
+{
+    message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
+    message->modified = true;
+}
+
 /* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
  * must call _notmuch_message_sync. */
 void
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index e46df9a8..6fc5b366 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -325,6 +325,10 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *from,
 				    const char *subject);
 
+void
+_notmuch_message_update_subject (notmuch_message_t *message,
+				 const char *subject);
+
 void
 _notmuch_message_upgrade_last_mod (notmuch_message_t *message);
 
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index ff37f6bd..fee3b043 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -83,4 +83,20 @@ test_json_nodes <<<"$output" \
                 'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
                 'reply-subject:["reply-headers"]["Subject"]="Re: Subject Unavailable"'
 
+test_begin_subtest "protected subject is not indexed by default"
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" ''
+
+test_begin_subtest "reindex message with protected header"
+test_expect_success 'notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "protected subject is indexed when cleartext is indexed"
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" 'id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "indexed protected subject is visible in search"
+output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.org')
+test_json_nodes <<<"$output" \
+                'subject:[0]["subject"]="This is a protected header"'
+
 test_done
-- 
2.20.1

[PATCH v2 11/17] test: protected headers should work when both encrypted and signed.

[Daniel Kahn Gillmor]

Up to this point, we've tested protected headers on messages that have
either been encrypted or signed, but not both.

This adds a couple tests of signed+encrypted messages, one where the
subject line is masked (outside subject line is "Subject Unavailable")
and another where it is not (outside Subject: matches inner Subject:)

See the discussion at
https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html#protected-headers
for more details about the nuances between signed, stripped, and
stubbed headers.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh                | 16 +++++++++
 .../encrypted-signed-not-masked.eml           | 34 +++++++++++++++++++
 .../protected-headers/encrypted-signed.eml    | 34 +++++++++++++++++++
 3 files changed, 84 insertions(+)
 create mode 100644 test/corpora/protected-headers/encrypted-signed-not-masked.eml
 create mode 100644 test/corpora/protected-headers/encrypted-signed.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index fee3b043..0a8d4bfa 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -99,4 +99,20 @@ output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.or
 test_json_nodes <<<"$output" \
                 'subject:[0]["subject"]="This is a protected header"'
 
+test_begin_subtest "verify protected header is both signed and encrypted"
+output=$(notmuch show --decrypt=true --format=json id:encrypted-signed@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={
+                   "signed":{"status": [{"status": "good", "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "created": 1525812676}],
+                   "encrypted": true, "headers": ["Subject"]},"decrypted": {"status": "full", "header-mask": {"Subject": "Subject Unavailable"}}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Rhinoceros dinner"'
+
+test_begin_subtest "verify protected header is signed even when not masked"
+output=$(notmuch show --decrypt=true --format=json id:encrypted-signed-not-masked@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={
+                   "signed":{"status": [{"status": "good", "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "created": 1525812676}],
+                   "encrypted": true, "headers": ["Subject"]},"decrypted": {"status": "full"}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="Rhinoceros dinner"'
+
 test_done
diff --git a/test/corpora/protected-headers/encrypted-signed-not-masked.eml b/test/corpora/protected-headers/encrypted-signed-not-masked.eml
new file mode 100644
index 00000000..8dfd7c39
--- /dev/null
+++ b/test/corpora/protected-headers/encrypted-signed-not-masked.eml
@@ -0,0 +1,34 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Rhinoceros dinner
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <encrypted-signed-not-masked@crypto.notmuchmail.org>
+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-----
+
+hIwDxE023q1UqxYBBADAJ03D4w48sefkQsBWXUc1spTljROjVN+y5a2yCKtYMt3M
+wWMeQyem5hwLpLYRCfeIzXCrlBfpZffuOkA5okGGVEWFvJ5a1kZNZnH5Wg0ccBp7
+KBGnJY0gS/BlrKK2Sjmk9Z3ww7GAgDGPbc7mc3Csj9G38UvneBdrQgm6kZR3GNLA
+6AGLN3KJETruI3Js6++aG+7tSkJ8Vo4WCVUR7oQROwF601X0QF/XghCoJCrx8B/1
+cw6Yb2wQj2nv3gw1rqWVsPVpAKsMc1yHx/2Vsee/VPtt4f67fSAMuJF3EJ6JkcK7
+tM761v69GoJGgvsie45pb1N2l/GfVMuwWU0wZhEsF7eXxqPzoE/kIGX1XIqleLaw
+On2kPSM5RgqV6gLOcw4WaFPi0oMbDhltNs72SV9cV6ZhhuwEQRq+u/K76NKLwte2
+R1JutAiuPZVF0WanmmiN6RbIpWOB5XxQfWagfr4vcf/03TaLP4hJMnqUdFMk20HP
+eI8TMQxkfryZK2Z6VxEBVdXhK05VEdkolmc4j9U+76A96Gd5zbYPApirkebmZatS
+X3rKKAiBqwWrFXi/7LNDoCwhRRmqDuHXruh3vZEcz+xiPfJh0G31GJQgIpE15Sv6
+trf20u3CXAFjHg9zPpSFV7uAOsqv7bg+xtG9PgN4aLCiVbXHsT0z6PAz+6K+SiKw
+QW8ZOtLikj5HyLAz/TDcsIShFaM3QHk2qq9RY10kmxlQVrf9Oyh3Wmc=
+=om0O
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/corpora/protected-headers/encrypted-signed.eml b/test/corpora/protected-headers/encrypted-signed.eml
new file mode 100644
index 00000000..c97d8c3c
--- /dev/null
+++ b/test/corpora/protected-headers/encrypted-signed.eml
@@ -0,0 +1,34 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Subject Unavailable
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <encrypted-signed@crypto.notmuchmail.org>
+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-----
+
+hIwDxE023q1UqxYBBADAJ03D4w48sefkQsBWXUc1spTljROjVN+y5a2yCKtYMt3M
+wWMeQyem5hwLpLYRCfeIzXCrlBfpZffuOkA5okGGVEWFvJ5a1kZNZnH5Wg0ccBp7
+KBGnJY0gS/BlrKK2Sjmk9Z3ww7GAgDGPbc7mc3Csj9G38UvneBdrQgm6kZR3GNLA
+6AGLN3KJETruI3Js6++aG+7tSkJ8Vo4WCVUR7oQROwF601X0QF/XghCoJCrx8B/1
+cw6Yb2wQj2nv3gw1rqWVsPVpAKsMc1yHx/2Vsee/VPtt4f67fSAMuJF3EJ6JkcK7
+tM761v69GoJGgvsie45pb1N2l/GfVMuwWU0wZhEsF7eXxqPzoE/kIGX1XIqleLaw
+On2kPSM5RgqV6gLOcw4WaFPi0oMbDhltNs72SV9cV6ZhhuwEQRq+u/K76NKLwte2
+R1JutAiuPZVF0WanmmiN6RbIpWOB5XxQfWagfr4vcf/03TaLP4hJMnqUdFMk20HP
+eI8TMQxkfryZK2Z6VxEBVdXhK05VEdkolmc4j9U+76A96Gd5zbYPApirkebmZatS
+X3rKKAiBqwWrFXi/7LNDoCwhRRmqDuHXruh3vZEcz+xiPfJh0G31GJQgIpE15Sv6
+trf20u3CXAFjHg9zPpSFV7uAOsqv7bg+xtG9PgN4aLCiVbXHsT0z6PAz+6K+SiKw
+QW8ZOtLikj5HyLAz/TDcsIShFaM3QHk2qq9RY10kmxlQVrf9Oyh3Wmc=
+=om0O
+-----END PGP MESSAGE-----
+--=-=-=--
-- 
2.20.1

[PATCH v2 12/17] test: after reindexing, only legitimate protected subjects are searchable

[Daniel Kahn Gillmor]

This test scans for all the possible protected headers (including
bogus/broken ones) that are present in the protected-headers corpus,
trying to make sure that only the ones that are not broken or
malformed show up in a search after re-indexing.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 0a8d4bfa..0c562c18 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -115,4 +115,13 @@ test_json_nodes <<<"$output" \
                    "encrypted": true, "headers": ["Subject"]},"decrypted": {"status": "full"}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="Rhinoceros dinner"'
 
+test_begin_subtest "reindex everything, ensure headers are as expected"
+notmuch reindex --decrypt=true from:test_suite@notmuchmail.org
+output=$(notmuch search --output=messages 'subject:"protected header" or subject:"Rhinoceros" or subject:"draft-melnikov-smime-header-signing" or subject:"valid"' | sort)
+test_expect_equal "$output" 'id:encrypted-signed-not-masked@crypto.notmuchmail.org
+id:encrypted-signed@crypto.notmuchmail.org
+id:nested-rfc822-message@crypto.notmuchmail.org
+id:protected-header@crypto.notmuchmail.org
+id:subjectless-protected-header@crypto.notmuchmail.org'
+
 test_done
-- 
2.20.1

[PATCH v2 13/17] test: try indexing nested messages and protected headers

[Daniel Kahn Gillmor]

We want to make sure that internally-forwarded messages don't end up
"bubbling up" when they aren't actually the cryptographic payload.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh                |  6 ++++
 ...pted-message-with-forwarded-attachment.eml | 33 +++++++++++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 test/corpora/protected-headers/encrypted-message-with-forwarded-attachment.eml

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 0c562c18..cbed3781 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -99,6 +99,12 @@ output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.or
 test_json_nodes <<<"$output" \
                 'subject:[0]["subject"]="This is a protected header"'
 
+test_begin_subtest "verify correct protected header when submessage exists"
+output=$(notmuch show --decrypt=true --format=json id:encrypted-message-with-forwarded-attachment@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "header-mask": {"Subject": "Subject Unavailable"}}}' \
+                'subject:[0][0][0]["headers"]["Subject"]="This is the cryptographic envelope subject"'
+
 test_begin_subtest "verify protected header is both signed and encrypted"
 output=$(notmuch show --decrypt=true --format=json id:encrypted-signed@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
diff --git a/test/corpora/protected-headers/encrypted-message-with-forwarded-attachment.eml b/test/corpora/protected-headers/encrypted-message-with-forwarded-attachment.eml
new file mode 100644
index 00000000..eea66a94
--- /dev/null
+++ b/test/corpora/protected-headers/encrypted-message-with-forwarded-attachment.eml
@@ -0,0 +1,33 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <encrypted-message-with-forwarded-attachment@crypto.notmuchmail.org>
+Subject: Subject Unavailable
+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-----
+
+hIwDxE023q1UqxYBBAC9RgjF0vsqVqHMB8fauhazs2XoTMKkANrDS6ECANm0wcvO
+tU1huRepG8ezoow/OgZ0Yd9y/zw6w+Frrx1PhVEr01lQsUdRq7INq2FRia015Q6Q
+eOgSv9Q8wg4Vcy9XD1wI2Un71nDvbNwqx+hiR9m8vhiWfXH1MvxVQUWcUocUMtLA
+uAEB+fx5ag3Qr42VAgyymvNrHJKtuhdj7CvdT/a5oVbZV7ilflFlYms7Wq0jSex+
+Jrb+/CnNLow4LehrOpf+IfgPumo0nBbseB17rAM9vtjNy+tHEqPsB0YFIpVR9FOp
+zJITbWeFyGbOd5vMk9xbEFbw58JR8PPqsYJK41RleU2QoPEO69hoV0tXzjby5JQZ
+2G/SrH+m9tggi3rWxHx9XuNKJP4iK9wZnO4k5DFaUXq6PGCYkgDi/K1RuUcJjcv7
+ob6Yp/cTLxHMmIS9VNNjUnnoaD71ndzYsZoaI6MTMX7/4eu5roeE3887NU5af/wS
+ep6POG8WFJzKwc4dvAPd0NBVojdrftJkYKONsYL5KN8TY8SqUPxiXReGwg2evQqb
+aGEU02zdRGYtmNSneGl20dJ39cHoW7B66ek9OQkgilSHQq4adPleq07r3HSv87jk
+xNYoQ7xH2fahqbosW8N5uI9L2sdGVmTBNZgejiNyZoUn47tFEt4Uocg=
+=/ZB1
+-----END PGP MESSAGE-----
+--=-=-=--
-- 
2.20.1

[PATCH v2 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected

[Daniel Kahn Gillmor]

We initially test only notmuch-search; tests for other functionality
come in different patchsets later.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T358-emacs-protected-headers.sh | 36 ++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100755 test/T358-emacs-protected-headers.sh

diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh
new file mode 100755
index 00000000..56ac06ca
--- /dev/null
+++ b/test/T358-emacs-protected-headers.sh
@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+
+test_description="emacs interface"
+. $(dirname "$0")/test-lib.sh || exit 1
+
+# testing protected headers with emacs
+add_gnupg_home
+add_email_corpus protected-headers
+
+test_begin_subtest "notmuch-search should show not unindexed protected subject header in emacs"
+test_emacs '(notmuch-search "id:protected-header@crypto.notmuchmail.org")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPECTED
+  2000-01-01 [1/1]   test_suite@notmuchmail.org  Subject Unavailable (encrypted inbox unread)
+End of search results.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+# protected headers should behave differently after re-indexing
+test_begin_subtest 'defaulting to indexing cleartext'
+test_expect_success 'notmuch config set index.decrypt true'
+test_begin_subtest 'try reindexing protected header message'
+test_expect_success 'notmuch reindex id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "notmuch-search should show indexed protected subject header in emacs"
+test_emacs '(notmuch-search "id:protected-header@crypto.notmuchmail.org")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPECTED
+  2000-01-01 [1/1]   test_suite@notmuchmail.org  This is a protected header (encrypted inbox unread)
+End of search results.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.20.1

Re: [PATCH v2 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected

[Rollins, Jameson]

On Sun, May 26 2019, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh
> new file mode 100755
> index 00000000..56ac06ca
> --- /dev/null
> +++ b/test/T358-emacs-protected-headers.sh
> @@ -0,0 +1,36 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs interface"

Rather generic test description, but since the output includes the test
name, which gives more context, it's probably ok.

jamie.

Re: [PATCH v3 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected

[Daniel Kahn Gillmor]

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

On Mon 2019-05-27 17:35:44 -0400, Daniel Kahn Gillmor wrote:
> We initially test only notmuch-search; tests for other functionality
> come in different patchsets later.
>
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

sorry, this patch (v3 14/17) is a minor update to the protected header
series, correcting a weak test suite title that jrollins pointed out.

The v3 patch here was supposed to be in-thread, but i botched the
--in-reply-to= when sending.  hopefully this message will stitch the
threads together.

          --dkg

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

stitching threads (v3 14/17)

[Daniel Kahn Gillmor]

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

hm, it appears that notmuch-emacs sends duplicate References: headers
during reply when i add that manually to the headers field during
compose.

and then when notmuch indexes a message, it only indexes the first
References: header it finds.

These are curious things i find as i try to stitch the threads together.

      --dkg

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

Re: stitching threads (v3 14/17)

[David Bremner]

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

> hm, it appears that notmuch-emacs sends duplicate References: headers
> during reply when i add that manually to the headers field during
> compose.
>
> and then when notmuch indexes a message, it only indexes the first
> References: header it finds.
>
> These are curious things i find as i try to stitch the threads together.

I was unable to duplicate this with notmuch-emacs 0.34.2 / emacs 27.1
(debian). So either it is fixed, or I need some help to reproduce it.

d

[PATCH v2 15/17] test: emacs/show: ensure that protected headers appear as expected

[Daniel Kahn Gillmor]

This tests notmuch-show; headers appear appropriately based on the
setting of notmuch-crypto-process-mime.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T358-emacs-protected-headers.sh | 36 +++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh
index 56ac06ca..a631223e 100755
--- a/test/T358-emacs-protected-headers.sh
+++ b/test/T358-emacs-protected-headers.sh
@@ -17,6 +17,40 @@ End of search results.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch-show should not show unindexed protected subject header in emacs when nm-c-process-mime is nil"
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+             (notmuch-show "id:protected-header@crypto.notmuchmail.org")
+             (test-output))'
+cat <<EOF >EXPECTED
+test_suite@notmuchmail.org (2000-01-01) (encrypted inbox)
+Subject: Subject Unavailable
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+
+[ multipart/encrypted ]
+[ Unknown encryption status ]
+[ application/pgp-encrypted ]
+[ application/octet-stream ]
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "notmuch-show should show protected subject header in emacs"
+test_emacs '(notmuch-show "id:protected-header@crypto.notmuchmail.org")
+	    (test-output)'
+cat <<EOF >EXPECTED
+test_suite@notmuchmail.org (2000-01-01) (encrypted inbox)
+Subject: This is a protected header
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+
+[ multipart/encrypted ]
+[ Decryption successful ]
+[ application/pgp-encrypted ]
+[ text/plain ]
+This is the sekrit message
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 # protected headers should behave differently after re-indexing
 test_begin_subtest 'defaulting to indexing cleartext'
 test_expect_success 'notmuch config set index.decrypt true'
@@ -28,7 +62,7 @@ test_emacs '(notmuch-search "id:protected-header@crypto.notmuchmail.org")
 	    (notmuch-test-wait)
 	    (test-output)'
 cat <<EOF >EXPECTED
-  2000-01-01 [1/1]   test_suite@notmuchmail.org  This is a protected header (encrypted inbox unread)
+  2000-01-01 [1/1]   test_suite@notmuchmail.org  This is a protected header (encrypted inbox)
 End of search results.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
-- 
2.20.1

[PATCH v2 16/17] test: reply (in cli and emacs) should protect indexed sensitive headers

[Daniel Kahn Gillmor]

These tests are currently broken!  When a protected subject is indexed
in the clear, it leaks in the reply headers :(

For emacs, we set up separate tests for when the protected header is
indexed in the clear and when it is unindexed.  neither case should
leak, but the former wasn't tested yet.

We will fix the two broken tests in a subsequent patch.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T356-protected-headers.sh       |  7 +++++
 test/T358-emacs-protected-headers.sh | 45 ++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index cbed3781..746c4760 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -99,6 +99,13 @@ output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.or
 test_json_nodes <<<"$output" \
                 'subject:[0]["subject"]="This is a protected header"'
 
+test_begin_subtest "indexed protected subject is not visible in reply header"
+test_subtest_known_broken
+output=$(notmuch reply --format=json 'id:protected-header@crypto.notmuchmail.org')
+test_json_nodes <<<"$output" \
+                'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
+                'reply-subject:["reply-headers"]["Subject"]="Re: Subject Unavailable"'
+
 test_begin_subtest "verify correct protected header when submessage exists"
 output=$(notmuch show --decrypt=true --format=json id:encrypted-message-with-forwarded-attachment@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh
index a631223e..765511d4 100755
--- a/test/T358-emacs-protected-headers.sh
+++ b/test/T358-emacs-protected-headers.sh
@@ -51,6 +51,29 @@ This is the sekrit message
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+# notmuch-emacs still leaks the subject line; as long as it leaks the
+# subject line, it should emit the external subject, not the protected
+# subject, even if it knows what the true subject is:
+test_begin_subtest "Reply within emacs to a message with protected headers, not leaking subject"
+test_emacs "(let ((message-hidden-headers '()))
+	    (notmuch-show \"id:protected-header@crypto.notmuchmail.org\")
+	    (notmuch-show-reply)
+	    (test-output))"
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: test_suite@notmuchmail.org
+Subject: Re: Subject Unavailable
+In-Reply-To: <protected-header@crypto.notmuchmail.org>
+Fcc: ${MAIL_DIR}/sent
+References: <protected-header@crypto.notmuchmail.org>
+--text follows this line--
+<#secure method=pgpmime mode=signencrypt>
+test_suite@notmuchmail.org writes:
+
+> This is the sekrit message
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 # protected headers should behave differently after re-indexing
 test_begin_subtest 'defaulting to indexing cleartext'
 test_expect_success 'notmuch config set index.decrypt true'
@@ -67,4 +90,26 @@ End of search results.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+# notmuch-emacs still leaks the subject line:
+test_begin_subtest "don't leak protected subject during reply, even if indexed"
+test_subtest_known_broken
+test_emacs "(let ((message-hidden-headers '()))
+	    (notmuch-show \"id:protected-header@crypto.notmuchmail.org\")
+	    (notmuch-show-reply)
+	    (test-output))"
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: test_suite@notmuchmail.org
+Subject: Re: Subject Unavailable
+In-Reply-To: <protected-header@crypto.notmuchmail.org>
+Fcc: ${MAIL_DIR}/sent
+References: <protected-header@crypto.notmuchmail.org>
+--text follows this line--
+<#secure method=pgpmime mode=signencrypt>
+test_suite@notmuchmail.org writes:
+
+> This is the sekrit message
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.20.1

[PATCH v2 17/17] cli/reply: pull proposed subject line from the message, not the index

[Daniel Kahn Gillmor]

Protected subject lines were being emitted in reply when the cleartext
of documents was indexed.  create_reply_message() was pulling the
subject line from the index, rather than pulling it from the
GMimeMessage object that it already has on hand.

This one-line fix to notmuch-reply.c solves that problem, and doesn't
cause any additional tests to fail.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 notmuch-reply.c                      | 2 +-
 test/T356-protected-headers.sh       | 1 -
 test/T358-emacs-protected-headers.sh | 1 -
 3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2689b247..46bab434 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -591,7 +591,7 @@ create_reply_message(void *ctx,
 				 from_addr);
     g_mime_object_set_header (GMIME_OBJECT (reply), "From", from_addr, NULL);
 
-    subject = notmuch_message_get_header (message, "subject");
+    subject = g_mime_message_get_subject (mime_message);
     if (subject) {
 	if (strncasecmp (subject, "Re:", 3))
 	    subject = talloc_asprintf (ctx, "Re: %s", subject);
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 746c4760..4af018f3 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -100,7 +100,6 @@ test_json_nodes <<<"$output" \
                 'subject:[0]["subject"]="This is a protected header"'
 
 test_begin_subtest "indexed protected subject is not visible in reply header"
-test_subtest_known_broken
 output=$(notmuch reply --format=json 'id:protected-header@crypto.notmuchmail.org')
 test_json_nodes <<<"$output" \
                 'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh
index 765511d4..c195d5c2 100755
--- a/test/T358-emacs-protected-headers.sh
+++ b/test/T358-emacs-protected-headers.sh
@@ -92,7 +92,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 # notmuch-emacs still leaks the subject line:
 test_begin_subtest "don't leak protected subject during reply, even if indexed"
-test_subtest_known_broken
 test_emacs "(let ((message-hidden-headers '()))
 	    (notmuch-show \"id:protected-header@crypto.notmuchmail.org\")
 	    (notmuch-show-reply)
-- 
2.20.1

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

[Rollins, Jameson]

On Sun, May 26 2019, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> Way back in id:20180511055544.13676-1-dkg@fifthhorseman.net, i
> proposed support for protected headers (in particular, for being able
> to read and search for subject lines of encrypted messages which
> protect the Subject).  Although that series was reviewed by Bremner, i
> never managed to get it in shape for merging.
>
> This is a revision of that series, applied against the current master,
> having taken into account those reviews and the current state of the
> notmuch codebase.  I'm hoping that we can get it into 0.29 before the
> feature freeze.

This series looks good to me.  All tests pass for me, and it behaves as
expected on my existing emails that include protected headers.  Very
good test coverage.  I don't see any issues with the code on a fairly
cursory code review.

Would be great to get this in before the 0.29 release.  Looking forward
to future autocrypt support based on this...

jamie.

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

[Daniel Kahn Gillmor]

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

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 XXXXX for however your
own installation identifies this series):

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

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

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

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

[David Bremner]

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

> Hi all--
>
> Way back in id:20180511055544.13676-1-dkg@fifthhorseman.net, i
> proposed support for protected headers (in particular, for being able
> to read and search for subject lines of encrypted messages which
> protect the Subject).  Although that series was reviewed by Bremner, i
> never managed to get it in shape for merging.
>
> This is a revision of that series, applied against the current master,
> having taken into account those reviews and the current state of the
> notmuch codebase.  I'm hoping that we can get it into 0.29 before the
> feature freeze.

It's in. 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.

d

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

[Daniel Kahn Gillmor]

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

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

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

[PATCH v3 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected

[Daniel Kahn Gillmor]

We initially test only notmuch-search; tests for other functionality
come in different patchsets later.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T358-emacs-protected-headers.sh | 36 ++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100755 test/T358-emacs-protected-headers.sh

diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh
new file mode 100755
index 00000000..5e97918f
--- /dev/null
+++ b/test/T358-emacs-protected-headers.sh
@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+
+test_description="protected headers in emacs interface"
+. $(dirname "$0")/test-lib.sh || exit 1
+
+# testing protected headers with emacs
+add_gnupg_home
+add_email_corpus protected-headers
+
+test_begin_subtest "notmuch-search should show not unindexed protected subject header in emacs"
+test_emacs '(notmuch-search "id:protected-header@crypto.notmuchmail.org")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPECTED
+  2000-01-01 [1/1]   test_suite@notmuchmail.org  Subject Unavailable (encrypted inbox unread)
+End of search results.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+# protected headers should behave differently after re-indexing
+test_begin_subtest 'defaulting to indexing cleartext'
+test_expect_success 'notmuch config set index.decrypt true'
+test_begin_subtest 'try reindexing protected header message'
+test_expect_success 'notmuch reindex id:protected-header@crypto.notmuchmail.org'
+
+test_begin_subtest "notmuch-search should show indexed protected subject header in emacs"
+test_emacs '(notmuch-search "id:protected-header@crypto.notmuchmail.org")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPECTED
+  2000-01-01 [1/1]   test_suite@notmuchmail.org  This is a protected header (encrypted inbox unread)
+End of search results.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.20.1