Protected headers in notmuch

Protected headers in notmuch

[Daniel Kahn Gillmor]

Traditionally, encrypted and signed e-mail covers only the body of the
message.  New standards are emerging that are capable of protecting
the headers as well.  In particular, Enigmail and an upcoming version
of K-9 mail both use the "Memory Hole" approach to encrypt the
Subject: header when sending encrypted mail.  It is awkward to receive
encrypted messages from those clients with notmuch, because all
notmuch sees is "Subject: Encrypted Message"

This series solves that problem specifically: it enables viewing (and
indexing and searching, if desired) of the cleartext of the encrypted
Subject:.  It also lays sensible groundwork for handling other
protected headers in the future.

For a discussion of protected headers and the various challenges and
opportunities they present, see my writeup here:

    https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html

What this series does *not* do (yet) is emit any protected headers
from notmuch itself.  I think the series can be applied without that,
because just consuming the protected headers and being able to work
with them is a win on its own terms.  The series is also careful not
to accidentally leak cleartext headers (e.g. in reply), so it should
be safe to adopt even if we don't immediately become capable of
emitting protected headers.

If we can land this series, i think the next steps along this
direction include:

 * emitting a protected Subject: line when sending mail via
   notmuch-emacs

 * restructuring messages that had protected headers so that any weird
   internal structure isn't clumsily visible (either the
   "force-display" part of a Memory Hole-structured message, or the
   wrapped message/rfc822 part encouraged by Melnikov's draft could be
   "skipped")

 * dealing with other protected headers

[PATCH 01/20] 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 | 112 +++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh         |  22 ++++++++
 2 files changed, 134 insertions(+)
 create mode 100644 test/json_check_nodes.py

diff --git a/test/json_check_nodes.py b/test/json_check_nodes.py
new file mode 100644
index 00000000..5e386aec
--- /dev/null
+++ b/test/json_check_nodes.py
@@ -0,0 +1,112 @@
+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: {} 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 there 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 $
+
+""".format(sys.argv[0]))
+
+
+# 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['address'])
+    try:
+        val = eval(e)
+    except SyntaxError:
+        fail = True
+        print("{}: syntax error on evaluation of object: {}".format(
+            expr['label'], e))
+        continue
+    except:
+        fail = True
+        print("{}: object not found: data{}".format(
+            expr['label'], expr['address']))
+        continue
+
+    if expr['type'] == '=':
+        try:
+            obj_val = json.loads(expr['val'])
+        except:
+            fail = True
+            print("{}: error evaluating value: {}".format(
+                expr['label'], expr['address']))
+            continue
+        if val != obj_val:
+            fail = True
+            print("{}: value not equal: data{} = {} != {}".format(
+                expr['label'], expr['address'], repr(val), repr(obj_val)))
+
+    elif expr['type'] == '!':
+        if not isinstance(val, (dict, list)):
+            fail = True
+            print("{}: not a dict or a list: data{}".format(
+                expr['label'], expr['address']))
+            continue
+        try:
+            idx = json.loads(expr['val'])
+            if idx in val:
+                fail = True
+                print("{}: {} contains key: {}[{}] = {}".format(
+                    expr['label'], type(val), e, expr['val'], val[idx]))
+        except SyntaxError:
+            fail = True
+            print("{}: syntax error on evaluation of value: {}".format(
+                expr['label'], expr['val']))
+            continue
+
+
+    elif expr['type'] is None:
+        print("{}: {}".format(expr['label'], val))
+
+
+if fail:
+    sys.exit(1)
+sys.exit(0)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 7e064021..18f34e85 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -497,6 +497,28 @@ test_sort_json () {
         "import sys, json; json.dump(sorted(json.load(sys.stdin)),sys.stdout)"
 }
 
+# test for json objects
+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.17.0

Re: [PATCH 01/20] test: new test framework to compare json parts

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
> +
> +if len(sys.argv) < 2:
> +    sys.exit("""usage: {} EXPR [EXPR]
> +

the useage message doesn't seem to work? I get

╭─ zancas:software/upstream/notmuch/test 
╰─ (git)-[master]-% python3 json_check_nodes.py          
Traceback (most recent call last):
  File "json_check_nodes.py", line 42, in <module>
    """.format(sys.argv[0]))
KeyError: '"c"'

I guess this is not tested with python2?

╭─ zancas:software/upstream/notmuch/test 
╰─ (git)-[master]-%  echo '["a", "b", {"c": 1}]' | python2 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"' 
Traceback (most recent call last):
  File "json_check_nodes.py", line 60, in <module>
    e = 'data{}'.format(expr['address'])
TypeError: '_sre.SRE_Match' object has no attribute '__getitem__'

The test suite currently supports python2 and python3 (or at least it's
supposed to).

d

Re: [PATCH 01/20] test: new test framework to compare json parts

[Daniel Kahn Gillmor]

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

On Tue 2018-06-05 22:06:07 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>> +
>> +if len(sys.argv) < 2:
>> +    sys.exit("""usage: {} EXPR [EXPR]
>> +
>
> the useage message doesn't seem to work? I get
>
> ╭─ zancas:software/upstream/notmuch/test 
> ╰─ (git)-[master]-% python3 json_check_nodes.py          
> Traceback (most recent call last):
>   File "json_check_nodes.py", line 42, in <module>
>     """.format(sys.argv[0]))
> KeyError: '"c"'
>
> I guess this is not tested with python2?
>
> ╭─ zancas:software/upstream/notmuch/test 
> ╰─ (git)-[master]-%  echo '["a", "b", {"c": 1}]' | python2 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"' 
> Traceback (most recent call last):
>   File "json_check_nodes.py", line 60, in <module>
>     e = 'data{}'.format(expr['address'])
> TypeError: '_sre.SRE_Match' object has no attribute '__getitem__'
>
> The test suite currently supports python2 and python3 (or at least it's
> supposed to).

thanks for noticing these incompatibilities with python2.  i've
corrected them in my working tree, and will include the fixes in the
next revision of this series.

   --dkg

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

Re: [PATCH 01/20] test: new test framework to compare json parts

[David Bremner]

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

> On Tue 2018-06-05 22:06:07 -0300, David Bremner wrote:
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>> +
>>> +if len(sys.argv) < 2:
>>> +    sys.exit("""usage: {} EXPR [EXPR]
>>> +
>>
>> the useage message doesn't seem to work? I get
>>
>> ╭─ zancas:software/upstream/notmuch/test 
>> ╰─ (git)-[master]-% python3 json_check_nodes.py          
>> Traceback (most recent call last):
>>   File "json_check_nodes.py", line 42, in <module>
>>     """.format(sys.argv[0]))
>> KeyError: '"c"'
>>
>> I guess this is not tested with python2?
>>
>> ╭─ zancas:software/upstream/notmuch/test 
>> ╰─ (git)-[master]-%  echo '["a", "b", {"c": 1}]' | python2 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"' 
>> Traceback (most recent call last):
>>   File "json_check_nodes.py", line 60, in <module>
>>     e = 'data{}'.format(expr['address'])
>> TypeError: '_sre.SRE_Match' object has no attribute '__getitem__'
>>
>> The test suite currently supports python2 and python3 (or at least it's
>> supposed to).
>
> thanks for noticing these incompatibilities with python2.  i've
> corrected them in my working tree, and will include the fixes in the
> next revision of this series.

Just to be clear, the first problem is (was?) also present with python3

d

Re: [PATCH 01/20] test: new test framework to compare json parts

[Daniel Kahn Gillmor]

On Wed 2018-06-06 13:21:13 -0300, David Bremner wrote:
> Just to be clear, the first problem is (was?) also present with python3

yep, got it :)

     --dkg

Re: [PATCH 01/20] test: new test framework to compare json parts

[Tomi Ollila]

On Wed, Jun 06 2018, Daniel Kahn Gillmor wrote:

> On Tue 2018-06-05 22:06:07 -0300, David Bremner wrote:
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>> +
>>> +if len(sys.argv) < 2:
>>> +    sys.exit("""usage: {} EXPR [EXPR]
>>> +
>>
>> the useage message doesn't seem to work? I get
>>
>> ╭─ zancas:software/upstream/notmuch/test 
>> ╰─ (git)-[master]-% python3 json_check_nodes.py          
>> Traceback (most recent call last):
>>   File "json_check_nodes.py", line 42, in <module>
>>     """.format(sys.argv[0]))
>> KeyError: '"c"'
>>
>> I guess this is not tested with python2?
>>
>> ╭─ zancas:software/upstream/notmuch/test 
>> ╰─ (git)-[master]-%  echo '["a", "b", {"c": 1}]' | python2 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"' 
>> Traceback (most recent call last):
>>   File "json_check_nodes.py", line 60, in <module>
>>     e = 'data{}'.format(expr['address'])
>> TypeError: '_sre.SRE_Match' object has no attribute '__getitem__'
>>
>> The test suite currently supports python2 and python3 (or at least it's
>> supposed to).
>
> thanks for noticing these incompatibilities with python2.  i've
> corrected them in my working tree, and will include the fixes in the
> next revision of this series.

If it is going to be python2 and python3 compatible, you could
add hashbang #!/usr/bin/env python 
and make the file executable.

From tests it still can be run w/ $NOTMUCH_PYTHON but it could
also work standalone should anyone(tm) want to do so...

Tomi

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

[PATCH 02/20] crypto: Avoid pretending to verify signatures on unsigned encrypted mail

[Daniel Kahn Gillmor]

Unsigned encrypted mail shows up with a weird empty signature list.
If we successfully decrypted and there was no signature in it, we
should just not show a sigstatus at all.

The documentation for g_mime_decrypt_result_get_signatures says:

    a GMimeSignatureList or NULL if the stream was not signed.
---
 mime-node.c         | 2 +-
 test/T350-crypto.sh | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 11df082b..74f40417 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -216,12 +216,12 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
     }
 
     node->decrypt_success = true;
-    node->verify_attempted = true;
 
     if (decrypt_result) {
 	/* This may be NULL if the part is not signed. */
 	node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
 	if (node->sig_list) {
+	    node->verify_attempted = true;
 	    g_object_ref (node->sig_list);
 	    set_signature_list_destructor (node);
 	}
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index a776ec35..b5067346 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -271,7 +271,6 @@ expected='[[[{"id": "XXXXX",
  "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "encstatus": [{"status": "good"}],
- "sigstatus": [],
  "content-type": "multipart/encrypted",
  "content": [{"id": 2,
  "content-type": "application/pgp-encrypted",
-- 
2.17.0

[PATCH 03/20] cli/show: pass the siglist directly to the sigstatus sprinter

[Daniel Kahn Gillmor]

This makes it easier to reuse format_part_sigstatus_sprinter() when we
have other places that we want to display a signature status.
---
 notmuch-show.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 9871159d..f0be8060 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -403,13 +403,11 @@ format_signature_errors (sprinter_t *sp, GMimeSignature *signature)
 
 /* Signature status sprinter (GMime 2.6) */
 static void
-format_part_sigstatus_sprinter (sprinter_t *sp, mime_node_t *node)
+format_part_sigstatus_sprinter (sprinter_t *sp, GMimeSignatureList *siglist)
 {
     /* Any changes to the JSON or S-Expression format should be
      * reflected in the file devel/schemata. */
 
-    GMimeSignatureList *siglist = node->sig_list;
-
     sp->begin_list (sp);
 
     if (!siglist) {
@@ -652,7 +650,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 
     if (node->verify_attempted) {
 	sp->map_key (sp, "sigstatus");
-	format_part_sigstatus_sprinter (sp, node);
+	format_part_sigstatus_sprinter (sp, node->sig_list);
     }
 
     sp->map_key (sp, "content-type");
-- 
2.17.0

[PATCH 04/20] util/crypto: _notmuch_message_crypto: tracks message-wide crypto state

[Daniel Kahn Gillmor]

E-mail encryption and signatures reported by notmuch are at the MIME
part level.  This makes sense in the dirty details, but for users we
need to have a per-message conception of the cryptographic state of
the e-mail.  (see
https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html for more
discussion of why this is important).

The object created in this patch is a useful for tracking the
cryptographic state of the underlying message as a whole, based on a
depth-first search of the message's MIME structure.
---
 util/crypto.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++
 util/crypto.h | 55 ++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+)

diff --git a/util/crypto.c b/util/crypto.c
index 9d3b6dad..b040cc56 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -219,3 +219,90 @@ _notmuch_crypto_decrypt (bool *attempted,
 #endif
     return ret;
 }
+
+
+_notmuch_message_crypto_t *
+_notmuch_message_crypto_new (void *ctx)
+{
+    return talloc_zero (ctx, _notmuch_message_crypto_t);
+}
+
+
+notmuch_status_t
+_notmuch_message_crypto_set_sig_list (_notmuch_message_crypto_t *msg_crypto, GMimeSignatureList *sigs)
+{
+    if (!msg_crypto)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    /* Signatures that arrive after a payload part during DFS are not
+     * part of the cryptographic envelope: */
+    if (msg_crypto->payload_encountered)
+	return NOTMUCH_STATUS_SUCCESS;
+
+    if (msg_crypto->sig_list)
+	g_object_unref (msg_crypto->sig_list);
+
+    msg_crypto->sig_list = sigs;
+    if (sigs)
+	g_object_ref (sigs);
+
+    if (msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL)
+	msg_crypto->signature_encrypted = true;
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+
+notmuch_status_t
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum)
+{
+    if (!msg_crypto || !payload)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    /* only fire on the first payload part encountered */
+    if (msg_crypto->payload_encountered)
+	return NOTMUCH_STATUS_SUCCESS;
+
+    /* the first child of multipart/encrypted that matches the
+     * encryption protocol should be "control information" metadata,
+     * not payload.  So we skip it. (see
+     * https://tools.ietf.org/html/rfc1847#page-8) */
+    if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum == GMIME_MULTIPART_ENCRYPTED_VERSION) {
+	const char *enc_type = g_mime_object_get_content_type_parameter (parent, "protocol");
+	GMimeContentType *ct = g_mime_object_get_content_type (payload);
+	if (ct) {
+	    const char *part_type = g_mime_content_type_get_mime_type (ct);
+	    if (part_type && strcmp (part_type, enc_type) == 0)
+		return NOTMUCH_STATUS_SUCCESS;
+	}
+    }
+
+    msg_crypto->payload_encountered = true;
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+
+notmuch_status_t
+_notmuch_message_crypto_successful_decryption (_notmuch_message_crypto_t *msg_crypto)
+{
+    if (!msg_crypto)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    if (!msg_crypto->payload_encountered)
+	msg_crypto->decryption_status = NOTMUCH_MESSAGE_DECRYPTED_FULL;
+    else if (msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_NONE)
+	msg_crypto->decryption_status = NOTMUCH_MESSAGE_DECRYPTED_PARTIAL;
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+
+void
+_notmuch_message_crypto_cleanup (_notmuch_message_crypto_t *msg_crypto)
+{
+    if (!msg_crypto)
+	return;
+    if (msg_crypto->sig_list)
+	g_object_unref (msg_crypto->sig_list);
+}
diff --git a/util/crypto.h b/util/crypto.h
index c384601c..02c8793a 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -34,4 +34,59 @@ _notmuch_crypto_get_gmime_ctx_for_protocol (_notmuch_crypto_t *crypto,
 void
 _notmuch_crypto_cleanup (_notmuch_crypto_t *crypto);
 
+
+
+/* The user probably wants to know if the entire message was in the
+ * clear.  When replying, the MUA probably wants to know whether there
+ * was any part decrypted in the message.  And when displaying to the
+ * user, we probably only want to display "encrypted message" if the
+ * entire message was covered by encryption. */
+typedef enum {
+    NOTMUCH_MESSAGE_DECRYPTED_NONE = 0,
+    NOTMUCH_MESSAGE_DECRYPTED_PARTIAL,
+    NOTMUCH_MESSAGE_DECRYPTED_FULL,
+} _notmuch_message_decryption_status_t;
+
+/* description of the cryptographic state of a given message overall;
+ * for use by simple user agents.
+ */
+typedef struct _notmuch_message_crypto {
+    /* encryption status: partial, full, none */
+    _notmuch_message_decryption_status_t decryption_status;
+    /* FIXME: can we show what key(s) a fully-encrypted message was
+     * encrypted to? This data is not necessarily cryptographically
+     * reliable; even when we decrypt, we might not know which public
+     * key was used (e.g. if we're using a session key). */
+
+    /* signature status of the whole message (either the whole message
+     * is signed, or it is not) -- this means that partially-signed
+     * messages will get no signature status. */
+    GMimeSignatureList * sig_list;
+    /* if part of the message was signed, and the MUA is clever, it
+     * can determine on its own exactly which part and try to make
+     * more sense of it. */
+
+    /* mark this flag once we encounter a payload (i.e. something that
+     * is not part of the cryptographic envelope) */
+    bool payload_encountered;
+
+    /* if both signed and encrypted, was the signature encrypted? */
+    bool signature_encrypted;
+} _notmuch_message_crypto_t;
+
+_notmuch_message_crypto_t *
+_notmuch_message_crypto_new (void *ctx);
+
+void
+_notmuch_message_crypto_cleanup (_notmuch_message_crypto_t *msg_crypto);
+
+notmuch_status_t
+_notmuch_message_crypto_set_sig_list (_notmuch_message_crypto_t *msg_crypto, GMimeSignatureList *sigs);
+
+notmuch_status_t
+_notmuch_message_crypto_successful_decryption (_notmuch_message_crypto_t *msg_crypto);
+
+notmuch_status_t
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum);
+
 #endif
-- 
2.17.0

Re: [PATCH 04/20] util/crypto: _notmuch_message_crypto: tracks message-wide crypto state

[David Bremner]

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

> +notmuch_status_t
> +_notmuch_message_crypto_set_sig_list (_notmuch_message_crypto_t *msg_crypto, GMimeSignatureList *sigs)
> +{

It's a bit confusing that nothing in this API/patch seems to use the
sig_list stored by this function

> +void
> +_notmuch_message_crypto_cleanup (_notmuch_message_crypto_t *msg_crypto)
> +{
> +    if (!msg_crypto)
> +	return;
> +    if (msg_crypto->sig_list)
> +	g_object_unref (msg_crypto->sig_list);
> +}

It _looks_ like you're planning on manually calling
_notmuch_message_crypto_cleanup.  In order to allow for hierarchical
de-allocation (i.e. non-explicit de-allocation, we need to call
talloc_set_destructor. There's a few examples in the the existing code.

Re: [PATCH 04/20] util/crypto: _notmuch_message_crypto: tracks message-wide crypto state

[Daniel Kahn Gillmor]

On Fri 2018-06-15 07:16:05 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +notmuch_status_t
>> +_notmuch_message_crypto_set_sig_list (_notmuch_message_crypto_t *msg_crypto, GMimeSignatureList *sigs)
>> +{
>
> It's a bit confusing that nothing in this API/patch seems to use the
> sig_list stored by this function

i've updated the commit message to explain that we will use it later :)

>> +void
>> +_notmuch_message_crypto_cleanup (_notmuch_message_crypto_t *msg_crypto)
>> +{
>> +    if (!msg_crypto)
>> +	return;
>> +    if (msg_crypto->sig_list)
>> +	g_object_unref (msg_crypto->sig_list);
>> +}
>
> It _looks_ like you're planning on manually calling
> _notmuch_message_crypto_cleanup.  In order to allow for hierarchical
> de-allocation (i.e. non-explicit de-allocation, we need to call
> talloc_set_destructor. There's a few examples in the the existing code.

ah, good call.  in the revised version, that should be taken care of.
hopefully i even did it right :) thanks for the heads-up!

       --dkg

[PATCH 05/20] cli: expose message-wide crypto status from mime-node

[Daniel Kahn Gillmor]

The mime node context (a per-message context) gains a cryptographic
status object, and the mime_node_t object itself can return a view on
that status to an interested party.

The status is not yet populated, and for now we can keep that view
read-only, so that it can only be populated/modified during MIME tree
traversal.
---
 mime-node.c      | 7 +++++++
 notmuch-client.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/mime-node.c b/mime-node.c
index 74f40417..cbff95d1 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -31,6 +31,7 @@ typedef struct mime_node_context {
     GMimeStream *stream;
     GMimeParser *parser;
     GMimeMessage *mime_message;
+    _notmuch_message_crypto_t *msg_crypto;
 
     /* Context provided by the caller. */
     _notmuch_crypto_t *crypto;
@@ -54,6 +55,12 @@ _mime_node_context_free (mime_node_context_t *res)
     return 0;
 }
 
+const _notmuch_message_crypto_t*
+mime_node_get_message_crypto_status (mime_node_t *node)
+{
+    return node->ctx->msg_crypto;
+}
+
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
 		_notmuch_crypto_t *crypto, mime_node_t **root_out)
diff --git a/notmuch-client.h b/notmuch-client.h
index 0985a7b0..dfc7c047 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -448,6 +448,9 @@ mime_node_child (mime_node_t *parent, int child);
 mime_node_t *
 mime_node_seek_dfs (mime_node_t *node, int n);
 
+const _notmuch_message_crypto_t*
+mime_node_get_message_crypto_status (mime_node_t *node);
+
 typedef enum dump_formats {
     DUMP_FORMAT_AUTO,
     DUMP_FORMAT_BATCH_TAG,
-- 
2.17.0

[PATCH 06/20] mime-node: track whole-message crypto state while walking the tree

[Daniel Kahn Gillmor]

Deliberately populate the message's cryptographic status while walking
the MIME tree from the CLI.
---
 mime-node.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index cbff95d1..6ecd121d 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -49,6 +49,9 @@ _mime_node_context_free (mime_node_context_t *res)
     if (res->stream)
 	g_object_unref (res->stream);
 
+    if (res->msg_crypto)
+	_notmuch_message_crypto_cleanup (res->msg_crypto);
+
     if (res->file)
 	fclose (res->file);
 
@@ -135,6 +138,8 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 	goto DONE;
     }
 
+    mctx->msg_crypto = _notmuch_message_crypto_new (mctx);
+
     mctx->crypto = crypto;
 
     /* Create the root node */
@@ -181,6 +186,7 @@ node_verify (mime_node_t *node, GMimeObject *part,
 	     g_mime_3_unused(GMimeCryptoContext *cryptoctx))
 {
     GError *err = NULL;
+    notmuch_status_t status;
 
     node->verify_attempted = true;
     node->sig_list = g_mime_multipart_signed_verify
@@ -194,6 +200,10 @@ node_verify (mime_node_t *node, GMimeObject *part,
 
     if (err)
 	g_error_free (err);
+
+    status = _notmuch_message_crypto_set_sig_list(node->ctx->msg_crypto, node->sig_list);
+    if (status) /* this is a warning, not an error */
+	fprintf (stderr, "Warning: failed to note signature status: %s.\n", notmuch_status_to_string (status));
 }
 
 /* Decrypt and optionally verify an encrypted mime node (GMime 2.6) */
@@ -203,6 +213,7 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 {
     GError *err = NULL;
     GMimeDecryptResult *decrypt_result = NULL;
+    notmuch_status_t status;
     GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
 
     if (! node->decrypted_child) {
@@ -223,6 +234,9 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
     }
 
     node->decrypt_success = true;
+    status = _notmuch_message_crypto_successful_decryption (node->ctx->msg_crypto);
+    if (status) /* this is a warning, not an error */
+	fprintf (stderr, "Warning: failed to note decryption status: %s.\n", notmuch_status_to_string (status));
 
     if (decrypt_result) {
 	/* This may be NULL if the part is not signed. */
@@ -231,6 +245,9 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 	    node->verify_attempted = true;
 	    g_object_ref (node->sig_list);
 	    set_signature_list_destructor (node);
+	    status = _notmuch_message_crypto_set_sig_list(node->ctx->msg_crypto, node->sig_list);
+	    if (status) /* this is a warning, not an error */
+		fprintf (stderr, "Warning: failed to note signature status: %s.\n", notmuch_status_to_string (status));
 	}
 	g_object_unref (decrypt_result);
     }
@@ -241,10 +258,11 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 }
 
 static mime_node_t *
-_mime_node_create (mime_node_t *parent, GMimeObject *part)
+_mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
 {
     mime_node_t *node = talloc_zero (parent, mime_node_t);
     GMimeCryptoContext *cryptoctx = NULL;
+    notmuch_status_t status;
 
     /* Set basic node properties */
     node->part = part;
@@ -282,7 +300,6 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	|| (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
 	GMimeContentType *content_type = g_mime_object_get_content_type (part);
 	const char *protocol = g_mime_content_type_get_parameter (content_type, "protocol");
-	notmuch_status_t status;
 	status = _notmuch_crypto_get_gmime_ctx_for_protocol (node->ctx->crypto,
 							     protocol, &cryptoctx);
 	if (status) /* this is a warning, not an error */
@@ -312,6 +329,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	} else {
 	    node_verify (node, part, cryptoctx);
 	}
+    } else {
+	status = _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);
+	if (status)
+	    fprintf (stderr, "Warning: failed to record potential crypto payload (%s).\n", notmuch_status_to_string (status));
     }
 
     return node;
@@ -339,7 +360,7 @@ mime_node_child (mime_node_t *parent, int child)
 	INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
 			g_type_name (G_OBJECT_TYPE (parent->part)));
     }
-    node = _mime_node_create (parent, sub);
+    node = _mime_node_create (parent, sub, child);
 
     if (child == parent->next_child && parent->next_part_num != -1) {
 	/* We're traversing in depth-first order.  Record the child's
-- 
2.17.0

Re: [PATCH 06/20] mime-node: track whole-message crypto state while walking the tree

[David Bremner]

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

> Deliberately populate the message's cryptographic status while walking
> the MIME tree from the CLI.
> ---
>  mime-node.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index cbff95d1..6ecd121d 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -49,6 +49,9 @@ _mime_node_context_free (mime_node_context_t *res)
>      if (res->stream)
>  	g_object_unref (res->stream);
>  
> +    if (res->msg_crypto)
> +	_notmuch_message_crypto_cleanup (res->msg_crypto);
> +

I think I see how you got here via code movement, but I think I prefer
the destructor approach for "library" code in util/

>  	}
> +    } else {
> +	status = _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchil

having _notmuch_message and _notmuch_message_crypto as prefixes is a bit
confusing; I momentarily expected this function to take a
notmuch_message_t * as it's first argument. I can live with the current
naming, but if you had any ideas for a non-colliding prefix, that might
save a few headaches in the long run.

>  static mime_node_t *
> -_mime_node_create (mime_node_t *parent, GMimeObject *part)
> +_mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
>  {

I guess that the additional argument is just to pass through to the call

> +	status = _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);

Maybe add a note to that effect in the commit message.

[PATCH 07/20] cli/show: emit new whole-message crypto status output

[Daniel Kahn Gillmor]

This allows MUAs that don't want to think about per-mime-part
cryptographic status to have a simple high-level overview of the
message's cryptographic state.

Sensibly structured encrypted and/or signed messages will work fine
with this.  The only requirement for the simplest encryption + signing
is that the message have all of its encryption and signing protection
(the "cryptographic envelope") in a contiguous set of MIME layers at
the very outside of the message itself.

This is because messages with some subparts signed or encrypted, but
with other subparts with no cryptographic protection is very difficult
to reason about, and even harder for the user to make sense of or work
with.

For further characterization of the Cryptographic Envelope and some of
the usability tradeoffs, see here:

   https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html#cryptographic-envelope
---
 devel/schemata      | 21 ++++++++++++++++++++-
 notmuch-show.c      | 27 +++++++++++++++++++++++++++
 test/T350-crypto.sh | 19 +++++++++++++++----
 test/T355-smime.sh  |  5 +++--
 4 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 42b1bcf3..6370eeac 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -14,7 +14,7 @@ are interleaved. Keys are printed as keywords (symbols preceded by a
 colon), e.g. (:id "123" :time 54321 :from "foobar"). Null is printed as
 nil, true as t and false as nil.
 
-This is version 4 of the structured output format.
+This is version 5 of the structured output format.
 
 Version history
 ---------------
@@ -34,6 +34,9 @@ v4
 - replace signature error integer bitmask with a set of flags for
   individual errors.
 
+v5
+- added message.crypto to identify overall message cryptographic state
+
 Common non-terminals
 --------------------
 
@@ -73,9 +76,25 @@ message = {
     tags:           [string*],
 
     headers:        headers,
+    crypto?:        crypto,   # omitted if crypto disabled, or if no part was signed or encrypted.
     body?:          [part]    # omitted if --body=false
 }
 
+# when showing the message, was any or all of it decrypted?
+msgdecstatus: "full"|"partial"
+
+# The overall cryptographic state of the message as a whole:
+crypto = {
+    signed?:    {
+                  status:      sigstatus,
+                  # was the set of signatures described under encrypted cover?
+                  encrypted:   bool,
+                },
+    decrypted?: {
+                  status: msgdecstatus,
+                }
+}
+
 # A MIME part (format_part_sprinter)
 part = {
     id:             int|string, # part id (currently DFS part number)
diff --git a/notmuch-show.c b/notmuch-show.c
index f0be8060..fea99bff 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -616,6 +616,33 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    format_part_sprinter (ctx, sp, mime_node_child (node, 0), true, include_html);
 	    sp->end (sp);
 	}
+
+	const _notmuch_message_crypto_t *msg_crypto = mime_node_get_message_crypto_status (node);
+	if (msg_crypto->sig_list ||
+	    msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_NONE) {
+	    sp->map_key (sp, "crypto");
+	    sp->begin_map (sp);
+	    if (msg_crypto->sig_list) {
+		sp->map_key (sp, "signed");
+		sp->begin_map (sp);
+		sp->map_key (sp, "status");
+		format_part_sigstatus_sprinter (sp, msg_crypto->sig_list);
+		if (msg_crypto->signature_encrypted) {
+		    sp->map_key (sp, "encrypted");
+		    sp->boolean (sp, msg_crypto->signature_encrypted);
+		}
+		sp->end (sp);
+	    }
+	    if (msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_NONE) {
+		sp->map_key (sp, "decrypted");
+		sp->begin_map (sp);
+		sp->map_key (sp, "status");
+		sp->string (sp, msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL ? "full" : "partial");
+		sp->end (sp);
+	    }
+	    sp->end (sp);
+	}
+
 	sp->end (sp);
 	return;
     }
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index b5067346..4c0f6f46 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -27,7 +27,7 @@ test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; t
 test_begin_subtest "signature verification"
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
-    | sed -e 's|"created": [1234567890]*|"created": 946728000|')
+    | sed -e 's|"created": [1234567890]*|"created": 946728000|g')
 expected='[[[{"id": "XXXXX",
  "match": true,
  "excluded": false,
@@ -35,6 +35,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
+ "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "fingerprint": "'$FINGERPRINT'"}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -75,6 +76,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
+ "crypto": {"signed": {"status": [{ "status": "bad", "keyid": "'$(echo $FINGERPRINT | cut -c 25-)'"}]}},
  "headers": {"Subject": "bad signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -144,7 +146,7 @@ echo "${FINGERPRINT}:6:" | gpg --no-tty --import-ownertrust >>"$GNUPGHOME"/trust
 gpg --no-tty --check-trustdb >>"$GNUPGHOME"/trust.log 2>&1
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
-    | sed -e 's|"created": [1234567890]*|"created": 946728000|')
+    | sed -e 's|"created": [1234567890]*|"created": 946728000|g')
 expected='[[[{"id": "XXXXX",
  "match": true,
  "excluded": false,
@@ -152,6 +154,8 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
+ "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "fingerprint": "'$FINGERPRINT'",
+                                    "userid": "Notmuch Test Suite <test_suite@notmuchmail.org> (INSECURE!)"}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -178,7 +182,7 @@ test_begin_subtest "signature verification with signer key unavailable"
 mv "${GNUPGHOME}"{,.bak}
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
-    | sed -e 's|"created": [1234567890]*|"created": 946728000|')
+    | sed -e 's|"created": [1234567890]*|"created": 946728000|g')
 expected='[[[{"id": "XXXXX",
  "match": true,
  "excluded": false,
@@ -186,6 +190,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
+ "crypto": {"signed": {"status": [{"errors": {"key-missing": true}, "keyid": "'$(echo $FINGERPRINT | cut -c 25-)'", "status": "error"}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -265,6 +270,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["encrypted","inbox"],
+ "crypto": {"decrypted": {"status": "full"}},
  "headers": {"Subject": "test encrypted message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -352,7 +358,7 @@ test_begin_subtest "decryption + signature verification"
 test_subtest_broken_gmime_2
 output=$(notmuch show --format=json --decrypt=true subject:"test encrypted message 002" \
     | notmuch_json_show_sanitize \
-    | sed -e 's|"created": [1234567890]*|"created": 946728000|')
+    | sed -e 's|"created": [1234567890]*|"created": 946728000|g')
 expected='[[[{"id": "XXXXX",
  "match": true,
  "excluded": false,
@@ -360,6 +366,10 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["encrypted","inbox"],
+ "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "fingerprint": "'$FINGERPRINT'",
+                                    "userid": "Notmuch Test Suite <test_suite@notmuchmail.org> (INSECURE!)"}],
+                       "encrypted": true },
+            "decrypted": {"status": "full"}},
  "headers": {"Subject": "test encrypted message 002",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -434,6 +444,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
+ "crypto": {"signed": {"status": [{"errors": {"key-revoked": true}, "keyid": "'$(echo $FINGERPRINT | cut -c 25-)'", "status": "error"}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index be45e3b1..d947e866 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -56,8 +56,8 @@ else
 fi
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
-    | sed -e 's|"created": [-1234567890]*|"created": 946728000|' \
-	  -e 's|"expires": [-1234567890]*|"expires": 424242424|' )
+    | sed -e 's|"created": [-1234567890]*|"created": 946728000|g' \
+	  -e 's|"expires": [-1234567890]*|"expires": 424242424|g' )
 expected='[[[{"id": "XXXXX",
  "match": true,
  "excluded": false,
@@ -65,6 +65,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
+ "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good",'$USERID' "expires": 424242424, "created": 946728000}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
-- 
2.17.0

Re: [PATCH 07/20] cli/show: emit new whole-message crypto status output

[David Bremner]

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

> +
> +	const _notmuch_message_crypto_t *msg_crypto = mime_node_get_message_crypto_status (node);
> +	if (msg_crypto->sig_list ||
> +	    msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_NONE) {
> +	    sp->map_key (sp, "crypto");

I believe the new stuff needs to guarded by

if (notmuch_format_version >= 5)

d

Re: [PATCH 07/20] cli/show: emit new whole-message crypto status output

[Daniel Kahn Gillmor]

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

On Fri 2018-06-15 20:47:59 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +
>> +	const _notmuch_message_crypto_t *msg_crypto = mime_node_get_message_crypto_status (node);
>> +	if (msg_crypto->sig_list ||
>> +	    msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_NONE) {
>> +	    sp->map_key (sp, "crypto");
>
> I believe the new stuff needs to guarded by
>
> if (notmuch_format_version >= 5)

hm, maybe it can all still be v4?  over in notmuch-client.h, it says:

    /* The current structured output format version.  Requests for format
     * versions above this will return an error.  Backwards-incompatible
     * changes such as removing map fields, changing the meaning of map
     * fields, or changing the meanings of list elements should increase
     * this.  New (required) map fields can be added without increasing
     * this.
     */
    #define NOTMUCH_FORMAT_CUR 4

i don't know exactly what "map fields" means here -- i don't think of
notmuch-show output as a "map" but maybe i'm using the terminology
wrong.

and, despite the comments above, the versioning actually looks like
this:

    Version history
    ---------------

    v1
    - First versioned schema release.
    - Added part.content-length and part.content-transfer-encoding fields.

    v2
    - Added the thread_summary.query field.

    v3
    - Replaced message.filename string with a list of filenames.
    - Added part.content-disposition field.

    v4
    - replace signature error integer bitmask with a set of flags for
      individual errors.


So we have bumped from 1 to 2 with just a simple addition in the past.
But maybe that was from before we knew better?

At any rate, since the only thing that we're doing is emitting
message.crypto, i think we can avoid bumping the version in this series.

That said, i think i need to wrap the block with a test for
(notmuch_format_version >= 4) in this case, right?

My new series will try out something like this, let me know what you
think!

          --dkg

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

Re: [PATCH 07/20] cli/show: emit new whole-message crypto status output

[David Bremner]

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

>
> So we have bumped from 1 to 2 with just a simple addition in the past.
> But maybe that was from before we knew better?
>

probably.

> At any rate, since the only thing that we're doing is emitting
> message.crypto, i think we can avoid bumping the version in this series.
>

agreed.

> That said, i think i need to wrap the block with a test for
> (notmuch_format_version >= 4) in this case, right?

yeah, I think so, since otherwise there's no real specification for the
schmema of e.g. version 2 + random stuff.

>
> My new series will try out something like this, let me know what you
> think!
>

I approve this message.

d

[PATCH 08/20] 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.
---
 notmuch-show.c    | 6 +++---
 test/T170-sexp.sh | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index fea99bff..4e918461 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -607,9 +607,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);
@@ -643,6 +640,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 c3dcf52a..a42004f8 100755
--- a/test/T170-sexp.sh
+++ b/test/T170-sexp.sh
@@ -5,12 +5,12 @@ 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\"))) ())))"
+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\")) :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\"))) ())))"
+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\")) :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")
@@ -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\"))) ())))"
+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\")) :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))))) ())))"
+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)))) :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.17.0

Re: [PATCH 08/20] cli/show: emit headers after emitting body

[David Bremner]

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

>  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\"))) ())))"
> +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\")) :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\")) ())))"

It would be cool if we could canonicalize (sort) these s-expressions,
but I didn't see a nice / easy way to do that. Elisp-wizards feel free
to jump in here...

d

[PATCH 09/20] 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.

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.
---
 util/crypto.c | 39 ++++++++++++++++++++++++++++++++++++++-
 util/crypto.h |  5 +++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/util/crypto.c b/util/crypto.c
index b040cc56..2135f677 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -256,6 +256,10 @@ _notmuch_message_crypto_set_sig_list (_notmuch_message_crypto_t *msg_crypto, GMi
 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;
 
@@ -270,7 +274,7 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
     if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum == GMIME_MULTIPART_ENCRYPTED_VERSION) {
 	const char *enc_type = g_mime_object_get_content_type_parameter (parent, "protocol");
 	GMimeContentType *ct = g_mime_object_get_content_type (payload);
-	if (ct) {
+	if (ct && enc_type) {
 	    const char *part_type = g_mime_content_type_get_mime_type (ct);
 	    if (part_type && strcmp (part_type, enc_type) == 0)
 		return NOTMUCH_STATUS_SUCCESS;
@@ -279,6 +283,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;
 }
 
@@ -305,4 +340,6 @@ _notmuch_message_crypto_cleanup (_notmuch_message_crypto_t *msg_crypto)
 	return;
     if (msg_crypto->sig_list)
 	g_object_unref (msg_crypto->sig_list);
+    if (msg_crypto->payload_subject)
+	talloc_free (msg_crypto->payload_subject);
 }
diff --git a/util/crypto.h b/util/crypto.h
index 02c8793a..0cea927c 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -70,6 +70,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.17.0

Re: [PATCH 09/20] util/crypto: add information about the payload part

[David Bremner]

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

> @@ -270,7 +274,7 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
>      if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum == GMIME_MULTIPART_ENCRYPTED_VERSION) {
>  	const char *enc_type = g_mime_object_get_content_type_parameter (parent, "protocol");
>  	GMimeContentType *ct = g_mime_object_get_content_type (payload);
> -	if (ct) {
> +	if (ct && enc_type) {
>  	    const char *part_type = g_mime_content_type_get_mime_type (ct);
>  	    if (part_type && strcmp (part_type, enc_type) == 0)
>  		return NOTMUCH_STATUS_SUCCESS;

I couldn't really understand why enc_type was defined earlier than it
was used here. Is this a bug fix?

d

Re: [PATCH 09/20] util/crypto: add information about the payload part

[Daniel Kahn Gillmor]

On Sun 2018-06-24 22:15:46 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> @@ -270,7 +274,7 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
>>      if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum == GMIME_MULTIPART_ENCRYPTED_VERSION) {
>>  	const char *enc_type = g_mime_object_get_content_type_parameter (parent, "protocol");
>>  	GMimeContentType *ct = g_mime_object_get_content_type (payload);
>> -	if (ct) {
>> +	if (ct && enc_type) {
>>  	    const char *part_type = g_mime_content_type_get_mime_type (ct);
>>  	    if (part_type && strcmp (part_type, enc_type) == 0)
>>  		return NOTMUCH_STATUS_SUCCESS;
>
> I couldn't really understand why enc_type was defined earlier than it
> was used here. Is this a bug fix?

whoops, you're right.  I've moved this particular edit to an earlier
patch in the series.  good catch, thanks!

      --dkg

[PATCH 10/20] 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.
---
 test/T356-protected-headers.sh                | 69 +++++++++++++++++++
 ...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, 302 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..9c6fe467
--- /dev/null
+++ b/test/T356-protected-headers.sh
@@ -0,0 +1,69 @@
+#!/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
+# Change this if we ship a new test key
+FINGERPRINT="5AEAB11F5E33DCE875DDB75B6D92612D94E46381"
+
+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:[0][0][0]!"crypto"' \
+                'subject:[0][0][0]["headers"]["Subject"]="encrypted message"'
+
+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"]="encrypted message"'
+
+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"]="encrypted message"'
+
+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:[0][0][0]!"crypto"' \
+                'subject:[0][0][0]["headers"]["Subject"]="encrypted message"'
+
+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] encrypted message"'
+
+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"]="encrypted message"'
+
+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..629637eb
--- /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: encrypted message
+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..50e2922d
--- /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: encrypted message
+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..e838e211
--- /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: encrypted message
+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..cf08ba30
--- /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: encrypted message
+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..d7a3db89
--- /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: encrypted message
+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..1a767bf9
--- /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: encrypted message
+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..6e242ef0
--- /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] encrypted message
+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.17.0

Re: [PATCH 10/20] cli/show: add tests for viewing protected headers

[David Bremner]

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

> +
> +# TODO:
> +#  * check S/MIME as well as PGP/MIME

Decrypting S/MIME would be good first step. Or is the feature there
(with gmime 3.0?) but tests missing? because T355-smime says there is no
S/MIME decryption.

> +#  * process headers protected by signature

> +
> +test_description='Message decryption with protected headers'
> +. $(dirname "$0")/test-lib.sh || exit 1
> +
> +##################################################
> +
> +add_gnupg_home
> +# Change this if we ship a new test key
> +FINGERPRINT="5AEAB11F5E33DCE875DDB75B6D92612D94E46381"

I wonder if it would be reasonable for add_gnupg_home to define FINGERPRINT

> +
> +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:[0][0][0]!"crypto"' \
> +                'subject:[0][0][0]["headers"]["Subject"]="encrypted message"'

maybe a pointer to where to find the docs for the json test syntax.

Re: [PATCH 10/20] cli/show: add tests for viewing protected headers

[Daniel Kahn Gillmor]

On Sun 2018-06-24 22:31:44 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +
>> +# TODO:
>> +#  * check S/MIME as well as PGP/MIME
>
> Decrypting S/MIME would be good first step. Or is the feature there
> (with gmime 3.0?) but tests missing? because T355-smime says there is no
> S/MIME decryption.

agreed, though you could argue that S/MIME signatures are relevant to
the cryptographic envelope, even without decryption.

regardless, i think that's orthogonal to this series, so i'm not going
to try to address it now.

>> +#  * process headers protected by signature
>
>> +
>> +test_description='Message decryption with protected headers'
>> +. $(dirname "$0")/test-lib.sh || exit 1
>> +
>> +##################################################
>> +
>> +add_gnupg_home
>> +# Change this if we ship a new test key
>> +FINGERPRINT="5AEAB11F5E33DCE875DDB75B6D92612D94E46381"
>
> I wonder if it would be reasonable for add_gnupg_home to define FINGERPRINT

fine with me, i'll include that in the upcoming revised series.

>> +
>> +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:[0][0][0]!"crypto"' \
>> +                'subject:[0][0][0]["headers"]["Subject"]="encrypted message"'
>
> maybe a pointer to where to find the docs for the json test syntax.

I'll stick a pointer in lib/test-lib.sh's definition of
test_json_nodes() because you suggest it, but i think it would be out of
place to do it here in a specifis test.

   --dkg

[PATCH 11/20] 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.
---
 notmuch-client.h               |  2 +-
 notmuch-reply.c                |  4 +++-
 notmuch-show.c                 | 11 +++++++----
 test/T356-protected-headers.sh |  3 ---
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index dfc7c047..73c8a163 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 75cf7ecb..fe02c590 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 4e918461..190e9128 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -196,7 +196,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. */
@@ -208,7 +208,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));
@@ -641,7 +644,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;
@@ -734,7 +737,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 9c6fe467..242ad105 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -23,12 +23,10 @@ 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"'
 
-
 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" \
@@ -61,7 +59,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.17.0

Re: [PATCH 11/20] cli/show: emit payload subject instead of outside subject

[David Bremner]

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

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

This is not really an issue with your patch per se, but do we actually
use this code for anything other than top level messages? I'm wondering
because of my experiments with storing message-document level headers
[1]. It seems difficult to look at the database when things are done at
the GMime level like this.

[1]      id:20180623014247.17834-1-david@tethera.net

>  test_json_nodes <<<"$output" \
>                  'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \
>                  'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"'
>  
> -

naughty white space change. Time to update your pre-commit hook?

d

Re: [PATCH 11/20] cli/show: emit payload subject instead of outside subject

[Daniel Kahn Gillmor]

On Thu 2018-06-28 21:40:04 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>>  
>>      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));
>
> This is not really an issue with your patch per se, but do we actually
> use this code for anything other than top level messages? I'm wondering
> because of my experiments with storing message-document level headers
> [1]. It seems difficult to look at the database when things are done at
> the GMime level like this.
>
> [1]      id:20180623014247.17834-1-david@tethera.net

hm, we might be also using this code (format_headers_sprinter() in
notmuch-show.c) for attached/forwarded messages.  what i wouldn't want
to happen is for an internal message to get its subject swapped out for
the top-level message's subject!

In the new draft of this series, i'm creating an
encrypted-message-with-forwarded-attachment to try to ensure that this
isn't a problem.  thanks for raising the issue.

      --dkg

[PATCH 12/20] 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.
---
 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 6370eeac..5e59b806 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -89,9 +89,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 190e9128..4cc1ce8c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -631,6 +631,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) {
@@ -638,6 +644,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);
 	    }
 	    sp->end (sp);
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 242ad105..c6838995 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -24,7 +24,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": "encrypted message"}}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"'
 
 test_begin_subtest "misplaced protected headers should not be made visible during decryption"
@@ -60,7 +60,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": "encrypted message"}}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="This is a message using draft-melnikov-smime-header-signing"'
 
 test_done
-- 
2.17.0

Re: [PATCH 12/20] cli/show: add information about which headers were protected

[David Bremner]

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

> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -89,9 +89,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,*}
>                  }
>  }

I'm not completely happy with either the update here to the schema file
without a version number bump or with the alternative of the output not
quite matching the schema. On the whole I think I'd prefer that the
schema update be done atomically, either before all the related changes
or after them.

[PATCH 13/20] test: add test for missing external subject

[Daniel Kahn Gillmor]

Adding another test to ensure that we handle it gracefully when no
external subject is present.
---
 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 c6838995..901ee60c 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -27,6 +27,12 @@ test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"Subject": "encrypted message"}}}' \
                 '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", "masked-headers": {"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.17.0

[PATCH 14/20] 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.
---
 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 901ee60c..67d2e0cb 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
@@ -69,4 +68,14 @@ test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"Subject": "encrypted message"}}}' \
                 '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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "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.17.0

Re: [PATCH 14/20] test: show cryptographic envelope information for signed mails

[David Bremner]

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

> +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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "status": "good"}], "headers": ["Subject"]}}'
> +

If possible, it would be good to avoid hardcoding the fingerprint, to
minimize the number of places we have to change when e.g. gpg starts
reject our 1K test key.

d

[PATCH 15/20] 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.
---
 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 67d2e0cb..687681ff 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -78,4 +78,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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "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", "masked-headers": {"Subject": "encrypted message"}}}' \
+                'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
+                'reply-subject:["reply-headers"]["Subject"]="Re: encrypted message"'
+
 test_done
-- 
2.17.0

[PATCH 16/20] cli: introduce flags for format_headers_sprinter

[Daniel Kahn Gillmor]

Rather than passing a boolean to indicate whether this is a reply to
format_headers_sprinter(), we use a flag field.  This will be used
shortly to allow clients to indicate that they can responsibly protect
the subject line.  This changeset has no functional change itself,
just modifying the types passed.
---
 devel/schemata   |  4 ++--
 notmuch-client.h | 12 +++++++++++-
 notmuch-reply.c  |  2 +-
 notmuch-show.c   |  9 +++++----
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 5e59b806..1ea4f798 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -131,7 +131,7 @@ part = {
     content-transfer-encoding?: string
 }
 
-# The headers of a message or part (format_headers_sprinter with reply = FALSE)
+# The headers of a message or part (format_headers_sprinter with flags = FORMAT_HEADERS_NORMAL)
 headers = {
     Subject:        string,
     From:           string,
@@ -223,7 +223,7 @@ reply = {
     original: message
 }
 
-# Reply headers (format_headers_sprinter with reply = TRUE)
+# Reply headers (format_headers_sprinter with flags = FORMAT_HEADERS_REPLY)
 reply_headers = {
     Subject:        string,
     From:           string,
diff --git a/notmuch-client.h b/notmuch-client.h
index 73c8a163..0af96986 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -230,9 +230,19 @@ format_part_sprinter (const void *ctx, struct sprinter *sp, mime_node_t *node,
 		      bool output_body,
 		      bool include_html);
 
+
+typedef enum {
+    /* typical "notmuch show" or other standard output: */
+    HEADERS_FORMAT_NORMAL = 0,
+    /* set only if this is being generated as a reply: */
+    HEADERS_FORMAT_REPLY = 1 << 0
+} notmuch_headers_format_flags;
+
+
 void
 format_headers_sprinter (struct sprinter *sp, GMimeMessage *message,
-			 bool reply, const _notmuch_message_crypto_t *msg_crypto);
+			 notmuch_headers_format_flags flags,
+			 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 fe02c590..749eac6d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -665,7 +665,7 @@ static int do_reply(notmuch_config_t *config,
 	    sp->map_key (sp, "reply-headers");
 	    /* FIXME: send msg_crypto here to avoid killing the
 	     * subject line on reply to encrypted messages! */
-	    format_headers_sprinter (sp, reply, true, NULL);
+	    format_headers_sprinter (sp, reply, HEADERS_FORMAT_REPLY, NULL);
 
 	    /* Start the original */
 	    sp->map_key (sp, "original");
diff --git a/notmuch-show.c b/notmuch-show.c
index 4cc1ce8c..799940f8 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -196,7 +196,8 @@ _is_from_line (const char *line)
 
 void
 format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
-			 bool reply, const _notmuch_message_crypto_t *msg_crypto)
+			 notmuch_headers_format_flags flags,
+			 const _notmuch_message_crypto_t *msg_crypto)
 {
     /* Any changes to the JSON or S-Expression format should be
      * reflected in the file devel/schemata. */
@@ -243,7 +244,7 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
 	sp->string (sp, reply_to_string);
     }
 
-    if (reply) {
+    if (flags & HEADERS_FORMAT_REPLY) {
 	sp->map_key (sp, "In-reply-to");
 	sp->string (sp, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to"));
 
@@ -665,7 +666,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, msg_crypto);
+	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), HEADERS_FORMAT_NORMAL, msg_crypto);
 
 	sp->end (sp);
 	return;
@@ -758,7 +759,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, NULL);
+	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), HEADERS_FORMAT_NORMAL, NULL);
 
 	sp->map_key (sp, "body");
 	sp->begin_list (sp);
-- 
2.17.0

[PATCH 17/20] cli/reply: add --protected-subject boolean flag

[Daniel Kahn Gillmor]

This flag indicates the intent of the client to protect the subject
line, which allows "notmuch reply" to safely emit the earlier
message's encrypted subject without risking leaking it in the clear in
the reply.

Obviously, it should only be used by a client that *will* protect the
subject line.  This feels clumsier than i'd like, but we really don't
want to be the ones who leak data on the wire that had been protected
otherwise, and this seems like a safe way to ensure that the MUA is
capable.
---
 doc/man1/notmuch-reply.rst     | 12 ++++++++++++
 notmuch-client.h               |  4 +++-
 notmuch-reply.c                | 20 ++++++++++++--------
 notmuch-show.c                 |  9 ++++++++-
 test/T356-protected-headers.sh |  7 +++++++
 5 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/doc/man1/notmuch-reply.rst b/doc/man1/notmuch-reply.rst
index c893ba04..08aadba6 100644
--- a/doc/man1/notmuch-reply.rst
+++ b/doc/man1/notmuch-reply.rst
@@ -70,6 +70,18 @@ Supported options for **reply** include
         order, and copy values from the first that contains something
         other than only the user's addresses.
 
+``--protected-subject=(true|false)``
+
+    Indicates that the replying client plans to protect (hide) the
+    subject in the subsequent reply.  When replying to an encrypted
+    message that itself has an encrypted subject, **notmuch**
+    **reply** needs to propose a subject for the new reply e-mail.  If
+    the client can handle protected subjects safely (if this flag is
+    set to ``true``), then the cleartext subject will be proposed.
+    Otherwise, the external (dummy) subject is proposed, to avoid
+    leaking the previously protected subject on reply. Defaults to
+    ``false``.
+
 ``--decrypt=(false|auto|true)``
 
     If ``true``, decrypt any MIME encrypted parts found in the
diff --git a/notmuch-client.h b/notmuch-client.h
index 0af96986..014fa064 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -235,7 +235,9 @@ typedef enum {
     /* typical "notmuch show" or other standard output: */
     HEADERS_FORMAT_NORMAL = 0,
     /* set only if this is being generated as a reply: */
-    HEADERS_FORMAT_REPLY = 1 << 0
+    HEADERS_FORMAT_REPLY = 1 << 0,
+    /* set only if the invoking MUA will responsibly protect the subject line */
+    HEADERS_FORMAT_PROTECTED_SUBJECT = 1 << 1
 } notmuch_headers_format_flags;
 
 
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 749eac6d..d1092ce9 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -612,7 +612,8 @@ static int do_reply(notmuch_config_t *config,
 		    notmuch_query_t *query,
 		    notmuch_show_params_t *params,
 		    int format,
-		    bool reply_all)
+		    bool reply_all,
+		    bool protected_subject)
 {
     GMimeMessage *reply;
     mime_node_t *node;
@@ -659,18 +660,19 @@ static int do_reply(notmuch_config_t *config,
 	    return 1;
 
 	if (format == FORMAT_JSON || format == FORMAT_SEXP) {
+	    notmuch_headers_format_flags flags = HEADERS_FORMAT_REPLY;
 	    sp->begin_map (sp);
 
-	    /* The headers of the reply message we've created */
-	    sp->map_key (sp, "reply-headers");
-	    /* FIXME: send msg_crypto here to avoid killing the
-	     * subject line on reply to encrypted messages! */
-	    format_headers_sprinter (sp, reply, HEADERS_FORMAT_REPLY, NULL);
-
 	    /* Start the original */
 	    sp->map_key (sp, "original");
 	    format_part_sprinter (config, sp, node, true, false);
 
+	    /* The headers of the reply message we've created */
+	    sp->map_key (sp, "reply-headers");
+	    if (protected_subject)
+		flags |= HEADERS_FORMAT_PROTECTED_SUBJECT;
+	    format_headers_sprinter (sp, reply, flags, mime_node_get_message_crypto_status (node));
+
 	    /* End */
 	    sp->end (sp);
 	} else {
@@ -699,6 +701,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
     char *query_string;
+    bool protected_subject = false;
     int opt_index;
     notmuch_show_params_t params = {
 	.part = -1,
@@ -715,6 +718,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "headers-only", FORMAT_HEADERS_ONLY },
 				  { 0, 0 } } },
 	{ .opt_int = &notmuch_format_version, .name = "format-version" },
+	{ .opt_bool = &protected_subject, .name = "protected-subject" },
 	{ .opt_keyword = &reply_all, .name = "reply-to", .keywords =
 	  (notmuch_keyword_t []){ { "all", true },
 				  { "sender", false },
@@ -764,7 +768,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    if (do_reply (config, query, &params, format, reply_all) != 0)
+    if (do_reply (config, query, &params, format, reply_all, protected_subject) != 0)
 	return EXIT_FAILURE;
 
     _notmuch_crypto_cleanup (&params.crypto);
diff --git a/notmuch-show.c b/notmuch-show.c
index 799940f8..88e1be7a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -209,8 +209,15 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
     sp->begin_map (sp);
 
     sp->map_key (sp, "Subject");
-    if (msg_crypto && msg_crypto->payload_subject) {
+    if (msg_crypto && msg_crypto->payload_subject &&
+	!(flags & HEADERS_FORMAT_REPLY))
 	sp->string (sp, msg_crypto->payload_subject);
+    else if ((msg_crypto && msg_crypto->payload_subject &&
+	      (flags & HEADERS_FORMAT_PROTECTED_SUBJECT))) {
+	if (strncasecmp (msg_crypto->payload_subject, "Re:", 3) == 0)
+	    sp->string (sp, msg_crypto->payload_subject);
+	else
+	    sp->string (sp, talloc_asprintf (local, "Re: %s", msg_crypto->payload_subject));
     } else
 	sp->string (sp, g_mime_message_get_subject (message));
 
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 687681ff..a77dae6d 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -85,4 +85,11 @@ test_json_nodes <<<"$output" \
                 'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
                 'reply-subject:["reply-headers"]["Subject"]="Re: encrypted message"'
 
+test_begin_subtest "emit protected subject in reply when client is safe"
+output=$(notmuch reply --decrypt=true --format=json --protected-subject id:protected-header@crypto.notmuchmail.org)
+test_json_nodes <<<"$output" \
+                'crypto:["original"]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"Subject": "encrypted message"}}}' \
+                'subject:["original"]["headers"]["Subject"]="This is a protected header"' \
+                'reply-subject:["reply-headers"]["Subject"]="Re: This is a protected header"'
+
 test_done
-- 
2.17.0

Re: [PATCH 17/20] cli/reply: add --protected-subject boolean flag

[David Bremner]

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

> This flag indicates the intent of the client to protect the subject
> line, which allows "notmuch reply" to safely emit the earlier
> message's encrypted subject without risking leaking it in the clear in
> the reply.
>
> Obviously, it should only be used by a client that *will* protect the
> subject line.  This feels clumsier than i'd like, but we really don't
> want to be the ones who leak data on the wire that had been protected
> otherwise, and this seems like a safe way to ensure that the MUA is
> capable.
> ---
>  doc/man1/notmuch-reply.rst     | 12 ++++++++++++
>  notmuch-client.h               |  4 +++-
>  notmuch-reply.c                | 20 ++++++++++++--------
>  notmuch-show.c                 |  9 ++++++++-
>  test/T356-protected-headers.sh |  7 +++++++
>  5 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/doc/man1/notmuch-reply.rst b/doc/man1/notmuch-reply.rst
> index c893ba04..08aadba6 100644
> --- a/doc/man1/notmuch-reply.rst
> +++ b/doc/man1/notmuch-reply.rst
> @@ -70,6 +70,18 @@ Supported options for **reply** include
>          order, and copy values from the first that contains something
>          other than only the user's addresses.
>  
> +``--protected-subject=(true|false)``
> +
> +    Indicates that the replying client plans to protect (hide) the
> +    subject in the subsequent reply.  When replying to an encrypted
> +    message that itself has an encrypted subject, **notmuch**
> +    **reply** needs to propose a subject for the new reply e-mail.  If
> +    the client can handle protected subjects safely (if this flag is
> +    set to ``true``), then the cleartext subject will be proposed.
> +    Otherwise, the external (dummy) subject is proposed, to avoid
> +    leaking the previously protected subject on reply. Defaults to
> +    ``false``.
> +
>  ``--decrypt=(false|auto|true)``

What about using a keyword argument like --protected=subject ? that
would allow easy future addition of protected headers by specifying e.g.

      --protected=subject --protected=references

>      If ``true``, decrypt any MIME encrypted parts found in the
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 0af96986..014fa064 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -235,7 +235,9 @@ typedef enum {
>      /* typical "notmuch show" or other standard output: */
>      HEADERS_FORMAT_NORMAL = 0,
>      /* set only if this is being generated as a reply: */
> -    HEADERS_FORMAT_REPLY = 1 << 0
> +    HEADERS_FORMAT_REPLY = 1 << 0,
> +    /* set only if the invoking MUA will responsibly protect the subject line */
> +    HEADERS_FORMAT_PROTECTED_SUBJECT = 1 << 1
>  } notmuch_headers_format_flags;

a minor improvement might be to end the list with a ',' from the start
to minimize diff.

>  
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 749eac6d..d1092ce9 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -612,7 +612,8 @@ static int do_reply(notmuch_config_t *config,
>  		    notmuch_query_t *query,
>  		    notmuch_show_params_t *params,
>  		    int format,
> -		    bool reply_all)
> +		    bool reply_all,
> +		    bool protected_subject)

similarly, maybe use the already defined flag enum rather than a boolean
here.

d

[PATCH 18/20] 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.
---
 lib/index.cc                   | 42 ++++++++++++++++++++++++++--------
 lib/message.cc                 |  8 +++++++
 lib/notmuch-private.h          |  4 ++++
 test/T356-protected-headers.sh | 20 ++++++++++++++++
 4 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 0ad683fa..db16b6f8 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,
 			    GMimeContentType *content_type,
-			    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;
@@ -404,6 +406,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) {
@@ -421,7 +425,8 @@ _index_mime_part (notmuch_message_t *message,
 		if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) {
 		    _index_encrypted_mime_part(message, indexopts,
 					       content_type,
-					       GMIME_MULTIPART_ENCRYPTED (part));
+					       GMIME_MULTIPART_ENCRYPTED (part),
+					       msg_crypto);
 		} else {
 		    if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) {
 			_notmuch_database_log (_notmuch_message_database (message),
@@ -430,8 +435,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_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;
     }
@@ -441,7 +451,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;
     }
@@ -518,7 +528,8 @@ static void
 _index_encrypted_mime_part (notmuch_message_t *message,
 			    notmuch_indexopts_t *indexopts,
 			    g_mime_3_unused(GMimeContentType *content_type),
-			    GMimeMultipartEncrypted *encrypted_data)
+			    GMimeMultipartEncrypted *encrypted_data,
+			    _notmuch_message_crypto_t *msg_crypto)
 {
     notmuch_status_t status;
     GError *err = NULL;
@@ -573,6 +584,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 HAVE_GMIME_SESSION_KEYS
 	if (get_sk) {
 	    status = notmuch_message_add_property (message, "session-key",
@@ -584,7 +599,8 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 #endif
 	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");
@@ -603,6 +619,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);
@@ -624,7 +641,14 @@ _notmuch_message_index_file (notmuch_message_t *message,
     subject = g_mime_message_get_subject (mime_message);
     _notmuch_message_gen_terms (message, "subject", subject);
 
-    _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);
+    }
+
+    _notmuch_message_crypto_cleanup (msg_crypto);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
diff --git a/lib/message.cc b/lib/message.cc
index b2067076..3f919180 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1135,6 +1135,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 4598577f..1351fecb 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -318,6 +318,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 a77dae6d..035b3e01 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -92,4 +92,24 @@ 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 "protected subject is indexed when cleartext is indexed"
+notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org
+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"
+notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org
+output=$(notmuch search --format=json 'id:protected-header@crypto.notmuchmail.org')
+test_json_nodes <<<"$output" \
+                'subject:[0]["subject"]="This is a protected header"'
+
+test_begin_subtest "protected subject is indexed when cleartext is indexed"
+notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org
+output=$(notmuch search --output=messages 'subject:"This is a protected header"')
+test_expect_equal "$output" 'id:protected-header@crypto.notmuchmail.org'
+
 test_done
-- 
2.17.0

Re: [PATCH 18/20] indexing: record protected subject when indexing cleartext

[Jameson Graef Rollins]

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

On Fri, May 11 2018, Daniel Kahn Gillmor <dkg@fifthhorseman.net> 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.
> ---
>  lib/index.cc                   | 42 ++++++++++++++++++++++++++--------
...
> diff --git a/lib/index.cc b/lib/index.cc
> index 0ad683fa..db16b6f8 100644
> --- a/lib/index.cc
> +++ b/lib/index.cc
...
> @@ -430,8 +435,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_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;
>      }

In 9088db76d89264b733f6b45e776d8952da237921 _notmuch_message_database
was exposed as notmuch_message_get_database, so this patch needs to be
updated:

diff --git a/lib/index.cc b/lib/index.cc
index 74e1ba43..18e712b1 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -438,7 +438,7 @@ _index_mime_part (notmuch_message_t *message,
            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_database (message),
+               _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);

This fix doesn't affect any of the subsequent patches in this series.

jamie.

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

[PATCH 19/20] 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 "encrypted message")
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.
---
 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 035b3e01..ba1d8c29 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -112,4 +112,20 @@ notmuch reindex --decrypt=true id:protected-header@crypto.notmuchmail.org
 output=$(notmuch search --output=messages 'subject:"This is a protected header"')
 test_expect_equal "$output" 'id:protected-header@crypto.notmuchmail.org'
 
+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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "created": 1525812676}],
+                   "encrypted": true, "headers": ["Subject"]},"decrypted": {"status": "full", "masked-headers": {"Subject": "encrypted message"}}}' \
+                '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": "5AEAB11F5E33DCE875DDB75B6D92612D94E46381", "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..d534e08a
--- /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: encrypted message
+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.17.0

[PATCH 20/20] 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.
---
 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 ba1d8c29..8addbdd0 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -128,4 +128,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.17.0

Re: Protected headers in notmuch

[Jameson Graef Rollins]

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

I've pushed a branch of this series rebased against notmuch/release (for
some reason master is currently a couple commits behind) and fixed to
reflect the exposure of notmuch_message_get_database:

https://gitlab.com/jrollins/notmuch/commits/protected-headers-fix

All tests pass.  Note it requires gmime 3.0 (which tripped me up for a
bit since notmuch still implicitly supports older gmime versions).

I haven't done a commit-by-commit review yet, but I am now using this
series and it works as advertised: messages with encrypted subjects are
searchable by the encrypted subject, and the encrypted subjects show up
correctly in all the clients I'm using (both CLI and emacs).

I strongly support the inclusion of this feature, particularly since
it's an important component of autocrypt, which I want even more.

jamie.

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

Re: Protected headers in notmuch

[David Bremner]

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

> I've pushed a branch of this series rebased against notmuch/release (for
> some reason master is currently a couple commits behind) and fixed to
> reflect the exposure of notmuch_message_get_database:
>
> https://gitlab.com/jrollins/notmuch/commits/protected-headers-fix
>
> All tests pass.  Note it requires gmime 3.0 (which tripped me up for a
> bit since notmuch still implicitly supports older gmime versions).
>

Depending on how much work that is to fix, it might be time to actually
rip out the pre-3.0 support; we deprecated it 7 months ago.

d

Re: Protected headers in notmuch

[Daniel Kahn Gillmor]

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

On Sat 2018-06-02 16:20:25 -0300, David Bremner wrote:
> Jameson Graef Rollins <jrollins@finestructure.net> writes:
>
>> I've pushed a branch of this series rebased against notmuch/release (for
>> some reason master is currently a couple commits behind) and fixed to
>> reflect the exposure of notmuch_message_get_database:
>>
>> https://gitlab.com/jrollins/notmuch/commits/protected-headers-fix
>>
>> All tests pass.  Note it requires gmime 3.0 (which tripped me up for a
>> bit since notmuch still implicitly supports older gmime versions).
>
> Depending on how much work that is to fix, it might be time to actually
> rip out the pre-3.0 support; we deprecated it 7 months ago.

as far as i'm concerned, i don't mind losing gmime support for versions
before 3.0.  for debian, we have 3.0 in stretch-backports, so anyone who
wants a backported notmuch can just build-depend on it directly.

are there other platforms where a current notmuch might be backported to
that don't have gmime 3.0 ?

     --dkg

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

Re: Protected headers in notmuch

[David Bremner]

David Bremner <david@tethera.net> writes:

> Jameson Graef Rollins <jrollins@finestructure.net> writes:
>
>> I've pushed a branch of this series rebased against notmuch/release (for
>> some reason master is currently a couple commits behind) and fixed to
>> reflect the exposure of notmuch_message_get_database:
>>
>> https://gitlab.com/jrollins/notmuch/commits/protected-headers-fix
>>
>> All tests pass.  Note it requires gmime 3.0 (which tripped me up for a
>> bit since notmuch still implicitly supports older gmime versions).
>>

I didn't get as far as testing this, but when you say it requires gmime
3.0, do you mean it needs 3.0 to provide the feature, or it needs 3.0 to
even build?

d

Re: Protected headers in notmuch

[Jameson Graef Rollins]

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

On Sat, Jun 02 2018, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> I've pushed a branch of this series rebased against notmuch/release (for
> some reason master is currently a couple commits behind) and fixed to
> reflect the exposure of notmuch_message_get_database:
>
> https://gitlab.com/jrollins/notmuch/commits/protected-headers-fix
>
> All tests pass.  Note it requires gmime 3.0 (which tripped me up for a
> bit since notmuch still implicitly supports older gmime versions).
>
> I haven't done a commit-by-commit review yet, but I am now using this
> series and it works as advertised: messages with encrypted subjects are
> searchable by the encrypted subject, and the encrypted subjects show up
> correctly in all the clients I'm using (both CLI and emacs).
>
> I strongly support the inclusion of this feature, particularly since
> it's an important component of autocrypt, which I want even more.

I've now done a commit-by-commit review and I think this is a very clean
patch series, with very good test suite coverage.  To the extent that I
have much to say on any of the code structure (not much) it all looks
good to me.

In particular I really like the introduction of the new "cryptographic
envelope" concept, and the new whole-message crypto status object that
goes with it.  I think this is a very solid idea that will be very
useful for clients going forward.  The implementation details seem solid
and well thought out to me, seeming to include all the useful info that
a client would need.

As for the protected headers, I like that they're just swapped in
seamlessly.  It also seems useful that the crypto envelope status
includes information about which headers were signed, and which headers
were masked by those that were encrypted.  It seems that there's just
enough information emitted about the overall crypto status and the
protected headers for any clients to be able to construct a useful UX
for users, but without any cruft that could potentially be confusing.

jamie.

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

Re: Protected headers in notmuch

[David Bremner]

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

> Traditionally, encrypted and signed e-mail covers only the body of the
> message.  New standards are emerging that are capable of protecting
> the headers as well.  In particular, Enigmail and an upcoming version
> of K-9 mail both use the "Memory Hole" approach to encrypt the
> Subject: header when sending encrypted mail.  It is awkward to receive
> encrypted messages from those clients with notmuch, because all
> notmuch sees is "Subject: Encrypted Message"
>

Sorry about the long pause, I finished reviewing the last 3 patches in
the series and have no further comments.

d