Stashed session keys

Stashed session keys

[Daniel Kahn Gillmor]

Now that cleartext indexing is merged, let's add the ability to stash
session keys!

Background
==========

Encrypted e-mail messages are "hybrid" encryption.  The message body
is encrypted with an ephemeral session key, and then that session key
is itself encrypted to the user's public key.

If an MUA retains (or obtains) a copy of the session key for a given
message, it can access the cleartext of that message without needing
any access to the user's private key material.

This offers possible wins in efficiency, usability, convenience *and*
security, as the series hopefully makes clear.

Decryption Policies
===================

At the end of the series, there are four sensible policies defined for
message decryption and stashing of session keys.  There are only two i
expect to see any widespread regular use: "auto", and "true".  But
hopefully the reasons for including the other two policies ("false"
and "nostash") are made clear by the series itself.

I'll replicate here the table this series adds to notmuch-config(1),
in describing the available values for index.try_decrypt:

   +------------------------+-------+------+---------+------+
   |                        | false | auto | nostash | true |
   +========================+=======+======+=========+======+
   | Index cleartext using  |       |  X   |    X    |  X   |
   | stashed session keys   |       |      |         |      |
   +------------------------+-------+------+---------+------+
   | Index cleartext        |       |      |    X    |  X   |
   | using secret keys      |       |      |         |      |
   +------------------------+-------+------+---------+------+
   | Stash session keys     |       |      |         |  X   |
   +------------------------+-------+------+---------+------+
   | Delete stashed session |   X   |      |         |      |
   | keys on reindex        |       |      |         |      |
   +------------------------+-------+------+---------+------+


Please let me know what you think!  I'd love feedback and critique.

Happy hacking,

       --dkg

[PATCH 01/18] mime-node: handle decrypt_result more safely

[Daniel Kahn Gillmor]

If (for whatever reason) we don't get a decrypt_result back, or it's
not structured the way we expect it to be, we shouldn't choke on it.
---
 mime-node.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index c3d5cb9b..28326e03 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -214,13 +214,15 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
     node->decrypt_success = true;
     node->verify_attempted = true;
 
-    /* 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) {
-	g_object_ref (node->sig_list);
-	set_signature_list_destructor (node);
+    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) {
+	    g_object_ref (node->sig_list);
+	    set_signature_list_destructor (node);
+	}
+	g_object_unref (decrypt_result);
     }
-    g_object_unref (decrypt_result);
 
  DONE:
     if (err)
-- 
2.14.2

[PATCH 02/18] crypto: add _notmuch_crypto_decrypt wrapper function

[Daniel Kahn Gillmor]

We will use this centralized function to consolidate the awkward
behavior around different gmime versions.

It's only invoked from two places: mime-node.c's
node_decrypt_and_verify() and lib/index.cc's
_index_encrypted_mime_part().

However, those two places have some markedly distinct logic, so the
interface for this _notmuch_crypto_decrypt function is going to get a
little bit clunky.  It's worthwhile, though, for the sake of keeping
these #if directives reasonably well-contained.
---
 lib/index.cc  |  8 ++------
 mime-node.c   |  8 ++------
 util/crypto.c | 18 ++++++++++++++++++
 util/crypto.h |  7 +++++--
 4 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 6e684f5f..19d03456 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -530,9 +530,9 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 
     notmuch = _notmuch_message_database (message);
 
+    GMimeCryptoContext* crypto_ctx = NULL;
 #if (GMIME_MAJOR_VERSION < 3)
     {
-	GMimeCryptoContext* crypto_ctx = NULL;
 	const char *protocol = NULL;
 	protocol = g_mime_content_type_get_parameter (content_type, "protocol");
 	status = _notmuch_crypto_get_gmime_ctx_for_protocol (&(indexopts->crypto),
@@ -546,13 +546,9 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 					      "property (%d)\n", status);
 	    return;
 	}
-	clear = g_mime_multipart_encrypted_decrypt(encrypted_data, crypto_ctx,
-						   NULL, &err);
     }
-#else
-    clear = g_mime_multipart_encrypted_decrypt(encrypted_data, GMIME_DECRYPT_NONE, NULL,
-					       NULL, &err);
 #endif
+    clear = _notmuch_crypto_decrypt (crypto_ctx, encrypted_data, NULL, &err);
     if (err) {
 	_notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n",
 			       err->domain, err->code, err->message);
diff --git a/mime-node.c b/mime-node.c
index 28326e03..7f116531 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -199,12 +199,8 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
     GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
 
     node->decrypt_attempted = true;
-    node->decrypted_child = g_mime_multipart_encrypted_decrypt
-#if (GMIME_MAJOR_VERSION < 3)
-	(encrypteddata, cryptoctx, &decrypt_result, &err);
-#else
-        (encrypteddata, GMIME_DECRYPT_NONE, NULL, &decrypt_result, &err);
-#endif
+    if (! node->decrypted_child)
+	node->decrypted_child = _notmuch_crypto_decrypt (cryptoctx, encrypteddata, &decrypt_result, &err);
     if (! node->decrypted_child) {
 	fprintf (stderr, "Failed to decrypt part: %s\n",
 		 err ? err->message : "no error explanation given");
diff --git a/util/crypto.c b/util/crypto.c
index 5c84282e..087536ec 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -138,3 +138,21 @@ void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t *crypto))
 {
 }
 #endif
+
+GMimeObject *
+_notmuch_crypto_decrypt (g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
+			 GMimeMultipartEncrypted *part,
+			 GMimeDecryptResult **decrypt_result,
+			 GError **err)
+{
+    GMimeObject *ret = NULL;
+
+#if (GMIME_MAJOR_VERSION < 3)
+    ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
+					     decrypt_result, err);
+#else
+    ret = g_mime_multipart_encrypted_decrypt(part, GMIME_DECRYPT_NONE, NULL,
+					     decrypt_result, err);
+#endif
+    return ret;
+}
diff --git a/util/crypto.h b/util/crypto.h
index 1ff0297d..d68634f3 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -2,10 +2,8 @@
 #define _CRYPTO_H
 
 #include <stdbool.h>
-#if (GMIME_MAJOR_VERSION < 3)
 #include "gmime-extra.h"
 #include "notmuch.h"
-#endif
 
 typedef struct _notmuch_crypto {
     bool verify;
@@ -17,6 +15,11 @@ typedef struct _notmuch_crypto {
 #endif
 } _notmuch_crypto_t;
 
+GMimeObject *
+_notmuch_crypto_decrypt (GMimeCryptoContext* crypto_ctx,
+			 GMimeMultipartEncrypted *part,
+			 GMimeDecryptResult **decrypt_result,
+			 GError **err);
 
 #if (GMIME_MAJOR_VERSION < 3)
 notmuch_status_t
-- 
2.14.2

[PATCH 03/18] crypto: use stashed session-key properties for decryption, if available

[Daniel Kahn Gillmor]

When doing any decryption, if the notmuch database knows of any
session keys associated with the message in question, try them before
defaulting to using default symmetric crypto.

The primary advantage this offers is a significant speedup when
rendering large encrypted threads ("notmuch show") if session keys
happen to be cached.

Additionally, it permits message composition without access to
asymmetric secret keys ("notmuch reply"); and it permits recovering a
cleartext index when reindexing after a "notmuch restore" for those
messages that already have a session key stored.

This does nothing if no session keys are stashed in the database,
which is fine.  Actually stashing session keys in the database will
come as a subsequent patch.

Note that this only works for GMime 2.6.21 and later (the session key
interface wasn't available before then).  I don't think we're ready
for this to be a minimum version requirement yet, so instead if you
build against a prior version of GMime, it simply won't try to use
stashed session keys.
---
 doc/man7/notmuch-properties.rst | 31 +++++++++++++++++++++++++++++++
 lib/index.cc                    |  2 +-
 mime-node.c                     | 13 ++++++++++---
 util/crypto.c                   | 31 ++++++++++++++++++++++++++++++-
 util/crypto.h                   |  3 ++-
 5 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 68121359..31d7a104 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -74,6 +74,35 @@ of its normal activity.
     **notmuch-config(1)**), then this property will not be set on that
     message.
 
+**session-key**
+
+    When **notmuch-show(1)** or **nomtuch-reply** encounters a message
+    with an encrypted part and ``--decrypt`` is set, if notmuch finds a
+    ``session-key=`` property associated with the message, it will try
+    that stashed session key for decryption.
+
+    Using a stashed session key with "notmuch show" will speed up
+    rendering of long encrypted threads.  It also allows the user to
+    destroy the secret part of any expired encryption-capable subkey
+    while still being able to read any retained messages for which
+    they have stashed the session key.  This enables truly deletable
+    e-mail, since (once the session key and asymmetric subkey are both
+    destroyed) there are no keys left that can be used to decrypt any
+    copy of the original message previously stored by an adversary.
+
+    However, access to the stashed session key for an encrypted message
+    permits full byte-for-byte reconstruction of the cleartext
+    message.  This includes attachments, cryptographic signatures, and
+    other material that cannot be reconstructed from the index alone.
+
+    The session key should be in the ASCII text form produced by
+    GnuPG.  For OpenPGP, that consists of a decimal representation of
+    the hash algorithm used (identified by number from RFC 4880,
+    e.g. 9 means AES-256) followed by a colon, followed by a
+    hexidecimal representation of the algorithm-specific key.  For
+    example, an AES-128 key might be stashed in a notmuch property as:
+    ``session-key=7:14B16AF65536C28AF209828DFE34C9E0``.
+
 SEE ALSO
 ========
 
@@ -83,5 +112,7 @@ SEE ALSO
 **notmuch-insert(1)**,
 **notmuch-new(1)**,
 **notmuch-reindex(1)**,
+**notmuch-reply(1)**,
 **notmuch-restore(1)**,
+**notmuch-show(1)**,
 ***notmuch-search-terms(7)**
diff --git a/lib/index.cc b/lib/index.cc
index 19d03456..6eb60f30 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -548,7 +548,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 	}
     }
 #endif
-    clear = _notmuch_crypto_decrypt (crypto_ctx, encrypted_data, NULL, &err);
+    clear = _notmuch_crypto_decrypt (message, crypto_ctx, encrypted_data, NULL, &err);
     if (err) {
 	_notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n",
 			       err->domain, err->code, err->message);
diff --git a/mime-node.c b/mime-node.c
index 7f116531..09170bfd 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -198,9 +198,16 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
     GMimeDecryptResult *decrypt_result = NULL;
     GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
 
-    node->decrypt_attempted = true;
-    if (! node->decrypted_child)
-	node->decrypted_child = _notmuch_crypto_decrypt (cryptoctx, encrypteddata, &decrypt_result, &err);
+    if (! node->decrypted_child) {
+	mime_node_t *parent;
+	for (parent = node; parent; parent = parent->parent)
+	    if (parent->envelope_file)
+		break;
+
+	node->decrypt_attempted = true;
+	node->decrypted_child = _notmuch_crypto_decrypt (parent ? parent->envelope_file : NULL,
+							 cryptoctx, encrypteddata, &decrypt_result, &err);
+    }
     if (! node->decrypted_child) {
 	fprintf (stderr, "Failed to decrypt part: %s\n",
 		 err ? err->message : "no error explanation given");
diff --git a/util/crypto.c b/util/crypto.c
index 087536ec..e014db5d 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -140,13 +140,42 @@ void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t *crypto))
 #endif
 
 GMimeObject *
-_notmuch_crypto_decrypt (g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
+_notmuch_crypto_decrypt (notmuch_message_t *message,
+			 g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
 			 GMimeMultipartEncrypted *part,
 			 GMimeDecryptResult **decrypt_result,
 			 GError **err)
 {
     GMimeObject *ret = NULL;
 
+    /* the versions of notmuch that can support session key decryption */
+#if (GMIME_MAJOR_VERSION >= 3 || (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21))
+    if (message) {
+	notmuch_message_properties_t *list = NULL;
+
+	for (list = notmuch_message_get_properties (message, "session-key", TRUE);
+	     notmuch_message_properties_valid (list); notmuch_message_properties_move_to_next (list)) {
+#if (GMIME_MAJOR_VERSION < 3)
+	    ret = g_mime_multipart_encrypted_decrypt_session (part,
+							      crypto_ctx,
+							      notmuch_message_properties_value (list),
+							      decrypt_result, err);
+#else
+	    ret = g_mime_multipart_encrypted_decrypt (part,
+						      GMIME_DECRYPT_NONE,
+						      notmuch_message_properties_value (list),
+						      decrypt_result, err);
+#endif
+	    if (ret)
+		break;
+	}
+	if (list)
+	    notmuch_message_properties_destroy (list);
+	if (ret)
+	    return ret;
+    }
+#endif
+
 #if (GMIME_MAJOR_VERSION < 3)
     ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
 					     decrypt_result, err);
diff --git a/util/crypto.h b/util/crypto.h
index d68634f3..0c62ac70 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
 } _notmuch_crypto_t;
 
 GMimeObject *
-_notmuch_crypto_decrypt (GMimeCryptoContext* crypto_ctx,
+_notmuch_crypto_decrypt (notmuch_message_t *message,
+			 GMimeCryptoContext* crypto_ctx,
 			 GMimeMultipartEncrypted *part,
 			 GMimeDecryptResult **decrypt_result,
 			 GError **err);
-- 
2.14.2

Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available

[Daniel Kahn Gillmor]

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

On Wed 2017-10-25 02:51:48 -0400, Daniel Kahn Gillmor wrote:
> diff --git a/util/crypto.c b/util/crypto.c
> index 087536ec..e014db5d 100644
> --- a/util/crypto.c
> +++ b/util/crypto.c
> @@ -140,13 +140,42 @@ void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t *crypto))
>  #endif
>  
>  GMimeObject *
> -_notmuch_crypto_decrypt (g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
> +_notmuch_crypto_decrypt (notmuch_message_t *message,
> +			 g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
>  			 GMimeMultipartEncrypted *part,
>  			 GMimeDecryptResult **decrypt_result,
>  			 GError **err)
>  {
>      GMimeObject *ret = NULL;
>  
> +    /* the versions of notmuch that can support session key decryption */
> +#if (GMIME_MAJOR_VERSION >= 3 || (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21))
> +    if (message) {
> +	notmuch_message_properties_t *list = NULL;
> +
> +	for (list = notmuch_message_get_properties (message, "session-key", TRUE);
> +	     notmuch_message_properties_valid (list); notmuch_message_properties_move_to_next (list)) {
> +#if (GMIME_MAJOR_VERSION < 3)
> +	    ret = g_mime_multipart_encrypted_decrypt_session (part,
> +							      crypto_ctx,
> +							      notmuch_message_properties_value (list),
> +							      decrypt_result, err);
> +#else
> +	    ret = g_mime_multipart_encrypted_decrypt (part,
> +						      GMIME_DECRYPT_NONE,
> +						      notmuch_message_properties_value (list),
> +						      decrypt_result, err);
> +#endif
> +	    if (ret)
> +		break;
> +	}
> +	if (list)
> +	    notmuch_message_properties_destroy (list);
> +	if (ret)
> +	    return ret;
> +    }
> +#endif
> +
>  #if (GMIME_MAJOR_VERSION < 3)
>      ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
>  					     decrypt_result, err);

In the change above, i realized that we might accidentally clobber the
GError of any intermediate failed decryption attempt, which would
produce a GLib warning to stderr.

In my revised/updated series ("session-keys" on
https://gitlab.com/dkg/notmuch), i clear err (if present) before each
attempted decryption.  This effectively throws away all errors except
for the last one, but i think that's the right thing to do -- we'll try
whatever we can for decrypting, but if the final decryption fails,
that's the error we'd want reported back anyway.

           --dkg the self-reviewer :)

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

Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available

[David Bremner]

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


> Note that this only works for GMime 2.6.21 and later (the session key
> interface wasn't available before then).  I don't think we're ready
> for this to be a minimum version requirement yet, so instead if you
> build against a prior version of GMime, it simply won't try to use
> stashed session keys.

Since you wrote this, I've deprecated GMime 2.6. I'm not sure that
changes anything here, but seems worth mentioning

> ---
>  doc/man7/notmuch-properties.rst | 31 +++++++++++++++++++++++++++++++
>  lib/index.cc                    |  2 +-
>  mime-node.c                     | 13 ++++++++++---
>  util/crypto.c                   | 31 ++++++++++++++++++++++++++++++-
>  util/crypto.h                   |  3 ++-
>  5 files changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
> index 68121359..31d7a104 100644
> --- a/doc/man7/notmuch-properties.rst
> +++ b/doc/man7/notmuch-properties.rst
> @@ -74,6 +74,35 @@ of its normal activity.
>      **notmuch-config(1)**), then this property will not be set on that
>      message.
>  
> +**session-key**
> +
> +    When **notmuch-show(1)** or **nomtuch-reply** encounters a message
> +    with an encrypted part and ``--decrypt`` is set, if notmuch finds a
> +    ``session-key=`` property associated with the message, it will try
> +    that stashed session key for decryption.
> +

Its a nitpick, but I don't really understand/like including = with the
property name.  That will break, e.g. for anyone attempting to use it
from the API.

> -    clear = _notmuch_crypto_decrypt (crypto_ctx, encrypted_data, NULL, &err);
> +    clear = _notmuch_crypto_decrypt (message, crypto_ctx, encrypted_data, NULL, &err);

The way the diff works out, I was pretty confused by seeing several
"wrong" calls to _notmuch_crypto_decrypt before the actual change. It
would be nice to telegraph that somehow, perhaps in the commit message.

>  {
>      GMimeObject *ret = NULL;
>  
> +    /* the versions of notmuch that can support session key decryption */
> +#if (GMIME_MAJOR_VERSION >= 3 || (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21))

Personally I would be fine with (and probably happier) only supporting
new features when using gmime-3.0.  Debugging crypto related stuff is
always hard (see recent discussion about _mime_node_create, where we
still don't know what's wrong, and are just papering over the problem),
and it seems worth striving for simplicity as much as possible.  I also
don't know how motivated gmime upstream is to fix bugs in 2.6; I could
certainly understand if the answer was "not very".

There is, by the way, a function notmuch_built_with that can be used to
introspect the library as to what optional features it is built
with. It's used in notmuch_config to report back to the user about the
presence of optional features.

Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available

[Daniel Kahn Gillmor]

Hi Bremner--

Thanks for the review!

On Tue 2017-11-14 09:02:08 -0400, David Bremner wrote:

> Since you wrote this, I've deprecated GMime 2.6. I'm not sure that
> changes anything here, but seems worth mentioning

well, i'm happy to hear that -- i've got no problem with deprecating
GMime 2.6, and would be fine with maintaining GMime 3.0 in
stretch-backports if that would make you feel more comfortable about the
decision.

Still, I'll be kind of bummed to have to rewrite this series to strip
out the 2.6 support: i originally wrote it only with 3.0 support, and
then went back in and added 2.6 support because at the time, you didn't
want to deprecate 2.6 :( our coding cadence isn't very well synced :/

> Its a nitpick, but I don't really understand/like including = with the
> property name.  That will break, e.g. for anyone attempting to use it
> from the API.

I don't mind changing the documentation to use ``session-key`` instead
of ``session-key=``.  *shrug*

> The way the diff works out, I was pretty confused by seeing several
> "wrong" calls to _notmuch_crypto_decrypt before the actual change. It
> would be nice to telegraph that somehow, perhaps in the commit message.

sure, i can add to the commit message that _notmuch_crypto_decrypt is
growing a new parameter.

> Personally I would be fine with (and probably happier) only supporting
> new features when using gmime-3.0.  Debugging crypto related stuff is
> always hard (see recent discussion about _mime_node_create, where we
> still don't know what's wrong, and are just papering over the problem),
> and it seems worth striving for simplicity as much as possible.  I also
> don't know how motivated gmime upstream is to fix bugs in 2.6; I could
> certainly understand if the answer was "not very".

I believe the answer is "not very" -- but if there are serious bugs (i
don't think we've talked about any of this stuff as bugs in gmime) then
we should probably try to raise them with him.

> There is, by the way, a function notmuch_built_with that can be used to
> introspect the library as to what optional features it is built
> with. It's used in notmuch_config to report back to the user about the
> presence of optional features.

Is there any naming convention for these features?  do you want me to
add a "session-key" label with a future revision of this branch?  or are
you asking for something else?

    --dkg

Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available

[David Bremner]

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

> I also
>> don't know how motivated gmime upstream is to fix bugs in 2.6; I could
>> certainly understand if the answer was "not very".
>
> I believe the answer is "not very" -- but if there are serious bugs (i
> don't think we've talked about any of this stuff as bugs in gmime) then
> we should probably try to raise them with him.

I think Jani tried a bit to narrow it down, but didn't succeed. Part of the
problem (which I suspect is endemic to crypto issues) is that we don't
have public test cases.

>
>> There is, by the way, a function notmuch_built_with that can be used to
>> introspect the library as to what optional features it is built
>> with. It's used in notmuch_config to report back to the user about the
>> presence of optional features.
>
> Is there any naming convention for these features?  do you want me to
> add a "session-key" label with a future revision of this branch?  or are
> you asking for something else?

It could be a followup, but yeah, if there is some feature that is
sometimes compiled in, and sometimes not, then it should be listed along
with the others. For whatever reason, the existing convention is
'session_key'

This discussion does make me think there should probably be a test in
configure that sets a corresponding feature macro
HAVE_GMIME_SESSION_KEYS, in a manner similar to HAVE_XAPIAN_COMPACT
(possibly just centralizing the version comparison currently used).

d

[PATCH 04/18] test/corpora: add an encrypted message for index decryption tests

[Daniel Kahn Gillmor]

---
 test/corpora/README                  |  5 +++++
 test/corpora/crypto/simple-encrypted | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 test/corpora/crypto/simple-encrypted

diff --git a/test/corpora/README b/test/corpora/README
index c9a35fed..05ea386d 100644
--- a/test/corpora/README
+++ b/test/corpora/README
@@ -12,3 +12,8 @@ broken
 
 html
   The html corpus contains html parts
+
+crypto
+  The crypto corpus contains encrypted messages for testing.
+  It should probably also contain signed messages in the future.
+  Please add them!
diff --git a/test/corpora/crypto/simple-encrypted b/test/corpora/crypto/simple-encrypted
new file mode 100644
index 00000000..6869972d
--- /dev/null
+++ b/test/corpora/crypto/simple-encrypted
@@ -0,0 +1,36 @@
+From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+To: dkg@fifthhorseman.net
+Subject: encrypted message
+Date: Mon, 22 Dec 2016 08:34:56 -0400
+Message-ID: <simple-encrypted@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-----
+
+hQIMAzt6p/AU5ptaAQ/+IDRx5dZWKjUz9qITP76w8OvtmTV9p871UoGWil1DSd3J
+dHgf56rXDWS73dzJ5EigevxLVMD3Xv8QEJBgMWb6yC+uR8ZdJ8h7hlE2lYyEg3Ch
+smqzcaYp2nKWw9JZQqubsMaeIgVu0exb4YE8g/qlUOL2mD64dXhnkJ68GGMmiEPJ
++d0H7fTMwstbxvKPKDmFJUztH43V2NSfxpeQUTi4iWmQtUGw6THYjjAWNFy7jVEE
+ozloT9W3ER5cRaXyKE4GWMBlUAOB0YwwsVnBU2JGUtTBzNHxQAbeoKrG6myrzmnC
+LhUNag0gMuNEbGR78XfWKpCbccEC1VLf9uUXze6yeuRXDuhfsvilKOH9MpaR9n+G
+JuYVAAobDc5wGOt5VGMka50ToaALrNt3FrWqni/0jqwqshEVKM3Kqzq6cSjh1TPf
+pfxE9aUDdvf+Nn16ZBGgyLox2NV4GkSQYq2ySzAk3XwLU80F0nCmtOV3EH+OMrv6
+sZI/Svwzaqzrs/w17cvpf0czXjE/N9V1MHdNtIkfb707WkO0l9/wtYvlrg/KyrU2
+FH5aecEO9VpMumgzBqP1MrrnzVlSM3kgRLIu06oshQYD+jjFn2YzvkwZ+pWoAxsQ
+ninQxoF8Ck2D7s8uGzx+HSQQpRVBM5AGfVdEkmzW/sZrlz63ZGUFh0FYqLl30rfS
+cQGqSoZz0ugTSxnTlg0nuzFmG7ylC1cx3dlSrnfv+l1azwLAr58ptoYZ0mO+D+Fy
+bePpCGoFAPKi0cZ/4eFlLKL7uYPmGeEo5Ku2wXU/SXtPfl4vRxzvPXTJTvD06vIx
+Dh1P0ChAJtxUjGGY6xkCHMY0
+=frmz
+-----END PGP MESSAGE-----
+--=-=-=--
-- 
2.14.2

[PATCH 05/18] crypto: Test restore of cleartext index from stashed session keys

[Daniel Kahn Gillmor]

If you've got a notmuch dump that includes stashed session keys for
every decrypted message, and you've got your message archive, you
should be able to get back to the same index that you had before.

Here we add a simple test that give some flavor of how that works.
---
 test/T357-index-decryption.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 22e716c6..11ea2074 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -156,6 +156,37 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+add_email_corpus crypto
+
+test_begin_subtest "indexing message fails when secret key not available"
+notmuch reindex --try-decrypt id:simple-encrypted@crypto.notmuchmail.org
+output=$(notmuch dump )
+expected='#notmuch-dump batch-tag:3 config,properties,tags
++encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org
+#= simple-encrypted@crypto.notmuchmail.org index.decryption=failure'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "cannot find cleartext index"
+output=$(notmuch search sekrit)
+expected=''
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "cleartext index recovery on reindexing with stashed session keys"
+notmuch restore <<EOF
+#notmuch-dump batch-tag:3 config,properties,tags
+#= simple-encrypted@crypto.notmuchmail.org session-key=9%3AFC09987F5F927CC0CC0EE80A96E4C5BBF4A499818FB591207705DFDDD6112CF9
+EOF
+notmuch reindex --try-decrypt id:simple-encrypted@crypto.notmuchmail.org
+output=$(notmuch search sekrit)
+expected='thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 
 # TODO: test removal of a message from the message store between
 # indexing and reindexing.
-- 
2.14.2

Re: [PATCH 05/18] crypto: Test restore of cleartext index from stashed session keys

[David Bremner]

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

> If you've got a notmuch dump that includes stashed session keys for
> every decrypted message, and you've got your message archive, you
> should be able to get back to the same index that you had before.
>

Out of curiousity, have you given any thought to what happens when
someone sends a message with the same message-id but a different
session-key? it seems like the user can potentially lose access to the
encrypted message.

Re: [PATCH 05/18] crypto: Test restore of cleartext index from stashed session keys

[Daniel Kahn Gillmor]

On Tue 2017-11-14 09:13:52 -0400, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> If you've got a notmuch dump that includes stashed session keys for
>> every decrypted message, and you've got your message archive, you
>> should be able to get back to the same index that you had before.
>
> Out of curiousity, have you given any thought to what happens when
> someone sends a message with the same message-id but a different
> session-key? it seems like the user can potentially lose access to the
> encrypted message.

yep!  I even have that case in my own mailbox due to messages i've sent
to schleuder encrypted mailing lists to which i'm also subscribed.

It works fine.  notmuch stashes both session keys against the message-id
(you can have multiple properties with the same name as long as they
have different values).  And upon decryption, it tries each session-key
in succession.  This is a little bit sloppy (maybe it would be less
sloppy to associate each message key with each version of the message
somehow?), but it's significantly simpler and basically unnoticeable
compared to the speedup gains provided by the rest of the series.

         --dkg

Re: [PATCH 05/18] crypto: Test restore of cleartext index from stashed session keys

[David Bremner]

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

> On Tue 2017-11-14 09:13:52 -0400, David Bremner wrote:
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>
>>> If you've got a notmuch dump that includes stashed session keys for
>>> every decrypted message, and you've got your message archive, you
>>> should be able to get back to the same index that you had before.
>>
>> Out of curiousity, have you given any thought to what happens when
>> someone sends a message with the same message-id but a different
>> session-key? it seems like the user can potentially lose access to the
>> encrypted message.
>
> yep!  I even have that case in my own mailbox due to messages i've sent
> to schleuder encrypted mailing lists to which i'm also subscribed.
>
> It works fine.  notmuch stashes both session keys against the message-id
> (you can have multiple properties with the same name as long as they
> have different values).  And upon decryption, it tries each session-key
> in succession.  This is a little bit sloppy (maybe it would be less
> sloppy to associate each message key with each version of the message
> somehow?), but it's significantly simpler and basically unnoticeable
> compared to the speedup gains provided by the rest of the series.
>
>          --dkg

Great!

d

[PATCH 06/18] lib: convert notmuch decryption policy to an enum

[Daniel Kahn Gillmor]

Future patches in this series will introduce new policies; this merely
readies the way for them.

We also convert --try-decrypt to a keyword argument instead of a boolean.
---
 lib/index.cc                  |  2 +-
 lib/indexopts.c               |  7 ++++---
 lib/notmuch.h                 | 14 ++++++++++++--
 mime-node.c                   |  4 ++--
 notmuch-client.h              |  9 +++++----
 notmuch-reply.c               |  6 +++++-
 notmuch-show.c                | 10 +++++++---
 notmuch.c                     | 13 ++++++++-----
 test/T357-index-decryption.sh | 12 ++++++------
 util/crypto.h                 |  2 +-
 10 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 6eb60f30..483fe31d 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -525,7 +525,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     notmuch_database_t * notmuch = NULL;
     GMimeObject *clear = NULL;
 
-    if (!indexopts || !notmuch_indexopts_get_try_decrypt (indexopts))
+    if (!indexopts || (notmuch_indexopts_get_try_decrypt (indexopts) == NOTMUCH_DECRYPT_FALSE))
 	return;
 
     notmuch = _notmuch_message_database (message);
diff --git a/lib/indexopts.c b/lib/indexopts.c
index 15c31d24..23557143 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -26,6 +26,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
     notmuch_indexopts_t *ret = talloc_zero (db, notmuch_indexopts_t);
     if (!ret)
 	return ret;
+    ret->crypto.decrypt = NOTMUCH_DECRYPT_FALSE;
 
     char * try_decrypt;
     notmuch_status_t err = notmuch_database_get_config (db, "index.try_decrypt", &try_decrypt);
@@ -36,7 +37,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
 	((!(strcasecmp(try_decrypt, "true"))) ||
 	 (!(strcasecmp(try_decrypt, "yes"))) ||
 	 (!(strcasecmp(try_decrypt, "1")))))
-	notmuch_indexopts_set_try_decrypt (ret, true);
+	notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_TRUE);
 
     free (try_decrypt);
     return ret;
@@ -44,7 +45,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
 
 notmuch_status_t
 notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
-				   notmuch_bool_t try_decrypt)
+				   notmuch_decryption_policy_t try_decrypt)
 {
     if (!indexopts)
 	return NOTMUCH_STATUS_NULL_POINTER;
@@ -52,7 +53,7 @@ notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
     return NOTMUCH_STATUS_SUCCESS;
 }
 
-notmuch_bool_t
+notmuch_decryption_policy_t
 notmuch_indexopts_get_try_decrypt (const notmuch_indexopts_t *indexopts)
 {
     if (!indexopts)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 2c5dcab5..26fe8f81 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2233,6 +2233,16 @@ notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 notmuch_indexopts_t *
 notmuch_database_get_default_indexopts (notmuch_database_t *db);
 
+/**
+ * Stating a policy about how to decrypt messages.
+ *
+ * See index.try_decrypt in notmuch-config(1) for more details.
+ */
+typedef enum {
+    NOTMUCH_DECRYPT_FALSE,
+    NOTMUCH_DECRYPT_TRUE,
+} notmuch_decryption_policy_t;
+
 /**
  * Specify whether to decrypt encrypted parts while indexing.
  *
@@ -2245,7 +2255,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db);
  */
 notmuch_status_t
 notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
-				   notmuch_bool_t try_decrypt);
+				   notmuch_decryption_policy_t try_decrypt);
 
 /**
  * Return whether to decrypt encrypted parts while indexing.
@@ -2253,7 +2263,7 @@ notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
  *
  * @since libnotmuch 5.1 (notmuch 0.26)
  */
-notmuch_bool_t
+notmuch_decryption_policy_t
 notmuch_indexopts_get_try_decrypt (const notmuch_indexopts_t *indexopts);
 
 /**
diff --git a/mime-node.c b/mime-node.c
index 09170bfd..eb6a16c0 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -270,7 +270,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
     }
 
 #if (GMIME_MAJOR_VERSION < 3)
-    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt)
+    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE))
 	|| (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");
@@ -286,7 +286,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 #endif
 
     /* Handle PGP/MIME parts */
-    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt) {
+    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE)) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 4, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
diff --git a/notmuch-client.h b/notmuch-client.h
index f7524e59..79b826c1 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -414,9 +414,10 @@ struct mime_node {
 
 /* Construct a new MIME node pointing to the root message part of
  * message. If crypto->verify is true, signed child parts will be
- * verified. If crypto->decrypt is true, encrypted child parts will be
- * decrypted.  If the crypto contexts (crypto->gpgctx or
- * crypto->pkcs7) are NULL, they will be lazily initialized.
+ * verified. If crypto->decrypt is NOTMUCH_DECRYPT_TRUE, encrypted
+ * child parts will be decrypted.  If the crypto contexts
+ * (crypto->gpgctx or crypto->pkcs7) are NULL, they will be lazily
+ * initialized.
  *
  * Return value:
  *
@@ -500,7 +501,7 @@ int notmuch_minimal_options (const char* subcommand_name,
 /* the state chosen by the user invoking one of the notmuch
  * subcommands that does indexing */
 struct _notmuch_client_indexing_cli_choices {
-    bool try_decrypt;
+    int try_decrypt;
     bool try_decrypt_set;
     notmuch_indexopts_t * opts;
 };
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2c7cc4eb..eec34bed 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -700,9 +700,11 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
     notmuch_show_params_t params = {
 	.part = -1,
+	.crypto = { .decrypt = NOTMUCH_DECRYPT_FALSE },
     };
     int format = FORMAT_DEFAULT;
     int reply_all = true;
+    bool decrypt = false;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -716,7 +718,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "all", true },
 				  { "sender", false },
 				  { 0, 0 } } },
-	{ .opt_bool = &params.crypto.decrypt, .name = "decrypt" },
+	{ .opt_bool = &decrypt, .name = "decrypt" },
 	{ .opt_inherit = notmuch_shared_options },
 	{ }
     };
@@ -726,6 +728,8 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
 
     notmuch_process_shared_options (argv[0]);
+    if (decrypt)
+	params.crypto.decrypt = NOTMUCH_DECRYPT_TRUE;
 
     notmuch_exit_if_unsupported_format ();
 
diff --git a/notmuch-show.c b/notmuch-show.c
index 7afd3947..7ee9685a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1083,11 +1083,13 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	.part = -1,
 	.omit_excluded = true,
 	.output_body = true,
+	.crypto = { .decrypt = NOTMUCH_DECRYPT_FALSE },
     };
     int format = NOTMUCH_FORMAT_NOT_SPECIFIED;
     bool exclude = true;
     bool entire_thread_set = false;
     bool single_message;
+    bool decrypt = false;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -1102,7 +1104,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_bool = &params.entire_thread, .name = "entire-thread",
 	  .present = &entire_thread_set },
 	{ .opt_int = &params.part, .name = "part" },
-	{ .opt_bool = &params.crypto.decrypt, .name = "decrypt" },
+	{ .opt_bool = &decrypt, .name = "decrypt" },
 	{ .opt_bool = &params.crypto.verify, .name = "verify" },
 	{ .opt_bool = &params.output_body, .name = "body" },
 	{ .opt_bool = &params.include_html, .name = "include-html" },
@@ -1116,9 +1118,11 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_process_shared_options (argv[0]);
 
-    /* decryption implies verification */
-    if (params.crypto.decrypt)
+    if (decrypt) {
+	params.crypto.decrypt = NOTMUCH_DECRYPT_TRUE;
+	/* decryption implies verification */
 	params.crypto.verify = true;
+    }
 
     /* specifying a part implies single message display */
     single_message = params.part >= 0;
diff --git a/notmuch.c b/notmuch.c
index 539ac58c..1abd8288 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -99,8 +99,11 @@ int notmuch_minimal_options (const char *subcommand_name,
 
 struct _notmuch_client_indexing_cli_choices indexing_cli_choices = { };
 const notmuch_opt_desc_t  notmuch_shared_indexing_options [] = {
-    { .opt_bool = &indexing_cli_choices.try_decrypt,
-      .present = &indexing_cli_choices.try_decrypt_set,
+    { .opt_keyword = &indexing_cli_choices.try_decrypt,
+      .present = &indexing_cli_choices.try_decrypt_set, .keywords =
+      (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
+			      { "true", NOTMUCH_DECRYPT_TRUE },
+			      { 0, 0 } },
       .name = "try-decrypt" },
     { }
 };
@@ -117,15 +120,15 @@ notmuch_process_shared_indexing_options (notmuch_database_t *notmuch, g_mime_3_u
 	    return NOTMUCH_STATUS_OUT_OF_MEMORY;
 	status = notmuch_indexopts_set_try_decrypt (indexing_cli_choices.opts, indexing_cli_choices.try_decrypt);
 	if (status != NOTMUCH_STATUS_SUCCESS) {
-	    fprintf (stderr, "Error: Failed to set try_decrypt to %s. (%s)\n",
-		     indexing_cli_choices.try_decrypt ? "True" : "False", notmuch_status_to_string (status));
+	    fprintf (stderr, "Error: Failed to set try_decrypt to %d. (%s)\n",
+		     indexing_cli_choices.try_decrypt, notmuch_status_to_string (status));
 	    notmuch_indexopts_destroy (indexing_cli_choices.opts);
 	    indexing_cli_choices.opts = NULL;
 	    return status;
 	}
     }
 #if (GMIME_MAJOR_VERSION < 3)
-    if (indexing_cli_choices.opts && notmuch_indexopts_get_try_decrypt (indexing_cli_choices.opts)) {
+    if (indexing_cli_choices.opts && notmuch_indexopts_get_try_decrypt (indexing_cli_choices.opts) == NOTMUCH_DECRYPT_TRUE) {
 	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
 	if (gpg_path && strcmp(gpg_path, "gpg"))
 	    fprintf (stderr, "Warning: deprecated crypto.gpg_path is set to '%s'\n"
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 11ea2074..670d1ae9 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -71,8 +71,8 @@ test_expect_equal \
 
 # try reinserting it with decryption, should appear again, but now we
 # have two copies of the message:
-test_begin_subtest "message cleartext is present after reinserting with --try-decrypt"
-notmuch insert --folder=sent --try-decrypt <<<"$contents"
+test_begin_subtest "message cleartext is present after reinserting with --try-decrypt=true"
+notmuch insert --folder=sent --try-decrypt=true <<<"$contents"
 output=$(notmuch search wumpus)
 expected='thread:0000000000000003   2000-01-01 [1/1(2)] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
 test_expect_equal \
@@ -93,8 +93,8 @@ test_expect_equal \
 # try inserting it with decryption, should appear as a single copy
 # (note: i think thread id skips 4 because of duplicate message-id
 # insertion, above)
-test_begin_subtest "message cleartext is present with insert --try-decrypt"
-notmuch insert --folder=sent --try-decrypt <<<"$contents"
+test_begin_subtest "message cleartext is present with insert --try-decrypt=true"
+notmuch insert --folder=sent --try-decrypt=true <<<"$contents"
 output=$(notmuch search wumpus)
 expected='thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
 test_expect_equal \
@@ -159,7 +159,7 @@ test_expect_equal \
 add_email_corpus crypto
 
 test_begin_subtest "indexing message fails when secret key not available"
-notmuch reindex --try-decrypt id:simple-encrypted@crypto.notmuchmail.org
+notmuch reindex --try-decrypt=true id:simple-encrypted@crypto.notmuchmail.org
 output=$(notmuch dump )
 expected='#notmuch-dump batch-tag:3 config,properties,tags
 +encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org
@@ -180,7 +180,7 @@ notmuch restore <<EOF
 #notmuch-dump batch-tag:3 config,properties,tags
 #= simple-encrypted@crypto.notmuchmail.org session-key=9%3AFC09987F5F927CC0CC0EE80A96E4C5BBF4A499818FB591207705DFDDD6112CF9
 EOF
-notmuch reindex --try-decrypt id:simple-encrypted@crypto.notmuchmail.org
+notmuch reindex --try-decrypt=true id:simple-encrypted@crypto.notmuchmail.org
 output=$(notmuch search sekrit)
 expected='thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)'
 test_expect_equal \
diff --git a/util/crypto.h b/util/crypto.h
index 0c62ac70..b23ca747 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -7,7 +7,7 @@
 
 typedef struct _notmuch_crypto {
     bool verify;
-    bool decrypt;
+    notmuch_decryption_policy_t decrypt;
 #if (GMIME_MAJOR_VERSION < 3)
     GMimeCryptoContext* gpgctx;
     GMimeCryptoContext* pkcs7ctx;
-- 
2.14.2

[PATCH 07/18] crypto: new decryption policy "auto"

[Daniel Kahn Gillmor]

This new automatic decryption policy should make it possible to
decrypt messages that we have stashed session keys for, without
incurring a call to the user's asymmetric keys.
---
 doc/man1/notmuch-config.rst   | 11 ++++++++---
 lib/index.cc                  |  3 ++-
 lib/indexopts.c               | 13 ++++++++-----
 lib/notmuch.h                 |  1 +
 mime-node.c                   |  7 ++++---
 notmuch-client.h              |  4 +++-
 notmuch.c                     |  3 ++-
 test/T357-index-decryption.sh | 12 +++++++++++-
 util/crypto.c                 |  8 +++++++-
 util/crypto.h                 |  3 ++-
 10 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 6961737f..14642062 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -142,9 +142,14 @@ The available configuration items are described below.
 
         **[STORED IN DATABASE]**
         When indexing an encrypted e-mail message, if this variable is
-        set to true, notmuch will try to decrypt the message and index
-        the cleartext.  Be aware that the index is likely sufficient
-        to reconstruct the cleartext of the message itself, so please
+        set to ``true``, notmuch will try to decrypt the message and
+        index the cleartext.  If ``auto``, it will try to index the
+        cleartext if a stashed session key is already known for the message,
+        but will not try to access your secret keys.  Use ``false`` to
+        avoid decrypting even when a session key is already known.
+
+        Be aware that the notmuch index is likely sufficient to
+        reconstruct the cleartext of the message itself, so please
         ensure that the notmuch message index is adequately protected.
         DO NOT USE ``index.try_decrypt=true`` without considering the
         security of your index.
diff --git a/lib/index.cc b/lib/index.cc
index 483fe31d..d9a0018c 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -548,7 +548,8 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 	}
     }
 #endif
-    clear = _notmuch_crypto_decrypt (message, crypto_ctx, encrypted_data, NULL, &err);
+    clear = _notmuch_crypto_decrypt (notmuch_indexopts_get_try_decrypt (indexopts),
+				     message, crypto_ctx, encrypted_data, NULL, &err);
     if (err) {
 	_notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n",
 			       err->domain, err->code, err->message);
diff --git a/lib/indexopts.c b/lib/indexopts.c
index 23557143..93a2c9eb 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -33,11 +33,14 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
     if (err)
 	return ret;
 
-    if (try_decrypt &&
-	((!(strcasecmp(try_decrypt, "true"))) ||
-	 (!(strcasecmp(try_decrypt, "yes"))) ||
-	 (!(strcasecmp(try_decrypt, "1")))))
-	notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_TRUE);
+    if (try_decrypt) {
+	if ((!(strcasecmp(try_decrypt, "true"))) ||
+	    (!(strcasecmp(try_decrypt, "yes"))) ||
+	    (!(strcasecmp(try_decrypt, "1"))))
+	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_TRUE);
+	else if (!strcasecmp(try_decrypt, "auto"))
+	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_AUTO);
+    }
 
     free (try_decrypt);
     return ret;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 26fe8f81..7b1c61ad 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2241,6 +2241,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db);
 typedef enum {
     NOTMUCH_DECRYPT_FALSE,
     NOTMUCH_DECRYPT_TRUE,
+    NOTMUCH_DECRYPT_AUTO,
 } notmuch_decryption_policy_t;
 
 /**
diff --git a/mime-node.c b/mime-node.c
index eb6a16c0..815c1787 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -205,7 +205,8 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 		break;
 
 	node->decrypt_attempted = true;
-	node->decrypted_child = _notmuch_crypto_decrypt (parent ? parent->envelope_file : NULL,
+	node->decrypted_child = _notmuch_crypto_decrypt (node->ctx->crypto->decrypt,
+							 parent ? parent->envelope_file : NULL,
 							 cryptoctx, encrypteddata, &decrypt_result, &err);
     }
     if (! node->decrypted_child) {
@@ -270,7 +271,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
     }
 
 #if (GMIME_MAJOR_VERSION < 3)
-    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE))
+    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt != NOTMUCH_DECRYPT_FALSE))
 	|| (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");
@@ -286,7 +287,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 #endif
 
     /* Handle PGP/MIME parts */
-    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE)) {
+    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt != NOTMUCH_DECRYPT_FALSE)) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 4, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
diff --git a/notmuch-client.h b/notmuch-client.h
index 79b826c1..6aec9974 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -415,7 +415,9 @@ struct mime_node {
 /* Construct a new MIME node pointing to the root message part of
  * message. If crypto->verify is true, signed child parts will be
  * verified. If crypto->decrypt is NOTMUCH_DECRYPT_TRUE, encrypted
- * child parts will be decrypted.  If the crypto contexts
+ * child parts will be decrypted using either stored session keys or
+ * asymmetric crypto.  If crypto->decrypt is NOTMUCH_DECRYPT_AUTO,
+ * only session keys will be tried.  If the crypto contexts
  * (crypto->gpgctx or crypto->pkcs7) are NULL, they will be lazily
  * initialized.
  *
diff --git a/notmuch.c b/notmuch.c
index 1abd8288..62706748 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -103,6 +103,7 @@ const notmuch_opt_desc_t  notmuch_shared_indexing_options [] = {
       .present = &indexing_cli_choices.try_decrypt_set, .keywords =
       (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
 			      { "true", NOTMUCH_DECRYPT_TRUE },
+			      { "auto", NOTMUCH_DECRYPT_AUTO },
 			      { 0, 0 } },
       .name = "try-decrypt" },
     { }
@@ -128,7 +129,7 @@ notmuch_process_shared_indexing_options (notmuch_database_t *notmuch, g_mime_3_u
 	}
     }
 #if (GMIME_MAJOR_VERSION < 3)
-    if (indexing_cli_choices.opts && notmuch_indexopts_get_try_decrypt (indexing_cli_choices.opts) == NOTMUCH_DECRYPT_TRUE) {
+    if (indexing_cli_choices.opts && notmuch_indexopts_get_try_decrypt (indexing_cli_choices.opts) != NOTMUCH_DECRYPT_FALSE) {
 	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
 	if (gpg_path && strcmp(gpg_path, "gpg"))
 	    fprintf (stderr, "Warning: deprecated crypto.gpg_path is set to '%s'\n"
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 670d1ae9..d453d568 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -140,6 +140,16 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+# ensure no session keys are present:
+test_begin_subtest 'reindex using only session keys'
+test_expect_success 'notmuch reindex --try-decrypt=auto tag:encrypted and property:index.decryption=success'
+test_begin_subtest "reindexed encrypted messages, decrypting only with session keys"
+output=$(notmuch search wumpus)
+expected=''
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 # and the same search, but by property ($expected is untouched):
 test_begin_subtest "emacs search by property with both messages unindexed"
 output=$(notmuch search property:index.decryption=success)
@@ -180,7 +190,7 @@ notmuch restore <<EOF
 #notmuch-dump batch-tag:3 config,properties,tags
 #= simple-encrypted@crypto.notmuchmail.org session-key=9%3AFC09987F5F927CC0CC0EE80A96E4C5BBF4A499818FB591207705DFDDD6112CF9
 EOF
-notmuch reindex --try-decrypt=true id:simple-encrypted@crypto.notmuchmail.org
+notmuch reindex --try-decrypt=auto id:simple-encrypted@crypto.notmuchmail.org
 output=$(notmuch search sekrit)
 expected='thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)'
 test_expect_equal \
diff --git a/util/crypto.c b/util/crypto.c
index e014db5d..9789f203 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -140,13 +140,16 @@ void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t *crypto))
 #endif
 
 GMimeObject *
-_notmuch_crypto_decrypt (notmuch_message_t *message,
+_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
+			 notmuch_message_t *message,
 			 g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
 			 GMimeMultipartEncrypted *part,
 			 GMimeDecryptResult **decrypt_result,
 			 GError **err)
 {
     GMimeObject *ret = NULL;
+    if (decrypt == NOTMUCH_DECRYPT_FALSE)
+	return NULL;
 
     /* the versions of notmuch that can support session key decryption */
 #if (GMIME_MAJOR_VERSION >= 3 || (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21))
@@ -176,6 +179,9 @@ _notmuch_crypto_decrypt (notmuch_message_t *message,
     }
 #endif
 
+    if (decrypt == NOTMUCH_DECRYPT_AUTO)
+	return ret;
+
 #if (GMIME_MAJOR_VERSION < 3)
     ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
 					     decrypt_result, err);
diff --git a/util/crypto.h b/util/crypto.h
index b23ca747..dc95b4ca 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
 } _notmuch_crypto_t;
 
 GMimeObject *
-_notmuch_crypto_decrypt (notmuch_message_t *message,
+_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
+			 notmuch_message_t *message,
 			 GMimeCryptoContext* crypto_ctx,
 			 GMimeMultipartEncrypted *part,
 			 GMimeDecryptResult **decrypt_result,
-- 
2.14.2

Re: [PATCH 07/18] crypto: new decryption policy "auto"

[Jameson Graef Rollins]

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

On Wed, Oct 25 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> diff --git a/util/crypto.h b/util/crypto.h
> index b23ca747..dc95b4ca 100644
> --- a/util/crypto.h
> +++ b/util/crypto.h
> @@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
>  } _notmuch_crypto_t;
>  
>  GMimeObject *
> -_notmuch_crypto_decrypt (notmuch_message_t *message,
> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
> +			 notmuch_message_t *message,
>  			 GMimeCryptoContext* crypto_ctx,
>  			 GMimeMultipartEncrypted *part,
>  			 GMimeDecryptResult **decrypt_result,

Why does _notmuch_crypt_decrypt need to have
"notmuch_decryption_policy_t decrypt" as an input argument?  Isn't
notmuch_decryption_policy_t already an attribute of the crypto_ctx?  Is
there some situation where the policy would differ from what's specified
in the crypto_ctx?

jamie.

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

Re: [PATCH 07/18] crypto: new decryption policy "auto"

[Daniel Kahn Gillmor]

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

On Sat 2017-11-11 15:14:03 -0800, Jameson Graef Rollins wrote:
> On Wed, Oct 25 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
>> diff --git a/util/crypto.h b/util/crypto.h
>> index b23ca747..dc95b4ca 100644
>> --- a/util/crypto.h
>> +++ b/util/crypto.h
>> @@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
>>  } _notmuch_crypto_t;
>>  
>>  GMimeObject *
>> -_notmuch_crypto_decrypt (notmuch_message_t *message,
>> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
>> +			 notmuch_message_t *message,
>>  			 GMimeCryptoContext* crypto_ctx,
>>  			 GMimeMultipartEncrypted *part,
>>  			 GMimeDecryptResult **decrypt_result,
>
> Why does _notmuch_crypt_decrypt need to have
> "notmuch_decryption_policy_t decrypt" as an input argument?  Isn't
> notmuch_decryption_policy_t already an attribute of the crypto_ctx?  Is
> there some situation where the policy would differ from what's specified
> in the crypto_ctx?

crypto_ctx here is just a GMimeCryptoContext, which doesn't know
anything about notmuch_decryption_policy_t.  Maybe i'm misunderstanding
your question?

I'd be happy to streamline the interface to this internal function, but
given that it's not an exported API, i'm not as concerned about things
like future cleanliness -- the notmuch source contains all invocations
of the function anywhere, so if we find a nicer way to streamline it in
the future, we can do that cleanup across the codebase in a single
commit.

Thanks for the review!

        --dkg

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

Re: [PATCH 07/18] crypto: new decryption policy "auto"

[Jameson Graef Rollins]

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

On Sun, Nov 12 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> On Sat 2017-11-11 15:14:03 -0800, Jameson Graef Rollins wrote:
>> On Wed, Oct 25 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
>>> diff --git a/util/crypto.h b/util/crypto.h
>>> index b23ca747..dc95b4ca 100644
>>> --- a/util/crypto.h
>>> +++ b/util/crypto.h
>>> @@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
>>>  } _notmuch_crypto_t;
>>>  
>>>  GMimeObject *
>>> -_notmuch_crypto_decrypt (notmuch_message_t *message,
>>> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
>>> +			 notmuch_message_t *message,
>>>  			 GMimeCryptoContext* crypto_ctx,
>>>  			 GMimeMultipartEncrypted *part,
>>>  			 GMimeDecryptResult **decrypt_result,
>>
>> Why does _notmuch_crypt_decrypt need to have
>> "notmuch_decryption_policy_t decrypt" as an input argument?  Isn't
>> notmuch_decryption_policy_t already an attribute of the crypto_ctx?  Is
>> there some situation where the policy would differ from what's specified
>> in the crypto_ctx?
>
> crypto_ctx here is just a GMimeCryptoContext, which doesn't know
> anything about notmuch_decryption_policy_t.  Maybe i'm misunderstanding
> your question?
>
> I'd be happy to streamline the interface to this internal function, but
> given that it's not an exported API, i'm not as concerned about things
> like future cleanliness -- the notmuch source contains all invocations
> of the function anywhere, so if we find a nicer way to streamline it in
> the future, we can do that cleanup across the codebase in a single
> commit.

I guess I'm confusing how things were before, when the crypto_ctx was a
notmuch-defined thing that included the GMimeCryptoContext.

It seems like _notmuch_crypto_t could just hold the GMimeCryptoContext,
as it does for earlier versions of GMime, which would make things easier
to pass around.  But this discussion is tangent to this patch series.

jamie.

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

Re: [PATCH 07/18] crypto: new decryption policy "auto"

[David Bremner]

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

>  GMimeObject *
> -_notmuch_crypto_decrypt (notmuch_message_t *message,
> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
> +			 notmuch_message_t *message,
>  			 g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
>  			 GMimeMultipartEncrypted *part,
>  			 GMimeDecryptResult **decrypt_result,
>  			 GError **err)
>  {
>      GMimeObject *ret = NULL;
> +    if (decrypt == NOTMUCH_DECRYPT_FALSE)
> +	return NULL;

I'm going to assume that all is well and no return value from
_notmuch_crypto_decrypt is used without guarding for NULL.

d

[PATCH 08/18] cli/reply: use decryption policy "auto" by default.

[Daniel Kahn Gillmor]

If the user doesn't specify --decrypt= at all, but a stashed session
key is known to notmuch, when replying to an encrypted message,
notmuch should just go ahead and decrypt.

The user can disable this at the command line with --decrypt=false,
though it's not clear why they would ever want to do that.
---
 completion/notmuch-completion.bash | 6 +++++-
 doc/man1/notmuch-reply.rst         | 6 +++++-
 notmuch-reply.c                    | 9 +++++----
 test/T357-index-decryption.sh      | 7 +++++++
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 7aae4297..5e408b9d 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -350,12 +350,16 @@ _notmuch_reply()
 	    COMPREPLY=( $( compgen -W "all sender" -- "${cur}" ) )
 	    return
 	    ;;
+	--decrypt)
+	    COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
+	    return
+	    ;;
     esac
 
     ! $split &&
     case "${cur}" in
 	-*)
-	    local options="--format= --format-version= --reply-to= --decrypt ${_notmuch_shared_options}"
+	    local options="--format= --format-version= --reply-to= --decrypt= ${_notmuch_shared_options}"
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
 	    ;;
diff --git a/doc/man1/notmuch-reply.rst b/doc/man1/notmuch-reply.rst
index b6aec3c8..ede77930 100644
--- a/doc/man1/notmuch-reply.rst
+++ b/doc/man1/notmuch-reply.rst
@@ -80,8 +80,12 @@ Supported options for **reply** include
         multipart/encrypted part will be replaced by the decrypted
         content.
 
+        If a session key is already known for the message, then it
+        will be decrypted automatically unless the user explicitly
+        sets ``--decrypt=false``.
+
         Decryption expects a functioning **gpg-agent(1)** to provide any
-        needed credentials. Without one, the decryption will fail.
+        needed credentials. Without one, the decryption will likely fail.
 
 See **notmuch-search-terms(7)** for details of the supported syntax for
 <search-terms>.
diff --git a/notmuch-reply.c b/notmuch-reply.c
index eec34bed..fd990a9a 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -700,11 +700,12 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
     notmuch_show_params_t params = {
 	.part = -1,
-	.crypto = { .decrypt = NOTMUCH_DECRYPT_FALSE },
+	.crypto = { .decrypt = NOTMUCH_DECRYPT_AUTO },
     };
     int format = FORMAT_DEFAULT;
     int reply_all = true;
     bool decrypt = false;
+    bool decrypt_set = false;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -718,7 +719,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "all", true },
 				  { "sender", false },
 				  { 0, 0 } } },
-	{ .opt_bool = &decrypt, .name = "decrypt" },
+	{ .opt_bool = &decrypt, .name = "decrypt", .present = &decrypt_set },
 	{ .opt_inherit = notmuch_shared_options },
 	{ }
     };
@@ -728,8 +729,8 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
 
     notmuch_process_shared_options (argv[0]);
-    if (decrypt)
-	params.crypto.decrypt = NOTMUCH_DECRYPT_TRUE;
+    if (decrypt_set)
+	params.crypto.decrypt = decrypt ? NOTMUCH_DECRYPT_TRUE : NOTMUCH_DECRYPT_FALSE;
 
     notmuch_exit_if_unsupported_format ();
 
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index d453d568..61360e42 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -197,6 +197,13 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+test_begin_subtest "notmuch reply should show cleartext if session key is present"
+output=$(notmuch reply id:simple-encrypted@crypto.notmuchmail.org | grep '^>')
+expected='> This is a top sekrit message.'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 
 # TODO: test removal of a message from the message store between
 # indexing and reindexing.
-- 
2.14.2

[PATCH 09/18] cli/show: use decryption policy "auto" by default.

[Daniel Kahn Gillmor]

When showing a message, if the user doesn't specify --decrypt= at all,
but a stashed session key is known to notmuch, notmuch should just go
ahead and try to decrypt the message with the session key (without
bothering the user for access to their asymmetric secret key).

The user can disable this at the command line with --decrypt=false if
they really don't want to look at the e-mail that they've asked
notmuch to show them.

and of course, "notmuch show --decrypt" still works for accessing the
user's secret keys if necessary.
---
 completion/notmuch-completion.bash |  4 ++--
 doc/man1/notmuch-show.rst          |  4 ++++
 notmuch-show.c                     | 17 +++++++++++------
 test/T357-index-decryption.sh      | 14 ++++++++++++++
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 5e408b9d..2703d542 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -517,7 +517,7 @@ _notmuch_show()
 	    COMPREPLY=( $( compgen -W "text json sexp mbox raw" -- "${cur}" ) )
 	    return
 	    ;;
-	--exclude|--body)
+	--exclude|--body|--decrypt)
 	    COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
 	    return
 	    ;;
@@ -526,7 +526,7 @@ _notmuch_show()
     ! $split &&
     case "${cur}" in
 	-*)
-	    local options="--entire-thread= --format= --exclude= --body= --format-version= --part= --verify --decrypt --include-html ${_notmuch_shared_options}"
+	    local options="--entire-thread= --format= --exclude= --body= --format-version= --part= --verify --decrypt= --include-html ${_notmuch_shared_options}"
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
 	    ;;
diff --git a/doc/man1/notmuch-show.rst b/doc/man1/notmuch-show.rst
index 7ba091cf..64caa7a6 100644
--- a/doc/man1/notmuch-show.rst
+++ b/doc/man1/notmuch-show.rst
@@ -123,6 +123,10 @@ Supported options for **show** include
         multipart/encrypted part will be replaced by the decrypted
         content.
 
+        If a session key is already known for the message, then it
+        will be decrypted automatically unless the user explicitly
+        sets ``--decrypt=false``.
+
         Decryption expects a functioning **gpg-agent(1)** to provide any
         needed credentials. Without one, the decryption will fail.
 
diff --git a/notmuch-show.c b/notmuch-show.c
index 7ee9685a..c8f5a48f 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1083,13 +1083,14 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	.part = -1,
 	.omit_excluded = true,
 	.output_body = true,
-	.crypto = { .decrypt = NOTMUCH_DECRYPT_FALSE },
+	.crypto = { .decrypt = NOTMUCH_DECRYPT_AUTO },
     };
     int format = NOTMUCH_FORMAT_NOT_SPECIFIED;
     bool exclude = true;
     bool entire_thread_set = false;
     bool single_message;
     bool decrypt = false;
+    bool decrypt_set = false;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -1104,7 +1105,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_bool = &params.entire_thread, .name = "entire-thread",
 	  .present = &entire_thread_set },
 	{ .opt_int = &params.part, .name = "part" },
-	{ .opt_bool = &decrypt, .name = "decrypt" },
+	{ .opt_bool = &decrypt, .name = "decrypt", .present = &decrypt_set },
 	{ .opt_bool = &params.crypto.verify, .name = "verify" },
 	{ .opt_bool = &params.output_body, .name = "body" },
 	{ .opt_bool = &params.include_html, .name = "include-html" },
@@ -1118,10 +1119,14 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_process_shared_options (argv[0]);
 
-    if (decrypt) {
-	params.crypto.decrypt = NOTMUCH_DECRYPT_TRUE;
-	/* decryption implies verification */
-	params.crypto.verify = true;
+    if (decrypt_set) {
+	if (decrypt) {
+	    params.crypto.decrypt = NOTMUCH_DECRYPT_TRUE;
+	    /* decryption implies verification */
+	    params.crypto.verify = true;
+	} else {
+	    params.crypto.decrypt = NOTMUCH_DECRYPT_FALSE;
+	}
     }
 
     /* specifying a part implies single message display */
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 61360e42..30bdf704 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -204,6 +204,20 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+test_begin_subtest "notmuch show should show cleartext if session key is present"
+output=$(notmuch show id:simple-encrypted@crypto.notmuchmail.org | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }')
+expected='This is a top sekrit message.'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "notmuch show should show nothing if decryption is explicitly disallowed"
+output=$(notmuch show --decrypt=false id:simple-encrypted@crypto.notmuchmail.org | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }')
+expected='Non-text part: application/octet-stream'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 
 # TODO: test removal of a message from the message store between
 # indexing and reindexing.
-- 
2.14.2

[PATCH 10/18] cli/show, reply: document use of stashed session keys in notmuch-properties

[Daniel Kahn Gillmor]

The stashed session keys are stored internally as notmuch properties.
So a user or developer who is reading about those properties might
want to understand how they fit into the bigger picture.

Note here that decrypting with a stored session key no longer needs
-decrypt for "notmuch show" and "notmuch reply".
---
 doc/man7/notmuch-properties.rst | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index 31d7a104..e2ab43be 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -77,9 +77,12 @@ of its normal activity.
 **session-key**
 
     When **notmuch-show(1)** or **nomtuch-reply** encounters a message
-    with an encrypted part and ``--decrypt`` is set, if notmuch finds a
-    ``session-key=`` property associated with the message, it will try
-    that stashed session key for decryption.
+    with an encrypted part, if notmuch finds a ``session-key=``
+    property associated with the message, it will try that stashed
+    session key for decryption.
+
+    If you do not want to use any stashed session keys that might be
+    present, you should pass those programs ``--decrypt=false``.
 
     Using a stashed session key with "notmuch show" will speed up
     rendering of long encrypted threads.  It also allows the user to
-- 
2.14.2

[PATCH 11/18] cli/new, insert, reindex: update documentation for --try-decrypt=auto

[Daniel Kahn Gillmor]

we also include --try-decrypt=auto in the tab completion.
---
 completion/notmuch-completion.bash |  6 +++---
 doc/man1/notmuch-insert.rst        | 16 ++++++++++------
 doc/man1/notmuch-new.rst           | 10 +++++++---
 doc/man1/notmuch-reindex.rst       | 23 ++++++++++++++---------
 4 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 2703d542..53d7380b 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -288,7 +288,7 @@ _notmuch_insert()
 	    return
 	    ;;
 	--try-decrypt)
-	    COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true false auto" -- "${cur}" ) )
 	    return
 	    ;;
     esac
@@ -320,7 +320,7 @@ _notmuch_new()
     $split &&
     case "${prev}" in
 	--try-decrypt)
-	    COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true false auto" -- "${cur}" ) )
 	    return
 	    ;;
     esac
@@ -442,7 +442,7 @@ _notmuch_reindex()
     $split &&
     case "${prev}" in
 	--try-decrypt)
-	    COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true false auto" -- "${cur}" ) )
 	    return
 	    ;;
     esac
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index e2bf37d0..a5505b5b 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -50,14 +50,18 @@ Supported options for **insert** include
     ``--no-hooks``
         Prevent hooks from being run.
 
-    ``--try-decrypt=(true|false)``
+    ``--try-decrypt=(true|auto|false)``
 
-        If true and the message is encrypted, try to decrypt the
-        message while indexing.  If decryption is successful, index
+        If ``true`` and the message is encrypted, try to decrypt the
+        message while indexing.  If ``auto``, and notmuch already
+        knows about a session key for the message, it will try
+        decrypting using that session key but will not try to access
+        the user's secret keys.  If decryption is successful, index
         the cleartext itself.  Either way, the message is always
-        stored to disk in its original form (ciphertext).  Be aware
-        that the index is likely sufficient to reconstruct the
-        cleartext of the message itself, so please ensure that the
+        stored to disk in its original form (ciphertext).
+
+        Be aware that the index is likely sufficient to reconstruct
+        the cleartext of the message itself, so please ensure that the
         notmuch message index is adequately protected. DO NOT USE
         ``--try-decrypt=true`` without considering the security of
         your index.
diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
index bc26aa48..d8cb77f5 100644
--- a/doc/man1/notmuch-new.rst
+++ b/doc/man1/notmuch-new.rst
@@ -43,11 +43,15 @@ Supported options for **new** include
     ``--quiet``
         Do not print progress or results.
 
-    ``--try-decrypt=(true|false)``
+    ``--try-decrypt=(true|auto|false)``
 
-        If true, when encountering an encrypted message, try to
+        If ``true``, when encountering an encrypted message, try to
         decrypt it while indexing.  If decryption is successful, index
-        the cleartext itself.  Be aware that the index is likely
+        the cleartext itself.  If ``auto``, try to use any session key
+        already known to belong to this message, but do not attempt to
+        use the user's secret keys.
+
+        Be aware that the index is likely
         sufficient to reconstruct the cleartext of the message itself,
         so please ensure that the notmuch message index is adequately
         protected.  DO NOT USE ``--try-decrypt=true`` without
diff --git a/doc/man1/notmuch-reindex.rst b/doc/man1/notmuch-reindex.rst
index 21f6c7a9..b15981a2 100644
--- a/doc/man1/notmuch-reindex.rst
+++ b/doc/man1/notmuch-reindex.rst
@@ -21,15 +21,20 @@ messages using the supplied options.
 
 Supported options for **reindex** include
 
-    ``--try-decrypt=(true|false)``
-
-        If true, when encountering an encrypted message, try to
-        decrypt it while reindexing.  If decryption is successful,
-        index the cleartext itself.  Be aware that the index is likely
-        sufficient to reconstruct the cleartext of the message itself,
-        so please ensure that the notmuch message index is adequately
-        protected. DO NOT USE ``--try-decrypt=true`` without
-        considering the security of your index.
+    ``--try-decrypt=(true|auto|false)``
+
+        If ``true``, when encountering an encrypted message, try to
+        decrypt it while reindexing.  If ``auto``, and notmuch already
+        knows about a session key for the message, it will try
+        decrypting using that session key but will not try to access
+        the user's secret keys.  If decryption is successful, index
+        the cleartext itself.
+
+        Be aware that the index is likely sufficient to reconstruct
+        the cleartext of the message itself, so please ensure that the
+        notmuch message index is adequately protected. DO NOT USE
+        ``--try-decrypt=true`` without considering the security of
+        your index.
 
         See also ``index.try_decrypt`` in **notmuch-config(1)**.
 
-- 
2.14.2

Re: [PATCH 11/18] cli/new, insert, reindex: update documentation for --try-decrypt=auto

[David Bremner]

>  
> -    ``--try-decrypt=(true|false)``
> +    ``--try-decrypt=(true|auto|false)``

I sympathize with Jamie's point about consistency here. I'm
not sure if this was already mentioned, but the inconsistency between
boolean and keyword arguments is a bit confusing also.

[PATCH 12/18] crypto: record whether an actual decryption attempt happened

[Daniel Kahn Gillmor]

In our consolidation of _notmuch_crypto_decrypt, the callers lost
track a little bit of whether any actual decryption was attempted.

Now that we have the more-subtle "auto" policy, it's possible that
_notmuch_crypto_decrypt could be called without having any actual
decryption take place.

This change lets the callers be a little bit smarter about whether or
not any decryption was actually attempted.
---
 lib/index.cc  | 17 ++++++++++++-----
 mime-node.c   |  4 ++--
 util/crypto.c |  7 ++++++-
 util/crypto.h |  3 ++-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index d9a0018c..e2701755 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -548,12 +548,19 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 	}
     }
 #endif
-    clear = _notmuch_crypto_decrypt (notmuch_indexopts_get_try_decrypt (indexopts),
+    bool attempted = false;
+    clear = _notmuch_crypto_decrypt (&attempted, notmuch_indexopts_get_try_decrypt (indexopts),
 				     message, crypto_ctx, encrypted_data, NULL, &err);
-    if (err) {
-	_notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n",
-			       err->domain, err->code, err->message);
-	g_error_free(err);
+    if (!attempted)
+	return;
+    if (err || !clear) {
+	if (err) {
+	    _notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n",
+				   err->domain, err->code, err->message);
+	    g_error_free(err);
+	} else {
+	    _notmuch_database_log (notmuch, "Failed to decrypt during indexing. (unknown error)\n");
+	}
 	/* Indicate that we failed to decrypt during indexing */
 	status = notmuch_message_add_property (message, "index.decryption", "failure");
 	if (status)
diff --git a/mime-node.c b/mime-node.c
index 815c1787..6d3d5f69 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -204,8 +204,8 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 	    if (parent->envelope_file)
 		break;
 
-	node->decrypt_attempted = true;
-	node->decrypted_child = _notmuch_crypto_decrypt (node->ctx->crypto->decrypt,
+	node->decrypted_child = _notmuch_crypto_decrypt (&node->decrypt_attempted,
+							 node->ctx->crypto->decrypt,
 							 parent ? parent->envelope_file : NULL,
 							 cryptoctx, encrypteddata, &decrypt_result, &err);
     }
diff --git a/util/crypto.c b/util/crypto.c
index 9789f203..ae6b94be 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -140,7 +140,8 @@ void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t *crypto))
 #endif
 
 GMimeObject *
-_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
+_notmuch_crypto_decrypt (bool *attempted,
+			 notmuch_decryption_policy_t decrypt,
 			 notmuch_message_t *message,
 			 g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
 			 GMimeMultipartEncrypted *part,
@@ -158,6 +159,8 @@ _notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
 
 	for (list = notmuch_message_get_properties (message, "session-key", TRUE);
 	     notmuch_message_properties_valid (list); notmuch_message_properties_move_to_next (list)) {
+	    if (attempted)
+		*attempted = true;
 #if (GMIME_MAJOR_VERSION < 3)
 	    ret = g_mime_multipart_encrypted_decrypt_session (part,
 							      crypto_ctx,
@@ -182,6 +185,8 @@ _notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
     if (decrypt == NOTMUCH_DECRYPT_AUTO)
 	return ret;
 
+    if (attempted)
+	*attempted = true;
 #if (GMIME_MAJOR_VERSION < 3)
     ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
 					     decrypt_result, err);
diff --git a/util/crypto.h b/util/crypto.h
index dc95b4ca..c384601c 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
 } _notmuch_crypto_t;
 
 GMimeObject *
-_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
+_notmuch_crypto_decrypt (bool *attempted,
+			 notmuch_decryption_policy_t decrypt,
 			 notmuch_message_t *message,
 			 GMimeCryptoContext* crypto_ctx,
 			 GMimeMultipartEncrypted *part,
-- 
2.14.2

[PATCH 13/18] cli/new, insert, reindex: change index.try_decrypt to "auto" by default

[Daniel Kahn Gillmor]

The new "auto" decryption policy is not only good for "notmuch show"
and "notmuch reindex".  It's also useful for indexing messages --
there's no good reason to not try to go ahead and index the cleartext
of a message that we have a stashed session key for.

This change updates the defaults and tunes the test suite to make sure
that they have taken effect.
---
 doc/man1/notmuch-config.rst   | 2 +-
 lib/indexopts.c               | 8 +++++---
 test/T357-index-decryption.sh | 4 ++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 14642062..ae7586f3 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -154,7 +154,7 @@ The available configuration items are described below.
         DO NOT USE ``index.try_decrypt=true`` without considering the
         security of your index.
 
-        Default: ``false``.
+        Default: ``auto``.
 
     **built_with.<name>**
 
diff --git a/lib/indexopts.c b/lib/indexopts.c
index 93a2c9eb..a61d6420 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -26,7 +26,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
     notmuch_indexopts_t *ret = talloc_zero (db, notmuch_indexopts_t);
     if (!ret)
 	return ret;
-    ret->crypto.decrypt = NOTMUCH_DECRYPT_FALSE;
+    ret->crypto.decrypt = NOTMUCH_DECRYPT_AUTO;
 
     char * try_decrypt;
     notmuch_status_t err = notmuch_database_get_config (db, "index.try_decrypt", &try_decrypt);
@@ -38,8 +38,10 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
 	    (!(strcasecmp(try_decrypt, "yes"))) ||
 	    (!(strcasecmp(try_decrypt, "1"))))
 	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_TRUE);
-	else if (!strcasecmp(try_decrypt, "auto"))
-	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_AUTO);
+	else if ((!(strcasecmp(try_decrypt, "false"))) ||
+		 (!(strcasecmp(try_decrypt, "no"))) ||
+		 (!(strcasecmp(try_decrypt, "0"))))
+	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_FALSE);
     }
 
     free (try_decrypt);
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 30bdf704..7fc59f1e 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -142,7 +142,7 @@ test_expect_equal \
 
 # ensure no session keys are present:
 test_begin_subtest 'reindex using only session keys'
-test_expect_success 'notmuch reindex --try-decrypt=auto tag:encrypted and property:index.decryption=success'
+test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success'
 test_begin_subtest "reindexed encrypted messages, decrypting only with session keys"
 output=$(notmuch search wumpus)
 expected=''
@@ -190,7 +190,7 @@ notmuch restore <<EOF
 #notmuch-dump batch-tag:3 config,properties,tags
 #= simple-encrypted@crypto.notmuchmail.org session-key=9%3AFC09987F5F927CC0CC0EE80A96E4C5BBF4A499818FB591207705DFDDD6112CF9
 EOF
-notmuch reindex --try-decrypt=auto id:simple-encrypted@crypto.notmuchmail.org
+notmuch reindex id:simple-encrypted@crypto.notmuchmail.org
 output=$(notmuch search sekrit)
 expected='thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)'
 test_expect_equal \
-- 
2.14.2

Re: [PATCH 13/18] cli/new, insert, reindex: change index.try_decrypt to "auto" by default

[David Bremner]

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

> The new "auto" decryption policy is not only good for "notmuch show"
> and "notmuch reindex".  It's also useful for indexing messages --
> there's no good reason to not try to go ahead and index the cleartext
> of a message that we have a stashed session key for.

I'm confused here. You talk about indexing other than reindex, but the
only tests that change are reindex? Is this meant to change "notmuch
new" behaviour?

d

Re: [PATCH 13/18] cli/new, insert, reindex: change index.try_decrypt to "auto" by default

[Daniel Kahn Gillmor]

On Thu 2017-11-16 08:40:41 -0400, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> The new "auto" decryption policy is not only good for "notmuch show"
>> and "notmuch reindex".  It's also useful for indexing messages --
>> there's no good reason to not try to go ahead and index the cleartext
>> of a message that we have a stashed session key for.
>
> I'm confused here. You talk about indexing other than reindex, but the
> only tests that change are reindex? Is this meant to change "notmuch
> new" behaviour?

the "auto" policy won't change the behavior of notmuch upon seeing a new
message (new, insert) from "false" -- all it does differently from
"false" is try to use session keys when they are available (and there's
no way for them to be available on a never-before-seen message).

   --dkg

[PATCH 14/18] cli/reindex: destroy stashed session keys when --try-decrypt=false

[Daniel Kahn Gillmor]

There are some situations where the user wants to get rid of the
cleartext index of a message.  For example, if they're indexing
encrypted messages normally, but suddenly they run across a message
that they really don't want any trace of in their index.

In that case, the natural thing to do is:

   notmuch reindex --try-decrypt=false id:whatever@example.biz

But of course, clearing the cleartext index without clearing the
stashed session key is just silly.  So we do the expected thing and
also destroy any stashed session keys while we're destroying the index
of the cleartext.

Note that stashed session keys are stored in the xapian database, but
xapian does not currently allow safe deletion (see
https://trac.xapian.org/ticket/742).

As a workaround, after removing session keys and cleartext material
from the database, the user probably should do something like "notmuch
compact" to try to purge whatever recoverable data is left in the
xapian freelist.  This problem really needs to be addressed within
xapian, though, if we want it fixed right.
---
 doc/man1/notmuch-reindex.rst  |  3 +++
 lib/message.cc                |  5 +++++
 test/T357-index-decryption.sh | 17 +++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/doc/man1/notmuch-reindex.rst b/doc/man1/notmuch-reindex.rst
index b15981a2..6f5e48a9 100644
--- a/doc/man1/notmuch-reindex.rst
+++ b/doc/man1/notmuch-reindex.rst
@@ -30,6 +30,9 @@ Supported options for **reindex** include
         the user's secret keys.  If decryption is successful, index
         the cleartext itself.
 
+        If ``false``, notmuch reindex will also delete any stashed
+        session keys for all messages matching the search terms.
+
         Be aware that the index is likely sufficient to reconstruct
         the cleartext of the message itself, so please ensure that the
         notmuch message index is adequately protected. DO NOT USE
diff --git a/lib/message.cc b/lib/message.cc
index 12743460..cfd99130 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -2002,6 +2002,11 @@ notmuch_message_reindex (notmuch_message_t *message,
     ret = notmuch_message_remove_all_properties_with_prefix (message, "index.");
     if (ret)
 	goto DONE; /* XXX TODO: distinguish from other error returns above? */
+    if (indexopts && notmuch_indexopts_get_try_decrypt (indexopts) == NOTMUCH_DECRYPT_FALSE) {
+	ret = notmuch_message_remove_all_properties (message, "session-key");
+	if (ret)
+	    goto DONE;
+    }
 
     /* re-add the filenames with the associated indexopts */
     for (; notmuch_filenames_valid (orig_filenames);
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 7fc59f1e..4f5a501a 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -218,6 +218,23 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+test_begin_subtest "purging stashed session keys should lose access to the cleartext"
+notmuch reindex --try-decrypt=false id:simple-encrypted@crypto.notmuchmail.org
+output=$(notmuch search sekrit)
+expected=''
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "and cleartext should be unrecoverable now that there are no stashed session keys"
+notmuch dump
+notmuch reindex --try-decrypt=true id:simple-encrypted@crypto.notmuchmail.org
+output=$(notmuch search sekrit)
+expected=''
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 
 # TODO: test removal of a message from the message store between
 # indexing and reindexing.
-- 
2.14.2

[PATCH 15/18] crypto: actually stash session keys when try-decrypt=true

[Daniel Kahn Gillmor]

If you're going to store the cleartext index of an encrypted message,
in most situations you might just as well store the session key.
Doing this storage has efficiency and recoverability advantages.

Combined with a schedule of regular OpenPGP subkey rotation and
destruction, this can also offer security benefits, like "deletable
e-mail", which is the store-and-forward analog to "forward secrecy".

But wait, i hear you saying, i have a special need to store cleartext
indexes but it's really bad for me to store session keys!  Maybe
(let's imagine) i get lots of e-mails with incriminating photos
attached, and i want to be able to search for them by the text in the
e-mail, but i don't want someone with access to the index to be
actually able to see the photos themselves.

Fret not, the next patch in this series will support your wacky
uncommon use case.
---
 doc/man1/notmuch-config.rst     | 10 ++++++----
 doc/man1/notmuch-insert.rst     | 13 +++++++------
 doc/man1/notmuch-new.rst        | 21 +++++++++++----------
 doc/man1/notmuch-reindex.rst    | 10 +++++-----
 doc/man7/notmuch-properties.rst |  4 ++++
 lib/index.cc                    | 16 +++++++++++++++-
 test/T357-index-decryption.sh   | 18 +++++++++++++++++-
 util/crypto.c                   | 16 +++++++++++++++-
 8 files changed, 80 insertions(+), 28 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index ae7586f3..5adde070 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -143,10 +143,12 @@ The available configuration items are described below.
         **[STORED IN DATABASE]**
         When indexing an encrypted e-mail message, if this variable is
         set to ``true``, notmuch will try to decrypt the message and
-        index the cleartext.  If ``auto``, it will try to index the
-        cleartext if a stashed session key is already known for the message,
-        but will not try to access your secret keys.  Use ``false`` to
-        avoid decrypting even when a session key is already known.
+        index the cleartext, stashing a copy of any discovered session
+        keys for the message.  If ``auto``, it will try to index the
+        cleartext if a stashed session key is already known for the message
+        (e.g. from a previous copy), but will not try to access your
+        secret keys.  Use ``false`` to avoid decrypting even when a
+        stashed session key is already present.
 
         Be aware that the notmuch index is likely sufficient to
         reconstruct the cleartext of the message itself, so please
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index a5505b5b..3e6f538d 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -53,12 +53,13 @@ Supported options for **insert** include
     ``--try-decrypt=(true|auto|false)``
 
         If ``true`` and the message is encrypted, try to decrypt the
-        message while indexing.  If ``auto``, and notmuch already
-        knows about a session key for the message, it will try
-        decrypting using that session key but will not try to access
-        the user's secret keys.  If decryption is successful, index
-        the cleartext itself.  Either way, the message is always
-        stored to disk in its original form (ciphertext).
+        message while indexing, storing any session keys discovered.
+        If ``auto``, and notmuch already knows about a session key for
+        the message, it will try decrypting using that session key but
+        will not try to access the user's secret keys.  If decryption
+        is successful, index the cleartext itself.  Either way, the
+        message is always stored to disk in its original form
+        (ciphertext).
 
         Be aware that the index is likely sufficient to reconstruct
         the cleartext of the message itself, so please ensure that the
diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
index d8cb77f5..351b7591 100644
--- a/doc/man1/notmuch-new.rst
+++ b/doc/man1/notmuch-new.rst
@@ -46,16 +46,17 @@ Supported options for **new** include
     ``--try-decrypt=(true|auto|false)``
 
         If ``true``, when encountering an encrypted message, try to
-        decrypt it while indexing.  If decryption is successful, index
-        the cleartext itself.  If ``auto``, try to use any session key
-        already known to belong to this message, but do not attempt to
-        use the user's secret keys.
-
-        Be aware that the index is likely
-        sufficient to reconstruct the cleartext of the message itself,
-        so please ensure that the notmuch message index is adequately
-        protected.  DO NOT USE ``--try-decrypt=true`` without
-        considering the security of your index.
+        decrypt it while indexing, and store any discovered session
+        keys.  If ``auto``, try to use any session key already known
+        to belong to this message, but do not attempt to use the
+        user's secret keys.  If decryption is successful, index the
+        cleartext of the message.
+
+        Be aware that the index is likely sufficient to reconstruct
+        the cleartext of the message itself, so please ensure that the
+        notmuch message index is adequately protected.  DO NOT USE
+        ``--try-decrypt=true`` without considering the security of
+        your index.
 
         See also ``index.try_decrypt`` in **notmuch-config(1)**.
 
diff --git a/doc/man1/notmuch-reindex.rst b/doc/man1/notmuch-reindex.rst
index 6f5e48a9..3b99b934 100644
--- a/doc/man1/notmuch-reindex.rst
+++ b/doc/man1/notmuch-reindex.rst
@@ -24,11 +24,11 @@ Supported options for **reindex** include
     ``--try-decrypt=(true|auto|false)``
 
         If ``true``, when encountering an encrypted message, try to
-        decrypt it while reindexing.  If ``auto``, and notmuch already
-        knows about a session key for the message, it will try
-        decrypting using that session key but will not try to access
-        the user's secret keys.  If decryption is successful, index
-        the cleartext itself.
+        decrypt it while reindexing, storing any session keys
+        discovered.  If ``auto``, and notmuch already knows about a
+        session key for the message, it will try decrypting using that
+        session key but will not try to access the user's secret keys.
+        If decryption is successful, index the cleartext itself.
 
         If ``false``, notmuch reindex will also delete any stashed
         session keys for all messages matching the search terms.
diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst
index e2ab43be..5dcbb4ae 100644
--- a/doc/man7/notmuch-properties.rst
+++ b/doc/man7/notmuch-properties.rst
@@ -98,6 +98,10 @@ of its normal activity.
     message.  This includes attachments, cryptographic signatures, and
     other material that cannot be reconstructed from the index alone.
 
+    See ``index.try_decrypt`` in **notmuch-config(1)** for more
+    details about how to set notmuch's policy on when to store session
+    keys.
+
     The session key should be in the ASCII text form produced by
     GnuPG.  For OpenPGP, that consists of a decimal representation of
     the hash algorithm used (identified by number from RFC 4880,
diff --git a/lib/index.cc b/lib/index.cc
index e2701755..06470919 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -549,11 +549,15 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     }
 #endif
     bool attempted = false;
+    GMimeDecryptResult *decrypt_result = NULL;
+    bool get_sk = (notmuch_indexopts_get_try_decrypt (indexopts) == NOTMUCH_DECRYPT_TRUE);
     clear = _notmuch_crypto_decrypt (&attempted, notmuch_indexopts_get_try_decrypt (indexopts),
-				     message, crypto_ctx, encrypted_data, NULL, &err);
+				     message, crypto_ctx, encrypted_data, get_sk ? &decrypt_result : NULL, &err);
     if (!attempted)
 	return;
     if (err || !clear) {
+	if (decrypt_result)
+	    g_object_unref (decrypt_result);
 	if (err) {
 	    _notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n",
 				   err->domain, err->code, err->message);
@@ -568,6 +572,16 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 					  "property (%d)\n", status);
 	return;
     }
+    if (decrypt_result) {
+	if (get_sk) {
+	    status = notmuch_message_add_property (message, "session-key",
+						   g_mime_decrypt_result_get_session_key (decrypt_result));
+	    if (status)
+		_notmuch_database_log (notmuch, "failed to add session-key "
+				       "property (%d)\n", status);
+	}
+	g_object_unref (decrypt_result);
+    }
     _index_mime_part (message, indexopts, clear);
     g_object_unref (clear);
 
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 4f5a501a..3ac8c43e 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -48,6 +48,14 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+test_begin_subtest "show the message body of the encrypted message"
+notmuch dump wumpus
+output=$(notmuch show wumpus | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }')
+expected='This is a test encrypted message with a wumpus.'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 
 test_begin_subtest "message should go away after deletion"
 # cache the message in an env var and remove it:
@@ -129,10 +137,18 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+# try a simple reindexx
+test_begin_subtest 'reindex in auto mode'
+test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success'
+test_begin_subtest "reindexed encrypted messages, should not have changed"
+output=$(notmuch search wumpus)
+test_expect_equal \
+    "$output" \
+    "$expected"
 
 # try to remove cleartext indexing
 test_begin_subtest 'reindex without cleartext'
-test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success'
+test_expect_success 'notmuch reindex --try-decrypt=false tag:encrypted and property:index.decryption=success'
 test_begin_subtest "reindexed encrypted messages, without cleartext"
 output=$(notmuch search wumpus)
 expected=''
diff --git a/util/crypto.c b/util/crypto.c
index ae6b94be..ba9bfa7b 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -188,10 +188,24 @@ _notmuch_crypto_decrypt (bool *attempted,
     if (attempted)
 	*attempted = true;
 #if (GMIME_MAJOR_VERSION < 3)
+#if (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21)
+    gboolean oldgetsk = g_mime_crypto_context_get_retrieve_session_key (crypto_ctx);
+    gboolean newgetsk = (decrypt_result);
+    if (newgetsk != oldgetsk)
+	/* This could return an error, but we can't do anything about it, so ignore it */
+	g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, newgetsk, NULL);
+#endif
     ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
 					     decrypt_result, err);
+#if (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21)
+    if (newgetsk != oldgetsk)
+	g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, oldgetsk, NULL);
+#endif
 #else
-    ret = g_mime_multipart_encrypted_decrypt(part, GMIME_DECRYPT_NONE, NULL,
+    GMimeDecryptFlags flags = GMIME_DECRYPT_NONE;
+    if (decrypt_result)
+	flags |= GMIME_DECRYPT_EXPORT_SESSION_KEY;
+    ret = g_mime_multipart_encrypted_decrypt(part, flags, NULL,
 					     decrypt_result, err);
 #endif
     return ret;
-- 
2.14.2

Re: [PATCH 15/18] crypto: actually stash session keys when try-decrypt=true

[David Bremner]

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

> +        Be aware that the index is likely sufficient to reconstruct
> +        the cleartext of the message itself, so please ensure that the
> +        notmuch message index is adequately protected.  DO NOT USE
> +        ``--try-decrypt=true`` without considering the security of
> +        your index.
>  

This is probably just my ignorance, but doesn't stashing session keys
change this from likely to certain? Is it possible we decrypt thing and
don't get session keys.

> +test_begin_subtest "show the message body of the encrypted message"
> +notmuch dump wumpus
> +output=$(notmuch show wumpus | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }')
> +expected='This is a test encrypted message with a wumpus.'
> +test_expect_equal \
> +    "$output" \
> +    "$expected"

I'd be happier if we didn't further entrench the text format in the test
suite. How hard would it be to use json output (+maybe python?) here? 

>  	*attempted = true;
>  #if (GMIME_MAJOR_VERSION < 3)
> +#if (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21)
> +    gboolean oldgetsk = g_mime_crypto_context_get_retrieve_session_key (crypto_ctx);
> +    gboolean newgetsk = (decrypt_result);
> +    if (newgetsk != oldgetsk)
> +	/* This could return an error, but we can't do anything about it, so ignore it */
> +	g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, newgetsk, NULL);
> +#endif
>      ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx,
>  					     decrypt_result, err);
> +#if (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21)
> +    if (newgetsk != oldgetsk)
> +	g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, oldgetsk, NULL);

I lost track a bit, but now there's at least 2 (maybe 3) repetitions of
this somewhat complicated test, and one more needed for
built_with.session_keys. HAVE_GMIME_SESSION_KEYS is looking better and
better.

Re: [PATCH 15/18] crypto: actually stash session keys when try-decrypt=true

[Daniel Kahn Gillmor]

On Thu 2017-11-16 08:53:14 -0400, David Bremner wrote:
> I'd be happier if we didn't further entrench the text format in the test
> suite. How hard would it be to use json output (+maybe python?) here? 

json output seems clunkier to me, and i don't think it's necessary for
the purposes of these tests.  Using python here isn't possible without
updating the python bindings to accomodate decryption policy, which
doesn't come until later in the series.

so i'd prefer to leave it as-is, but i wouldn't object if someone wanted
to propose a good patch to these tests that uses json.

          --dkg

Re: [PATCH 15/18] crypto: actually stash session keys when try-decrypt=true

[David Bremner]

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

> On Thu 2017-11-16 08:53:14 -0400, David Bremner wrote:
>> I'd be happier if we didn't further entrench the text format in the test
>> suite. How hard would it be to use json output (+maybe python?) here? 
>
> json output seems clunkier to me, and i don't think it's necessary for
> the purposes of these tests.  Using python here isn't possible without
> updating the python bindings to accomodate decryption policy, which
> doesn't come until later in the series.
>
> so i'd prefer to leave it as-is, but i wouldn't object if someone wanted
> to propose a good patch to these tests that uses json.
>
>           --dkg

At some point I had the idea that we should get rid of the text output
format.  Looking at e.g. notmuch-show.c the stuff related to text output
is not as ad hoc as I remember, so maybe I should just give up on that
idea.

d

[PATCH 16/18] crypto: add --try-decrypt=nostash to avoid stashing session keys

[Daniel Kahn Gillmor]

Here's the configuration choice for people who want a cleartext index,
but don't want stashed session keys.

Interestingly, this "nostash" decryption policy is actually the same
policy that should be used by "notmuch show" and "notmuch reply",
since they never modify the index or database when they are invoked
with --decrypt.

We take advantage of this parallel to tune the behavior of those
programs so that we're not requesting session keys from GnuPG during
"show" and "reply" that we would then otherwise just throw away.
---
 completion/notmuch-completion.bash |  6 +++---
 doc/man1/notmuch-config.rst        | 10 ++++++++--
 doc/man1/notmuch-insert.rst        | 11 +++++++----
 doc/man1/notmuch-new.rst           | 11 +++++++----
 doc/man1/notmuch-reindex.rst       | 11 +++++++----
 lib/indexopts.c                    |  2 ++
 lib/notmuch.h                      |  1 +
 notmuch-reply.c                    |  2 +-
 notmuch-show.c                     |  3 ++-
 notmuch.c                          |  1 +
 test/T357-index-decryption.sh      | 26 ++++++++++++++++++++++++++
 util/crypto.c                      |  4 ++--
 12 files changed, 67 insertions(+), 21 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 53d7380b..29256210 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -288,7 +288,7 @@ _notmuch_insert()
 	    return
 	    ;;
 	--try-decrypt)
-	    COMPREPLY=( $( compgen -W "true false auto" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true false auto nostash" -- "${cur}" ) )
 	    return
 	    ;;
     esac
@@ -320,7 +320,7 @@ _notmuch_new()
     $split &&
     case "${prev}" in
 	--try-decrypt)
-	    COMPREPLY=( $( compgen -W "true false auto" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true false auto nostash" -- "${cur}" ) )
 	    return
 	    ;;
     esac
@@ -442,7 +442,7 @@ _notmuch_reindex()
     $split &&
     case "${prev}" in
 	--try-decrypt)
-	    COMPREPLY=( $( compgen -W "true false auto" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true false auto nostash" -- "${cur}" ) )
 	    return
 	    ;;
     esac
diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 5adde070..d9e22653 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -141,6 +141,9 @@ The available configuration items are described below.
     **index.try_decrypt**
 
         **[STORED IN DATABASE]**
+
+        One of ``false``, ``auto``, ``nostash``, or ``true``.
+
         When indexing an encrypted e-mail message, if this variable is
         set to ``true``, notmuch will try to decrypt the message and
         index the cleartext, stashing a copy of any discovered session
@@ -150,11 +153,14 @@ The available configuration items are described below.
         secret keys.  Use ``false`` to avoid decrypting even when a
         stashed session key is already present.
 
+        ``nostash`` is the same as ``true`` except that it will not
+        stash newly-discovered session keys in the database.
+
         Be aware that the notmuch index is likely sufficient to
         reconstruct the cleartext of the message itself, so please
         ensure that the notmuch message index is adequately protected.
-        DO NOT USE ``index.try_decrypt=true`` without considering the
-        security of your index.
+        DO NOT USE ``index.try_decrypt=true`` or ``index-only``
+        without considering the security of your index.
 
         Default: ``auto``.
 
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index 3e6f538d..bf659a3d 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -50,10 +50,10 @@ Supported options for **insert** include
     ``--no-hooks``
         Prevent hooks from being run.
 
-    ``--try-decrypt=(true|auto|false)``
+    ``--try-decrypt=(true|nostash|auto|false)``
 
         If ``true`` and the message is encrypted, try to decrypt the
-        message while indexing, storing any session keys discovered.
+        message while indexing, stashing any session keys discovered.
         If ``auto``, and notmuch already knows about a session key for
         the message, it will try decrypting using that session key but
         will not try to access the user's secret keys.  If decryption
@@ -61,11 +61,14 @@ Supported options for **insert** include
         message is always stored to disk in its original form
         (ciphertext).
 
+        ``nostash`` is the same as ``true`` except that it will not
+        stash newly-discovered session keys in the database.
+
         Be aware that the index is likely sufficient to reconstruct
         the cleartext of the message itself, so please ensure that the
         notmuch message index is adequately protected. DO NOT USE
-        ``--try-decrypt=true`` without considering the security of
-        your index.
+        ``--try-decrypt=true`` or ``nostash`` without considering
+        the security of your index.
 
         See also ``index.try_decrypt`` in **notmuch-config(1)**.
 
diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
index 351b7591..2660988d 100644
--- a/doc/man1/notmuch-new.rst
+++ b/doc/man1/notmuch-new.rst
@@ -43,20 +43,23 @@ Supported options for **new** include
     ``--quiet``
         Do not print progress or results.
 
-    ``--try-decrypt=(true|auto|false)``
+    ``--try-decrypt=(true|nostash|auto|false)``
 
         If ``true``, when encountering an encrypted message, try to
-        decrypt it while indexing, and store any discovered session
+        decrypt it while indexing, and stash any discovered session
         keys.  If ``auto``, try to use any session key already known
         to belong to this message, but do not attempt to use the
         user's secret keys.  If decryption is successful, index the
         cleartext of the message.
 
+        ``nostash`` is the same as ``true`` except that it will not
+        stash newly-discovered session keys in the database.
+
         Be aware that the index is likely sufficient to reconstruct
         the cleartext of the message itself, so please ensure that the
         notmuch message index is adequately protected.  DO NOT USE
-        ``--try-decrypt=true`` without considering the security of
-        your index.
+        ``--try-decrypt=true`` or ``nostash`` without considering
+        the security of your index.
 
         See also ``index.try_decrypt`` in **notmuch-config(1)**.
 
diff --git a/doc/man1/notmuch-reindex.rst b/doc/man1/notmuch-reindex.rst
index 3b99b934..7883e0cb 100644
--- a/doc/man1/notmuch-reindex.rst
+++ b/doc/man1/notmuch-reindex.rst
@@ -21,23 +21,26 @@ messages using the supplied options.
 
 Supported options for **reindex** include
 
-    ``--try-decrypt=(true|auto|false)``
+    ``--try-decrypt=(true|nostash|auto|false)``
 
         If ``true``, when encountering an encrypted message, try to
-        decrypt it while reindexing, storing any session keys
+        decrypt it while reindexing, stashing any session keys
         discovered.  If ``auto``, and notmuch already knows about a
         session key for the message, it will try decrypting using that
         session key but will not try to access the user's secret keys.
         If decryption is successful, index the cleartext itself.
 
+        ``nostash`` is the same as ``true`` except that it will not
+        stash newly-discovered session keys in the database.
+
         If ``false``, notmuch reindex will also delete any stashed
         session keys for all messages matching the search terms.
 
         Be aware that the index is likely sufficient to reconstruct
         the cleartext of the message itself, so please ensure that the
         notmuch message index is adequately protected. DO NOT USE
-        ``--try-decrypt=true`` without considering the security of
-        your index.
+        ``--try-decrypt=true`` or ``nostash`` without considering
+        the security of your index.
 
         See also ``index.try_decrypt`` in **notmuch-config(1)**.
 
diff --git a/lib/indexopts.c b/lib/indexopts.c
index a61d6420..e698dcea 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -42,6 +42,8 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
 		 (!(strcasecmp(try_decrypt, "no"))) ||
 		 (!(strcasecmp(try_decrypt, "0"))))
 	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_FALSE);
+	else if (!strcasecmp(try_decrypt, "nostash"))
+	    notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_NOSTASH);
     }
 
     free (try_decrypt);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7b1c61ad..103a7091 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2242,6 +2242,7 @@ typedef enum {
     NOTMUCH_DECRYPT_FALSE,
     NOTMUCH_DECRYPT_TRUE,
     NOTMUCH_DECRYPT_AUTO,
+    NOTMUCH_DECRYPT_NOSTASH,
 } notmuch_decryption_policy_t;
 
 /**
diff --git a/notmuch-reply.c b/notmuch-reply.c
index fd990a9a..5cdf642b 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -730,7 +730,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_process_shared_options (argv[0]);
     if (decrypt_set)
-	params.crypto.decrypt = decrypt ? NOTMUCH_DECRYPT_TRUE : NOTMUCH_DECRYPT_FALSE;
+	params.crypto.decrypt = decrypt ? NOTMUCH_DECRYPT_NOSTASH : NOTMUCH_DECRYPT_FALSE;
 
     notmuch_exit_if_unsupported_format ();
 
diff --git a/notmuch-show.c b/notmuch-show.c
index c8f5a48f..4e22424b 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1121,7 +1121,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 
     if (decrypt_set) {
 	if (decrypt) {
-	    params.crypto.decrypt = NOTMUCH_DECRYPT_TRUE;
+	    /* we do not need or want to ask for session keys */
+	    params.crypto.decrypt = NOTMUCH_DECRYPT_NOSTASH;
 	    /* decryption implies verification */
 	    params.crypto.verify = true;
 	} else {
diff --git a/notmuch.c b/notmuch.c
index 62706748..2f43ad28 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -104,6 +104,7 @@ const notmuch_opt_desc_t  notmuch_shared_indexing_options [] = {
       (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
 			      { "true", NOTMUCH_DECRYPT_TRUE },
 			      { "auto", NOTMUCH_DECRYPT_AUTO },
+			      { "nostash", NOTMUCH_DECRYPT_NOSTASH },
 			      { 0, 0 } },
       .name = "try-decrypt" },
     { }
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 3ac8c43e..8aef4354 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -182,6 +182,32 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+test_begin_subtest "index cleartext without keeping session keys"
+test_expect_success "notmuch reindex --try-decrypt=nostash tag:blarney"
+
+test_begin_subtest "Ensure that the indexed terms are present"
+output=$(notmuch search wumpus)
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "show one of the messages with --decrypt"
+output=$(notmuch show --decrypt thread:0000000000000001 | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }')
+expected='This is a test encrypted message with a wumpus.'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "Ensure that we cannot show the message without --decrypt"
+output=$(notmuch show thread:0000000000000001 | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }')
+expected='Non-text part: application/octet-stream'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+
+
+
 add_email_corpus crypto
 
 test_begin_subtest "indexing message fails when secret key not available"
diff --git a/util/crypto.c b/util/crypto.c
index ba9bfa7b..ac018005 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -190,7 +190,7 @@ _notmuch_crypto_decrypt (bool *attempted,
 #if (GMIME_MAJOR_VERSION < 3)
 #if (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21)
     gboolean oldgetsk = g_mime_crypto_context_get_retrieve_session_key (crypto_ctx);
-    gboolean newgetsk = (decrypt_result);
+    gboolean newgetsk = (decrypt == NOTMUCH_DECRYPT_TRUE && decrypt_result);
     if (newgetsk != oldgetsk)
 	/* This could return an error, but we can't do anything about it, so ignore it */
 	g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, newgetsk, NULL);
@@ -203,7 +203,7 @@ _notmuch_crypto_decrypt (bool *attempted,
 #endif
 #else
     GMimeDecryptFlags flags = GMIME_DECRYPT_NONE;
-    if (decrypt_result)
+    if (decrypt == NOTMUCH_DECRYPT_TRUE && decrypt_result)
 	flags |= GMIME_DECRYPT_EXPORT_SESSION_KEY;
     ret = g_mime_multipart_encrypted_decrypt(part, flags, NULL,
 					     decrypt_result, err);
-- 
2.14.2

Re: [PATCH 16/18] crypto: add --try-decrypt=nostash to avoid stashing session keys

[Daniel Kahn Gillmor]

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

On Wed 2017-10-25 02:52:01 -0400, Daniel Kahn Gillmor wrote:
> -        DO NOT USE ``index.try_decrypt=true`` without considering the
> -        security of your index.
> +        DO NOT USE ``index.try_decrypt=true`` or ``index-only``
> +        without considering the security of your index.

jrollins helpfully caught that i'd let the previous, worse name of
"nostash" ("index-only") sneak into this commit.  I've fixed it in my
local copy of this branch, which can be found as "session-keys" at
https://gitlab.com/dkg/notmuch (current commit ID
ac7a7bb931b68a17dff0b4b782a2bdeced4e779c)

I'll wait to send revised patches to the list once i've incorporated any
other reviews folks want to send my way.

      --dkg

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

Re: [PATCH 16/18] crypto: add --try-decrypt=nostash to avoid stashing session keys

[David Bremner]

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

> +        DO NOT USE ``index.try_decrypt=true`` or ``index-only``
> +        without considering the security of your index.

is index-only a typo there?

As a future improvement it would be nice to reduce some of the
documentation cut and paste for common options, perhaps with include
files? As long as the solution isn't worse than the problem of course.

> +test_expect_equal \
> +    "$output" \
> +    "$expected"
> +
> +
> +
> +

Is there some reason for all those blank lines?

[PATCH 17/18] docs: clean up documentation about decryption policies

[Daniel Kahn Gillmor]

Now that the range of sensible decryption policies has come into full
view, we take a bit of space to document the distinctions.

Most people will use either "auto" or "true" -- but we provide "false"
and "nostash" to handle use cases that might reasonably be requested.

Note also that these can be combined in sensible ways.  Like, if your
mail comes in regularly to a service that doesn't have access to your
secret keys, but does have access to your index, and you feel
comfortable adding selected encrypted messages to the index after
you've read them, you could stay in "auto" normally, and then when you
find yourself reading an indexable message (e.g. one you want to be
able to search for in the future, and that you don't mind exposing to
whatever entities have access to your inde), you can do:

    notmuch reindex --try-decrypt=true id:whatever@example.biz

That leaves your default the same (still "auto") but you get the
cleartext index and stashed session key benefits for that particular
message.
---
 doc/man1/notmuch-config.rst | 44 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index d9e22653..6b7d5db2 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -142,7 +142,9 @@ The available configuration items are described below.
 
         **[STORED IN DATABASE]**
 
-        One of ``false``, ``auto``, ``nostash``, or ``true``.
+        Policy for decrypting encrypted messages during indexing.
+        Must be one of: ``false``, ``auto``, ``nostash``, or
+        ``true``.
 
         When indexing an encrypted e-mail message, if this variable is
         set to ``true``, notmuch will try to decrypt the message and
@@ -156,11 +158,40 @@ The available configuration items are described below.
         ``nostash`` is the same as ``true`` except that it will not
         stash newly-discovered session keys in the database.
 
-        Be aware that the notmuch index is likely sufficient to
-        reconstruct the cleartext of the message itself, so please
-        ensure that the notmuch message index is adequately protected.
-        DO NOT USE ``index.try_decrypt=true`` or ``index-only``
-        without considering the security of your index.
+        From the command line (i.e. during **notmuch-new(1)**,
+        **notmuch-insert(1)**, or **notmuch-reindex**), the user can
+        override the database's stored decryption policy with the
+        ``--try-decrypt=`` option.
+
+        Here is a table that summarizes the functionality of each of
+        these policies:
+
+        +------------------------+-------+------+---------+------+
+        |                        | false | auto | nostash | true |
+        +========================+=======+======+=========+======+
+        | Index cleartext using  |       |  X   |    X    |  X   |
+        | stashed session keys   |       |      |         |      |
+        +------------------------+-------+------+---------+------+
+        | Index cleartext        |       |      |    X    |  X   |
+        | using secret keys      |       |      |         |      |
+        +------------------------+-------+------+---------+------+
+        | Stash session keys     |       |      |         |  X   |
+        +------------------------+-------+------+---------+------+
+        | Delete stashed session |   X   |      |         |      |
+        | keys on reindex        |       |      |         |      |
+        +------------------------+-------+------+---------+------+
+
+        Stashed session keys are kept in the database as properties
+        associated with the message.  See ``session-key`` in
+        **notmuch-properties(7)** for more details about how they can
+        be useful.
+
+        Be aware that the notmuch index itself (whether session keys
+        are stashed or not) is likely sufficient to reconstruct a
+        close approximation of the cleartext of the message itself, so
+        please ensure that the notmuch message index is adequately
+        protected.  DO NOT SET ``index.try_decrypt`` to ``true`` or
+        ``index-only`` without considering the security of your index.
 
         Default: ``auto``.
 
@@ -200,5 +231,6 @@ SEE ALSO
 **notmuch-restore(1)**,
 **notmuch-search(1)**,
 **notmuch-search-terms(7)**,
+**notmuch-properties(7)**,
 **notmuch-show(1)**,
 **notmuch-tag(1)**
-- 
2.14.2

[PATCH 18/18] python: add try_decrypt argument to Database.index_file()

[Daniel Kahn Gillmor]

We adopt a pythonic idiom here with an optional argument, rather than
exposing the user to the C indexopts object directly.
---
 bindings/python/notmuch/database.py | 46 +++++++++++++++++++++++++++++++++++--
 bindings/python/notmuch/globals.py  |  5 ++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 1279804a..bbce5cc2 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -28,6 +28,7 @@ from .globals import (
     _str,
     NotmuchDatabaseP,
     NotmuchDirectoryP,
+    NotmuchIndexoptsP,
     NotmuchMessageP,
     NotmuchTagsP,
 )
@@ -72,6 +73,9 @@ class Database(object):
     MODE = Enum(['READ_ONLY', 'READ_WRITE'])
     """Constants: Mode in which to open the database"""
 
+    DECRYPTION_POLICY = Enum(['FALSE', 'TRUE', 'AUTO', 'NOSTASH'])
+    """Constants: policies for decrypting messages during indexing"""
+
     """notmuch_database_get_directory"""
     _get_directory = nmlib.notmuch_database_get_directory
     _get_directory.argtypes = [NotmuchDatabaseP, c_char_p, POINTER(NotmuchDirectoryP)]
@@ -400,13 +404,25 @@ class Database(object):
         # return the Directory, init it with the absolute path
         return Directory(abs_dirpath, dir_p, self)
 
+    _get_default_indexopts = nmlib.notmuch_database_get_default_indexopts
+    _get_default_indexopts.argtypes = [NotmuchDatabaseP]
+    _get_default_indexopts.restype = NotmuchIndexoptsP
+
+    _indexopts_set_try_decrypt = nmlib.notmuch_indexopts_set_try_decrypt
+    _indexopts_set_try_decrypt.argtypes = [NotmuchIndexoptsP, c_uint]
+    _indexopts_set_try_decrypt.restype = None
+
+    _indexopts_destroy = nmlib.notmuch_indexopts_destroy
+    _indexopts_destroy.argtypes = [NotmuchIndexoptsP]
+    _indexopts_destroy.restype = None
+
     _index_file = nmlib.notmuch_database_index_file
     _index_file.argtypes = [NotmuchDatabaseP, c_char_p,
                              c_void_p,
                              POINTER(NotmuchMessageP)]
     _index_file.restype = c_uint
 
-    def index_file(self, filename, sync_maildir_flags=False):
+    def index_file(self, filename, sync_maildir_flags=False, try_decrypt=None):
         """Adds a new message to the database
 
         :param filename: should be a path relative to the path of the
@@ -427,6 +443,23 @@ class Database(object):
             API. You might want to look into the underlying method
             :meth:`Message.maildir_flags_to_tags`.
 
+        :param try_decrypt: If the message contains any encrypted
+            parts, and try_decrypt is set to
+            :attr:`DECRYPTION_POLICY`.TRUE, notmuch will try to
+            decrypt the message and index the cleartext, stashing any
+            discovered session keys.  If it is set to
+            :attr:`DECRYPTION_POLICY`.FALSE, it will never try to
+            decrypt during indexing.  If it is set to
+            :attr:`DECRYPTION_POLICY`.AUTO, then it will try to use
+            any stashed session keys it knows about, but will not try
+            to access the user's secret keys.
+            :attr:`DECRYPTION_POLICY`.NOSTASH behaves the same as
+            :attr:`DECRYPTION_POLICY`.TRUE except that no session keys
+            are stashed in the database.  If try_decrypt is set to
+            None (the default), then the database itself will decide
+            whether to decrypt, based on the `index.try_decrypt`
+            configuration setting (see notmuch-config(1)).
+
         :returns: On success, we return
 
            1) a :class:`Message` object that can be used for things
@@ -454,10 +487,19 @@ class Database(object):
               :attr:`STATUS`.READ_ONLY_DATABASE
                       Database was opened in read-only mode so no message can
                       be added.
+
         """
         self._assert_db_is_initialized()
         msg_p = NotmuchMessageP()
-        status = self._index_file(self._db, _str(filename), c_void_p(None), byref(msg_p))
+        indexopts = c_void_p(None)
+        if try_decrypt is not None:
+            indexopts = self._get_default_indexopts(self._db)
+            self._indexopts_set_try_decrypt(indexopts, try_decrypt)
+
+        status = self._index_file(self._db, _str(filename), indexopts, byref(msg_p))
+
+        if indexopts:
+            self._indexopts_destroy(indexopts)
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
             raise NotmuchError(status)
diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
index b1eec2cf..71426c84 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -88,3 +88,8 @@ NotmuchDirectoryP = POINTER(NotmuchDirectoryS)
 class NotmuchFilenamesS(Structure):
     pass
 NotmuchFilenamesP = POINTER(NotmuchFilenamesS)
+
+
+class NotmuchIndexoptsS(Structure):
+    pass
+NotmuchIndexoptsP = POINTER(NotmuchIndexoptsS)
-- 
2.14.2

Re: [PATCH 18/18] python: add try_decrypt argument to Database.index_file()

[David Bremner]

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

> @@ -454,10 +487,19 @@ class Database(object):
>                :attr:`STATUS`.READ_ONLY_DATABASE
>                        Database was opened in read-only mode so no message can
>                        be added.
> +

looks like a random blank line

>          """
>          self._assert_db_is_initialized()
>          msg_p = NotmuchMessageP()
> -        status = self._index_file(self._db, _str(filename), c_void_p(None), byref(msg_p))
> +        indexopts = c_void_p(None)
> +        if try_decrypt is not None:
> +            indexopts = self._get_default_indexopts(self._db)
> +            self._indexopts_set_try_decrypt(indexopts, try_decrypt)
> +
> +        status = self._index_file(self._db, _str(filename), indexopts, byref(msg_p))
> +
> +        if indexopts:
> +            self._indexopts_destroy(indexopts)
>  
>          if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
>              raise NotmuchError(status)
> diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
> index b1eec2cf..71426c84 100644
> --- a/bindings/python/notmuch/globals.py
> +++ b/bindings/python/notmuch/globals.py
> @@ -88,3 +88,8 @@ NotmuchDirectoryP = POINTER(NotmuchDirectoryS)
>  class NotmuchFilenamesS(Structure):
>      pass
>  NotmuchFilenamesP = POINTER(NotmuchFilenamesS)
> +
> +
> +class NotmuchIndexoptsS(Structure):
> +    pass
> +NotmuchIndexoptsP = POINTER(NotmuchIndexoptsS)
> -- 
> 2.14.2

I think this new bindings functionality needs a test.

Re: [PATCH 18/18] python: add try_decrypt argument to Database.index_file()

[Daniel Kahn Gillmor]

On Thu 2017-11-16 09:06:09 -0400, David Bremner wrote:
> I think this new bindings functionality needs a test.

agreed, the python bindings do need to be added to the test suite (this
is also true in the newer version of the series).

I'm happy to add those tests as a condition of getting the python
bindings merged, but i hope they won't block the review and merge of the
rest of the series :)

      --dkg

Re: Stashed session keys

[Daniel Kahn Gillmor]

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

On Wed 2017-10-25 02:51:45 -0400, Daniel Kahn Gillmor wrote:
> Now that cleartext indexing is merged, let's add the ability to stash
> session keys!

I'd really appreciate feedback on this series.  It works for me, and i'm
using it.  I don't want this to take another two years to land, if
possible.  And it blocks additional improvements that i would like to
eventually land as well.

So any review of any part of this series would be appreciated.  I want
to make notmuch a mail client that provides actually usable encrypted
mail.

Regards,

      --dkg

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

Re: Stashed session keys

[Jameson Graef Rollins]

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

On Wed, Oct 25 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> Now that cleartext indexing is merged, let's add the ability to stash
> session keys!

I have reviewed and tested this series, and it seems solidly implemented
and very well motivated.  I have been using it regularly for a couple
weeks now and have found no issues with it's usage, and have noticed the
considerable speed up when viewing encrypted threads (as much as x8 for
show on a thread of just 8 encrypted messages).  I fully support it's
integration unconditionally.

It should be emphasized that this series is actually fairly critical for
good support of message encryption.  Without this series it's actually
possible to completely lose access to encrypted mail if one were to
rotate/replace their encryption key, which one might reasonably be
expected to do.  Without access to the original encryption key or the
message session key, there is no way to access the contents of an
encrypted message.  If, however, the session key is stashed in the
index, the original encryption key can be destroyed and the message can
still be accessed.  Daniel likes to think of this in terms of being able
to "delete" encrypted messages in the wild (via deletion of the original
encryption key) whereas I like to think of it in terms of preserving
access to received encrypted messages after key rotation.  Both benefits
hold, though, obviously.

>    +------------------------+-------+------+---------+------+
>    |                        | false | auto | nostash | true |
>    +========================+=======+======+=========+======+
>    | Index cleartext using  |       |  X   |    X    |  X   |
>    | stashed session keys   |       |      |         |      |
>    +------------------------+-------+------+---------+------+
>    | Index cleartext        |       |      |    X    |  X   |
>    | using secret keys      |       |      |         |      |
>    +------------------------+-------+------+---------+------+
>    | Stash session keys     |       |      |         |  X   |
>    +------------------------+-------+------+---------+------+
>    | Delete stashed session |   X   |      |         |      |
>    | keys on reindex        |       |      |         |      |
>    +------------------------+-------+------+---------+------+

I think these policies cover all potential use cases that I can see.
However, there will need to be further work on the UX to make things
flow more smoothly.

I've been using this series with index.try_decrypt set to 'true', which
causes encrypted messages to be indexed on new.  I do this because I
don't want to be bothered to manually initiate indexing of encrypted
messages.  However, since my mail retrieval and indexing happen in the
background, this has the unfortunate side effect that I am occasionally
presented with a gpg agent prompt at random unexpected times.  Ideally,
one would leave index.try_decrypt set to 'auto', and there would be an
easy/automatic way to prompt reindexing when the user is interacting
directly with their MUA.  I haven't decided what's the best way to do
that yet, but something like the following happening automatically at
inbox view might do the trick:

  notmuch reindex --try-decrypt=true (tag:inbox AND tag:encrypted)

Finally, I think it would be worthwhile to resolve the disparity between
the usage of "decrypt" and "try-decrypt" in the CLI and config options.
I'm not sure why we're using different terms in different contexts, even
though the meanings are essentially the same.  A follow-up patch series
changing "try-decrypt" -> "decrypt" would probably be in order.

But these are next steps.  The series in question here is absolutely
ready, and needed, as is.

jamie.

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

Re: Stashed session keys

[Daniel Kahn Gillmor]

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

On Sat 2017-11-11 15:31:36 -0800, Jameson Graef Rollins wrote:
> I have reviewed and tested this series, and it seems solidly
> implemented and very well motivated.  I have been using it regularly
> for a couple weeks now and have found no issues with it's usage, and
> have noticed the considerable speed up when viewing encrypted threads
> (as much as x8 for show on a thread of just 8 encrypted messages).  I
> fully support it's integration unconditionally.

thanks for the review, the testing, and the reportback.  I'm glad to
hear that it's giving you the same sort of speedups that it gives me.

> Daniel likes to think of this in terms of being able to "delete"
> encrypted messages in the wild (via deletion of the original
> encryption key) whereas I like to think of it in terms of preserving
> access to received encrypted messages after key rotation.  Both
> benefits hold, though, obviously.

yes, these are different ways of looking at the same key management
strategy.

> I think these policies cover all potential use cases that I can see.
> However, there will need to be further work on the UX to make things
> flow more smoothly.

Agreed.  The goal of this series is to provide the framework that can be
used to build smoother UX, but it doesn't get all the way to providing
the smoothest possible UX.  Such is the nature of toolkit development.

> I haven't decided what's the best way to do that yet, but something
> like the following happening automatically at inbox view might do the
> trick:
>
>   notmuch reindex --try-decrypt=true (tag:inbox AND tag:encrypted)

This seems like a reasonable way to ensure that your long-term, personal
secret keys only get accessed when you are interactively working with
your mail user agent.

You might be able to target the reindex even more narrowly by adding
something like "AND not property:index-decryption=success"

> Finally, I think it would be worthwhile to resolve the disparity between
> the usage of "decrypt" and "try-decrypt" in the CLI and config options.
> I'm not sure why we're using different terms in different contexts, even
> though the meanings are essentially the same.  A follow-up patch series
> changing "try-decrypt" -> "decrypt" would probably be in order.

If this series lands, i'd be happy to supply such an term-normalizing
series for subsequent consideration.

If people feel that this term normalization is the main blocker to
landing this series, i could try to rebase it with a different UI terms,
but rebasing the series for this change feels like busy-work to me (and
would be more effort than a simple normalization patch on top).  i'd
rather spend my limited notmuch hacking+reviewing time providing useful
features.

      --dkg

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

Re: Stashed session keys

[Jameson Graef Rollins]

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

On Sun, Nov 12 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
>> Finally, I think it would be worthwhile to resolve the disparity between
>> the usage of "decrypt" and "try-decrypt" in the CLI and config options.
>> I'm not sure why we're using different terms in different contexts, even
>> though the meanings are essentially the same.  A follow-up patch series
>> changing "try-decrypt" -> "decrypt" would probably be in order.
>
> If this series lands, i'd be happy to supply such an term-normalizing
> series for subsequent consideration.
>
> If people feel that this term normalization is the main blocker to
> landing this series, i could try to rebase it with a different UI terms,
> but rebasing the series for this change feels like busy-work to me (and
> would be more effort than a simple normalization patch on top).  i'd
> rather spend my limited notmuch hacking+reviewing time providing useful
> features.

I don't think it's a blocker at all, but I do think the term
normalization should happen before the next release, since I think we
should change "try-decrypt" -> "decrypt", which would affect things
already in master.  But I very much hope this series will land before
the next release, in which case I think it's fine to wait to do the
normalization after this series lands.

jamie.

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

Re: Stashed session keys

[Daniel Kahn Gillmor]

On Sun 2017-11-12 11:51:11 +0800, Daniel Kahn Gillmor wrote:
> On Sat 2017-11-11 15:31:36 -0800, Jameson Graef Rollins wrote:
>> I haven't decided what's the best way to do that yet, but something
>> like the following happening automatically at inbox view might do the
>> trick:
>>
>>   notmuch reindex --try-decrypt=true (tag:inbox AND tag:encrypted)
>
> This seems like a reasonable way to ensure that your long-term, personal
> secret keys only get accessed when you are interactively working with
> your mail user agent.
>
> You might be able to target the reindex even more narrowly by adding
> something like "AND not property:index-decryption=success"

Sorry, this should be "AND not property:index.decryption=success"

       --dkg

Re: Stashed session keys

[meskio]

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

Quoting Daniel Kahn Gillmor (2017-10-25 08:51:45)
> Now that cleartext indexing is merged, let's add the ability to stash
> session keys!

Nice feature. I'm using it and it works fine. I notice some speed up, improving 
the painfulness of reading long encrypted threads in alot. And I like to don't 
be able to have around my old private keys.

I implemented some support for it in alot (using the patch I just sent adding 
notmuch_message_get_property to the python binding):
https://github.com/meskio/alot/tree/session-key

Thanks for the work.

-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEs7M6f/ZpXzXMAQR+Urj1rJei2oYFAloMwpQACgkQUrj1rJei
2oaqEA//fd+80zknJJJSuZ/zWdO9nHU0w6zW/9DCMupyu8H+j/xMY24Yj7AC9Kz+
5zZ4VuiGt+4vvc9YHjqbzOM1P8+GXhwOZ3W+xzNzVTspiIeOSZUH9+7iKFIzLImH
QLMGQ3bDPr0NWDhuFIeWF05Pe/4Bn2ZaaSOpKiw2avnhJ6eS5VZ1124LLdJB3XTf
OZ2Tu3sJDwtVkMarHDp47palnl6MsEhT8Ej/UAAaP1w/UiYF7E3FrNZbcsbA2azW
cKpLjwgbMW5xb5ONm0wh5NHSYjrVQcfrJK4Dy9s/lQrJaOsddp63l8yFhxmHPNwy
ID45ZYbDdxi9Rj8mFAYFw3PcAI+Wwlm/KXgjqW28yL0C2eNGMXeOVIiIzZGx1FHs
rNU2gYUxq1w3imDCdtbWmzJxPKIPGoEufv0EtqvS5aYgDn1YmBA6u7/SCpCSQ8z9
o/ldufEvlgdSLbEfxWX5v/6eaPY6L/Q9cOasCx0cwAvE0jBajnUt87Enywfy75iW
uT8p8xaShLPUnHF+awb3I4mqZy3+eiHgI8MUAyuXIyC0TAx6t6WzdGBUX0wqduHJ
MNU891Ne+nuR0YP9Z1A1JCozVjwrNdKZg5LQNIwyyAQGM5p2vS8vEoaroQk+v/N3
qttE8Y4DQnxU8v5SyMmWCTAjtZMkhpEHhqZ19EC+2lnamCghJVc=
=vzxa
-----END PGP SIGNATURE-----

Re: Stashed session keys

[Daniel Kahn Gillmor]

Hi meskio--

On Wed 2017-11-15 23:41:28 +0100, meskio wrote:
> Nice feature. I'm using it and it works fine. I notice some speed up, improving 
> the painfulness of reading long encrypted threads in alot. And I like to don't 
> be able to have around my old private keys.

cool, i'm glad it's working for you!

> I implemented some support for it in alot (using the patch I just sent adding 
> notmuch_message_get_property to the python binding):
> https://github.com/meskio/alot/tree/session-key

very nice :)

     --dkg