allow indexing cleartext of encrypted messages

allow indexing cleartext of encrypted messages

[Daniel Kahn Gillmor]

Notmuch currently doesn't index the cleartext of encrypted mail.  This
is the right choice by default, because the index is basically
cleartext-equivalent, and we wouldn't want every indexed mailstore to
leak the contents of its encrypted mails.

However, if a notmuch user has their index in a protected location,
they may prefer the convenience of being able to search the contents
of (at least some of) their encrypted mail.

This series of patches enables notmuch to index the cleartext of
specific encrypted messages when they're being added via "notmuch new"
or "notmuch insert", via a new --try-decrypt flag.

If --try-decrypt is used, and decryption is successful for part of a
message, the message gets an additional "index-decrypted" tag.  If
decryption of part of a message fails, the message gets an additional
"index-decryption-failed" tag.

This tagging approach should allow people to figure out which messages
have been indexed in the clear (or not), and can be used to
selectively reindex them in batch with something like:

----------------
#!/usr/bin/env python3

'''notmuch-reindex.py -- a quick and dirty pythonic mechanism to
re-index specific messages in a notmuch database.  This should
probably be properly implemented as a subcommand for /usr/bin/notmuch
itself'''

import notmuch
import sys

d = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)

query = sys.argv[1]

q = d.create_query(query)

for m in q.search_messages():
    mainfilename = m.get_filename()
    origtags = m.get_tags()
    tags = []
    for t in origtags:
        if t not in ['index-decrypted', 'index-decryption-failed']:
            tags += [t]
    d.begin_atomic()
    for f in m.get_filenames():
        d.remove_message(f)
    (newm,stat) = d.add_message(mainfilename, try_decrypt=True)
    for tag in tags:
        newm.add_tag(tag)
    d.end_atomic()
----------------

A couple key points:

 * There is some code duplication between crypto.c (for the
   notmuch-client) and lib/database.cc and lib/index.cc (for the
   library) because both parts of the codebase use gmime to handle the
   decryption.  I don't want to contaminate the libnotmuch API with
   gmime implementation details, so i don't quite see how to reuse the
   code cleanly.  I'd love suggestions on how to reduce the
   duplications.

 * the libnotmuch API is extended with
   notmuch_database_add_message_try_decrypt().  This should probably
   ultimately be more general, because there are a few additional
   knobs that i can imagine fiddling at indexing time.  For example:

    * verifying cryptographic signatures and storing something about
      those verifications in the notmuch db
     
    * extracting OpenPGP session key information for a given message
      and storing it in a lookaside table in the notmuch db, so that
      it's possible to securely destroy old encryption-capable keys
      and still have local access to the cleartext of the remaining
      messages.

   Some of these additional features might be orthogonal to one
   another as well.  I welcome suggestions for how to improve the API
   so that we don't end up with a combinatorial explosion of
   n_d_add_message_foo() functions.

 * To properly complete this patch series, i think i want to make
   notmuch-reindex.c and add a reindex subcommand, also with a
   --try-decrypt option.  It's not clear to me if the right approach
   for that is to have a C implementation of the python script above
   without modifying libnotmuch, or if i should start by creating a
   notmuch_message_reindex function in libnotmuch, with a try_decrypt
   flag.  Again, suggestions welcome.

 * Is the tagging approach the right thing to do to record success or
   failure of decryption at index time?  Is there a better approach?

[PATCH 1/9] reorganize indexing of multipart/signed and multipart/encrypted

[Daniel Kahn Gillmor]

This prepares the codebase for a cleaner changeset for dealing with
indexing some encrypted messages in the clear.
---
 lib/index.cc | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index f166aef..2fa6616 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -333,27 +333,25 @@ _index_mime_part (notmuch_message_t *message,
 	GMimeMultipart *multipart = GMIME_MULTIPART (part);
 	int i;
 
-	if (GMIME_IS_MULTIPART_SIGNED (multipart))
-	  _notmuch_message_add_term (message, "tag", "signed");
-
-	if (GMIME_IS_MULTIPART_ENCRYPTED (multipart))
-	  _notmuch_message_add_term (message, "tag", "encrypted");
-
-	for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
-	    if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
-		/* Don't index the signature. */
-		if (i == 1)
-		    continue;
-		if (i > 1)
-		    _notmuch_database_log (_notmuch_message_database (message),
-					  "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
-	    }
-	    if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
-		/* Don't index encrypted parts. */
-		continue;
-	    }
+	if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
+	    _notmuch_message_add_term (message, "tag", "signed");
+	    /* FIXME: should we try to validate the signature? */
+	    
+	    /* FIXME: is it always just the first part that is signed in
+	     all multipart/signed messages?*/
 	    _index_mime_part (message,
-			      g_mime_multipart_get_part (multipart, i));
+			      g_mime_multipart_get_part (multipart, 0));
+	    
+	    if (g_mime_multipart_get_count (multipart) > 2)
+		_notmuch_database_log (_notmuch_message_database (message),
+				       "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
+	} else if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
+	    /* Don't index encrypted parts */
+	} else {
+	    for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
+		_index_mime_part (message,
+				  g_mime_multipart_get_part (multipart, i));
+	    }
 	}
 	return;
     }
-- 
2.6.2

[PATCH 2/9] Add a lazily-initialized crypto context to notmuch_database_t

[Daniel Kahn Gillmor]

This is in large part a duplicate of parts of crypto.c, but that code
is in the client (outside the library), and we don't want to entangle
the libgmime API with the libnotmuch API.

I welcome better proposals for how to share this code explicitly
between the library and the client.
---
 lib/database-private.h |  1 +
 lib/database.cc        | 42 ++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch-private.h  |  8 ++++++++
 3 files changed, 51 insertions(+)

diff --git a/lib/database-private.h b/lib/database-private.h
index 3fb10f7..1bf76c5 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -171,6 +171,7 @@ struct _notmuch_database {
      * notmuch_database_new_revision. */
     unsigned long revision;
     const char *uuid;
+    GMimeCryptoContext *gpg_crypto_ctx;
 
     Xapian::QueryParser *query_parser;
     Xapian::TermGenerator *term_gen;
diff --git a/lib/database.cc b/lib/database.cc
index 3b342f1..13b0bad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -995,6 +995,8 @@ notmuch_database_open_verbose (const char *path,
 	notmuch->uuid = talloc_strdup (
 	    notmuch, notmuch->xapian_db->get_uuid ().c_str ());
 
+	notmuch->gpg_crypto_ctx = NULL;
+	
 	notmuch->query_parser = new Xapian::QueryParser;
 	notmuch->term_gen = new Xapian::TermGenerator;
 	notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));
@@ -1090,6 +1092,11 @@ notmuch_database_close (notmuch_database_t *notmuch)
     delete notmuch->last_mod_range_processor;
     notmuch->last_mod_range_processor = NULL;
 
+    if (notmuch->gpg_crypto_ctx) {
+	g_object_unref (notmuch->gpg_crypto_ctx);
+	notmuch->gpg_crypto_ctx = NULL;
+    }
+    
     return status;
 }
 
@@ -2386,6 +2393,41 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
     return status;
 }
 
+notmuch_private_status_t
+_notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
+					   const char *protocol,
+					   GMimeCryptoContext **crypto_ctx)
+{
+    if (! protocol)
+	return NOTMUCH_PRIVATE_STATUS_MALFORMED_CRYPTO_PROTOCOL;
+    
+    /* As per RFC 1847 section 2.1: "the [protocol] value token is
+     * comprised of the type and sub-type tokens of the Content-Type".
+     * As per RFC 1521 section 2: "Content-Type values, subtypes, and
+     * parameter names as defined in this document are
+     * case-insensitive."  Thus, we use strcasecmp for the protocol.
+     */
+    if (strcasecmp (protocol, "application/pgp-signature") == 0 ||
+	strcasecmp (protocol, "application/pgp-encrypted") == 0) {
+	if (! notmuch->gpg_crypto_ctx) {
+	    /* FIXME: how do we allow for configuring the gpg binary
+	     * here? how would this config get into the library?  Is
+	     * this an option we can set on the database object?  Or
+	     * elsewhere?  */
+	    notmuch->gpg_crypto_ctx = g_mime_gpg_context_new (NULL, "gpg");
+	    if (! notmuch->gpg_crypto_ctx)
+		return NOTMUCH_PRIVATE_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
+
+	    g_mime_gpg_context_set_use_agent ((GMimeGpgContext *) notmuch->gpg_crypto_ctx, TRUE);
+	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) notmuch->gpg_crypto_ctx, FALSE);
+	}
+	*crypto_ctx = notmuch->gpg_crypto_ctx;
+	return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+    } else {
+	return NOTMUCH_PRIVATE_STATUS_UNKNOWN_CRYPTO_PROTOCOL;
+    }
+}
+
 notmuch_status_t
 notmuch_database_add_message (notmuch_database_t *notmuch,
 			      const char *filename,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 5dd4770..f6fd36a 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -141,6 +141,9 @@ typedef enum _notmuch_private_status {
     /* Then add our own private values. */
     NOTMUCH_PRIVATE_STATUS_TERM_TOO_LONG = NOTMUCH_STATUS_LAST_STATUS,
     NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND,
+    NOTMUCH_PRIVATE_STATUS_MALFORMED_CRYPTO_PROTOCOL,
+    NOTMUCH_PRIVATE_STATUS_FAILED_CRYPTO_CONTEXT_CREATION,
+    NOTMUCH_PRIVATE_STATUS_UNKNOWN_CRYPTO_PROTOCOL,
 
     NOTMUCH_PRIVATE_STATUS_LAST_STATUS
 } notmuch_private_status_t;
@@ -239,6 +242,11 @@ _notmuch_database_filename_to_direntry (void *ctx,
 					notmuch_find_flags_t flags,
 					char **direntry);
 
+notmuch_private_status_t
+_notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
+					   const char *protocol,
+					   GMimeCryptoContext **crypto_ctx);
+
 /* directory.cc */
 
 notmuch_directory_t *
-- 
2.6.2

[PATCH 3/9] index encrypted parts when the message is flagged appropriately

[Daniel Kahn Gillmor]

We add a new message flag that indicates desire for indexing the
cleartext of a given message.

If that flag is set while indexing, we'll try to descend into it.

If we can decrypt, we tag the message with index-decrypted.

If we can't decrypt (or recognize the encrypted type of mail), we tag
with decryption-failed.

Note that a single message may be tagged with "encrypted" and
"index-decrypted" and "decryption-failed".  For example, consider a
message that includes multiple layers of encryption.  It is
automatically tagged with "encrypted".  If we decrypt the outer layer
("index-decrypted"), but fail on the inner layer
("decryption-failed").
---
 lib/index.cc  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/notmuch.h |  5 +++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/lib/index.cc b/lib/index.cc
index 2fa6616..bd028cb 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -300,6 +300,10 @@ _index_address_list (notmuch_message_t *message,
     }
 }
 
+static void
+_index_encrypted_mime_part (notmuch_message_t *message, GMimeContentType *content_type,
+			    GMimeMultipartEncrypted *part);
+
 /* Callback to generate terms for each mime part of a message. */
 static void
 _index_mime_part (notmuch_message_t *message,
@@ -346,7 +350,10 @@ _index_mime_part (notmuch_message_t *message,
 		_notmuch_database_log (_notmuch_message_database (message),
 				       "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
 	} else if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
-	    /* Don't index encrypted parts */
+	    _notmuch_message_add_term (message, "tag", "encrypted");
+	    if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_INDEX_DECRYPTED)) {
+		_index_encrypted_mime_part(message, content_type, GMIME_MULTIPART_ENCRYPTED (part));
+	    }
 	} else {
 	    for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
 		_index_mime_part (message,
@@ -431,6 +438,49 @@ _index_mime_part (notmuch_message_t *message,
     }
 }
 
+/* descend (if possible) into the cleartext part of an encrypted MIME
+ * part while indexing. */
+static void
+_index_encrypted_mime_part (notmuch_message_t *message,
+			    GMimeContentType *content_type,
+			    GMimeMultipartEncrypted *encrypted_data)
+{
+    notmuch_private_status_t status;
+    GMimeCryptoContext* crypto_ctx = NULL;
+    const char *protocol = g_mime_content_type_get_parameter (content_type, "protocol");
+    GError *err = NULL;
+    notmuch_database_t * notmuch = _notmuch_message_database (message);
+    GMimeObject *clear = NULL;
+    
+    status = _notmuch_database_get_crypto_for_protocol (notmuch, protocol,
+							&crypto_ctx);
+    if (status) {
+	_notmuch_database_log (notmuch, "Warning: setup failed for decrypting "
+			       "during indexing. (%d)\n", status);
+	_notmuch_message_add_term (message, "tag", "index-decryption-failed");
+	return;
+    }
+
+    /* we don't need the GMimeDecryptResult, because we're not looking
+     * at validating signatures, and we don't care about indexing who
+     * the message was ostensibly encrypted to.
+     */
+    clear = g_mime_multipart_encrypted_decrypt(encrypted_data, crypto_ctx,
+					       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);
+	/* Indicate that we failed to decrypt during indexing */
+	_notmuch_message_add_term (message, "tag", "index-decryption-failed");
+	return;
+    }
+    _index_mime_part (message, clear);
+    g_object_unref (clear);
+    
+    _notmuch_message_add_term (message, "tag", "index-decrypted");
+}
+
 notmuch_status_t
 _notmuch_message_index_file (notmuch_message_t *message,
 			     notmuch_message_file_t *message_file)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 310a8b8..e7085b7 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1357,6 +1357,11 @@ typedef enum _notmuch_message_flag {
      * thread ID.
      */
     NOTMUCH_MESSAGE_FLAG_GHOST,
+    
+    /* Some part(s) of this message is encrypted, but the message is
+     * indexed in the clear.
+     */
+    NOTMUCH_MESSAGE_FLAG_INDEX_DECRYPTED
 } notmuch_message_flag_t;
 
 /**
-- 
2.6.2

[PATCH 4/9] Add new n_d_add_message_try_decrypt (analogous to to n_d_add_message)

[Daniel Kahn Gillmor]

When adding a message to the database, optionally try to decrypt the
message and index the cleartext.

Note that when a message is retrieved from the database, it will not
have this flag attached to it necessarily (though users can inspect
the tags that were attached during decryption/indexing)
---
 lib/database.cc | 31 ++++++++++++++++++++++++++++---
 lib/notmuch.h   | 19 +++++++++++++++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 13b0bad..62bc6d9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2429,9 +2429,10 @@ _notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
 }
 
 notmuch_status_t
-notmuch_database_add_message (notmuch_database_t *notmuch,
-			      const char *filename,
-			      notmuch_message_t **message_ret)
+_notmuch_database_add_message_with_options (notmuch_database_t *notmuch,
+					    const char *filename,
+					    notmuch_bool_t decrypt,
+					    notmuch_message_t **message_ret)
 {
     notmuch_message_file_t *message_file;
     notmuch_message_t *message = NULL;
@@ -2550,6 +2551,8 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 	    date = _notmuch_message_file_get_header (message_file, "date");
 	    _notmuch_message_set_header_values (message, date, from, subject);
 
+	    notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_INDEX_DECRYPTED, decrypt);
+
 	    ret = _notmuch_message_index_file (message, message_file);
 	    if (ret)
 		goto DONE;
@@ -2587,6 +2590,28 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
     return ret;
 }
 
+
+notmuch_status_t
+notmuch_database_add_message (notmuch_database_t *notmuch,
+			      const char *filename,
+			      notmuch_message_t **message_ret)
+{
+    return _notmuch_database_add_message_with_options (notmuch, filename,
+						       false,
+						       message_ret);
+    
+}
+notmuch_status_t
+notmuch_database_add_message_try_decrypt (notmuch_database_t *notmuch,
+					  const char *filename,
+					  notmuch_message_t **message_ret)
+{
+    return _notmuch_database_add_message_with_options (notmuch, filename,
+						       true,
+						       message_ret);
+    
+}
+
 notmuch_status_t
 notmuch_database_remove_message (notmuch_database_t *notmuch,
 				 const char *filename)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e7085b7..809a2ea 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -570,6 +570,25 @@ notmuch_status_t
 notmuch_database_add_message (notmuch_database_t *database,
 			      const char *filename,
 			      notmuch_message_t **message);
+/**
+ * Add a new message to the given notmuch database or associate an
+ * additional filename with an existing message.
+ * 
+ * This does the same thing as notmuch_database_add_message except
+ * that it if part of the message is encrypted, it also tries to
+ * decrypt the message and index the cleartext version if it can.
+ * 
+ * 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 THIS FUNCTION
+ * without considering the security of your index.
+ * 
+ * FIXME: document new error codes here.
+ */
+notmuch_status_t
+notmuch_database_add_message_try_decrypt (notmuch_database_t *database,
+					  const char *filename,
+					  notmuch_message_t **message);
 
 /**
  * Remove a message filename from the given notmuch database. If the
-- 
2.6.2

[PATCH 5/9] Enable cleartext indexing in python bindings

[Daniel Kahn Gillmor]

The python add_message function now includes a try_decrypt flag to
permit indexing the cleartext of the mail where possible.
---
 bindings/python/notmuch/database.py | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 5b58e09..8e25a05 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -388,8 +388,13 @@ class Database(object):
     _add_message.argtypes = [NotmuchDatabaseP, c_char_p,
                              POINTER(NotmuchMessageP)]
     _add_message.restype = c_uint
-
-    def add_message(self, filename, sync_maildir_flags=False):
+    
+    _add_message_try_decrypt = nmlib.notmuch_database_add_message_try_decrypt
+    _add_message_try_decrypt.argtypes = [NotmuchDatabaseP, c_char_p,
+                                         POINTER(NotmuchMessageP)]
+    _add_message_try_decrypt.restype = c_uint
+    
+    def add_message(self, filename, sync_maildir_flags=False, try_decrypt=False):
         """Adds a new message to the database
 
         :param filename: should be a path relative to the path of the
@@ -410,6 +415,13 @@ 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 hasn't been seen by the
+            index before, and it includes any encrypted parts, try to
+            index the cleartext if set to `True`.  Note that this
+            means that the cleartext of the message is effectively
+            stored in the database index, so DO NOT SET THIS FLAG
+            without considering the security of your index.
+
         :returns: On success, we return
 
            1) a :class:`Message` object that can be used for things
@@ -437,10 +449,14 @@ 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._add_message(self._db, _str(filename), byref(msg_p))
+        if (try_decrypt):
+            status = self._add_message_try_decrypt(self._db, _str(filename), byref(msg_p))
+        else:
+            status = self._add_message(self._db, _str(filename), byref(msg_p))
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
             raise NotmuchError(status)
-- 
2.6.2

[PATCH 6/9] search for a reasonable gpg implementation

[Daniel Kahn Gillmor]

When the notmuch database needs to find gpg, have it search reasonable
paths first.
---
 lib/database.cc | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/database.cc b/lib/database.cc
index 62bc6d9..d0e8800 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2393,6 +2393,17 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
     return status;
 }
 
+static const char*
+_notmuch_database_get_gpg_path (notmuch_database_t *notmuch)
+{
+#define try_gpg_path(z) if (!access(z, X_OK)) return z
+    try_gpg_path("/usr/bin/gpg2");
+    try_gpg_path("/bin/gpg2");
+    try_gpg_path("/usr/bin/gpg");
+    try_gpg_path("/bin/gpg");
+    return NULL;
+}
+
 notmuch_private_status_t
 _notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
 					   const char *protocol,
@@ -2414,7 +2425,7 @@ _notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
 	     * here? how would this config get into the library?  Is
 	     * this an option we can set on the database object?  Or
 	     * elsewhere?  */
-	    notmuch->gpg_crypto_ctx = g_mime_gpg_context_new (NULL, "gpg");
+	    notmuch->gpg_crypto_ctx = g_mime_gpg_context_new (NULL, _notmuch_database_get_gpg_path(notmuch));
 	    if (! notmuch->gpg_crypto_ctx)
 		return NOTMUCH_PRIVATE_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
 
-- 
2.6.2

[PATCH 7/9] add a gpg_path value for notmuch_database_t

[Daniel Kahn Gillmor]

Exposing this to the user of the library lets the user point to
arbitrary gpg executables when trying to decrypt.
---
 lib/database-private.h |  3 ++
 lib/database.cc        | 93 +++++++++++++++++++++++++++++++++++++++++++-------
 lib/notmuch.h          | 31 +++++++++++++++++
 3 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index 1bf76c5..9a35044 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -171,6 +171,9 @@ struct _notmuch_database {
      * notmuch_database_new_revision. */
     unsigned long revision;
     const char *uuid;
+
+    /* can be NULL, meaning "try to find gpg2 or gpg if possible" */
+    char *gpg_path;
     GMimeCryptoContext *gpg_crypto_ctx;
 
     Xapian::QueryParser *query_parser;
diff --git a/lib/database.cc b/lib/database.cc
index d0e8800..c40ce77 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -996,6 +996,7 @@ notmuch_database_open_verbose (const char *path,
 	    notmuch, notmuch->xapian_db->get_uuid ().c_str ());
 
 	notmuch->gpg_crypto_ctx = NULL;
+	notmuch->gpg_path = NULL;
 	
 	notmuch->query_parser = new Xapian::QueryParser;
 	notmuch->term_gen = new Xapian::TermGenerator;
@@ -1096,6 +1097,10 @@ notmuch_database_close (notmuch_database_t *notmuch)
 	g_object_unref (notmuch->gpg_crypto_ctx);
 	notmuch->gpg_crypto_ctx = NULL;
     }
+    if (notmuch->gpg_path) {
+	talloc_free(notmuch->gpg_path);
+	notmuch->gpg_path = NULL;
+    }
     
     return status;
 }
@@ -2393,17 +2398,6 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
     return status;
 }
 
-static const char*
-_notmuch_database_get_gpg_path (notmuch_database_t *notmuch)
-{
-#define try_gpg_path(z) if (!access(z, X_OK)) return z
-    try_gpg_path("/usr/bin/gpg2");
-    try_gpg_path("/bin/gpg2");
-    try_gpg_path("/usr/bin/gpg");
-    try_gpg_path("/bin/gpg");
-    return NULL;
-}
-
 notmuch_private_status_t
 _notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
 					   const char *protocol,
@@ -2425,7 +2419,7 @@ _notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
 	     * here? how would this config get into the library?  Is
 	     * this an option we can set on the database object?  Or
 	     * elsewhere?  */
-	    notmuch->gpg_crypto_ctx = g_mime_gpg_context_new (NULL, _notmuch_database_get_gpg_path(notmuch));
+	    notmuch->gpg_crypto_ctx = g_mime_gpg_context_new (NULL, notmuch_database_get_gpg_path(notmuch));
 	    if (! notmuch->gpg_crypto_ctx)
 		return NOTMUCH_PRIVATE_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
 
@@ -2752,3 +2746,78 @@ notmuch_database_status_string (const notmuch_database_t *notmuch)
 {
     return notmuch->status_string;
 }
+
+
+static notmuch_bool_t
+_find_in_path(const char* path)
+{
+    char *c = NULL, *save = NULL, *tok;
+    size_t n;
+    int dfd = -1;
+    notmuch_bool_t ret = FALSE;
+    
+    n = confstr(_CS_PATH, NULL, 0);
+    c = (char*)talloc_size(NULL, n);
+    if (!c)
+	return FALSE;
+    confstr(_CS_PATH, c, n);
+
+    tok = strtok_r(c, ":", &save);
+    while (tok) {
+	dfd = open(tok, O_DIRECTORY | O_RDONLY);
+	if (dfd != -1) {
+	    if (!faccessat(dfd, path, X_OK, 0)) {
+		ret = TRUE;
+		goto done;
+	    }
+	    close(dfd);
+	}
+	tok = strtok_r(NULL, ":", &save);
+    }
+done:
+    if (dfd != -1)
+	close(dfd);
+    if (c)
+	talloc_free(c);
+    return ret;
+}
+
+notmuch_status_t
+notmuch_database_set_gpg_path (notmuch_database_t *notmuch, const char* path)
+{
+    /* return success if this matches what is already configured */
+    if ((!path && !notmuch->gpg_path) ||
+	(path && notmuch->gpg_path && 0 == strcmp(path, notmuch->gpg_path)))
+	return NOTMUCH_STATUS_SUCCESS;
+    
+    if (!path && !_find_in_path(path))
+	return NOTMUCH_STATUS_FILE_ERROR;
+
+    /* clear any existing gpg_crypto_ctx, since things are changing */
+    if (notmuch->gpg_crypto_ctx) {
+	g_object_unref (notmuch->gpg_crypto_ctx);
+	notmuch->gpg_crypto_ctx = NULL;
+    }
+
+    if (notmuch->gpg_path) {
+	talloc_free(notmuch->gpg_path);
+	notmuch->gpg_path = NULL;
+    }
+
+    if (path)
+	notmuch->gpg_path = talloc_strdup (notmuch, path);
+    
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+const char*
+notmuch_database_get_gpg_path (const notmuch_database_t *notmuch)
+{
+    if (notmuch->gpg_path)
+	return notmuch->gpg_path;
+
+#define try_gpg_path(z) if (_find_in_path(z)) return z
+    try_gpg_path("gpg2");
+    try_gpg_path("gpg");
+    return NULL;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 809a2ea..e9cfed3 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -591,6 +591,37 @@ notmuch_database_add_message_try_decrypt (notmuch_database_t *database,
 					  notmuch_message_t **message);
 
 /**
+ * Tell the notmuch database where to find GnuPG.
+ *
+ * This is only useful when notmuch might try to use GnuPG to decrypt
+ * MIME parts (see for example
+ * notmuch_database_add_message_try_decrypt).  The argument needs to
+ * be an executable version of gpg.
+ * 
+ * If this function has never been invoked, notmuch will try to find
+ * gpg in reasonable places.
+ *
+ * This value is not currently stored in the database on disk, it is
+ * only used for this notmuch_database_t while it exists.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: the path was accepted and will be used.
+ * 
+ * NOTMUCH_STATUS_FILE_ERROR: the path given either wasn't found or
+ *      wasn't executable.
+ */
+notmuch_status_t
+notmuch_database_set_gpg_path (notmuch_database_t *database, const char* path);
+			       
+/**
+ * Find out where the notmuch database will try to find gpg if it
+ * needs to use it.
+ */
+const char*
+notmuch_database_get_gpg_path (const notmuch_database_t *database);
+
+/**
  * Remove a message filename from the given notmuch database. If the
  * message has no more filenames, remove the message.
  *
-- 
2.6.2

[PATCH 8/9] add --try-decrypt to notmuch insert

[Daniel Kahn Gillmor]

allow an incoming message to be delivered while indexing the
cleartext.

This requires the secret keys for the message to be available.  For
the moment, the most functional approach is to ensure that gpg-agent
is running and knows about any secret keys that might be useful to
decrypt incoming mail.

Any additional recommendations for how to phrase the caveat for this
option are welcome.

If ~/.notmuch-config contains crypto.gpg_path, pass it into the
database.
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-insert.rst        | 11 +++++++++++
 notmuch-insert.c                   | 21 ++++++++++++++++++---
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index cc58392..4bc9040 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -224,7 +224,7 @@ _notmuch_insert()
     ! $split &&
     case "${cur}" in
 	--*)
-	    local options="--create-folder --folder= --keep --no-hooks ${_notmuch_shared_options}"
+	    local options="--create-folder --folder= --keep --no-hooks --try-decrypt ${_notmuch_shared_options}"
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
 	    return
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index 2c9c0d0..9c76b30 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -50,6 +50,17 @@ Supported options for **insert** include
     ``--no-hooks``
         Prevent hooks from being run.
 
+    ``--try-decrypt``
+
+        If the message is encrypted, try to decrypt the message while
+        indexing.  If decryption is successful, index the cleartext
+        itself.  The message is 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 THIS FLAG without considering the
+        security of your index.
+
 EXIT STATUS
 ===========
 
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 5205c17..9742574 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -378,12 +378,15 @@ FAIL:
  */
 static notmuch_status_t
 add_file (notmuch_database_t *notmuch, const char *path, tag_op_list_t *tag_ops,
-	  notmuch_bool_t synchronize_flags, notmuch_bool_t keep)
+	  notmuch_bool_t synchronize_flags, notmuch_bool_t keep, notmuch_bool_t try_decrypt)
 {
     notmuch_message_t *message;
     notmuch_status_t status;
 
-    status = notmuch_database_add_message (notmuch, path, &message);
+    if (try_decrypt)
+	status = notmuch_database_add_message_try_decrypt (notmuch, path, &message);
+    else
+	status = notmuch_database_add_message (notmuch, path, &message);
     if (status == NOTMUCH_STATUS_SUCCESS) {
 	status = tag_op_list_apply (message, tag_ops, 0);
 	if (status) {
@@ -455,6 +458,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_bool_t create_folder = FALSE;
     notmuch_bool_t keep = FALSE;
     notmuch_bool_t no_hooks = FALSE;
+    notmuch_bool_t try_decrypt = FALSE;
     notmuch_bool_t synchronize_flags;
     const char *maildir;
     char *newpath;
@@ -466,6 +470,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	{ NOTMUCH_OPT_BOOLEAN, &create_folder, "create-folder", 0, 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &keep, "keep", 0, 0 },
 	{ NOTMUCH_OPT_BOOLEAN,  &no_hooks, "no-hooks", 'n', 0 },
+	{ NOTMUCH_OPT_BOOLEAN,  &try_decrypt, "try-decrypt", 0, 0 },
 	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
 	{ NOTMUCH_OPT_END, 0, 0, 0, 0 }
     };
@@ -545,8 +550,18 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
+    if (try_decrypt) {
+	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
+	status = notmuch_database_set_gpg_path(notmuch, gpg_path);
+	if (status)
+	    fprintf (stderr, "Warning: failed to set database gpg_path to '%s' (%s)\n",
+		     gpg_path ? gpg_path : "(NULL)",
+		     notmuch_status_to_string (status));
+    }
+	
+
     /* Index the message. */
-    status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep);
+    status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep, try_decrypt);
 
     /* Commit changes. */
     close_status = notmuch_database_destroy (notmuch);
-- 
2.6.2

[PATCH 9/9] add --try-decrypt to notmuch new

[Daniel Kahn Gillmor]

Try to decrypt newly-discovered messages while indexing them.

If ~/.notmuch-config contains crypto.gpg_path, and gpg is needed, it
will be used to find gpg while indexing.
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-new.rst           | 10 ++++++++++
 notmuch-new.c                      | 18 +++++++++++++++++-
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 4bc9040..214f776 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -247,7 +247,7 @@ _notmuch_new()
 
     case "${cur}" in
 	-*)
-	    local options="--no-hooks --quiet ${_notmuch_shared_options}"
+	    local options="--no-hooks --try-decrypt --quiet ${_notmuch_shared_options}"
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "${options}" -- ${cur}) )
 	    ;;
diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
index 787ed78..cf08021 100644
--- a/doc/man1/notmuch-new.rst
+++ b/doc/man1/notmuch-new.rst
@@ -43,6 +43,16 @@ Supported options for **new** include
     ``--quiet``
         Do not print progress or results.
 
+    ``--try-decrypt``
+
+        For each message, if it is encrypted, try to decrypt it while
+        indexing.  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 THIS FLAG without considering the
+        security of your index.
+
 SEE ALSO
 ========
 
diff --git a/notmuch-new.c b/notmuch-new.c
index d45d0af..e5903b8 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -49,6 +49,7 @@ typedef struct {
     size_t new_tags_length;
     const char **new_ignore;
     size_t new_ignore_length;
+    notmuch_bool_t try_decrypt;
 
     int total_files;
     int processed_files;
@@ -260,7 +261,10 @@ add_file (notmuch_database_t *notmuch, const char *filename,
     if (status)
 	goto DONE;
 
-    status = notmuch_database_add_message (notmuch, filename, &message);
+    if (state->try_decrypt)
+	status = notmuch_database_add_message_try_decrypt (notmuch, filename, &message);
+    else
+	status = notmuch_database_add_message (notmuch, filename, &message);
     switch (status) {
     /* Success. */
     case NOTMUCH_STATUS_SUCCESS:
@@ -930,6 +934,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     add_files_state_t add_files_state = {
 	.verbosity = VERBOSITY_NORMAL,
 	.debug = FALSE,
+	.try_decrypt = FALSE,
 	.output_is_a_tty = isatty (fileno (stdout)),
     };
     struct timeval tv_start;
@@ -951,6 +956,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	{ NOTMUCH_OPT_BOOLEAN,  &verbose, "verbose", 'v', 0 },
 	{ NOTMUCH_OPT_BOOLEAN,  &add_files_state.debug, "debug", 'd', 0 },
 	{ NOTMUCH_OPT_BOOLEAN,  &no_hooks, "no-hooks", 'n', 0 },
+	{ NOTMUCH_OPT_BOOLEAN,  &add_files_state.try_decrypt, "try-decrypt", 0, 0 },
 	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
 	{ 0, 0, 0, 0, 0 }
     };
@@ -1068,6 +1074,16 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     if (notmuch == NULL)
 	return EXIT_FAILURE;
 
+    if (add_files_state.try_decrypt) {
+	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
+	status = notmuch_database_set_gpg_path(notmuch, gpg_path);
+	if (status)
+	    fprintf (stderr, "Warning: failed to set database gpg_path to '%s' (%s)\n",
+		     gpg_path ? gpg_path : "(NULL)",
+		     notmuch_status_to_string (status));
+    }
+
+    
     /* Set up our handler for SIGINT. We do this after having
      * potentially done a database upgrade we this interrupt handler
      * won't support. */
-- 
2.6.2

Re: [PATCH 2/9] Add a lazily-initialized crypto context to notmuch_database_t

[David Bremner]

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

> This is in large part a duplicate of parts of crypto.c, but that code
> is in the client (outside the library), and we don't want to entangle
> the libgmime API with the libnotmuch API.
>
> I welcome better proposals for how to share this code explicitly
> between the library and the client.

Maybe I miss something obvious, but util/libutil.a is exactly there for
sharing code between the library and the client.

perhaps something like "gmime-extra.c" to go with {talloc,zlib}-extra.c

I didn't look at the code yet, just the commentary.

d

Re: [PATCH 2/9] Add a lazily-initialized crypto context to notmuch_database_t

[Daniel Kahn Gillmor]

On Fri 2015-12-11 09:03:05 -0500, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> This is in large part a duplicate of parts of crypto.c, but that code
>> is in the client (outside the library), and we don't want to entangle
>> the libgmime API with the libnotmuch API.
>>
>> I welcome better proposals for how to share this code explicitly
>> between the library and the client.
>
> Maybe I miss something obvious, but util/libutil.a is exactly there for
> sharing code between the library and the client.

You didn't miss anything obvious -- i did!  Thanks for pointing that
out, i'll take a look at normalizing these bits for my second draft.

> perhaps something like "gmime-extra.c" to go with {talloc,zlib}-extra.c

right, sounds good.

> I didn't look at the code yet, just the commentary.

many thanks for the review, that's why i wrote the commentary :)

After a couple nights of sleep, i have a proposal to fix one of the
open questions too, which i'll follow up on shortly here.

     --dkg

Re: allow indexing cleartext of encrypted messages

[Daniel Kahn Gillmor]

On Wed 2015-12-09 22:39:37 -0500, Daniel Kahn Gillmor wrote:
>  * the libnotmuch API is extended with
>    notmuch_database_add_message_try_decrypt().  This should probably
>    ultimately be more general, because there are a few additional
>    knobs that i can imagine fiddling at indexing time.  For example:
>
>     * verifying cryptographic signatures and storing something about
>       those verifications in the notmuch db
>      
>     * extracting OpenPGP session key information for a given message
>       and storing it in a lookaside table in the notmuch db, so that
>       it's possible to securely destroy old encryption-capable keys
>       and still have local access to the cleartext of the remaining
>       messages.
>
>    Some of these additional features might be orthogonal to one
>    another as well.  I welcome suggestions for how to improve the API
>    so that we don't end up with a combinatorial explosion of
>    n_d_add_message_foo() functions.

I have a proposal for how to do this better:

I'll introduce a notmuch_index_options_t, with the usual constructors
and destructors and a couple functions:

  notmuch_index_options_set_try_decrypt()
  notmuch_index_options_get_try_decrypt()
  notmuch_index_options_set_gpg_path()
  notmuch_index_options_get_gpg_path()

Then i'll add:

  notmuch_database_add_message_with_options(db, fname, options, &message)

If we add new indexing features, they can be set directly in the
index_options object (including features that might be more complex than
a string or a bool, like a chain of command-line filters).

a few nice features of this approach:

 * The user of the library can craft a set of index options and repeat
   it easily, and the options can contain cached/lazily-initialized
   things (like GMimeCryptoContexts) if needed.

 * The user can index different messages with different options if they
   prefer (no need to set the options on the database object itself)

 * the capability of the indexing features in the library is visible
   directly in the exposed API.

any thoughts on this?

    --dkg

Re: [PATCH 1/9] reorganize indexing of multipart/signed and multipart/encrypted

[Tomi Ollila]

On Thu, Dec 10 2015, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> This prepares the codebase for a cleaner changeset for dealing with
> indexing some encrypted messages in the clear.
> ---
>  lib/index.cc | 38 ++++++++++++++++++--------------------
>  1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/lib/index.cc b/lib/index.cc
> index f166aef..2fa6616 100644
> --- a/lib/index.cc
> +++ b/lib/index.cc
> @@ -333,27 +333,25 @@ _index_mime_part (notmuch_message_t *message,
>  	GMimeMultipart *multipart = GMIME_MULTIPART (part);
>  	int i;
>  
> -	if (GMIME_IS_MULTIPART_SIGNED (multipart))
> -	  _notmuch_message_add_term (message, "tag", "signed");
> -
> -	if (GMIME_IS_MULTIPART_ENCRYPTED (multipart))
> -	  _notmuch_message_add_term (message, "tag", "encrypted");
> -
> -	for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
> -	    if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
> -		/* Don't index the signature. */
> -		if (i == 1)
> -		    continue;
> -		if (i > 1)
> -		    _notmuch_database_log (_notmuch_message_database (message),
> -					  "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
> -	    }
> -	    if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
> -		/* Don't index encrypted parts. */
> -		continue;
> -	    }
> +	if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
> +	    _notmuch_message_add_term (message, "tag", "signed");
> +	    /* FIXME: should we try to validate the signature? */
> +	    
> +	    /* FIXME: is it always just the first part that is signed in
> +	     all multipart/signed messages?*/
>  	    _index_mime_part (message,
> -			      g_mime_multipart_get_part (multipart, i));
> +			      g_mime_multipart_get_part (multipart, 0));
> +	    
> +	    if (g_mime_multipart_get_count (multipart) > 2)
> +		_notmuch_database_log (_notmuch_message_database (message),
> +				       "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n");
> +	} else if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
> +	    /* Don't index encrypted parts */

This could have the  _notmuch_message_add_term (message, "tag", "encrypted");
that was added in next patch file ?

(i saw your latest proposal, here i am commenting so that the relevant
changes can be done in nest round...)


> +	} else {
> +	    for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
> +		_index_mime_part (message,
> +				  g_mime_multipart_get_part (multipart, i));
> +	    }
>  	}
>  	return;
>      }
> -- 
> 2.6.2

Re: [PATCH 2/9] Add a lazily-initialized crypto context to notmuch_database_t

[Tomi Ollila]

On Thu, Dec 10 2015, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> This is in large part a duplicate of parts of crypto.c, but that code
> is in the client (outside the library), and we don't want to entangle
> the libgmime API with the libnotmuch API.
>
> I welcome better proposals for how to share this code explicitly
> between the library and the client.
> ---
>  lib/database-private.h |  1 +
>  lib/database.cc        | 42 ++++++++++++++++++++++++++++++++++++++++++
>  lib/notmuch-private.h  |  8 ++++++++
>  3 files changed, 51 insertions(+)
>
> diff --git a/lib/database-private.h b/lib/database-private.h
> index 3fb10f7..1bf76c5 100644
> --- a/lib/database-private.h
> +++ b/lib/database-private.h
> @@ -171,6 +171,7 @@ struct _notmuch_database {
>       * notmuch_database_new_revision. */
>      unsigned long revision;
>      const char *uuid;
> +    GMimeCryptoContext *gpg_crypto_ctx;
>  
>      Xapian::QueryParser *query_parser;
>      Xapian::TermGenerator *term_gen;
> diff --git a/lib/database.cc b/lib/database.cc
> index 3b342f1..13b0bad 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -995,6 +995,8 @@ notmuch_database_open_verbose (const char *path,
>  	notmuch->uuid = talloc_strdup (
>  	    notmuch, notmuch->xapian_db->get_uuid ().c_str ());
>  
> +	notmuch->gpg_crypto_ctx = NULL;
> +	
>  	notmuch->query_parser = new Xapian::QueryParser;
>  	notmuch->term_gen = new Xapian::TermGenerator;
>  	notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));
> @@ -1090,6 +1092,11 @@ notmuch_database_close (notmuch_database_t *notmuch)
>      delete notmuch->last_mod_range_processor;
>      notmuch->last_mod_range_processor = NULL;
>  
> +    if (notmuch->gpg_crypto_ctx) {
> +	g_object_unref (notmuch->gpg_crypto_ctx);
> +	notmuch->gpg_crypto_ctx = NULL;
> +    }
> +    
>      return status;
>  }
>  
> @@ -2386,6 +2393,41 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
>      return status;
>  }
>  
> +notmuch_private_status_t
> +_notmuch_database_get_crypto_for_protocol (notmuch_database_t *notmuch,
> +					   const char *protocol,
> +					   GMimeCryptoContext **crypto_ctx)

^^^^ static function ^^^

Re: [PATCH 6/9] search for a reasonable gpg implementation

[Tomi Ollila]

On Thu, Dec 10 2015, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> When the notmuch database needs to find gpg, have it search reasonable
> paths first.
> ---
>  lib/database.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 62bc6d9..d0e8800 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -2393,6 +2393,17 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
>      return status;
>  }
>  
> +static const char*
> +_notmuch_database_get_gpg_path (notmuch_database_t *notmuch)
> +{
> +#define try_gpg_path(z) if (!access(z, X_OK)) return z
> +    try_gpg_path("/usr/bin/gpg2");
> +    try_gpg_path("/bin/gpg2");
> +    try_gpg_path("/usr/bin/gpg");
> +    try_gpg_path("/bin/gpg");
> +    return NULL;
> +}

If this were to survive longer, BSD folks would like to have /usr/local/bin checked...
(i don't know (yet) about os x

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Tomi Ollila]

On Thu, Dec 10 2015, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> Exposing this to the user of the library lets the user point to
> arbitrary gpg executables when trying to decrypt.
> ---
>  lib/database-private.h |  3 ++
>  lib/database.cc        | 93 +++++++++++++++++++++++++++++++++++++++++++-------
>  lib/notmuch.h          | 31 +++++++++++++++++
>  3 files changed, 115 insertions(+), 12 deletions(-)
>
......

> +
> +static notmuch_bool_t
> +_find_in_path(const char* path)
> +{
> +    char *c = NULL, *save = NULL, *tok;
> +    size_t n;
> +    int dfd = -1;
> +    notmuch_bool_t ret = FALSE;
> +    
> +    n = confstr(_CS_PATH, NULL, 0);
> +    c = (char*)talloc_size(NULL, n);
> +    if (!c)
> +	return FALSE;
> +    confstr(_CS_PATH, c, n);
> +
> +    tok = strtok_r(c, ":", &save);
> +    while (tok) {
> +	dfd = open(tok, O_DIRECTORY | O_RDONLY);
> +	if (dfd != -1) {
> +	    if (!faccessat(dfd, path, X_OK, 0)) {
> +		ret = TRUE;
> +		goto done;
> +	    }
> +	    close(dfd);
> +	}
> +	tok = strtok_r(NULL, ":", &save);
> +    }

The above code finds gpg/gpg2 (when called w/ these args) from
_CS_PATH (seems to be /bin:/usr/bin by default in linux (tried to
look how this set in *BSD -- initially it looks like /usr/local/bin
not included but... maybe we let them to complain if this is the case
... :/)
... anyway, the full found path is not set anywhere -- how is it found
when used (exec*p() using $PATH? :O)

> +done:
> +    if (dfd != -1)
> +	close(dfd);
> +    if (c)
> +	talloc_free(c);
> +    return ret;
> +}
> +
> +notmuch_status_t
> +notmuch_database_set_gpg_path (notmuch_database_t *notmuch, const char* path)
> +{
> +    /* return success if this matches what is already configured */
> +    if ((!path && !notmuch->gpg_path) ||
> +	(path && notmuch->gpg_path && 0 == strcmp(path, notmuch->gpg_path)))
> +	return NOTMUCH_STATUS_SUCCESS;
> +    
> +    if (!path && !_find_in_path(path))
> +	return NOTMUCH_STATUS_FILE_ERROR;
> +
> +    /* clear any existing gpg_crypto_ctx, since things are changing */
> +    if (notmuch->gpg_crypto_ctx) {
> +	g_object_unref (notmuch->gpg_crypto_ctx);
> +	notmuch->gpg_crypto_ctx = NULL;
> +    }
> +
> +    if (notmuch->gpg_path) {
> +	talloc_free(notmuch->gpg_path);
> +	notmuch->gpg_path = NULL;
> +    }
> +
> +    if (path)
> +	notmuch->gpg_path = talloc_strdup (notmuch, path);
> +    
> +    return NOTMUCH_STATUS_SUCCESS;
> +}
> +
> +const char*
> +notmuch_database_get_gpg_path (const notmuch_database_t *notmuch)
> +{
> +    if (notmuch->gpg_path)
> +	return notmuch->gpg_path;
> +
> +#define try_gpg_path(z) if (_find_in_path(z)) return z
> +    try_gpg_path("gpg2");
> +    try_gpg_path("gpg");
> +    return NULL;
> +}

Re: allow indexing cleartext of encrypted messages

[Tomi Ollila]

On Fri, Dec 11 2015, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> On Wed 2015-12-09 22:39:37 -0500, Daniel Kahn Gillmor wrote:
>>  * the libnotmuch API is extended with
>>    notmuch_database_add_message_try_decrypt().  This should probably
>>    ultimately be more general, because there are a few additional
>>    knobs that i can imagine fiddling at indexing time.  For example:
>>
>>     * verifying cryptographic signatures and storing something about
>>       those verifications in the notmuch db
>>      
>>     * extracting OpenPGP session key information for a given message
>>       and storing it in a lookaside table in the notmuch db, so that
>>       it's possible to securely destroy old encryption-capable keys
>>       and still have local access to the cleartext of the remaining
>>       messages.
>>
>>    Some of these additional features might be orthogonal to one
>>    another as well.  I welcome suggestions for how to improve the API
>>    so that we don't end up with a combinatorial explosion of
>>    n_d_add_message_foo() functions.
>
> I have a proposal for how to do this better:
>
> I'll introduce a notmuch_index_options_t, with the usual constructors
> and destructors and a couple functions:
>
>   notmuch_index_options_set_try_decrypt()
>   notmuch_index_options_get_try_decrypt()
>   notmuch_index_options_set_gpg_path()
>   notmuch_index_options_get_gpg_path()
>
> Then i'll add:
>
>   notmuch_database_add_message_with_options(db, fname, options, &message)
>
> If we add new indexing features, they can be set directly in the
> index_options object (including features that might be more complex than
> a string or a bool, like a chain of command-line filters).
>
> a few nice features of this approach:
>
>  * The user of the library can craft a set of index options and repeat
>    it easily, and the options can contain cached/lazily-initialized
>    things (like GMimeCryptoContexts) if needed.
>
>  * The user can index different messages with different options if they
>    prefer (no need to set the options on the database object itself)
>
>  * the capability of the indexing features in the library is visible
>    directly in the exposed API.
>
> any thoughts on this?

sounds good (on paper) (*)

>
>     --dkg

Tomi

(*) deliberately declined to write 'looks good' >;) (but it's good)

Re: [PATCH 6/9] search for a reasonable gpg implementation

[J. Lewis Muir]

On 12/11/15 3:56 PM, Tomi Ollila wrote:
>> +static const char*
>> +_notmuch_database_get_gpg_path (notmuch_database_t *notmuch)
>> +{
>> +#define try_gpg_path(z) if (!access(z, X_OK)) return z
>> +    try_gpg_path("/usr/bin/gpg2");
>> +    try_gpg_path("/bin/gpg2");
>> +    try_gpg_path("/usr/bin/gpg");
>> +    try_gpg_path("/bin/gpg");
>> +    return NULL;
>> +}
>
> If this were to survive longer, BSD folks would like to have
> /usr/local/bin checked...
> (i don't know (yet) about os x

I'm not following closely, but seeing paths to programs hard coded in
the source never seems like a good idea; invariably someone will have
the program in an unanticipated location.  I'm using pkgsrc on OS X,
and my gpg is at /opt/pkg/bin/gpg.  How about a Notmuch configuration
file item specifying the location of the program?  Or if not that, how
about a configuration option at build time to specify the location of
gpg that then gets hard coded in the source?  Or if not that, how about
an environment variable that will specify the location of the program
(e.g. like OpenSSH's SSH_ASKPASS environment variable)?

Regards,

Lewis

Re: [PATCH 6/9] search for a reasonable gpg implementation

[Daniel Kahn Gillmor]

On Fri 2015-12-11 17:18:52 -0500, J. Lewis Muir wrote:
> On 12/11/15 3:56 PM, Tomi Ollila wrote:
>>> +static const char*
>>> +_notmuch_database_get_gpg_path (notmuch_database_t *notmuch)
>>> +{
>>> +#define try_gpg_path(z) if (!access(z, X_OK)) return z
>>> +    try_gpg_path("/usr/bin/gpg2");
>>> +    try_gpg_path("/bin/gpg2");
>>> +    try_gpg_path("/usr/bin/gpg");
>>> +    try_gpg_path("/bin/gpg");
>>> +    return NULL;
>>> +}
>>
>> If this were to survive longer, BSD folks would like to have
>> /usr/local/bin checked...
>> (i don't know (yet) about os x
>
> I'm not following closely, but seeing paths to programs hard coded in
> the source never seems like a good idea; invariably someone will have
> the program in an unanticipated location.  I'm using pkgsrc on OS X,
> and my gpg is at /opt/pkg/bin/gpg.  How about a Notmuch configuration
> file item specifying the location of the program?  Or if not that, how
> about a configuration option at build time to specify the location of
> gpg that then gets hard coded in the source?  Or if not that, how about
> an environment variable that will specify the location of the program
> (e.g. like OpenSSH's SSH_ASKPASS environment variable)?

Yes, the path search is genericized later in the series (7/9).  clearly
i should have done even more commit squashing to avoid confusing people
with this distraction.  point clearly taken, though; i will avoid
hard-coding any paths.  thanks!

            --dkg

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Daniel Kahn Gillmor]

On Fri 2015-12-11 17:02:33 -0500, Tomi Ollila wrote:
> The above code finds gpg/gpg2 (when called w/ these args) from
> _CS_PATH (seems to be /bin:/usr/bin by default in linux (tried to
> look how this set in *BSD -- initially it looks like /usr/local/bin
> not included but... maybe we let them to complain if this is the case
> ... :/)
> ... anyway, the full found path is not set anywhere -- how is it found
> when used (exec*p() using $PATH? :O)

Hm, according to exec(3):

   Special semantics for execlp() and execvp()
       The execlp(), execvp(), and execvpe() functions duplicate the
       actions of the shell in searching for an executable file if the
       specified filename does not contain a slash (/) character.  The
       file is sought in the colon-separated list of directory pathnames
       specified in the PATH environment variable.  If this variable
       isn't defined, the path list defaults to the current directory
       followed by the list of directories returned by
       confstr(_CS_PATH).  (This confstr(3) call typically returns the
       value "/bin:/usr/bin".)

So this code probably also ought to be searching $PATH as well.  yuck.
You'd think there would be a commonly-available function for doing this
specific check without having to actually try to exec() something.

         --dkg

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[J. Lewis Muir]

On 12/9/15 9:39 PM, Daniel Kahn Gillmor wrote:
> +static notmuch_bool_t
> +_find_in_path(const char* path)
> +{
> +    char *c = NULL, *save = NULL, *tok;
> +    size_t n;
> +    int dfd = -1;
> +    notmuch_bool_t ret = FALSE;
> +
> +    n = confstr(_CS_PATH, NULL, 0);
> +    c = (char*)talloc_size(NULL, n);
> +    if (!c)
> +	return FALSE;
> +    confstr(_CS_PATH, c, n);
> +
> +    tok = strtok_r(c, ":", &save);
> +    while (tok) {
> +	dfd = open(tok, O_DIRECTORY | O_RDONLY);
> +	if (dfd != -1) {
> +	    if (!faccessat(dfd, path, X_OK, 0)) {
> +		ret = TRUE;
> +		goto done;
> +	    }
> +	    close(dfd);
> +	}
> +	tok = strtok_r(NULL, ":", &save);
> +    }
> +done:
> +    if (dfd != -1)
> +	close(dfd);
> +    if (c)
> +	talloc_free(c);
> +    return ret;
> +}

I guess I still don't get it.  Why even have a _find_in_path function?
Why not just expect the gpg executable path to have already been
specified somehow (e.g. Notmuch configuration file, build-time constant,
or environment variable)?

Regards,

Lewis

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Daniel Kahn Gillmor]

On Fri 2015-12-11 17:35:58 -0500, J. Lewis Muir wrote:
> I guess I still don't get it.  Why even have a _find_in_path function?
> Why not just expect the gpg executable path to have already been
> specified somehow (e.g. Notmuch configuration file, build-time constant,
> or environment variable)?

This is happening in the library, which doesn't read the config file,
and doesn't depend on the environment.  if a user tells the library "use
this as your gpg executable", it seems nice to fail early for them.

It's also nice if we want to have a default that says something like
"use gpg2 if it's available, but gpg otherwise".

     --dkg

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Tomi Ollila]

On Sat, Dec 12 2015, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> On Fri 2015-12-11 17:02:33 -0500, Tomi Ollila wrote:
>> The above code finds gpg/gpg2 (when called w/ these args) from
>> _CS_PATH (seems to be /bin:/usr/bin by default in linux (tried to
>> look how this set in *BSD -- initially it looks like /usr/local/bin
>> not included but... maybe we let them to complain if this is the case
>> ... :/)
>> ... anyway, the full found path is not set anywhere -- how is it found
>> when used (exec*p() using $PATH? :O)
>
> Hm, according to exec(3):
>
>    Special semantics for execlp() and execvp()
>        The execlp(), execvp(), and execvpe() functions duplicate the
>        actions of the shell in searching for an executable file if the
>        specified filename does not contain a slash (/) character.  The
>        file is sought in the colon-separated list of directory pathnames
>        specified in the PATH environment variable.  If this variable
>        isn't defined, the path list defaults to the current directory
>        followed by the list of directories returned by
>        confstr(_CS_PATH).  (This confstr(3) call typically returns the
>        value "/bin:/usr/bin".)
>
> So this code probably also ought to be searching $PATH as well.  yuck.
> You'd think there would be a commonly-available function for doing this
> specific check without having to actually try to exec() something.

Indeed.

Shells seems to be smart and do pre-check through PATH (but not 
_CS_PATH...(**)) and then exec just with the found path. I straced
one execlp() usage and it naïvely attempted to execve() through the
name appended to path (if getting ENOENT); when execve() succeeds
it does not return...

I originally had a fast thought that it was a security feature not
use PATH (which user can screw up ;/), but some safer path set.
Perhaps that is not necessary...

Perhaps the search should go through PATH if it is defined and
if not defined then use confstr(_CS_PATH). If PATH is empty
then not use confstr (but use current dir !!11!!?++?+?? (*))

One thing I forgot from the review:

the #define try_gpg_path(z) is done inside {}:s, so
before closing } there could be
#undef try_gpg_path (to make the "liveness" of it match the scope)


(*) what is the yuckiness of this:

  $ PATH= ls
  zsh: command not found: ls

  $ bash -c 'PATH= ls'
  bash: ls: No such file or directory

  $ echo '#!/bin/echo this is ./ls ?' >! ./ls
  $ chmod 755 ./ls

  $ PATH= ls
  this is ./ls ? ls -F

  $ PATH= command ls
  this is ./ls ? ls

  $ bash -c 'PATH= ls'
  this is ./ls ? ls

  $ rm ./ls

(**) unset PATH had the same effect in bash and dash but zsh did not 
     check ./ls after 'unset PATH'

-- I personally think we could just ignore empty components in PATH
(being it like PATH=, PATH=:/path/to, PATH=/path/to: and finally
PATH=/path/to::/path/to) (or then not, just be as compliant (and stupid)
as the PATH handling is).

Tomi

>
>          --dkg

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[David Bremner]

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

> On Fri 2015-12-11 17:02:33 -0500, Tomi Ollila wrote:
>> The above code finds gpg/gpg2 (when called w/ these args) from
>> _CS_PATH (seems to be /bin:/usr/bin by default in linux (tried to
>> look how this set in *BSD -- initially it looks like /usr/local/bin
>> not included but... maybe we let them to complain if this is the case
>> ... :/)
>> ... anyway, the full found path is not set anywhere -- how is it found
>> when used (exec*p() using $PATH? :O)
>
> Hm, according to exec(3):
>
>    Special semantics for execlp() and execvp()
>        The execlp(), execvp(), and execvpe() functions duplicate the
>        actions of the shell in searching for an executable file if the
>        specified filename does not contain a slash (/) character.  The
>        file is sought in the colon-separated list of directory pathnames
>        specified in the PATH environment variable.  If this variable
>        isn't defined, the path list defaults to the current directory
>        followed by the list of directories returned by
>        confstr(_CS_PATH).  (This confstr(3) call typically returns the
>        value "/bin:/usr/bin".)
>
> So this code probably also ought to be searching $PATH as well.  yuck.
> You'd think there would be a commonly-available function for doing this
> specific check without having to actually try to exec() something.

Without weighing in on the advisibility of searching for gpg in $PATH,
there is a glib function g_find_program_in_path. We're already linking
to glib (because of gmime mainly, but it's used other places as well).

The other point that occurs to me is that libgpgme solves this same
problem in src/posix-util.c. It also seems to search path, at least
optionally, although only if it cannot find gpgconf.

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Tomi Ollila]

On Sun, Dec 13 2015, David Bremner <david@tethera.net> wrote:

> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> On Fri 2015-12-11 17:02:33 -0500, Tomi Ollila wrote:
>>> The above code finds gpg/gpg2 (when called w/ these args) from
>>> _CS_PATH (seems to be /bin:/usr/bin by default in linux (tried to
>>> look how this set in *BSD -- initially it looks like /usr/local/bin
>>> not included but... maybe we let them to complain if this is the case
>>> ... :/)
>>> ... anyway, the full found path is not set anywhere -- how is it found
>>> when used (exec*p() using $PATH? :O)
>>
>> Hm, according to exec(3):
>>
>>    Special semantics for execlp() and execvp()
>>        The execlp(), execvp(), and execvpe() functions duplicate the
>>        actions of the shell in searching for an executable file if the
>>        specified filename does not contain a slash (/) character.  The
>>        file is sought in the colon-separated list of directory pathnames
>>        specified in the PATH environment variable.  If this variable
>>        isn't defined, the path list defaults to the current directory
>>        followed by the list of directories returned by
>>        confstr(_CS_PATH).  (This confstr(3) call typically returns the
>>        value "/bin:/usr/bin".)
>>
>> So this code probably also ought to be searching $PATH as well.  yuck.
>> You'd think there would be a commonly-available function for doing this
>> specific check without having to actually try to exec() something.
>
> Without weighing in on the advisibility of searching for gpg in $PATH,
> there is a glib function g_find_program_in_path. We're already linking
> to glib (because of gmime mainly, but it's used other places as well).

glib2-2.38 (glib/gutils.c) seems to look in PATH, and if 
g_getenv ("PATH") == NULL uses hardcoded path "/bin:/usr/bin:.";
(it us "security" feature to have '.' last...)

If rest is TL;DR; I'd suggest we use this... since libgpgme has implemented
it IMO too late for use in 2016 (or do additional compat function?)

> The other point that occurs to me is that libgpgme solves this same
> problem in src/posix-util.c. It also seems to search path, at least
> optionally, although only if it cannot find gpgconf.

On Fedora 20 I looked gpgme-1.3.2 sources -- in there I could not find
this search using PATH... gpg-1.3.2 is released 2012-05-02...

Jessie (lib)gpgme 1.5.1 (2014-07-30) seems to have the code David mentioned...
(btw. I was suprisingly hard to search Debian packages; IIRC it was easier)

Ubuntu 14.04 LTS has (lib)gpgme 1.4.3 (ubuntu5). I did not download that
source... but 1.5.1 NEWS indicates that this PATH search has arrived to
1.5.0 (2014-05-21)

This search is different from glib2-version that if getenv("PATH") == NULL
search only "/bin:/usr/bin"

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Tomi Ollila]

On Sun, Dec 13 2015, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Sun, Dec 13 2015, David Bremner <david@tethera.net> wrote:
>
>>
>> Without weighing in on the advisibility of searching for gpg in $PATH,
>> there is a glib function g_find_program_in_path. We're already linking
>> to glib (because of gmime mainly, but it's used other places as well).
>
> glib2-2.38 (glib/gutils.c) seems to look in PATH, and if 
> g_getenv ("PATH") == NULL uses hardcoded path "/bin:/usr/bin:.";
> (it us "security" feature to have '.' last...)
>
> If rest is TL;DR; I'd suggest we use this... since libgpgme has implemented
> it IMO too late for use in 2016 (or do additional compat function?)

Actually now that I sent this mail it kept rolling on my mind... If anyone
else than me (and libgpgme?) thinks that '.' should not be in search path
we could do

if (getenv("PATH") == NULL) {
   path_set = true;
   setenv("PATH", "/bin:/usr/bin", 1); // XXX *BSD configurability //
}
else path_set = false;

... g_find_program_in_path("gpg2")
... g_find_program_in_path("gpg")

if (path_set) {
  unsetenv("PATH");

---

I also thought of examining the return value starting with ./ but
(current or) future version of g_find_program_in_path() might
canonicalize the returned path...


Tomi


>
>> The other point that occurs to me is that libgpgme solves this same
>> problem in src/posix-util.c. It also seems to search path, at least
>> optionally, although only if it cannot find gpgconf.
>
> On Fedora 20 I looked gpgme-1.3.2 sources -- in there I could not find
> this search using PATH... gpg-1.3.2 is released 2012-05-02...
>
> Jessie (lib)gpgme 1.5.1 (2014-07-30) seems to have the code David mentioned...
> (btw. I was suprisingly hard to search Debian packages; IIRC it was easier)
>
> Ubuntu 14.04 LTS has (lib)gpgme 1.4.3 (ubuntu5). I did not download that
> source... but 1.5.1 NEWS indicates that this PATH search has arrived to
> 1.5.0 (2014-05-21)
>
> This search is different from glib2-version that if getenv("PATH") == NULL
> search only "/bin:/usr/bin"

Re: [PATCH 7/9] add a gpg_path value for notmuch_database_t

[Daniel Kahn Gillmor]

On Sun 2015-12-13 06:17:07 -0500, Tomi Ollila wrote:
> Actually now that I sent this mail it kept rolling on my mind... If anyone
> else than me (and libgpgme?) thinks that '.' should not be in search path
> we could do

fwiw, i agree that . should *not* be in the search path.

> if (getenv("PATH") == NULL) {
>    path_set = true;
>    setenv("PATH", "/bin:/usr/bin", 1); // XXX *BSD configurability //
> }
> else path_set = false;
>
> ... g_find_program_in_path("gpg2")
> ... g_find_program_in_path("gpg")
>
> if (path_set) {
>   unsetenv("PATH");

I'm game for something like this, but i've got a queue of patches i'm
about to send that would provide a different place to make this change,
so i'm not making it now.  please keep this in mind, though :)

> I also thought of examining the return value starting with ./ but
> (current or) future version of g_find_program_in_path() might
> canonicalize the returned path...

i'm not sure what this suggestion means -- do you mean checking to see
whether the returned value started with ./ ?  If so, I agree that this
seems like a not very robust way to protect against this problem.

Should we maybe also be reporting this as a bug against
g_find_program_in_path ?

                       --dkg