notmuch show --decrypt=stash

notmuch show --decrypt=stash

[Daniel Kahn Gillmor]

This is a revision of the series initially introduced in
id:20171212025225.11854-1-dkg@fifthhorseman.net, with minor updates to
accomodate the recent release of notmuch 0.26 (yay!)

This series allows "notmuch show" to index the cleartext and stash the
session keys of an encrypted message while displaying it.

Background
----------

As of notmuch 0.26, cleartext indexing and session-keys make working
with encrypted e-mail significantly easier in notmuch.  However, the
underlying requirement is that at the time of message ingestion (and
"notmuch new" in particular), the user is likely to have access to
their long-term secret keys.

In practice, many people using GnuPG today have their secret keys
locked behind a passphrase, or on a smartcard, and also run "notmuch
new" in some sort of scheduled, backgrounded process.

The result is that for users with this workflow, GnuPG prompts for
their passphrase (or wants to make use of their smartcard) at
unpredictable times, depending on when their mail delivery happens,
and on how many encrypted messages they receive.  This is both
unfriendly and bad for security (we should not train users to approve
random prompts for secret key access when nothing they're doing
interactively seems to warrant it).

Outline
-------

For a friendlier experience, some users may prefer incoming encrypted
mail to stay in their inbox *without* being decrypted, until they
choose to look at it.  At the moment that they're looking at it, their
MUA is in the foreground and they're interacting with it, so being
prompted for their password or smartcard interactively makes sense at
that time.

This series makes it possible for this interaction to to actually
decrypt the message, index it, and stash any session keys the first
time the user interacts with the message through "notmuch show".

This is not a workflow that every MUA will choose to use (e.g. users
whose decryption-capable secret key is already cheaply available
without hassling the user at "notmuch new" shouldn't use it), but it
is a sensible workflow for some users that notmuch should support.

Furthermore, it is a more efficient use of secret key material -- a
user that wants to stash session keys of a message, but whose
long-term decryption secret key is on a smartcard should only be obliged
to trigger the smartcard once per message, ever.

Implementation details
----------------------

The most controversial part of this series is that it makes "notmuch
show" potentially not a read-only operation on the database.  This is
a tradeoff that the users of this workflow will need to consider,
since they are explicitly asking "notmuch show" to potentially modify
their index.

Note that i've made this R/O-to-R/W switch fairly coarse.  If the user
requests --decrypt=stash, then "notmuch show" will operate on a
read/write database, regardless of whether the message is actually
encrypted.  I used this coarse approach because i couldn't figure out
a safe way to reopen an existing read-only database in read-write
mode.  If someone more clever with Xapian than me wants to suggest a
way to do this in a more fine-grained fashion, i'd welcome patches or
pointers.  That said, if users don't like the idea of "notmuch show"
using the db read/write, they can also just not use --decrypt=stash :)

I welcome review and feedback.

  --dkg

[PATCH v2 1/5] lib: expose notmuch_message_get_database()

[Daniel Kahn Gillmor]

We've had _notmuch_message_database() internally for a while, and it's
useful.  It turns out to be useful on the other side of the library
interface as well (i'll use it later in this series for "notmuch
show"), so we expose it publicly now.
---
 lib/index.cc            | 10 +++++-----
 lib/message-property.cc |  4 ++--
 lib/message.cc          | 14 +++++++-------
 lib/notmuch-private.h   |  2 --
 lib/notmuch.h           |  8 ++++++++
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 0ad683fa..22ca9ec1 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -385,7 +385,7 @@ _index_mime_part (notmuch_message_t *message,
     const char *charset;
 
     if (! part) {
-	_notmuch_database_log (_notmuch_message_database (message),
+	_notmuch_database_log (notmuch_message_get_database (message),
 			      "Warning: Not indexing empty mime part.\n");
 	return;
     }
@@ -411,7 +411,7 @@ _index_mime_part (notmuch_message_t *message,
 					 g_mime_multipart_get_part (multipart, i));
 		    continue;
 		} else if (i != GMIME_MULTIPART_SIGNED_CONTENT) {
-		    _notmuch_database_log (_notmuch_message_database (message),
+		    _notmuch_database_log (notmuch_message_get_database (message),
 					   "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
 		}
 	    }
@@ -424,7 +424,7 @@ _index_mime_part (notmuch_message_t *message,
 					       GMIME_MULTIPART_ENCRYPTED (part));
 		} else {
 		    if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) {
-			_notmuch_database_log (_notmuch_message_database (message),
+			_notmuch_database_log (notmuch_message_get_database (message),
 					       "Warning: Unexpected extra parts of multipart/encrypted.\n");
 		    }
 		}
@@ -447,7 +447,7 @@ _index_mime_part (notmuch_message_t *message,
     }
 
     if (! (GMIME_IS_PART (part))) {
-	_notmuch_database_log (_notmuch_message_database (message),
+	_notmuch_database_log (notmuch_message_get_database (message),
 			      "Warning: Not indexing unknown mime part: %s.\n",
 			      g_type_name (G_OBJECT_TYPE (part)));
 	return;
@@ -528,7 +528,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     if (!indexopts || (notmuch_indexopts_get_decrypt_policy (indexopts) == NOTMUCH_DECRYPT_FALSE))
 	return;
 
-    notmuch = _notmuch_message_database (message);
+    notmuch = notmuch_message_get_database (message);
 
     GMimeCryptoContext* crypto_ctx = NULL;
 #if (GMIME_MAJOR_VERSION < 3)
diff --git a/lib/message-property.cc b/lib/message-property.cc
index 35eaf3c6..2e44a386 100644
--- a/lib/message-property.cc
+++ b/lib/message-property.cc
@@ -44,7 +44,7 @@ _notmuch_message_modify_property (notmuch_message_t *message, const char *key, c
     notmuch_status_t status;
     char *term = NULL;
 
-    status = _notmuch_database_ensure_writable (_notmuch_message_database (message));
+    status = _notmuch_database_ensure_writable (notmuch_message_get_database (message));
     if (status)
 	return status;
 
@@ -92,7 +92,7 @@ _notmuch_message_remove_all_properties (notmuch_message_t *message, const char *
     notmuch_status_t status;
     const char * term_prefix;
 
-    status = _notmuch_database_ensure_writable (_notmuch_message_database (message));
+    status = _notmuch_database_ensure_writable (notmuch_message_get_database (message));
     if (status)
 	return status;
 
diff --git a/lib/message.cc b/lib/message.cc
index d5db89b6..0886b22d 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -268,7 +268,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch,
 
 	doc_id = _notmuch_database_generate_doc_id (notmuch);
     } catch (const Xapian::Error &error) {
-	_notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n",
+	_notmuch_database_log(notmuch_message_get_database (message), "A Xapian exception occurred creating message: %s\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = true;
 	*status_ret = NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION;
@@ -495,7 +495,7 @@ _notmuch_message_ensure_message_file (notmuch_message_t *message)
 	return;
 
     message->message_file = _notmuch_message_file_open_ctx (
-	_notmuch_message_database (message), message, filename);
+	notmuch_message_get_database (message), message, filename);
 }
 
 const char *
@@ -525,7 +525,7 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header)
 		return talloc_strdup (message, value.c_str ());
 
 	} catch (Xapian::Error &error) {
-	    _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading header: %s\n",
+	    _notmuch_database_log(notmuch_message_get_database (message), "A Xapian exception occurred when reading header: %s\n",
 		     error.get_msg().c_str());
 	    message->notmuch->exception_reported = true;
 	    return NULL;
@@ -646,7 +646,7 @@ _notmuch_message_remove_indexed_terms (notmuch_message_t *message)
 	    notmuch_database_t *notmuch = message->notmuch;
 
 	    if (!notmuch->exception_reported) {
-		_notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n",
+		_notmuch_database_log(notmuch_message_get_database (message), "A Xapian exception occurred creating message: %s\n",
 				      error.get_msg().c_str());
 		notmuch->exception_reported = true;
 	    }
@@ -1042,7 +1042,7 @@ notmuch_message_get_date (notmuch_message_t *message)
     try {
 	value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP);
     } catch (Xapian::Error &error) {
-	_notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading date: %s\n",
+	_notmuch_database_log(notmuch_message_get_database (message), "A Xapian exception occurred when reading date: %s\n",
 		 error.get_msg().c_str());
 	message->notmuch->exception_reported = true;
 	return 0;
@@ -1908,7 +1908,7 @@ notmuch_message_destroy (notmuch_message_t *message)
 }
 
 notmuch_database_t *
-_notmuch_message_database (notmuch_message_t *message)
+notmuch_message_get_database (notmuch_message_t *message)
 {
     return message->notmuch;
 }
@@ -1985,7 +1985,7 @@ notmuch_message_reindex (notmuch_message_t *message,
     /* strdup it because the metadata may be invalidated */
     orig_thread_id = talloc_strdup (message, orig_thread_id);
 
-    notmuch = _notmuch_message_database (message);
+    notmuch = notmuch_message_get_database (message);
 
     ret = _notmuch_database_ensure_writable (notmuch);
     if (ret)
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 1093429c..426c02a8 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -532,8 +532,6 @@ _notmuch_message_id_parse (void *ctx, const char *message_id, const char **next)
 void
 _notmuch_message_add_reply (notmuch_message_t *message,
 			    notmuch_message_t *reply);
-notmuch_database_t *
-_notmuch_message_database (notmuch_message_t *message);
 
 void
 _notmuch_message_remove_unprefixed_terms (notmuch_message_t *message);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7275d270..da3767bf 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1345,6 +1345,14 @@ notmuch_messages_destroy (notmuch_messages_t *messages);
 notmuch_tags_t *
 notmuch_messages_collect_tags (notmuch_messages_t *messages);
 
+/**
+ * Get the database associated with this message.
+ *
+ * @since libnotmuch 5.2 (notmuch 0.27)
+ */
+notmuch_database_t *
+notmuch_message_get_database (notmuch_message_t *message);
+
 /**
  * Get the message ID of 'message'.
  *
-- 
2.15.1

[PATCH v2 2/5] properties: add notmuch_message_count_properties

[Daniel Kahn Gillmor]

The user can already do this manually, of course, but (a) it's nice to
have a convenience function, and (b) exposing this interface means
that someone more clever with a _notmuch_string_map_t than i am can
write a more efficient version if they like, and it will just
accelerate the users of the convenience function.
---
 lib/message-property.cc | 25 +++++++++++++++++++++++++
 lib/notmuch.h           | 16 ++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/message-property.cc b/lib/message-property.cc
index 2e44a386..7894016c 100644
--- a/lib/message-property.cc
+++ b/lib/message-property.cc
@@ -36,6 +36,31 @@ notmuch_message_get_property (notmuch_message_t *message, const char *key, const
     return NOTMUCH_STATUS_SUCCESS;
 }
 
+notmuch_status_t
+notmuch_message_count_properties (notmuch_message_t *message, const char *key, unsigned int *count)
+{
+    if (! count || ! key || ! message)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    notmuch_string_map_t *map;
+    map = _notmuch_message_property_map (message);
+    if (! map)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    notmuch_string_map_iterator_t *matcher = _notmuch_string_map_iterator_create (map, key, true);
+    if (! matcher)
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+    *count = 0;
+    while (_notmuch_string_map_iterator_valid (matcher)) {
+	(*count)++;
+	_notmuch_string_map_iterator_move_to_next (matcher);
+    }
+
+    _notmuch_string_map_iterator_destroy (matcher);
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
 static notmuch_status_t
 _notmuch_message_modify_property (notmuch_message_t *message, const char *key, const char *value,
 				  bool delete_it)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index da3767bf..d5df0246 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1890,6 +1890,22 @@ typedef struct _notmuch_string_map_iterator notmuch_message_properties_t;
 notmuch_message_properties_t *
 notmuch_message_get_properties (notmuch_message_t *message, const char *key, notmuch_bool_t exact);
 
+/**
+ * Return the number of properties named "key" belonging to the specific message.
+ *
+ * @param[in] message  The message to examine
+ * @param[in] key      key to count
+ * @param[out] count   The number of matching properties associated with this message.
+ *
+ * @returns
+ *
+ * NOTMUCH_STATUS_SUCCESS: successful count, possibly some other error.
+ *
+ * @since libnotmuch 5.2 (notmuch 0.27)
+ */
+notmuch_status_t
+notmuch_message_count_properties (notmuch_message_t *message, const char *key, unsigned int *count);
+
 /**
  * Is the given *properties* iterator pointing at a valid (key,value)
  * pair.
-- 
2.15.1

[PATCH v2 3/5] cli: write session keys to database, if asked to do so

[Daniel Kahn Gillmor]

If the decryption policy is NOTMUCH_DECRYPT_TRUE, that means we want
to stash session keys in the database.  Note that there is currently
no way from the command line to set it this way, though, so it is not
yet included in the test suite.
---
 mime-node.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 11df082b..75b79f98 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -197,16 +197,18 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
     GError *err = NULL;
     GMimeDecryptResult *decrypt_result = NULL;
     GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
+    notmuch_message_t *message = NULL;
 
     if (! node->decrypted_child) {
-	mime_node_t *parent;
-	for (parent = node; parent; parent = parent->parent)
-	    if (parent->envelope_file)
+	for (mime_node_t *parent = node; parent; parent = parent->parent)
+	    if (parent->envelope_file) {
+		message = parent->envelope_file;
 		break;
+	    }
 
 	node->decrypted_child = _notmuch_crypto_decrypt (&node->decrypt_attempted,
 							 node->ctx->crypto->decrypt,
-							 parent ? parent->envelope_file : NULL,
+							 message,
 							 cryptoctx, encrypteddata, &decrypt_result, &err);
     }
     if (! node->decrypted_child) {
@@ -225,6 +227,20 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 	    g_object_ref (node->sig_list);
 	    set_signature_list_destructor (node);
 	}
+
+#if HAVE_GMIME_SESSION_KEYS
+	if (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE && message) {
+	    notmuch_database_t *db = notmuch_message_get_database (message);
+	    const char *sk = g_mime_decrypt_result_get_session_key (decrypt_result);
+	    if (db && sk) {
+		notmuch_status_t status;
+		status = notmuch_message_add_property (message, "session-key", sk);
+		if (status)
+		    fprintf (stderr, "Failed to stash session key in the database (%d) %s\n",
+			     status, notmuch_status_to_string (status));
+	    }
+	}
+#endif
 	g_object_unref (decrypt_result);
     }
 
-- 
2.15.1

Re: [PATCH v2 3/5] cli: write session keys to database, if asked to do so

[David Bremner]

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

> +
> +#if HAVE_GMIME_SESSION_KEYS
> +	if (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE && message) {
> +	    notmuch_database_t *db = notmuch_message_get_database (message);
> +	    const char *sk = g_mime_decrypt_result_get_session_key (decrypt_result);
> +	    if (db && sk) {
> +		notmuch_status_t status;
> +		status = notmuch_message_add_property (message, "session-key", sk);
> +		if (status)
> +		    fprintf (stderr, "Failed to stash session key in the database (%d) %s\n",
> +			     status, notmuch_status_to_string (status));
> +	    }
> +	}
> +#endif

As a nit, I don't really like sk as a variable name.

It might be worth definining a "print_status_message", along the lines
of print_status_query in status.c and using it here and in the next
commit. That would also handle any use of _notmuch_database_log.

[PATCH v2 4/5] cli/show: reindex when we learned new session keys about a message

[Daniel Kahn Gillmor]

If the number of session keys for a given message increased after
running "notmuch show" then we just learned something new that might
let us do automatic decryption.  We should reindex this message using
our newfound knowledge.
---
 notmuch-show.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/notmuch-show.c b/notmuch-show.c
index 9871159d..90e45cd9 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -873,6 +873,11 @@ show_message (void *ctx,
     void *local = talloc_new (ctx);
     mime_node_t *root, *part;
     notmuch_status_t status;
+    unsigned int session_keys = 0;
+    notmuch_status_t session_key_count_error = NOTMUCH_STATUS_SUCCESS;
+
+    if (params->crypto.decrypt == NOTMUCH_DECRYPT_TRUE)
+	session_key_count_error = notmuch_message_count_properties (message, "session-key", &session_keys);
 
     status = mime_node_open (local, message, &(params->crypto), &root);
     if (status)
@@ -880,6 +885,21 @@ show_message (void *ctx,
     part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
     if (part)
 	status = format->part (local, sp, part, indent, params);
+
+    if (params->crypto.decrypt == NOTMUCH_DECRYPT_TRUE && session_key_count_error == NOTMUCH_STATUS_SUCCESS) {
+	unsigned int new_session_keys = 0;
+	if (notmuch_message_count_properties (message, "session-key", &new_session_keys) == NOTMUCH_STATUS_SUCCESS &&
+	    new_session_keys > session_keys) {
+	    /* try a quiet re-indexing */
+	    notmuch_indexopts_t *indexopts = notmuch_database_get_default_indexopts (notmuch_message_get_database (message));
+	    if (indexopts) {
+		notmuch_indexopts_set_decrypt_policy (indexopts, NOTMUCH_DECRYPT_AUTO);
+		status = notmuch_message_reindex (message, indexopts);
+		if (status)
+		    fprintf (stderr, "Error re-indexing message with --decrypt=stash. (%d) %s\n", status, notmuch_status_to_string (status));
+	    }
+	}
+    }
   DONE:
     talloc_free (local);
     return status;
-- 
2.15.1

Re: [PATCH v2 4/5] cli/show: reindex when we learned new session keys about a message

[David Bremner]

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

> +
> +    if (params->crypto.decrypt == NOTMUCH_DECRYPT_TRUE && session_key_count_error == NOTMUCH_STATUS_SUCCESS) {
> +	unsigned int new_session_keys = 0;
> +	if (notmuch_message_count_properties (message, "session-key", &new_session_keys) == NOTMUCH_STATUS_SUCCESS &&
> +	    new_session_keys > session_keys) {
> +	    /* try a quiet re-indexing */
> +	    notmuch_indexopts_t *indexopts = notmuch_database_get_default_indexopts (notmuch_message_get_database (message));
> +	    if (indexopts) {
> +		notmuch_indexopts_set_decrypt_policy (indexopts, NOTMUCH_DECRYPT_AUTO);
> +		status = notmuch_message_reindex (message, indexopts);
> +		if (status)
> +		    fprintf (stderr, "Error re-indexing message with --decrypt=stash. (%d) %s\n", status, notmuch_status_to_string (status));
> +	    }
> +	}
> +    }

I'm wondering about the lack of #if HAVE_GMIME_SESSION_KEYS here.  Are
you relying here on the number of session keys not increasing when
running a binary without session key support? Is there some advantage to
doing it this way? It seems a bit harder to reason about.

Re: [PATCH v2 4/5] cli/show: reindex when we learned new session keys about a message

[Daniel Kahn Gillmor]

On Tue 2018-05-01 22:36:31 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +
>> +    if (params->crypto.decrypt == NOTMUCH_DECRYPT_TRUE && session_key_count_error == NOTMUCH_STATUS_SUCCESS) {
>> +	unsigned int new_session_keys = 0;
>> +	if (notmuch_message_count_properties (message, "session-key", &new_session_keys) == NOTMUCH_STATUS_SUCCESS &&
>> +	    new_session_keys > session_keys) {
>> +	    /* try a quiet re-indexing */
>> +	    notmuch_indexopts_t *indexopts = notmuch_database_get_default_indexopts (notmuch_message_get_database (message));
>> +	    if (indexopts) {
>> +		notmuch_indexopts_set_decrypt_policy (indexopts, NOTMUCH_DECRYPT_AUTO);
>> +		status = notmuch_message_reindex (message, indexopts);
>> +		if (status)
>> +		    fprintf (stderr, "Error re-indexing message with --decrypt=stash. (%d) %s\n", status, notmuch_status_to_string (status));
>> +	    }
>> +	}
>> +    }
>
> I'm wondering about the lack of #if HAVE_GMIME_SESSION_KEYS here.  Are
> you relying here on the number of session keys not increasing when
> running a binary without session key support? Is there some advantage to
> doing it this way? It seems a bit harder to reason about.

yes, i'm relying on the number of session keys not increasing if we
don't know how to extract the session keys.  I suppose i could also
imagine some other way that session keys get supplied during a "show",
but then i guess that'd make the reindex moot, which would be a Bad
Thing.

I avoided #if'ing out these sections because i thought the general
strategy was to avoid preprocessor shenanigans where possible, as it
makes the code harder to think about given the combinatorial explosion
of #define options.  But I don't mind wrapping the hunks of this patch
in an #if if the consensus is that this approach is preferable.

   --dkg

Re: [PATCH v2 4/5] cli/show: reindex when we learned new session keys about a message

[Daniel Kahn Gillmor]

On Wed 2018-05-09 14:42:35 -0400, Daniel Kahn Gillmor wrote:
> I avoided #if'ing out these sections because i thought the general
> strategy was to avoid preprocessor shenanigans where possible, as it
> makes the code harder to think about given the combinatorial explosion
> of #define options.  But I don't mind wrapping the hunks of this patch
> in an #if if the consensus is that this approach is preferable.

Note that in v3 of this series, i did *not* wrap this section in #if
blocks.  Let me know if you'd prefer a v4 that does so, or (if you
prefer, and it's the only thing preventing a merge) feel free to add the
wrapping them directly as you merge.

Thanks!

        --dkg

[PATCH v2 5/5] cli/show: enable --decrypt=stash

[Daniel Kahn Gillmor]

Add fancy new feature, which makes "notmuch show" capable of actually
indexing messages that it just decrypted.

This enables a workflow where messages can come in in the background
and be indexed using "--decrypt=auto".  But when showing an encrypted
message for the first time, it gets automatically indexed.

This is something of a departure for "notmuch show" -- in particular,
because it requires read/write access to the database.  However, this
might be a common use case -- people get mail delivered and indexed in
the background, but only want access to their secret key to happen
when they're directly interacting with notmuch itself.

In such a scenario, they couldn't search newly-delivered, encrypted
messages, but they could search for them once they've read them.

Documentation of this new feature also uses a table form, similar to
that found in the description of index.decrypt in notmuch-config(1).

A notmuch UI that wants to facilitate this workflow while also
offering an interactive search interface might instead make use of
these additional commands while the user is at the console:

Count received encrypted messages (if > 0, there are some things we
haven't yet tried to index, and therefore can't yet search):

     notmuch count tag:encrypted and \
         not property:index.decryption=success and \
         not property:index.decryption=failure

Reindex those messages:

     notmuch reindex --try-decrypt=true tag:encrypted and \
         not property:index.decryption=success and \
         not property:index.decryption=failure
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-show.rst          | 40 ++++++++++++++++++++++++++++++++------
 notmuch-show.c                     |  9 +++++++--
 test/T357-index-decryption.sh      | 18 +++++++++++++++++
 4 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 249b9664..15425697 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -522,7 +522,7 @@ _notmuch_show()
 	    return
 	    ;;
         --decrypt)
-	    COMPREPLY=( $( compgen -W "true auto false" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "true auto false stash" -- "${cur}" ) )
 	    return
 	    ;;
     esac
diff --git a/doc/man1/notmuch-show.rst b/doc/man1/notmuch-show.rst
index 2b825ccc..66e8024c 100644
--- a/doc/man1/notmuch-show.rst
+++ b/doc/man1/notmuch-show.rst
@@ -110,7 +110,7 @@ Supported options for **show** include
     supported with --format=json and --format=sexp), and the
     multipart/signed part will be replaced by the signed data.
 
-``--decrypt=(false|auto|true)``
+``--decrypt=(false|auto|true|stash)``
     If ``true``, decrypt any MIME encrypted parts found in the
     selected content (i.e. "multipart/encrypted" parts). Status of
     the decryption will be reported (currently only supported
@@ -118,17 +118,45 @@ Supported options for **show** include
     decryption the multipart/encrypted part will be replaced by
     the decrypted content.
 
+    ``stash`` behaves like ``true``, but upon successful decryption it
+    will also stash the message's session key in the database, and
+    index the cleartext of the message, enabling automatic decryption
+    in the future.
+
     If ``auto``, and a session key is already known for the
     message, then it will be decrypted, but notmuch will not try
     to access the user's keys.
 
     Use ``false`` to avoid even automatic decryption.
 
-    Non-automatic decryption expects a functioning
-    **gpg-agent(1)** to provide any needed credentials. Without
-    one, the decryption will fail.
-
-    Note: ``true`` implies --verify.
+    Non-automatic decryption (``stash`` or ``true``, in the absence of
+    a stashed session key) expects a functioning **gpg-agent(1)** to
+    provide any needed credentials. Without one, the decryption will
+    fail.
+
+    Note: setting either ``true`` or ``stash`` here implies
+    ``--verify``.
+
+    Here is a table that summarizes each of these policies:
+
+    +------------------------+-------+------+------+-------+
+    |                        | false | auto | true | stash |
+    +========================+=======+======+======+=======+
+    | Show cleartext if      |       |  X   |  X   |   X   |
+    | session key is         |       |      |      |       |
+    | already known          |       |      |      |       |
+    +------------------------+-------+------+------+-------+
+    | Use secret keys to     |       |      |  X   |   X   |
+    | show cleartext         |       |      |      |       |
+    +------------------------+-------+------+------+-------+
+    | Stash any newly        |       |      |      |   X   |
+    | recovered session keys,|       |      |      |       |
+    | reindexing message if  |       |      |      |       |
+    | found                  |       |      |      |       |
+    +------------------------+-------+------+------+-------+
+
+    Note: ``--decrypt=stash`` requires a writable database.
+    Otherwise, ``notmuch show`` operates entirely in read-only mode.
 
     Default: ``auto``
 
diff --git a/notmuch-show.c b/notmuch-show.c
index 90e45cd9..b09d1f4b 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1124,6 +1124,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
 				  { "auto", NOTMUCH_DECRYPT_AUTO },
 				  { "true", NOTMUCH_DECRYPT_NOSTASH },
+				  { "stash", NOTMUCH_DECRYPT_TRUE },
 				  { 0, 0 } } },
 	{ .opt_bool = &params.crypto.verify, .name = "verify" },
 	{ .opt_bool = &params.output_body, .name = "body" },
@@ -1139,7 +1140,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_process_shared_options (argv[0]);
 
     /* explicit decryption implies verification */
-    if (params.crypto.decrypt == NOTMUCH_DECRYPT_NOSTASH)
+    if (params.crypto.decrypt == NOTMUCH_DECRYPT_NOSTASH ||
+	params.crypto.decrypt == NOTMUCH_DECRYPT_TRUE)
 	params.crypto.verify = true;
 
     /* specifying a part implies single message display */
@@ -1202,8 +1204,11 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     params.crypto.gpgpath = notmuch_config_get_crypto_gpg_path (config);
 #endif
 
+    notmuch_database_mode_t mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
+    if (params.crypto.decrypt == NOTMUCH_DECRYPT_TRUE)
+	mode = NOTMUCH_DATABASE_MODE_READ_WRITE;
     if (notmuch_database_open (notmuch_config_get_database_path (config),
-			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
+			       mode, &notmuch))
 	return EXIT_FAILURE;
 
     notmuch_exit_if_unmatched_db_uuid (notmuch);
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 2b8e05b8..6e7dc74e 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -80,6 +80,24 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+# show the message using stashing decryption
+test_begin_subtest "stash decryption during show"
+output=$(notmuch show --decrypt=stash tag:encrypted subject:002 | 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 "search should now show the contents"
+output=$(notmuch search wumpus)
+expected='thread:0000000000000003   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
+if [ $NOTMUCH_HAVE_GMIME_SESSION_KEYS -eq 0 ]; then
+    test_subtest_known_broken
+fi
+test_expect_equal \
+    "$output" \
+    "$expected"
+
 # 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 --decrypt=true"
-- 
2.15.1

Re: [PATCH v2 5/5] cli/show: enable --decrypt=stash

[David Bremner]

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

> +
> +    Note: ``--decrypt=stash`` requires a writable database.
> +    Otherwise, ``notmuch show`` operates entirely in read-only mode.

I would rephrase this as "requires write access to the database";
otherwise it sounds like "writable" (or lack) is persistent property of
databases.


> +# show the message using stashing decryption
> +test_begin_subtest "stash decryption during show"
> +output=$(notmuch show --decrypt=stash tag:encrypted subject:002 | 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"
> +

This is a bit hard to follow. I think it would be better to isolate this
kind of parsing in a function in test-lib.sh; then at least the name
would suggest the intent.

> +test_begin_subtest "search should now show the contents"

I think the point is not that it _shows_ the contents, but that it finds
them

> +output=$(notmuch search wumpus)
> +expected='thread:0000000000000003   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox unread)'
> +if [ $NOTMUCH_HAVE_GMIME_SESSION_KEYS -eq 0 ]; then
> +    test_subtest_known_broken
> +fi
> +test_expect_equal \
> +    "$output" \
> +    "$expected"

Re: notmuch show --decrypt=stash

[Daniel Kahn Gillmor]

On Tue 2018-01-09 18:12:23 -0500, Daniel Kahn Gillmor wrote:
> This is a revision of the series initially introduced in
> id:20171212025225.11854-1-dkg@fifthhorseman.net, with minor updates to
> accomodate the recent release of notmuch 0.26 (yay!)
>
> This series allows "notmuch show" to index the cleartext and stash the
> session keys of an encrypted message while displaying it.

I'd really appreciate review for this series!  I think it adds useful
functionality to notmuch, enabling a particular workflow for people to
use the cleartext index who don't want mail being decrypted in the
background.

Please take a look and let me know what you think!

       --dkg

notmuch show --decrypt=stash

[Daniel Kahn Gillmor]

This series allows "notmuch show" to index the cleartext and stash the
session keys of an encrypted message while displaying it.

Because it uses a keyword argument to --decrypt for "notmuch show", It
needs to be applied *after* the series with the subject:

    Encourage explicit arguments for --decrypt in "show" and "reply"


Background
----------

The cleartext indexing and session-keys series make working with
encrypted e-mail significantly easier in notmuch.  However, their
underlying assumption is that at the time of message ingestion (and
"notmuch new" in particular), the user is likely to have access to
their long-term secret keys.

In practice, many people using GnuPG today have their secret keys
locked behind a passphrase, or on a smartcard, and also run "notmuch
new" in some sort of scheduled, backgrounded process.

The result is that for users with this workflow, GnuPG prompts for
their passphrase (or to trigger their smartcard) at unpredictable
times, depending on when their mail delivery happens, and on how many
encrypted messages they receive.  This is both unfriendly and bad for
security (we should not train users to approve random prompts for
secret key access when nothing they're doing interactively seems to
warrant it).

Outline
-------

For a friendlier experience, some users may prefer incoming encrypted
mail to stay in their inbox *without* being decrypted, until they
choose to look at it.  At the moment that they're looking at it, their
MUA is in the foreground and they're interacting with it, so being
prompted for their password or smartcard interactively makes sense at
that time.

This series makes it possible for this interaction to to actually
decrypt the message, index it, and stash any session keys the first
time the user interacts with the message through "notmuch show".

This is not a workflow that every MUA will choose to use (e.g. users
whose decryption-capable secret key is already cheaply available
without hassling the user at "notmuch new" shouldn't use it), but it
is a sensible workflow for some users that notmuch should support.

Furthermore, it is a more efficient use of secret key material -- a
user that wants to stash session keys of a message, but whose
long-term decryption secret key is on a smartcard should only be obliged
to trigger the smartcard once per message, ever.

Implementation details
----------------------

The most controversial part of this series is that it makes "notmuch
show" potentially not a read-only operation on the database.  This is
a tradeoff that the users of this workflow will need to consider,
since they are explicitly asking "notmuch show" to potentially modify
their index.

Note that i've made this R/O-to-R/W switch fairly coarse.  If the user
requests --decrypt=stash, then "notmuch show" will operate on a
read/write database, regardless of whether the message is actually
encrypted.  I used this coarse approach because i couldn't figure out
a safe way to reopen an existing read-only database in read-write
mode.  If someone more clever with Xapian than me wants to suggest a
way to do this in a more fine-grained fashion, i'd welcome patches or
pointers.

I welcome review and feedback.

  --dkg