Allow indexing cleartext of encrypted messages (v4)

Allow indexing cleartext of encrypted messages (v4)

[Daniel Kahn Gillmor]

This is the fourth draft of the series that enables indexing cleartext
of encrypted message parts.

previous versions start at:

v1:  id:1449718786-28000-1-git-send-email-dkg@fifthhorseman.net
v2:  id:1453258369-7366-1-git-send-email-dkg@fifthhorseman.net
v3:  id:1454272801-23623-1-git-send-email-dkg@fifthhorseman.net

differs from v3 in that it uses Bremner's "message properties" to
record its information instead of trampling on the user-visible tag
space.

It depends also on one additional fix i pushed to the "message
properties" series, allowing notmuch queries with a "has:" prefix to
search the property namespace.  In particular:

 id:1467969336-7605-1-git-send-email-dkg@fifthhorseman.net

I welcome feedback!

[PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[Daniel Kahn Gillmor]

This is a utility function we can use to see whether an executable is
available.
---
 util/Makefile.local |  2 +-
 util/search-path.c  | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 util/search-path.h  | 24 ++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 util/search-path.c
 create mode 100644 util/search-path.h

diff --git a/util/Makefile.local b/util/Makefile.local
index 905f237..8b2b91b 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -5,7 +5,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
-		$(dir)/util.c
+		$(dir)/util.c $(dir)/search-path.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/search-path.c b/util/search-path.c
new file mode 100644
index 0000000..9da21cb
--- /dev/null
+++ b/util/search-path.c
@@ -0,0 +1,50 @@
+#include "search-path.h"
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+
+notmuch_bool_t
+test_for_executable (const char *exename)
+{
+    char *path = NULL, *save = NULL, *tok;
+    notmuch_bool_t ret = FALSE;
+
+    if (strchr (exename, '/')) {
+	if (0 == access (exename, X_OK))
+	    return TRUE;
+	else
+	    return FALSE;
+    }
+
+    path = getenv ("PATH");
+    if (path)
+	path = strdup (path);
+    else {
+	size_t n = confstr (_CS_PATH, NULL, 0);
+	path = (char *) malloc (n);
+	if (! path)
+	    return FALSE;
+	confstr (_CS_PATH, path, n);
+    }
+
+    tok = strtok_r (path, ":", &save);
+    while (tok) {
+	int dir_fd = open (tok, O_DIRECTORY | O_RDONLY);
+	if (dir_fd != -1) {
+	    int access = faccessat (dir_fd, exename, X_OK, 0);
+	    close (dir_fd);
+	    if (access == 0) {
+		ret = TRUE;
+		break;
+	    }
+	}
+	tok = strtok_r (NULL, ":", &save);
+    }
+    if (path)
+	free (path);
+    return ret;
+}
diff --git a/util/search-path.h b/util/search-path.h
new file mode 100644
index 0000000..14c4d14
--- /dev/null
+++ b/util/search-path.h
@@ -0,0 +1,24 @@
+#ifndef _SEARCH_PATH_H
+#define _SEARCH_PATH_H
+
+#include "notmuch.h"
+
+/* can an executable be found with the given name?
+ *
+ * Return TRUE only if we can find something to execute with the
+ * associated name.
+ *
+ * if the name has a '/' in it, we look for it directly with
+ * access(exename, X_OK).
+ *
+ * otherwise, we look for it in $PATH (or in confstr(_CS_PATH), if
+ * $PATH is unset).
+ *
+ * This should match the logic for execvp (as well as matching user
+ * expectations, hopefully).
+ */
+
+notmuch_bool_t
+test_for_executable (const char *exename);
+
+#endif
-- 
2.8.1

Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[David Bremner]

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

> This is a utility function we can use to see whether an executa>
> +    if (strchr (exename, '/')) {
> +	if (0 == access (exename, X_OK))
> +	    return TRUE;
> +	else
> +	    return FALSE;
> +    }

Should we distinguish between relative and absolute paths here?  I can't
think of any security implications, but I'm wondering if a relative path
is likely just a user error.

> +	path = (char *) malloc (n);
> +	if (! path)
> +	    return FALSE;

I kindof hate hiding the error here, although I agree it's
unlikely. What about the unixy return 0 ok, 1 not found -1 error?

> +	confstr (_CS_PATH, path, n);
> +    }
> +
> +    tok = strtok_r (path, ":", &save);
> +    while (tok) {

I guess it's fine to modify path here, but another option is
strtok_len (in string-util.h)

Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[Daniel Kahn Gillmor]

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

On Fri 2016-08-12 01:51:16 -0400, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> This is a utility function we can use to see whether an executa>
>> +    if (strchr (exename, '/')) {
>> +	if (0 == access (exename, X_OK))
>> +	    return TRUE;
>> +	else
>> +	    return FALSE;
>> +    }
>
> Should we distinguish between relative and absolute paths here?  I can't
> think of any security implications, but I'm wondering if a relative path
> is likely just a user error.

I don't think a relative path is necessarily a user error.  I certainly
use relative paths myself from time to time.

>> +	path = (char *) malloc (n);
>> +	if (! path)
>> +	    return FALSE;
>
> I kindof hate hiding the error here, although I agree it's
> unlikely. What about the unixy return 0 ok, 1 not found -1 error?
>
>> +	confstr (_CS_PATH, path, n);
>> +    }
>> +
>> +    tok = strtok_r (path, ":", &save);
>> +    while (tok) {
>
> I guess it's fine to modify path here, but another option is
> strtok_len (in string-util.h)

I'm ok with both of these changes -- do you want to propose a variant
for this patch?

thanks for going through and trying to get this stuff building again.

    --dkg

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

Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[David Bremner]

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

>>
>> Should we distinguish between relative and absolute paths here?  I can't
>> think of any security implications, but I'm wondering if a relative path
>> is likely just a user error.
>
> I don't think a relative path is necessarily a user error.  I certainly
> use relative paths myself from time to time.

As configuration values? That seems quite fragile, since it depends on
the current working directory when notmuch is run.

d

Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[Daniel Kahn Gillmor]

On Fri 2016-08-12 03:38:53 -0400, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>>> Should we distinguish between relative and absolute paths here?  I can't
>>> think of any security implications, but I'm wondering if a relative path
>>> is likely just a user error.
>>
>> I don't think a relative path is necessarily a user error.  I certainly
>> use relative paths myself from time to time.
>
> As configuration values? That seems quite fragile, since it depends on
> the current working directory when notmuch is run.

rarely!  but sometimes i do it because i'm testing things in strange
ways, and it can be a bit frustrating to have a tool second-guess me
when it seems like i ought to be able to drop the same string i'm using
on the command line into the configuration.

I don't feel strongly, though.  if you want to say that bare words found
in the $PATH and absolute filenames (starting with /) are fine in the
notmuch config but relative paths are not, i'd be ok with that :)

        --dkg

Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[Tomi Ollila]

On Fri, Aug 12 2016, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> On Fri 2016-08-12 03:38:53 -0400, David Bremner wrote:
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>
>>>> Should we distinguish between relative and absolute paths here?  I can't
>>>> think of any security implications, but I'm wondering if a relative path
>>>> is likely just a user error.
>>>
>>> I don't think a relative path is necessarily a user error.  I certainly
>>> use relative paths myself from time to time.
>>
>> As configuration values? That seems quite fragile, since it depends on
>> the current working directory when notmuch is run.
>
> rarely!  but sometimes i do it because i'm testing things in strange
> ways, and it can be a bit frustrating to have a tool second-guess me
> when it seems like i ought to be able to drop the same string i'm using
> on the command line into the configuration.
>
> I don't feel strongly, though.  if you want to say that bare words found
> in the $PATH and absolute filenames (starting with /) are fine in the
> notmuch config but relative paths are not, i'd be ok with that :)

From consistency point of view, current patch not checking it being
absolute might prevail -- I don't see database.path being checked for
being absolute...

The probability for user error is pretty small there -- if there is
typo/thinko there things usually just starts failing. Security is
easier to break elsewhere than here (e.g. borken PATH...)

I'd keep the current implementation of test_for_executable()...

Tomi

>
>         --dkg

Re: [PATCH v4 01/16] add util/search-path.{c, h} to test for executables in $PATH

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> The probability for user error is pretty small there -- if there is
> typo/thinko there things usually just starts failing. Security is
> easier to break elsewhere than here (e.g. borken PATH...)
>
> I'd keep the current implementation of test_for_executable()...
>
> Tomi
>

OK

[PATCH v4 02/16] Move crypto.c into libutil

[Daniel Kahn Gillmor]

This prepares us for using the crypto object in both the library and
the client.

i've prefixed notmuch_crypto with _ to indicate that while this can be
built into the library when needed, it's not something to be exported
or used externally.
---
 Makefile.local      |   1 -
 crypto.c            | 134 --------------------------------------------------
 mime-node.c         |  12 ++---
 notmuch-client.h    |  23 ++-------
 notmuch-reply.c     |   2 +-
 notmuch-show.c      |   2 +-
 util/Makefile.local |   2 +-
 util/crypto.c       | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 util/crypto.h       |  25 ++++++++++
 9 files changed, 175 insertions(+), 164 deletions(-)
 delete mode 100644 crypto.c
 create mode 100644 util/crypto.c
 create mode 100644 util/crypto.h

diff --git a/Makefile.local b/Makefile.local
index f2ad0c1..c13ba76 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -293,7 +293,6 @@ notmuch_client_srcs =		\
 	sprinter-text.c		\
 	query-string.c		\
 	mime-node.c		\
-	crypto.c		\
 	tag-util.c
 
 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
diff --git a/crypto.c b/crypto.c
deleted file mode 100644
index 3e8ce7c..0000000
--- a/crypto.c
+++ /dev/null
@@ -1,134 +0,0 @@
-/* notmuch - Not much of an email program, (just index and search)
- *
- * Copyright © 2012 Jameson Rollins
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see https://www.gnu.org/licenses/ .
- *
- * Authors: Jameson Rollins <jrollins@finestructure.net>
- */
-
-#include "notmuch-client.h"
-
-/* Create a GPG context (GMime 2.6) */
-static notmuch_crypto_context_t *
-create_gpg_context (notmuch_crypto_t *crypto)
-{
-    notmuch_crypto_context_t *gpgctx;
-
-    if (crypto->gpgctx)
-	return crypto->gpgctx;
-
-    /* TODO: GMimePasswordRequestFunc */
-    gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath : "gpg");
-    if (! gpgctx) {
-	fprintf (stderr, "Failed to construct gpg context.\n");
-	return NULL;
-    }
-    crypto->gpgctx = gpgctx;
-
-    g_mime_gpg_context_set_use_agent ((GMimeGpgContext *) gpgctx, TRUE);
-    g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) gpgctx, FALSE);
-
-    return gpgctx;
-}
-
-/* Create a PKCS7 context (GMime 2.6) */
-static notmuch_crypto_context_t *
-create_pkcs7_context (notmuch_crypto_t *crypto)
-{
-    notmuch_crypto_context_t *pkcs7ctx;
-
-    if (crypto->pkcs7ctx)
-	return crypto->pkcs7ctx;
-
-    /* TODO: GMimePasswordRequestFunc */
-    pkcs7ctx = g_mime_pkcs7_context_new (NULL);
-    if (! pkcs7ctx) {
-	fprintf (stderr, "Failed to construct pkcs7 context.\n");
-	return NULL;
-    }
-    crypto->pkcs7ctx = pkcs7ctx;
-
-    g_mime_pkcs7_context_set_always_trust ((GMimePkcs7Context *) pkcs7ctx,
-					   FALSE);
-
-    return pkcs7ctx;
-}
-static const struct {
-    const char *protocol;
-    notmuch_crypto_context_t *(*get_context) (notmuch_crypto_t *crypto);
-} protocols[] = {
-    {
-	.protocol = "application/pgp-signature",
-	.get_context = create_gpg_context,
-    },
-    {
-	.protocol = "application/pgp-encrypted",
-	.get_context = create_gpg_context,
-    },
-    {
-	.protocol = "application/pkcs7-signature",
-	.get_context = create_pkcs7_context,
-    },
-    {
-	.protocol = "application/x-pkcs7-signature",
-	.get_context = create_pkcs7_context,
-    },
-};
-
-/* for the specified protocol return the context pointer (initializing
- * if needed) */
-notmuch_crypto_context_t *
-notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol)
-{
-    notmuch_crypto_context_t *cryptoctx = NULL;
-    size_t i;
-
-    if (! protocol) {
-	fprintf (stderr, "Cryptographic protocol is empty.\n");
-	return cryptoctx;
-    }
-
-    /* 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.
-     */
-    for (i = 0; i < ARRAY_SIZE (protocols); i++) {
-	if (strcasecmp (protocol, protocols[i].protocol) == 0)
-	    return protocols[i].get_context (crypto);
-    }
-
-    fprintf (stderr, "Unknown or unsupported cryptographic protocol %s.\n",
-	     protocol);
-
-    return NULL;
-}
-
-int
-notmuch_crypto_cleanup (notmuch_crypto_t *crypto)
-{
-    if (crypto->gpgctx) {
-	g_object_unref (crypto->gpgctx);
-	crypto->gpgctx = NULL;
-    }
-
-    if (crypto->pkcs7ctx) {
-	g_object_unref (crypto->pkcs7ctx);
-	crypto->pkcs7ctx = NULL;
-    }
-
-    return 0;
-}
diff --git a/mime-node.c b/mime-node.c
index c9b8233..14873ce 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -33,7 +33,7 @@ typedef struct mime_node_context {
     GMimeMessage *mime_message;
 
     /* Context provided by the caller. */
-    notmuch_crypto_t *crypto;
+    _notmuch_crypto_t *crypto;
 } mime_node_context_t;
 
 static int
@@ -56,7 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
 
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
-		notmuch_crypto_t *crypto, mime_node_t **root_out)
+		_notmuch_crypto_t *crypto, mime_node_t **root_out)
 {
     const char *filename = notmuch_message_get_filename (message);
     mime_node_context_t *mctx;
@@ -151,7 +151,7 @@ set_signature_list_destructor (mime_node_t *node)
 /* Verify a signed mime node (GMime 2.6) */
 static void
 node_verify (mime_node_t *node, GMimeObject *part,
-	     notmuch_crypto_context_t *cryptoctx)
+	     GMimeCryptoContext *cryptoctx)
 {
     GError *err = NULL;
 
@@ -172,7 +172,7 @@ node_verify (mime_node_t *node, GMimeObject *part,
 /* Decrypt and optionally verify an encrypted mime node (GMime 2.6) */
 static void
 node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
-			 notmuch_crypto_context_t *cryptoctx)
+			 GMimeCryptoContext *cryptoctx)
 {
     GError *err = NULL;
     GMimeDecryptResult *decrypt_result = NULL;
@@ -207,7 +207,7 @@ static mime_node_t *
 _mime_node_create (mime_node_t *parent, GMimeObject *part)
 {
     mime_node_t *node = talloc_zero (parent, mime_node_t);
-    notmuch_crypto_context_t *cryptoctx = NULL;
+    GMimeCryptoContext *cryptoctx = NULL;
 
     /* Set basic node properties */
     node->part = part;
@@ -244,7 +244,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	|| (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
 	GMimeContentType *content_type = g_mime_object_get_content_type (part);
 	const char *protocol = g_mime_content_type_get_parameter (content_type, "protocol");
-	cryptoctx = notmuch_crypto_get_context (node->ctx->crypto, protocol);
+	cryptoctx = _notmuch_crypto_get_gmime_context (node->ctx->crypto, protocol);
     }
 
     /* Handle PGP/MIME parts */
diff --git a/notmuch-client.h b/notmuch-client.h
index 9ce2aef..a65bb6d 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -30,10 +30,6 @@
 
 #include <gmime/gmime.h>
 
-typedef GMimeCryptoContext notmuch_crypto_context_t;
-/* This is automatically included only since gmime 2.6.10 */
-#include <gmime/gmime-pkcs7-context.h>
-
 #include "notmuch.h"
 
 /* This is separate from notmuch-private.h because we're trying to
@@ -53,6 +49,7 @@ typedef GMimeCryptoContext notmuch_crypto_context_t;
 #include <ctype.h>
 
 #include "talloc-extra.h"
+#include "crypto.h"
 
 #define unused(x) x __attribute__ ((unused))
 
@@ -70,21 +67,13 @@ typedef struct notmuch_show_format {
 			      const struct notmuch_show_params *params);
 } notmuch_show_format_t;
 
-typedef struct notmuch_crypto {
-    notmuch_crypto_context_t* gpgctx;
-    notmuch_crypto_context_t* pkcs7ctx;
-    notmuch_bool_t verify;
-    notmuch_bool_t decrypt;
-    const char *gpgpath;
-} notmuch_crypto_t;
-
 typedef struct notmuch_show_params {
     notmuch_bool_t entire_thread;
     notmuch_bool_t omit_excluded;
     notmuch_bool_t output_body;
     notmuch_bool_t raw;
     int part;
-    notmuch_crypto_t crypto;
+    _notmuch_crypto_t crypto;
     notmuch_bool_t include_html;
 } notmuch_show_params_t;
 
@@ -167,12 +156,6 @@ typedef struct _notmuch_config notmuch_config_t;
 void
 notmuch_exit_if_unsupported_format (void);
 
-notmuch_crypto_context_t *
-notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol);
-
-int
-notmuch_crypto_cleanup (notmuch_crypto_t *crypto);
-
 int
 notmuch_count_command (notmuch_config_t *config, int argc, char *argv[]);
 
@@ -423,7 +406,7 @@ struct mime_node {
  */
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
-		notmuch_crypto_t *crypto, mime_node_t **node_out);
+		_notmuch_crypto_t *crypto, mime_node_t **node_out);
 
 /* Return a new MIME node for the requested child part of parent.
  * parent will be used as the talloc context for the returned child
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 4951373..42aef47 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -862,7 +862,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
     if (reply_format_func (config, config, query, &params, reply_all, sp) != 0)
 	return EXIT_FAILURE;
 
-    notmuch_crypto_cleanup (&params.crypto);
+    _notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
diff --git a/notmuch-show.c b/notmuch-show.c
index 22fa655..8ebf4ff 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1171,7 +1171,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	ret = do_show (config, query, format, sprinter, &params);
     }
 
-    notmuch_crypto_cleanup (&params.crypto);
+    _notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
diff --git a/util/Makefile.local b/util/Makefile.local
index 8b2b91b..7590618 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -5,7 +5,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
-		$(dir)/util.c $(dir)/search-path.c
+		$(dir)/util.c $(dir)/search-path.c $(dir)/crypto.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/crypto.c b/util/crypto.c
new file mode 100644
index 0000000..48c20bb
--- /dev/null
+++ b/util/crypto.c
@@ -0,0 +1,138 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2012 Jameson Rollins
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see https://www.gnu.org/licenses/ .
+ *
+ * Authors: Jameson Rollins <jrollins@finestructure.net>
+ *          Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+ */
+
+#include "notmuch.h"
+#include "crypto.h"
+#include <string.h>
+
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
+/* Create a GPG context (GMime 2.6) */
+static GMimeCryptoContext*
+create_gpg_context (_notmuch_crypto_t *crypto)
+{
+    GMimeCryptoContext *gpgctx;
+
+    if (crypto->gpgctx) {
+	return crypto->gpgctx;
+    }
+
+    /* TODO: GMimePasswordRequestFunc */
+    gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath : "gpg");
+    if (! gpgctx) {
+	fprintf (stderr, "Failed to construct gpg context.\n");
+	return NULL;
+    }
+    crypto->gpgctx = gpgctx;
+
+    g_mime_gpg_context_set_use_agent ((GMimeGpgContext *) gpgctx, TRUE);
+    g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) gpgctx, FALSE);
+
+    return crypto->gpgctx;
+}
+
+/* Create a PKCS7 context (GMime 2.6) */
+static notmuch_crypto_context_t *
+create_pkcs7_context (notmuch_crypto_t *crypto)
+{
+    notmuch_crypto_context_t *pkcs7ctx;
+
+    if (crypto->pkcs7ctx)
+	return crypto->pkcs7ctx;
+
+    /* TODO: GMimePasswordRequestFunc */
+    pkcs7ctx = g_mime_pkcs7_context_new (NULL);
+    if (! pkcs7ctx) {
+	fprintf (stderr, "Failed to construct pkcs7 context.\n");
+	return NULL;
+    }
+    crypto->pkcs7ctx = pkcs7ctx;
+
+    g_mime_pkcs7_context_set_always_trust ((GMimePkcs7Context *) pkcs7ctx,
+					   FALSE);
+
+    return crypto->pkcs7ctx;
+}
+static const struct {
+    const char *protocol;
+    GMimeCryptoContext *(*get_context) (_notmuch_crypto_t *crypto);
+} protocols[] = {
+    {
+	.protocol = "application/pgp-signature",
+	.get_context = create_gpg_context,
+    },
+    {
+	.protocol = "application/pgp-encrypted",
+	.get_context = create_gpg_context,
+    },
+    {
+	.protocol = "application/pkcs7-signature",
+	.get_context = create_pkcs7_context,
+    },
+    {
+	.protocol = "application/x-pkcs7-signature",
+	.get_context = create_pkcs7_context,
+    },
+};
+
+/* for the specified protocol return the context pointer (initializing
+ * if needed) */
+GMimeCryptoContext *
+_notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char *protocol)
+{
+    GMimeCryptoContext *cryptoctx = NULL;
+    size_t i;
+
+    if (! protocol) {
+	fprintf (stderr, "Cryptographic protocol is empty.\n");
+	return cryptoctx;
+    }
+
+    /* 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.
+     */
+    for (i = 0; i < ARRAY_SIZE (protocols); i++) {
+	if (strcasecmp (protocol, protocols[i].protocol) == 0)
+	    return protocols[i].get_context (crypto);
+    }
+
+    fprintf (stderr, "Unknown or unsupported cryptographic protocol %s.\n",
+	     protocol);
+
+    return NULL;
+}
+
+void
+_notmuch_crypto_cleanup (_notmuch_crypto_t *crypto)
+{
+    if (crypto->gpgctx) {
+	g_object_unref (crypto->gpgctx);
+	crypto->gpgctx = NULL;
+    }
+
+    if (crypto->pkcs7ctx) {
+	g_object_unref (crypto->pkcs7ctx);
+	crypto->pkcs7ctx = NULL;
+    }
+}
diff --git a/util/crypto.h b/util/crypto.h
new file mode 100644
index 0000000..d4a51e8
--- /dev/null
+++ b/util/crypto.h
@@ -0,0 +1,25 @@
+#ifndef _CRYPTO_H
+#define _CRYPTO_H
+
+#include "notmuch.h"
+#include <gmime/gmime.h>
+/* This is automatically included only since gmime 2.6.10 */
+#include <gmime/gmime-pkcs7-context.h>
+
+typedef struct _notmuch_crypto {
+    GMimeCryptoContext* gpgctx;
+    GMimeCryptoContext* pkcs7ctx;
+    notmuch_bool_t verify;
+    notmuch_bool_t decrypt;
+    const char *gpgpath;
+} _notmuch_crypto_t;
+
+
+GMimeCryptoContext *
+_notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char *protocol);
+
+void
+_notmuch_crypto_cleanup (_notmuch_crypto_t *crypto);
+
+
+#endif
-- 
2.8.1

Re: [PATCH v4 02/16] Move crypto.c into libutil

[David Bremner]

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

> This prepares us for using the crypto object in both the library and
> the client.
>
> i've prefixed notmuch_crypto with _ to indicate that while this can be
> built into the library when needed, it's not something to be exported
> or used externally.

I started to review this series on top of [1], but I didn't get
much past here because of the build failure. I guess there will be at
least one more small change needed because of the alternative way I
ended up doing "remove all properties".  I find

% git rebase -i -x "make test"

an effective way of making sure things are in a good state after every
patch. At least it's effective when I remember to do it; just before
writing this, I discovered a rebasing error in [1] that caused a similar
build failure. 

[1]: id:1470491559-3946-1-git-send-email-david@tethera.net

Re: [PATCH v4 02/16] Move crypto.c into libutil

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
> +++ b/util/crypto.c
> @@ -0,0 +1,138 @@

> +#include "notmuch.h"

It feels wrong to me for a file in util/ to include notmuch.h.  It seems
the same situation holds with search-path.h.  There it seems we use
notmuch_bool_t (although I'm not convinced that's the right return
type). If that's the only reason maybe we should either factor out the
definition or just return ints.

> +/* Create a PKCS7 context (GMime 2.6) */
> +static notmuch_crypto_context_t *
> +create_pkcs7_context (notmuch_crypto_t *crypto)
> +{
> +    notmuch_crypto_context_t *pkcs7ctx;

I guess this is leftover, and causes the build failure.

> +
> +#include "notmuch.h"

Same questions notmuch.h here.

d

Re: [PATCH v4 02/16] Move crypto.c into libutil

[Tomi Ollila]

On Fri, Aug 12 2016, David Bremner <david@tethera.net> wrote:

> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>> +++ b/util/crypto.c
>> @@ -0,0 +1,138 @@
>
>> +#include "notmuch.h"
>
> It feels wrong to me for a file in util/ to include notmuch.h.  It seems
> the same situation holds with search-path.h.  There it seems we use
> notmuch_bool_t (although I'm not convinced that's the right return
> type). If that's the only reason maybe we should either factor out the
> definition or just return ints.

util/search-path.c is easy, change to int or bool (and include stdbool.h,
as parse-time-string.c does)

util/crypto.c is harder. it uses many more \bnotmuch_.* types. perhaps this
could be moved to lib/ instead ?



>
>> +/* Create a PKCS7 context (GMime 2.6) */
>> +static notmuch_crypto_context_t *
>> +create_pkcs7_context (notmuch_crypto_t *crypto)
>> +{
>> +    notmuch_crypto_context_t *pkcs7ctx;
>
> I guess this is leftover, and causes the build failure.
>
>> +
>> +#include "notmuch.h"
>
> Same questions notmuch.h here.
>
> d
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 02/16] Move crypto.c into libutil

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Fri, Aug 12 2016, David Bremner <david@tethera.net> wrote:
>
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>> +++ b/util/crypto.c
>>> @@ -0,0 +1,138 @@
>>
>>> +#include "notmuch.h"
>>
>> It feels wrong to me for a file in util/ to include notmuch.h.  It seems
>> the same situation holds with search-path.h.  There it seems we use
>> notmuch_bool_t (although I'm not convinced that's the right return
>> type). If that's the only reason maybe we should either factor out the
>> definition or just return ints.
>
> util/search-path.c is easy, change to int or bool (and include stdbool.h,
> as parse-time-string.c does)
>
> util/crypto.c is harder. it uses many more \bnotmuch_.* types. perhaps this
> could be moved to lib/ instead ?

I guess the issue is we don't want to export these functions as ppart of
the API, but we do want to use them in the CLI.  I _thought_ that
util/crypto.c only (or mainly) used _notmuch_crypto_t, which is defined
in crypto.h. Given the various constraints, I think that is probably
OK. Some kind of purism about naming things in util/ might suggest they
not be called notmuch, but that is probably silly (although I admit I
had that thought).

d

[PATCH v4 03/16] make shared crypto code behave library-like

[Daniel Kahn Gillmor]

If we're going to reuse the crypto code across both the library and
the client, then it needs to report error states properly and not
write to stderr.
---
 lib/database.cc |  6 ++++
 lib/notmuch.h   | 17 +++++++++++
 mime-node.c     |  7 ++++-
 util/crypto.c   | 89 ++++++++++++++++++++++++++++-----------------------------
 util/crypto.h   |  6 ++--
 5 files changed, 76 insertions(+), 49 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3bdbd07..9d6b6f2 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -369,6 +369,12 @@ notmuch_status_to_string (notmuch_status_t status)
 	return "Operation requires a database upgrade";
     case NOTMUCH_STATUS_PATH_ERROR:
 	return "Path supplied is illegal for this function";
+    case NOTMUCH_STATUS_MALFORMED_CRYPTO_PROTOCOL:
+	return "Crypto protocol missing, malformed, or unintelligible";
+    case NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION:
+	return "Crypto engine initialization failure";
+    case NOTMUCH_STATUS_UNKNOWN_CRYPTO_PROTOCOL:
+	return "Unknown crypto protocol";
     default:
     case NOTMUCH_STATUS_LAST_STATUS:
 	return "Unknown error status value";
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0d667f5..aaed516 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -185,6 +185,23 @@ typedef enum _notmuch_status {
      */
     NOTMUCH_STATUS_ILLEGAL_ARGUMENT,
     /**
+     * A MIME object claimed to have cryptographic protection which
+     * notmuch tried to handle, but the protocol was not specified in
+     * an intelligible way.
+     */
+    NOTMUCH_STATUS_MALFORMED_CRYPTO_PROTOCOL,
+    /**
+     * Notmuch attempted to do crypto processing, but could not
+     * initialize the engine needed to do so.
+     */
+    NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION,
+    /**
+     * A MIME object claimed to have cryptographic protection, and
+     * notmuch attempted to process it, but the specific protocol was
+     * something that notmuch doesn't know how to handle.
+     */
+    NOTMUCH_STATUS_UNKNOWN_CRYPTO_PROTOCOL,
+    /**
      * Not an actual status value. Just a way to find out how many
      * valid status values there are.
      */
diff --git a/mime-node.c b/mime-node.c
index 14873ce..717a122 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -244,7 +244,12 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	|| (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
 	GMimeContentType *content_type = g_mime_object_get_content_type (part);
 	const char *protocol = g_mime_content_type_get_parameter (content_type, "protocol");
-	cryptoctx = _notmuch_crypto_get_gmime_context (node->ctx->crypto, protocol);
+	notmuch_status_t status;
+	status = _notmuch_crypto_get_gmime_ctx_for_protocol (node->ctx->crypto,
+							     protocol, &cryptoctx);
+	if (status) /* this is a warning, not an error */
+	    fprintf (stderr, "Warning: %s (%s).\n", notmuch_status_to_string (status),
+		     protocol ? protocol : "(NULL)");
     }
 
     /* Handle PGP/MIME parts */
diff --git a/util/crypto.c b/util/crypto.c
index 48c20bb..cce5cbc 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -25,86 +25,86 @@
 
 #define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
 
-/* Create a GPG context (GMime 2.6) */
-static GMimeCryptoContext*
-create_gpg_context (_notmuch_crypto_t *crypto)
+/* Create or pass on a GPG context (GMime 2.6) */
+static notmuch_status_t
+get_gpg_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
 {
-    GMimeCryptoContext *gpgctx;
+    if (ctx == NULL || crypto == NULL)
+	return NOTMUCH_STATUS_NULL_POINTER;
 
     if (crypto->gpgctx) {
-	return crypto->gpgctx;
+	*ctx = crypto->gpgctx;
+	return NOTMUCH_STATUS_SUCCESS;
     }
 
     /* TODO: GMimePasswordRequestFunc */
-    gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath : "gpg");
-    if (! gpgctx) {
-	fprintf (stderr, "Failed to construct gpg context.\n");
-	return NULL;
+    crypto->gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath : "gpg");
+    if (! crypto->gpgctx) {
+	return NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
     }
-    crypto->gpgctx = gpgctx;
 
-    g_mime_gpg_context_set_use_agent ((GMimeGpgContext *) gpgctx, TRUE);
-    g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) gpgctx, FALSE);
+    g_mime_gpg_context_set_use_agent ((GMimeGpgContext *) crypto->gpgctx, TRUE);
+    g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) crypto->gpgctx, FALSE);
 
-    return crypto->gpgctx;
+    *ctx = crypto->gpgctx;
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
-/* Create a PKCS7 context (GMime 2.6) */
-static notmuch_crypto_context_t *
-create_pkcs7_context (notmuch_crypto_t *crypto)
+/* Create or pass on a PKCS7 context (GMime 2.6) */
+static notmuch_status_t 
+get_pkcs7_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
 {
-    notmuch_crypto_context_t *pkcs7ctx;
+    if (ctx == NULL || crypto == NULL)
+	return NOTMUCH_STATUS_NULL_POINTER;
 
-    if (crypto->pkcs7ctx)
-	return crypto->pkcs7ctx;
+    if (crypto->pkcs7ctx) {
+	*ctx = crypto->pkcs7ctx;
+	return NOTMUCH_STATUS_SUCCESS;
+    }
 
     /* TODO: GMimePasswordRequestFunc */
-    pkcs7ctx = g_mime_pkcs7_context_new (NULL);
-    if (! pkcs7ctx) {
-	fprintf (stderr, "Failed to construct pkcs7 context.\n");
-	return NULL;
+    crypto->pkcs7ctx = g_mime_pkcs7_context_new (NULL);
+    if (! crypto->pkcs7ctx) {
+	return NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
     }
-    crypto->pkcs7ctx = pkcs7ctx;
 
-    g_mime_pkcs7_context_set_always_trust ((GMimePkcs7Context *) pkcs7ctx,
+    g_mime_pkcs7_context_set_always_trust ((GMimePkcs7Context *) crypto->pkcs7ctx,
 					   FALSE);
 
-    return crypto->pkcs7ctx;
+    *ctx = crypto->pkcs7ctx;
+    return NOTMUCH_STATUS_SUCCESS;
 }
 static const struct {
     const char *protocol;
-    GMimeCryptoContext *(*get_context) (_notmuch_crypto_t *crypto);
+    notmuch_status_t (*get_context) (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx);
 } protocols[] = {
     {
 	.protocol = "application/pgp-signature",
-	.get_context = create_gpg_context,
+	.get_context = get_gpg_context,
     },
     {
 	.protocol = "application/pgp-encrypted",
-	.get_context = create_gpg_context,
+	.get_context = get_gpg_context,
     },
     {
 	.protocol = "application/pkcs7-signature",
-	.get_context = create_pkcs7_context,
+	.get_context = get_pkcs7_context,
     },
     {
 	.protocol = "application/x-pkcs7-signature",
-	.get_context = create_pkcs7_context,
+	.get_context = get_pkcs7_context,
     },
 };
 
 /* for the specified protocol return the context pointer (initializing
  * if needed) */
-GMimeCryptoContext *
-_notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char *protocol)
+notmuch_status_t
+_notmuch_crypto_get_gmime_ctx_for_protocol (_notmuch_crypto_t *crypto,
+					    const char *protocol,
+					    GMimeCryptoContext **ctx)
 {
-    GMimeCryptoContext *cryptoctx = NULL;
-    size_t i;
-
-    if (! protocol) {
-	fprintf (stderr, "Cryptographic protocol is empty.\n");
-	return cryptoctx;
-    }
+    if (! protocol)
+	return NOTMUCH_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".
@@ -112,15 +112,12 @@ _notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char *protoc
      * parameter names as defined in this document are
      * case-insensitive."  Thus, we use strcasecmp for the protocol.
      */
-    for (i = 0; i < ARRAY_SIZE (protocols); i++) {
+    for (size_t i = 0; i < ARRAY_SIZE (protocols); i++) {
 	if (strcasecmp (protocol, protocols[i].protocol) == 0)
-	    return protocols[i].get_context (crypto);
+	    return protocols[i].get_context (crypto, ctx);
     }
 
-    fprintf (stderr, "Unknown or unsupported cryptographic protocol %s.\n",
-	     protocol);
-
-    return NULL;
+    return NOTMUCH_STATUS_UNKNOWN_CRYPTO_PROTOCOL;
 }
 
 void
diff --git a/util/crypto.h b/util/crypto.h
index d4a51e8..7cb0a39 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -15,8 +15,10 @@ typedef struct _notmuch_crypto {
 } _notmuch_crypto_t;
 
 
-GMimeCryptoContext *
-_notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char *protocol);
+notmuch_status_t
+_notmuch_crypto_get_gmime_ctx_for_protocol (_notmuch_crypto_t *crypto,
+					    const char *protocol,
+					    GMimeCryptoContext **ctx);
 
 void
 _notmuch_crypto_cleanup (_notmuch_crypto_t *crypto);
-- 
2.8.1

Re: [PATCH v4 03/16] make shared crypto code behave library-like

[David Bremner]

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

>  lib/notmuch.h   | 17 +++++++++++

It seems this patch needs rebasing, it doesn't apply anymore (and my git
the data to do it easily).  Reviewing it statically, the patch looks ok.

[PATCH v4 04/16] Provide _notmuch_crypto_{set,get}_gpg_path

[Daniel Kahn Gillmor]

Use functions to access the gpg_path for a _notmuch_crypto_t object.
This lets us return sensible defaults based on the state of the user's
machine.

If the passed-in _notmuch_crypto_t is NULL, then just return the
system's default choice of gpg.
---
 notmuch-reply.c | 13 ++++++++++---
 notmuch-show.c  | 12 ++++++++++--
 util/crypto.c   | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
 util/crypto.h   |  8 +++++++-
 4 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 42aef47..d0b4a0d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -790,13 +790,15 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	.crypto = {
 	    .verify = FALSE,
 	    .decrypt = FALSE,
-	    .gpgpath = NULL
+	    .gpg_path = NULL
 	}
     };
     int format = FORMAT_DEFAULT;
     int reply_all = TRUE;
     struct sprinter *sp = NULL;
-
+    notmuch_status_t status;
+    const char *gpg_path = NULL;
+    
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
 	  (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
@@ -845,7 +847,12 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    params.crypto.gpgpath = notmuch_config_get_crypto_gpg_path (config);
+    gpg_path = notmuch_config_get_crypto_gpg_path (config);
+    status = _notmuch_crypto_set_gpg_path (&(params.crypto), gpg_path);
+    if (status != NOTMUCH_STATUS_SUCCESS) {
+	fprintf (stderr, "Error: could not set gpg_path to '%s'.\n", gpg_path);
+	return EXIT_FAILURE;
+    }
 
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
diff --git a/notmuch-show.c b/notmuch-show.c
index 8ebf4ff..60411d0 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1006,13 +1006,15 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	.crypto = {
 	    .verify = FALSE,
 	    .decrypt = FALSE,
-	    .gpgpath = NULL
+	    .gpg_path = NULL
 	},
 	.include_html = FALSE
     };
     int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
     int exclude = EXCLUDE_TRUE;
     int entire_thread = ENTIRE_THREAD_DEFAULT;
+    notmuch_status_t status;
+    const char *gpg_path = NULL;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
@@ -1130,7 +1132,13 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    params.crypto.gpgpath = notmuch_config_get_crypto_gpg_path (config);
+
+    gpg_path = notmuch_config_get_crypto_gpg_path (config);
+    status = _notmuch_crypto_set_gpg_path (&(params.crypto), gpg_path);
+    if (status != NOTMUCH_STATUS_SUCCESS) {
+	fprintf (stderr, "Error: could not set gpg_path to '%s'.\n", gpg_path);
+	return EXIT_FAILURE;
+    }
 
     if (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
diff --git a/util/crypto.c b/util/crypto.c
index cce5cbc..9766c2c 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -21,7 +21,9 @@
 
 #include "notmuch.h"
 #include "crypto.h"
+#include "search-path.h"
 #include <string.h>
+#include <talloc.h>
 
 #define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
 
@@ -38,7 +40,7 @@ get_gpg_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
     }
 
     /* TODO: GMimePasswordRequestFunc */
-    crypto->gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath : "gpg");
+    crypto->gpgctx = g_mime_gpg_context_new (NULL, _notmuch_crypto_get_gpg_path (crypto));
     if (! crypto->gpgctx) {
 	return NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
     }
@@ -51,7 +53,7 @@ get_gpg_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
 }
 
 /* Create or pass on a PKCS7 context (GMime 2.6) */
-static notmuch_status_t 
+static notmuch_status_t
 get_pkcs7_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
 {
     if (ctx == NULL || crypto == NULL)
@@ -76,7 +78,7 @@ get_pkcs7_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
 }
 static const struct {
     const char *protocol;
-    notmuch_status_t (*get_context) (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx);
+    notmuch_status_t (*get_context)(_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx);
 } protocols[] = {
     {
 	.protocol = "application/pgp-signature",
@@ -120,6 +122,45 @@ _notmuch_crypto_get_gmime_ctx_for_protocol (_notmuch_crypto_t *crypto,
     return NOTMUCH_STATUS_UNKNOWN_CRYPTO_PROTOCOL;
 }
 
+const char *
+_notmuch_crypto_get_gpg_path (const _notmuch_crypto_t *crypto)
+{
+    if (crypto && crypto->gpg_path)
+	return crypto->gpg_path;
+
+    if (test_for_executable ("gpg2")) return "gpg2";
+    if (test_for_executable ("gpg")) return "gpg";
+    return NULL;
+}
+
+notmuch_status_t
+_notmuch_crypto_set_gpg_path (_notmuch_crypto_t *crypto, const char *gpg_path)
+{
+    /* return success if this matches what is already configured */
+    if ((! gpg_path && ! crypto->gpg_path) ||
+	(gpg_path && crypto->gpg_path && 0 == strcmp (gpg_path, crypto->gpg_path)))
+	return NOTMUCH_STATUS_SUCCESS;
+
+    if (! gpg_path && ! test_for_executable (gpg_path))
+	return NOTMUCH_STATUS_FILE_ERROR;
+
+    /* clear any existing gpgctx, since things are changing */
+    if (crypto->gpgctx) {
+	g_object_unref (crypto->gpgctx);
+	crypto->gpgctx = NULL;
+    }
+
+    if (crypto->gpg_path) {
+	talloc_free (crypto->gpg_path);
+	crypto->gpg_path = NULL;
+    }
+
+    if (gpg_path)
+	crypto->gpg_path = talloc_strdup (NULL, gpg_path);
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
 void
 _notmuch_crypto_cleanup (_notmuch_crypto_t *crypto)
 {
@@ -132,4 +173,7 @@ _notmuch_crypto_cleanup (_notmuch_crypto_t *crypto)
 	g_object_unref (crypto->pkcs7ctx);
 	crypto->pkcs7ctx = NULL;
     }
+
+    talloc_free (crypto->gpg_path);
+    crypto->gpg_path = NULL;
 }
diff --git a/util/crypto.h b/util/crypto.h
index 7cb0a39..70fc8ef 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -11,7 +11,7 @@ typedef struct _notmuch_crypto {
     GMimeCryptoContext* pkcs7ctx;
     notmuch_bool_t verify;
     notmuch_bool_t decrypt;
-    const char *gpgpath;
+    char *gpg_path;
 } _notmuch_crypto_t;
 
 
@@ -20,6 +20,12 @@ _notmuch_crypto_get_gmime_ctx_for_protocol (_notmuch_crypto_t *crypto,
 					    const char *protocol,
 					    GMimeCryptoContext **ctx);
 
+notmuch_status_t
+_notmuch_crypto_set_gpg_path (_notmuch_crypto_t *crypto, const char *gpg_path);
+
+const char *
+_notmuch_crypto_get_gpg_path (const _notmuch_crypto_t *crypto);
+
 void
 _notmuch_crypto_cleanup (_notmuch_crypto_t *crypto);
 
-- 
2.8.1

Re: [PATCH v4 04/16] Provide _notmuch_crypto_{set,get}_gpg_path

[David Bremner]

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


> +notmuch_status_t
> +_notmuch_crypto_set_gpg_path (_notmuch_crypto_t *crypto, const char *gpg_path)
> +{
[snip]
> +    if (! gpg_path && ! test_for_executable (gpg_path))
> +	return NOTMUCH_STATUS_FILE_ERROR;

this looks strange. Maybe the "!" in front of gpg_path might be
spurious. Or maybe you mean || ?

> -    const char *gpgpath;
> +    char *gpg_path;
>  } _notmuch_crypto_t;

Why is const dropped here?
>  

[PATCH v4 05/16] Choose the default gpg_path with _notmuch_crypto_get_gpg_path (NULL)

[Daniel Kahn Gillmor]

This way we're only choosing a default in one place.
---
 notmuch-config.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index e5d42a0..9e4601a 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -417,9 +417,8 @@ notmuch_config_open (void *ctx,
 	g_error_free (error);
     }
 
-    if (notmuch_config_get_crypto_gpg_path (config) == NULL) {
-	notmuch_config_set_crypto_gpg_path (config, "gpg");
-    }
+    if (notmuch_config_get_crypto_gpg_path (config) == NULL)
+	notmuch_config_set_crypto_gpg_path (config, _notmuch_crypto_get_gpg_path (NULL));
     
     /* Whenever we know of configuration sections that don't appear in
      * the configuration file, we add some comments to help the user
-- 
2.8.1

[PATCH v4 06/16] Prefer gpg2 in the test suite if available

[Daniel Kahn Gillmor]

Now that the notmuch client prefers gpg2 if available, having the test
suite use the same preference makes it more likely to validate as
expected.

Be warned that the final test in T350-crypto.sh fails with an infinite
loop in gpg if you're using an unpatched GnuPG 2.1.10, due to an
upstream GnuPG bug: https://bugs.gnupg.org/gnupg/issue2187.  In
debian, this is resolved in 2.1.10-3
---
 test/README         |  2 +-
 test/T030-config.sh |  2 +-
 test/T040-setup.sh  |  2 +-
 test/T350-crypto.sh | 16 ++++++++--------
 test/test-lib.sh    | 10 +++++++++-
 5 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/test/README b/test/README
index 104a120..b4489da 100644
--- a/test/README
+++ b/test/README
@@ -23,7 +23,7 @@ that you know if you break anything.
   - emacs(1)
   - emacsclient(1)
   - gdb(1)
-  - gpg(1)
+  - gpg(1) or gpg2(1)
   - python(1)
 
 If your system lacks these tools or have older, non-upgreable versions
diff --git a/test/T030-config.sh b/test/T030-config.sh
index 0915abd..9eb9294 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -54,7 +54,7 @@ new.tags=unread;inbox;
 new.ignore=
 search.exclude_tags=
 maildir.synchronize_flags=true
-crypto.gpg_path=gpg
+crypto.gpg_path=$GPG
 foo.string=this is another string value
 foo.list=this;is another;list value;
 built_with.compact=something
diff --git a/test/T040-setup.sh b/test/T040-setup.sh
index 021f2d0..afc7bc0 100755
--- a/test/T040-setup.sh
+++ b/test/T040-setup.sh
@@ -29,7 +29,7 @@ new.tags=foo;bar;
 new.ignore=
 search.exclude_tags=baz;
 maildir.synchronize_flags=true
-crypto.gpg_path=gpg
+crypto.gpg_path=$GPG
 built_with.compact=something
 built_with.field_processor=something
 built_with.retry_lock=something"
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 3656cce..4bc15bc 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -12,11 +12,11 @@ add_gnupg_home ()
     local output
     [ -d ${GNUPGHOME} ] && return
     mkdir -m 0700 "$GNUPGHOME"
-    gpg --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc >"$GNUPGHOME"/import.log 2>&1
+    $GPG --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc >"$GNUPGHOME"/import.log 2>&1
     test_debug "cat $GNUPGHOME/import.log"
-    if (gpg --quick-random --version >/dev/null 2>&1) ; then
+    if ($GPG --quick-random --version >/dev/null 2>&1) ; then
 	echo quick-random >> "$GNUPGHOME"/gpg.conf
-    elif (gpg --debug-quick-random --version >/dev/null 2>&1) ; then
+    elif ($GPG --debug-quick-random --version >/dev/null 2>&1) ; then
 	echo debug-quick-random >> "$GNUPGHOME"/gpg.conf
     fi
     echo no-emit-version >> "$GNUPGHOME"/gpg.conf
@@ -26,7 +26,7 @@ add_gnupg_home ()
 
 add_gnupg_home
 # get key fingerprint
-FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10)
+FINGERPRINT=$($GPG --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10)
 
 test_expect_success 'emacs delivery of signed message' \
 'emacs_fcc_message \
@@ -67,8 +67,8 @@ test_expect_equal_json \
 
 test_begin_subtest "signature verification with full owner trust"
 # give the key full owner trust
-echo "${FINGERPRINT}:6:" | gpg --no-tty --import-ownertrust >>"$GNUPGHOME"/trust.log 2>&1
-gpg --no-tty --check-trustdb >>"$GNUPGHOME"/trust.log 2>&1
+echo "${FINGERPRINT}:6:" | $GPG --no-tty --import-ownertrust >>"$GNUPGHOME"/trust.log 2>&1
+$GPG --no-tty --check-trustdb >>"$GNUPGHOME"/trust.log 2>&1
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
@@ -325,8 +325,8 @@ Notmuch Test Suite key revocation (automated) $(date '+%F_%T%z')
 y
 
 " \
-    | gpg --no-tty --quiet --command-fd 0 --armor --gen-revoke "0x${FINGERPRINT}!" 2>/dev/null \
-    | gpg --no-tty --quiet --import
+    | $GPG --no-tty --quiet --command-fd 0 --armor --gen-revoke "0x${FINGERPRINT}!" 2>/dev/null \
+    | $GPG --no-tty --quiet --import
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
diff --git a/test/test-lib.sh b/test/test-lib.sh
index aac0343..5c14d1e 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -86,6 +86,13 @@ unset GREP_OPTIONS
 # For emacsclient
 unset ALTERNATE_EDITOR
 
+# choose the preferred GnuPG binary:
+if command -v gpg2 > /dev/null; then
+    GPG=gpg2
+else
+    GPG=gpg
+fi
+
 # Convenience
 #
 # A regexp to match 5 and 40 hexdigits
@@ -1172,6 +1179,7 @@ test_emacs () {
 				$load_emacs_tests \
 				--eval '(setq server-name \"$server_name\")' \
 				--eval '(server-start)' \
+				--eval '(setq epg-gpg-program \"$GPG\")' \
 				--eval '(orphan-watchdog $$)'" || return
 		EMACS_SERVER="$server_name"
 		# wait until the emacs server is up
@@ -1368,7 +1376,7 @@ test_declare_external_prereq dtach
 test_declare_external_prereq emacs
 test_declare_external_prereq ${TEST_EMACSCLIENT}
 test_declare_external_prereq gdb
-test_declare_external_prereq gpg
+test_declare_external_prereq gpg2 || test_declare_external_prereq gpg
 test_declare_external_prereq openssl
 test_declare_external_prereq gpgsm
 test_declare_external_prereq ${NOTMUCH_PYTHON}
-- 
2.8.1

Re: [PATCH v4 06/16] Prefer gpg2 in the test suite if available

[David Bremner]

patches 5 and 6 look fine (although I currently can't test them).

[PATCH v4 07/16] create a notmuch_indexopts_t index options object

[Daniel Kahn Gillmor]

This is currently mostly a wrapper around _notmuch_crypto_t that keeps
its internals private and doesn't expose any of the GMime API.
However, non-crypto indexing options might also be added later to
indexopts (e.g. filters or other transformations).
---
 lib/Makefile.local    |  1 +
 lib/indexopts.c       | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch-private.h |  7 +++++
 lib/notmuch.h         | 63 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+)
 create mode 100644 lib/indexopts.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index c012ed1..56a5f5a 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -41,6 +41,7 @@ libnotmuch_c_srcs =		\
 	$(dir)/sha1.c		\
 	$(dir)/built-with.c	\
 	$(dir)/string-map.c    \
+	$(dir)/indexopts.c	\
 	$(dir)/tags.c
 
 libnotmuch_cxx_srcs =		\
diff --git a/lib/indexopts.c b/lib/indexopts.c
new file mode 100644
index 0000000..da36e2b
--- /dev/null
+++ b/lib/indexopts.c
@@ -0,0 +1,72 @@
+/* indexopts.c - options for indexing messages
+ *
+ * Copyright © 2015 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+ */
+
+#include "notmuch-private.h"
+
+notmuch_indexopts_t *
+notmuch_indexopts_create ()
+{
+    notmuch_indexopts_t *ret;
+    
+    ret = talloc_zero (NULL, notmuch_indexopts_t);
+
+    return ret;
+}
+
+notmuch_status_t
+notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
+				   notmuch_bool_t try_decrypt)
+{
+    if (!indexopts)
+	return NOTMUCH_STATUS_NULL_POINTER;
+    indexopts->crypto.decrypt = try_decrypt;
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+notmuch_bool_t
+notmuch_indexopts_get_try_decrypt (const notmuch_indexopts_t *indexopts)
+{
+    if (!indexopts)
+	return FALSE;
+    return indexopts->crypto.decrypt;
+}
+
+notmuch_status_t
+notmuch_indexopts_set_gpg_path (notmuch_indexopts_t *indexopts,
+				const char *gpg_path)
+{
+    if (!indexopts)
+	return NOTMUCH_STATUS_NULL_POINTER;
+    return _notmuch_crypto_set_gpg_path (&(indexopts->crypto), gpg_path);
+}
+
+const char*
+notmuch_indexopts_get_gpg_path (const notmuch_indexopts_t *indexopts)
+{
+    if (!indexopts)
+	return NULL;
+    return _notmuch_crypto_get_gpg_path (&(indexopts->crypto));
+}
+
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *indexopts)
+{
+    talloc_free (indexopts);
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 65f7ead..8b2aede 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -51,6 +51,7 @@ NOTMUCH_BEGIN_DECLS
 #include "xutil.h"
 #include "error_util.h"
 #include "string-util.h"
+#include "crypto.h"
 
 #pragma GCC visibility push(hidden)
 
@@ -594,6 +595,12 @@ _notmuch_thread_create (void *ctx,
 			notmuch_exclude_t omit_exclude,
 			notmuch_sort_t sort);
 
+/* indexopts.c */
+
+typedef struct _notmuch_indexopts {
+    _notmuch_crypto_t crypto;
+} notmuch_indexopts_t;
+
 NOTMUCH_END_DECLS
 
 #ifdef __cplusplus
diff --git a/lib/notmuch.h b/lib/notmuch.h
index aaed516..b92d969 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -229,6 +229,7 @@ typedef struct _notmuch_tags notmuch_tags_t;
 typedef struct _notmuch_directory notmuch_directory_t;
 typedef struct _notmuch_filenames notmuch_filenames_t;
 typedef struct _notmuch_config_list notmuch_config_list_t;
+typedef struct _notmuch_indexopts notmuch_indexopts_t;
 #endif /* __DOXYGEN__ */
 
 /**
@@ -2091,6 +2092,68 @@ notmuch_config_list_destroy (notmuch_config_list_t *config_list);
  */
 notmuch_bool_t
 notmuch_built_with (const char *name);
+
+/**
+ * Create a notmuch_indexopts_t object.
+ *
+ * This object describes options on how indexing can happen when a
+ * message is added to the index.
+ */
+notmuch_indexopts_t *
+notmuch_indexopts_create ();
+
+/**
+ * Specify whether to decrypt encrypted parts while indexing.
+ *
+ * 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 SET THIS FLAG TO TRUE
+ * without considering the security of your index.
+ */
+notmuch_status_t
+notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
+				   notmuch_bool_t try_decrypt);
+
+/**
+ * Return whether to decrypt encrypted parts while indexing.
+ * see notmuch_indexopts_set_try_decrypt.
+ */
+notmuch_bool_t
+notmuch_indexopts_get_try_decrypt (const notmuch_indexopts_t *indexopts);
+
+/**
+ * Specify the name (or name and path) of the gpg executable, in case
+ * GnuPG needs to be used during indexing.  The default should usually
+ * be fine.
+ *
+ * Passing NULL to this will reset it to the default.
+ *
+ * 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_indexopts_set_gpg_path (notmuch_indexopts_t *indexopts,
+				const char *gpg_path);
+
+/**
+ * Return the name (possibly including path) of the gpg executable to
+ * be used in case GnuPG needs to be used during indexing.
+ *
+ * see notmuch_indexopts_set_gpg_path
+ */
+const char*
+notmuch_indexopts_get_gpg_path (const notmuch_indexopts_t *indexopts);
+
+/**
+ * Destroy a notmuch_indexopts_t object.
+ */
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *options);
+
 /* @} */
 
 NOTMUCH_END_DECLS
-- 
2.8.1

[PATCH v4 08/16] 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 | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 8c14554..1c030a6 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -333,27 +333,26 @@ _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 */
+	    _notmuch_message_add_term (message, "tag", "encrypted");
+	} 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.8.1

Re: [PATCH v4 08/16] reorganize indexing of multipart/signed and multipart/encrypted

[David Bremner]

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

> +	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)) {

I'm confused about this warning, or more importantly about whether there
is an indexing behaviour change here. In particular the extra parts are
indexed in a for loop which is now in the else branch of this if, so as
far as I can tell, it won't be run for multipart/signed

[PATCH v4 09/16] index encrypted parts when asked.

[Daniel Kahn Gillmor]

If we see index options that ask us to decrypt when indexing a
message, and we encounter an encrypted part, we'll try to descend into
it.

If we can decrypt, we add the property index-decryption=success.

If we can't decrypt (or recognize the encrypted type of mail), we add
the property index-decryption=failure.

Note that a single message may have both values of the
"index-decryption" property: "success" and "failure".  For example,
consider a message that includes multiple layers of encryption.  If we
manage to decrypt the outer layer ("index-decryption=success"), but
fail on the inner layer ("index-decryption=failure").
---
 lib/database.cc       |  3 ++-
 lib/index.cc          | 74 ++++++++++++++++++++++++++++++++++++++++++++++++---
 lib/notmuch-private.h |  1 +
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 9d6b6f2..4e01f12 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2426,6 +2426,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, ret2;
     notmuch_private_status_t private_status;
     notmuch_bool_t is_ghost = false;
+    notmuch_indexopts_t *indexopts = NULL;
 
     const char *date, *header;
     const char *from, *to, *subject;
@@ -2538,7 +2539,7 @@ 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);
 
-	    ret = _notmuch_message_index_file (message, message_file);
+	    ret = _notmuch_message_index_file (message, indexopts, message_file);
 	    if (ret)
 		goto DONE;
 	} else {
diff --git a/lib/index.cc b/lib/index.cc
index 1c030a6..a579c42 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -300,9 +300,14 @@ _index_address_list (notmuch_message_t *message,
     }
 }
 
+static void
+_index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts,
+			    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,
+		  notmuch_indexopts_t *indexopts,
 		  GMimeObject *part)
 {
     GMimeStream *stream, *filter;
@@ -340,17 +345,19 @@ _index_mime_part (notmuch_message_t *message,
 	    /* FIXME: is it always just the first part that is signed in
 	     all multipart/signed messages?*/
 	    _index_mime_part (message,
+			      indexopts,
 			      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 */
 	    _notmuch_message_add_term (message, "tag", "encrypted");
+	    _index_encrypted_mime_part(message, indexopts, content_type, GMIME_MULTIPART_ENCRYPTED (part));
 	} else {
 	    for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
 		_index_mime_part (message,
+				  indexopts,
 				  g_mime_multipart_get_part (multipart, i));
 	    }
 	}
@@ -362,7 +369,7 @@ _index_mime_part (notmuch_message_t *message,
 
 	mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (part));
 
-	_index_mime_part (message, g_mime_message_get_mime_part (mime_message));
+	_index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message));
 
 	return;
     }
@@ -432,8 +439,69 @@ _index_mime_part (notmuch_message_t *message,
     }
 }
 
+/* descend (if desired) into the cleartext part of an encrypted MIME
+ * part while indexing. */
+static void
+_index_encrypted_mime_part (notmuch_message_t *message,
+			    notmuch_indexopts_t *indexopts,
+			    GMimeContentType *content_type,
+			    GMimeMultipartEncrypted *encrypted_data)
+{
+    notmuch_status_t status;
+    GMimeCryptoContext* crypto_ctx = NULL;
+    const char *protocol = NULL;
+    GError *err = NULL;
+    notmuch_database_t * notmuch = NULL;
+    GMimeObject *clear = NULL;
+
+    if (!indexopts || !notmuch_indexopts_get_try_decrypt (indexopts))
+	return;
+
+    protocol = g_mime_content_type_get_parameter (content_type, "protocol");
+    notmuch = _notmuch_message_database (message);
+    
+    status = _notmuch_crypto_get_gmime_ctx_for_protocol (&(indexopts->crypto),
+							 protocol, &crypto_ctx);
+    if (status) {
+	_notmuch_database_log (notmuch, "Warning: setup failed for decrypting "
+			       "during indexing. (%d)\n", status);
+	status = notmuch_message_add_property (message, "index-decryption", "failure");
+	if (status)
+	    _notmuch_database_log (notmuch, "failed to add index-decryption "
+				   "property (%d)\n", status);
+	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 */
+	status = notmuch_message_add_property (message, "index-decryption", "failure");
+	if (status)
+	    _notmuch_database_log (notmuch, "failed to add index-decryption "
+				   "property (%d)\n", status);
+	return;
+    }
+    _index_mime_part (message, indexopts, clear);
+    g_object_unref (clear);
+    
+    status = notmuch_message_add_property (message, "index-decryption", "success");
+    if (status)
+	_notmuch_database_log (notmuch, "failed to add index-decryption "
+			       "property (%d)\n", status);
+
+}
+
 notmuch_status_t
 _notmuch_message_index_file (notmuch_message_t *message,
+			     notmuch_indexopts_t *indexopts,
 			     notmuch_message_file_t *message_file)
 {
     GMimeMessage *mime_message;
@@ -463,7 +531,7 @@ _notmuch_message_index_file (notmuch_message_t *message,
     subject = g_mime_message_get_subject (mime_message);
     _notmuch_message_gen_terms (message, "subject", subject);
 
-    _index_mime_part (message, g_mime_message_get_mime_part (mime_message));
+    _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message));
 
     return NOTMUCH_STATUS_SUCCESS;
 }
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 8b2aede..01b65ba 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -431,6 +431,7 @@ _notmuch_message_file_get_header (notmuch_message_file_t *message,
 
 notmuch_status_t
 _notmuch_message_index_file (notmuch_message_t *message,
+			     notmuch_indexopts_t *indexopts,
 			     notmuch_message_file_t *message_file);
 
 /* messages.c */
-- 
2.8.1

Re: [PATCH v4 09/16] index encrypted parts when asked.

[David Bremner]

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

> +    status = _notmuch_crypto_get_gmime_ctx_for_protocol (&(indexopts->crypto),
> +							 protocol, &crypto_ctx);
> +    if (status) {
> +	_notmuch_database_log (notmuch, "Warning: setup failed for decrypting "
> +			       "during indexing. (%d)\n", status);
> +	status = notmuch_message_add_property (message, "index-decryption", "failure");
> +	if (status)
> +	    _notmuch_database_log (notmuch, "failed to add index-decryption "
> +				   "property (%d)\n", status);
> +	return;
> +    }

Currently the only correct usage of _notmuch_database_log is the
following pattern

          _notmuch_database_log (notmuch, "Cannot write to a read-only database.\n");
          return NOTMUCH_STATUS_READ_ONLY_DATABASE;

In particular, the log buffer is only one line, and the caller needs to
know to retrieve it.

I agree it's not ideal, but I doubt you want to delay your stuff in
order to extend/fix the internal logging API.


d

Re: [PATCH v4 09/16] index encrypted parts when asked.

[Daniel Kahn Gillmor]

On Thu 2016-07-14 15:59:15 +0200, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> +    status = _notmuch_crypto_get_gmime_ctx_for_protocol (&(indexopts->crypto),
>> +							 protocol, &crypto_ctx);
>> +    if (status) {
>> +	_notmuch_database_log (notmuch, "Warning: setup failed for decrypting "
>> +			       "during indexing. (%d)\n", status);
>> +	status = notmuch_message_add_property (message, "index-decryption", "failure");
>> +	if (status)
>> +	    _notmuch_database_log (notmuch, "failed to add index-decryption "
>> +				   "property (%d)\n", status);
>> +	return;
>> +    }
>
> Currently the only correct usage of _notmuch_database_log is the
> following pattern
>
>           _notmuch_database_log (notmuch, "Cannot write to a read-only database.\n");
>           return NOTMUCH_STATUS_READ_ONLY_DATABASE;
>
> In particular, the log buffer is only one line, and the caller needs to
> know to retrieve it.
>
> I agree it's not ideal, but I doubt you want to delay your stuff in
> order to extend/fix the internal logging API.

I understand that the internal log is currently only a single line, but
following the usage pattern you describe isn't useful for this case.

What is your suggested fix?  a given message could have multiple parts,
some of which are decryptable and others of which are not.

It makes no sense to stop indexing a message just because one of the
parts failed to decrypt, so i'm not going to immediately return.

I'm willing to accept that only the last log message will make it out to
the caller, and i could track whether anything has been written to the
log and change the return value in that case.  would that be acceptable?

    --dkg

Re: [PATCH v4 09/16] index encrypted parts when asked.

[David Bremner]

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

> It makes no sense to stop indexing a message just because one of the
> parts failed to decrypt, so i'm not going to immediately return.
>
> I'm willing to accept that only the last log message will make it out to
> the caller, and i could track whether anything has been written to the
> log and change the return value in that case.  would that be acceptable?
>

That sounds like an improvement. Other options I can think of

      - accumulate an error string. With talloc_asprintf_append, this is
      not _too_ terrible. Making a second logging function [1] that
      didn't clear the log buffer but appended would maybe make sense
      (aside from contradicting what I said in the previous message).
      This would still need some status return to alert the caller.

    - Pass a logging callback; this requires API changes.  We
      already have a such a callback for notmuch_database_compact.
      

While thinking about this, I noticed several suspect uses of
_notmuch_database_log in current index.cc, at least in
_index_mime_part. These are probably my fault, resulting from 
blindly replacing printfs.

[1]: untested:

void
_notmuch_database_log_append (notmuch_database_t *notmuch,
		      const char *format,
		      ...)
{
    va_list va_args;

    va_start (va_args, format);

    if (notmuch->status_string)
	notmuch->status_string = talloc_vasprintf_append (notmuch->status_string, format, va_args)
    else
	notmuch->status_string = talloc_vasprintf (notmuch, format, va_args);
    
    va_end (va_args);
}

Re: [PATCH v4 09/16] index encrypted parts when asked.

[Daniel Kahn Gillmor]

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

On Fri 2016-07-15 02:23:20 +0200, David Bremner wrote:
>       - accumulate an error string. With talloc_asprintf_append, this is
>       not _too_ terrible. Making a second logging function [1] that
>       didn't clear the log buffer but appended would maybe make sense
>       (aside from contradicting what I said in the previous message).
>       This would still need some status return to alert the caller.
 [...]
> [1]: untested:
>
> void
> _notmuch_database_log_append (notmuch_database_t *notmuch,
> 		      const char *format,
> 		      ...)
> {
>     va_list va_args;
>
>     va_start (va_args, format);
>
>     if (notmuch->status_string)
> 	notmuch->status_string = talloc_vasprintf_append (notmuch->status_string, format, va_args)
>     else
> 	notmuch->status_string = talloc_vasprintf (notmuch, format, va_args);
>     
>     va_end (va_args);
> }

If Someone™ were to publish a changeset with this feature, i'd certainly
make use of it in this series.

thanks for the suggestion, David.

       --dkg

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

Re: [PATCH v4 09/16] index encrypted parts when asked.

[David Bremner]

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

> +    if (status) {
> +	_notmuch_database_log (notmuch, "Warning: setup failed for decrypting "
> +			       "during indexing. (%d)\n", status);
> +	status = notmuch_message_add_property (message, "index-decryption", "failure");
> +	if (status)
> +	    _notmuch_database_log (notmuch, "failed to add index-decryption "
> +				   "property (%d)\n", status);
> +	return;
> +    }

Just as a reminder, _notmuch_database_log_append is now available for
your logging needs. Other than that, this patch looks ok.

[PATCH v4 10/16] Add n_d_add_message_with_indexopts (extension of n_d_add_message)

[Daniel Kahn Gillmor]

Expose the ability to ask for index options via the library interface.
This _add_message_with_indexopts function is now a generalized form of
the older _add_message.  It lets you specify parameters and
configurations that can affect the indexing, like indexing encrypted
messages in the clear should the user choose to do so.

We also adjust the tests so that we test the extended function
returning bad values (since the non-extended function just calls the
extended one).
---
 lib/database.cc     | 20 ++++++++++++++++----
 lib/notmuch.h       | 14 ++++++++++++++
 test/T070-insert.sh |  4 ++--
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 4e01f12..92507b8 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2417,16 +2417,16 @@ _notmuch_database_link_message (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_indexopts (notmuch_database_t *notmuch,
+					     const char *filename,
+					     notmuch_indexopts_t *indexopts,
+					     notmuch_message_t **message_ret)
 {
     notmuch_message_file_t *message_file;
     notmuch_message_t *message = NULL;
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, ret2;
     notmuch_private_status_t private_status;
     notmuch_bool_t is_ghost = false;
-    notmuch_indexopts_t *indexopts = NULL;
 
     const char *date, *header;
     const char *from, *to, *subject;
@@ -2576,6 +2576,18 @@ 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_indexopts (notmuch, filename,
+							NULL,
+							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 b92d969..66b3503 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -605,6 +605,20 @@ 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 using specified
+ * options.
+ *
+ * This does the same thing as notmuch_database_add_message except
+ * that it passes a pre-configured set of indexing options to indicate
+ * how the specific message should be indexed.
+ */
+notmuch_status_t
+notmuch_database_add_message_with_indexopts (notmuch_database_t *database,
+					     const char *filename,
+					     notmuch_indexopts_t *indexopts,
+					     notmuch_message_t **message);
 
 /**
  * Remove a message filename from the given notmuch database. If the
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index e7ec6a6..557f9d5 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -192,14 +192,14 @@ for code in OUT_OF_MEMORY XAPIAN_EXCEPTION FILE_NOT_EMAIL \
 gen_insert_msg
 cat <<EOF > index-file-$code.gdb
 set breakpoint pending on
-break notmuch_database_add_message
+break notmuch_database_add_message_with_indexopts
 commands
 return NOTMUCH_STATUS_$code
 continue
 end
 run
 EOF
-test_begin_subtest "error exit when add_message returns $code"
+test_begin_subtest "error exit when add_message_with_indexopts returns $code"
 gdb --batch-silent --return-child-result -x index-file-$code.gdb \
     --args notmuch insert  < $gen_msg_filename
 test_expect_equal $? 1
-- 
2.8.1

Re: [PATCH v4 10/16] Add n_d_add_message_with_indexopts (extension of n_d_add_message)

[David Bremner]

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

> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index b92d969..66b3503 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -605,6 +605,20 @@ 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 using specified
> + * options.
> + *
> + * This does the same thing as notmuch_database_add_message except
> + * that it passes a pre-configured set of indexing options to indicate
> + * how the specific message should be indexed.
> + */
> +notmuch_status_t
> +notmuch_database_add_message_with_indexopts (notmuch_database_t *database,
> +					     const char *filename,
> +					     notmuch_indexopts_t *indexopts,
> +					     notmuch_message_t **message);
>  

I'm a bit surprised that no new return values are possible / documented
here.

d

[PATCH v4 11/16] 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, and gpg is needed for
indexing, the configuration option will be used to find gpg.
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-insert.rst        | 11 +++++++++++
 notmuch-insert.c                   | 32 +++++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 78047b5..1e4b2cc 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 131f09e..eec0eb5 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -378,12 +378,13 @@ 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_indexopts_t *indexopts)
 {
     notmuch_message_t *message;
     notmuch_status_t status;
 
-    status = notmuch_database_add_message (notmuch, path, &message);
+    status = notmuch_database_add_message_with_indexopts (notmuch, path, indexopts, &message);
     if (status == NOTMUCH_STATUS_SUCCESS) {
 	status = tag_op_list_apply (message, tag_ops, 0);
 	if (status) {
@@ -455,17 +456,20 @@ 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;
     int opt_index;
     unsigned int i;
+    notmuch_indexopts_t *indexopts;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_STRING, &folder, "folder", 0, 0 },
 	{ 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 +549,29 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
+    indexopts = notmuch_indexopts_create ();
+    if (!indexopts) {
+	fprintf (stderr, "Error: could not create index options.\n");
+	return EXIT_FAILURE;
+    }
+    status = notmuch_indexopts_set_try_decrypt (indexopts, try_decrypt);
+    if (status != NOTMUCH_STATUS_SUCCESS) {
+	fprintf (stderr, "Error: Failed to set try_decrypt to %s. (%s)\n",
+		 try_decrypt ? "True" : "False", notmuch_status_to_string (status));
+	notmuch_indexopts_destroy (indexopts);
+	return EXIT_FAILURE;
+    }
+    if (try_decrypt) {
+	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
+	status = notmuch_indexopts_set_gpg_path (indexopts, 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, indexopts);
 
     /* Commit changes. */
     close_status = notmuch_database_destroy (notmuch);
@@ -577,5 +602,6 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	notmuch_run_hook (db_path, "post-insert");
     }
 
+    notmuch_indexopts_destroy (indexopts);
     return status ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.8.1

Re: [PATCH v4 11/16] add --try-decrypt to notmuch insert

[David Bremner]

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

> +    status = notmuch_indexopts_set_try_decrypt (indexopts, try_decrypt);
> +    if (status != NOTMUCH_STATUS_SUCCESS) {
> +	fprintf (stderr, "Error: Failed to set try_decrypt to %s. (%s)\n",
> +		 try_decrypt ? "True" : "False", notmuch_status_to_string (status));
> +	notmuch_indexopts_destroy (indexopts);
> +	return EXIT_FAILURE;
> +    }

This patch looks fine. I wondered whether it might be useful to add a
"print_status_indexopts" function to status.c to reduce some of the
boilerplate. I think the current way is only a bit longer, so probably
not worth the extra obfuscation, but feel free to add such a function if
you like; it's mainly interesting if there is potential secondary info
from notmuch_database_status_string.

d

[PATCH v4 12/16] add --try-decrypt to notmuch new

[Daniel Kahn Gillmor]

Try to decrypt any encrypted parts of newly-discovered messages while
indexing them.  The cleartext of any successfully-decrypted messages
will be indexed, with tags applied in the same form as from notmuch
insert --try-decrypt.

If ~/.notmuch-config contains crypto.gpg_path, and gpg is needed for
indexing, the configuration option will be used to find gpg.
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-new.rst           | 10 ++++++++++
 notmuch-new.c                      | 30 +++++++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 1e4b2cc..a6a5a60 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 c55dea7..e495557 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_indexopts_t *indexopts;
 
     int total_files;
     int processed_files;
@@ -260,7 +261,8 @@ add_file (notmuch_database_t *notmuch, const char *filename,
     if (status)
 	goto DONE;
 
-    status = notmuch_database_add_message (notmuch, filename, &message);
+    status = notmuch_database_add_message_with_indexopts (notmuch, filename,
+							  state->indexopts, &message);
     switch (status) {
     /* Success. */
     case NOTMUCH_STATUS_SUCCESS:
@@ -930,6 +932,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     add_files_state_t add_files_state = {
 	.verbosity = VERBOSITY_NORMAL,
 	.debug = FALSE,
+	.indexopts = NULL,
 	.output_is_a_tty = isatty (fileno (stdout)),
     };
     struct timeval tv_start;
@@ -943,6 +946,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     unsigned int i;
     notmuch_bool_t timer_is_active = FALSE;
     notmuch_bool_t no_hooks = FALSE;
+    notmuch_bool_t try_decrypt = FALSE;
     notmuch_bool_t quiet = FALSE, verbose = FALSE;
     notmuch_status_t status;
 
@@ -951,6 +955,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,  &try_decrypt, "try-decrypt", 0, 0 },
 	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
 	{ 0, 0, 0, 0, 0 }
     };
@@ -1068,6 +1073,28 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     if (notmuch == NULL)
 	return EXIT_FAILURE;
 
+    add_files_state.indexopts = notmuch_indexopts_create ();
+    if (!add_files_state.indexopts) {
+	fprintf (stderr, "Error: could not create index options.\n");
+	return EXIT_FAILURE;
+    }
+    status = notmuch_indexopts_set_try_decrypt (add_files_state.indexopts, try_decrypt);
+    if (status != NOTMUCH_STATUS_SUCCESS) {
+	fprintf (stderr, "Error: Failed to set try_decrypt to %s. (%s)\n",
+		 try_decrypt ? "True" : "False", notmuch_status_to_string (status));
+	notmuch_indexopts_destroy (add_files_state.indexopts);
+	return EXIT_FAILURE;
+    }
+    if (try_decrypt) {
+	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
+	status = notmuch_indexopts_set_gpg_path (add_files_state.indexopts, 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. */
@@ -1151,5 +1178,6 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     if (!no_hooks && !ret && !interrupted)
 	ret = notmuch_run_hook (db_path, "post-new");
 
+    notmuch_indexopts_destroy (add_files_state.indexopts);
     return ret || interrupted ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.8.1

Re: [PATCH v4 12/16] add --try-decrypt to notmuch new

[David Bremner]

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

> Try to decrypt any encrypted parts of newly-discovered messages while
> indexing them.  The cleartext of any successfully-decrypted messages
> will be indexed, with tags applied in the same form as from notmuch
> insert --try-decrypt.

s/tags/properties/?  At this point I'm a bit lost, and not sure where
those properties are documented, but this commit message is a bit
strange since it kindof implies the reader is likely to know what those
are.

> diff --git a/notmuch-new.c b/notmuch-new.c
> index c55dea7..e495557 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_indexopts_t *indexopts;

I'm only reading the patch here, but is this variable actually used?

>      int total_files;
>      int processed_files;
> @@ -260,7 +261,8 @@ add_file (notmuch_database_t *notmuch, const char *filename,
>      if (status)
>  	goto DONE;

[PATCH v4 13/16] add indexopts to notmuch python bindings.

[Daniel Kahn Gillmor]

Make notmuch indexing options are not available in python as
the notmuch.Indexopts class.  Users can do something like:

 import notmuch
 d = notmuch.Database()
 indexopts = notmuch.Indexopts(try_decrypt=true)
 d.add_message(fname, indexopts=indexopts)
---
 bindings/python/notmuch/__init__.py  |  1 +
 bindings/python/notmuch/database.py  | 24 ++++++---
 bindings/python/notmuch/globals.py   |  5 ++
 bindings/python/notmuch/indexopts.py | 97 ++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 7 deletions(-)
 create mode 100644 bindings/python/notmuch/indexopts.py

diff --git a/bindings/python/notmuch/__init__.py b/bindings/python/notmuch/__init__.py
index cf627ff..5c19532 100644
--- a/bindings/python/notmuch/__init__.py
+++ b/bindings/python/notmuch/__init__.py
@@ -54,6 +54,7 @@ Copyright 2010-2011 Sebastian Spaeth <Sebastian@SSpaeth.de>
 from .database import Database
 from .directory import Directory
 from .filenames import Filenames
+from .indexopts import Indexopts
 from .message import Message
 from .messages import Messages
 from .query import Query
diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 67fb1c4..a13b9bb 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -29,6 +29,7 @@ from .globals import (
     NotmuchDirectoryP,
     NotmuchMessageP,
     NotmuchTagsP,
+    NotmuchIndexoptsP,
 )
 from .errors import (
     STATUS,
@@ -383,12 +384,13 @@ class Database(object):
         # return the Directory, init it with the absolute path
         return Directory(abs_dirpath, dir_p, self)
 
-    _add_message = nmlib.notmuch_database_add_message
-    _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_with_indexopts = nmlib.notmuch_database_add_message_with_indexopts
+    _add_message_with_indexopts.argtypes = [NotmuchDatabaseP, c_char_p,
+                                            NotmuchIndexoptsP,
+                                            POINTER(NotmuchMessageP)]
+    _add_message_with_indexopts.restype = c_uint
+        
+    def add_message(self, filename, sync_maildir_flags=False, indexopts=None):
         """Adds a new message to the database
 
         :param filename: should be a path relative to the path of the
@@ -409,6 +411,9 @@ class Database(object):
             API. You might want to look into the underlying method
             :meth:`Message.maildir_flags_to_tags`.
 
+        :param indexopts: a nomtuch.Indexopts object indicating custom
+            options desired for indexing.
+
         :returns: On success, we return
 
            1) a :class:`Message` object that can be used for things
@@ -436,10 +441,15 @@ 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))
+
+        io = None
+        if indexopts is not None:
+            io = indexopts._indexopts
+        status = self._add_message_with_indexopts(self._db, _str(filename), io, byref(msg_p))
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
             raise NotmuchError(status)
diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
index b1eec2c..71426c8 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -88,3 +88,8 @@ NotmuchDirectoryP = POINTER(NotmuchDirectoryS)
 class NotmuchFilenamesS(Structure):
     pass
 NotmuchFilenamesP = POINTER(NotmuchFilenamesS)
+
+
+class NotmuchIndexoptsS(Structure):
+    pass
+NotmuchIndexoptsP = POINTER(NotmuchIndexoptsS)
diff --git a/bindings/python/notmuch/indexopts.py b/bindings/python/notmuch/indexopts.py
new file mode 100644
index 0000000..b0d4603
--- /dev/null
+++ b/bindings/python/notmuch/indexopts.py
@@ -0,0 +1,97 @@
+"""
+This file is part of notmuch.
+
+Notmuch is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by the
+Free Software Foundation, either version 3 of the License, or (at your
+option) any later version.
+
+Notmuch is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with notmuch.  If not, see <http://www.gnu.org/licenses/>.
+
+Copyright 2015 Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+"""
+from ctypes import c_char_p, c_bool, c_int
+from .globals import (
+    nmlib,
+    NotmuchIndexoptsP,
+)
+from .errors import (
+    STATUS,
+    NullPointerError,
+    NotInitializedError,
+)
+
+
+class Indexopts(object):
+    """Represents available options for notmuch indexing.
+    """
+
+    # create
+    _create = nmlib.notmuch_indexopts_create
+    _create.argtypes = []
+    _create.restype = NotmuchIndexoptsP
+
+    def __init__(self, try_decrypt=False, gpg_path=None):
+        """
+        :param try_decrypt: True if notmuch should try to decrypt messages
+             while indexing, and index the cleartext.
+
+        :param gpg_path: the name or path to the preferred GnuPG binary.
+        """
+        self._indexopts = Indexopts._create()
+        self.gpg_path = gpg_path
+        self.try_decrypt = try_decrypt
+
+    # try_decrypt
+    _get_try_decrypt = nmlib.notmuch_indexopts_get_try_decrypt
+    _get_try_decrypt.argtypes = [NotmuchIndexoptsP]
+    _get_try_decrypt.restype = bool
+
+    _set_try_decrypt = nmlib.notmuch_indexopts_set_try_decrypt
+    _set_try_decrypt.argtypes = [NotmuchIndexoptsP, c_bool]
+    _set_try_decrypt.restype = c_int
+
+    @property
+    def try_decrypt(self):
+        return Indexopts._get_try_decrypt(self._indexopts)
+
+    @try_decrypt.setter
+    def try_decrypt(self, try_decrypt):
+        status = Indexopts._set_try_decrypt(self._indexopts, try_decrypt)
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+
+    # gpg_path
+    _get_gpg_path = nmlib.notmuch_indexopts_get_gpg_path
+    _get_gpg_path.argtypes = [NotmuchIndexoptsP]
+    _get_gpg_path.restype = c_char_p
+
+    _set_gpg_path = nmlib.notmuch_indexopts_set_gpg_path
+    _set_gpg_path.argtypes = [NotmuchIndexoptsP, c_char_p]
+    _set_gpg_path.restype = c_int
+
+    @property
+    def gpg_path(self):
+        return Indexopts._get_gpg_path(self._indexopts)
+
+    @gpg_path.setter
+    def gpg_path(self, gpg_path):
+        status = Indexopts._set_gpg_path(self._indexopts, gpg_path)
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+    
+
+    _destroy = nmlib.notmuch_indexopts_destroy
+    _destroy.argtypes = [NotmuchIndexoptsP]
+    _destroy.restype = None
+
+    def __del__(self):
+        """Close and free the indexopts"""
+        if self._indexopts:
+            self._destroy(self._indexopts)
-- 
2.8.1

Re: [PATCH v4 13/16] add indexopts to notmuch python bindings.

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>  
> +        :param indexopts: a nomtuch.Indexopts object indicating custom
> +            options desired for indexing.
> +

typo

>                        be added.
> +
spurious whitespace change?

otherwise the patch looks fine.

d

[PATCH v4 14/16] test indexing cleartext version of delivered messages.

[Daniel Kahn Gillmor]

This requires a bit of reorganization:

 * add_gnupg_home gets moved to test-lib.sh, and

 * we allow passing --long-arguments to "notmuch new" via
   emacs_fcc_message
---
 test/T350-crypto.sh           | 15 -------------
 test/T357-index-decryption.sh | 49 +++++++++++++++++++++++++++++++++++++++++++
 test/test-lib.sh              | 26 ++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 16 deletions(-)
 create mode 100755 test/T357-index-decryption.sh

diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 4bc15bc..50cc526 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -7,21 +7,6 @@
 test_description='PGP/MIME signature verification and decryption'
 . ./test-lib.sh || exit 1
 
-add_gnupg_home ()
-{
-    local output
-    [ -d ${GNUPGHOME} ] && return
-    mkdir -m 0700 "$GNUPGHOME"
-    $GPG --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc >"$GNUPGHOME"/import.log 2>&1
-    test_debug "cat $GNUPGHOME/import.log"
-    if ($GPG --quick-random --version >/dev/null 2>&1) ; then
-	echo quick-random >> "$GNUPGHOME"/gpg.conf
-    elif ($GPG --debug-quick-random --version >/dev/null 2>&1) ; then
-	echo debug-quick-random >> "$GNUPGHOME"/gpg.conf
-    fi
-    echo no-emit-version >> "$GNUPGHOME"/gpg.conf
-}
-
 ##################################################
 
 add_gnupg_home
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
new file mode 100755
index 0000000..27d6885
--- /dev/null
+++ b/test/T357-index-decryption.sh
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+
+# TODO: test index-decryption-failed
+
+test_description='indexing decrypted mail'
+. ./test-lib.sh || exit 1
+
+##################################################
+
+add_gnupg_home
+# get key fingerprint
+FINGERPRINT=$($GPG --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10)
+
+# create a test encrypted message
+test_expect_success 'emacs delivery of encrypted message' \
+'emacs_fcc_message \
+    "test encrypted message for cleartext index 001" \
+    "This is a test encrypted message with a wumpus.\n" \
+    "(mml-secure-message-encrypt)"'
+
+test_begin_subtest "search for unindexed cleartext"
+output=$(notmuch search wumpus)
+expected=''
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+# create a test encrypted message that is indexed in the clear
+test_expect_success 'emacs delivery of encrypted message' \
+'emacs_fcc_message --try-decrypt \
+    "test encrypted message for cleartext index 002" \
+    "This is a test encrypted message with a wumpus.\n" \
+    "(mml-secure-message-encrypt)"'
+
+test_begin_subtest "emacs delivery of encrypted message, indexed cleartext"
+output=$(notmuch search wumpus)
+expected='thread:0000000000000002   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (encrypted inbox)'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+# and the same search, but by property ($expected is untouched):
+test_begin_subtest "emacs search by property for one message"
+output=$(notmuch search has:index-decryption=success)
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 5c14d1e..cb12ed9 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -93,6 +93,21 @@ else
     GPG=gpg
 fi
 
+add_gnupg_home ()
+{
+    local output
+    [ -d ${GNUPGHOME} ] && return
+    mkdir -m 0700 "$GNUPGHOME"
+    $GPG --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc >"$GNUPGHOME"/import.log 2>&1
+    test_debug "cat $GNUPGHOME/import.log"
+    if ($GPG --quick-random --version >/dev/null 2>&1) ; then
+	echo quick-random >> "$GNUPGHOME"/gpg.conf
+    elif ($GPG --debug-quick-random --version >/dev/null 2>&1) ; then
+	echo debug-quick-random >> "$GNUPGHOME"/gpg.conf
+    fi
+    echo no-emit-version >> "$GNUPGHOME"/gpg.conf
+}
+
 # Convenience
 #
 # A regexp to match 5 and 40 hexdigits
@@ -525,8 +540,17 @@ emacs_deliver_message ()
 # Accepts arbitrary extra emacs/elisp functions to modify the message
 # before sending, which is useful to doing things like attaching files
 # to the message and encrypting/signing.
+#
+# If any GNU-style long-arguments (like --quiet or --try-decrypt) are
+# at the head of the argument list, they are sent directly to "notmuch
+# new" after message delivery
 emacs_fcc_message ()
 {
+    local nmn_args=''
+    while [[ "$1" =~ ^-- ]]; do
+        nmn_args="$nmn_args $1"
+        shift
+    done
     local subject="$1"
     local body="$2"
     shift 2
@@ -545,7 +569,7 @@ emacs_fcc_message ()
 	   (insert \"${body}\")
 	   $@
 	   (notmuch-mua-send-and-exit))" || return 1
-    notmuch new >/dev/null
+    notmuch new $nmn_args >/dev/null
 }
 
 # Generate a corpus of email and add it to the database.
-- 
2.8.1

Re: [PATCH v4 14/16] test indexing cleartext version of delivered messages.

[David Bremner]

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

> This requires a bit of reorganization:
>
>  * add_gnupg_home gets moved to test-lib.sh, and
>
>  * we allow passing --long-arguments to "notmuch new" via
>    emacs_fcc_message

LGTM

d

[PATCH v4 15/16] added notmuch_message_reindex

[Daniel Kahn Gillmor]

This new function asks the database to reindex a given message, using
the supplied indexopts.

This can be used, for example, to index the cleartext of an encrypted
message.

My initial inclination for this implementation was to remove all the
indexed terms for a given message's body, and then to add them back
in.

Unfortunately, that doesn't appear to be possible due to the way we're
using xapian.  I could find no way to distinguish terms which were
added during indexing of the message body from other terms associated
with the document.  As a result, we just save the tags and properties,
remove the message from the database entirely, and add it back into
the database in full, re-adding tags and properties as needed.
---
 lib/message.cc | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/notmuch.h  |  14 ++++++++
 2 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/lib/message.cc b/lib/message.cc
index 9d3e807..ab807b7 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -557,7 +557,9 @@ void
 _notmuch_message_remove_terms (notmuch_message_t *message, const char *prefix)
 {
     Xapian::TermIterator i;
-    size_t prefix_len = strlen (prefix);
+    size_t prefix_len = 0;
+
+    prefix_len = strlen (prefix);
 
     while (1) {
 	i = message->doc.termlist_begin ();
@@ -1847,3 +1849,107 @@ _notmuch_message_frozen (notmuch_message_t *message)
 {
     return message->frozen;
 }
+
+notmuch_status_t
+notmuch_message_reindex (notmuch_message_t *message,
+			 notmuch_indexopts_t *indexopts)
+{
+    notmuch_database_t *notmuch = NULL;
+    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, status;
+    notmuch_tags_t *tags = NULL;
+    notmuch_message_properties_t *properties = NULL;
+    notmuch_filenames_t *filenames, *orig_filenames = NULL;
+    const char *filename = NULL, *tag = NULL, *propkey = NULL;
+    notmuch_message_t *newmsg = NULL;
+    notmuch_bool_t readded = FALSE, skip;
+    const char *autotags[] = {
+		    "attachment",
+		    "encrypted",
+		    "signed" };
+    const char *autoproperties[] = { "index-decryption" };
+
+    if (message == NULL)
+	return NOTMUCH_STATUS_NULL_POINTER;
+    
+    notmuch = _notmuch_message_database (message);
+
+    /* cache tags, properties, and filenames */
+    tags = notmuch_message_get_tags (message);
+    properties = notmuch_message_get_properties (message, "", FALSE);
+    filenames = notmuch_message_get_filenames (message);
+    orig_filenames = notmuch_message_get_filenames (message);
+    
+    /* walk through filenames, removing them until the message is gone */
+    for ( ; notmuch_filenames_valid (filenames);
+	  notmuch_filenames_move_to_next (filenames)) {
+	filename = notmuch_filenames_get (filenames);
+
+	ret = notmuch_database_remove_message (notmuch, filename);
+	if (ret != NOTMUCH_STATUS_SUCCESS &&
+	    ret != NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID)
+	    return ret;
+    }
+    if (ret != NOTMUCH_STATUS_SUCCESS)
+	return ret;
+    
+    /* re-add the filenames with the associated indexopts */
+    for (; notmuch_filenames_valid (orig_filenames);
+	 notmuch_filenames_move_to_next (orig_filenames)) {
+	filename = notmuch_filenames_get (orig_filenames);
+
+	status = notmuch_database_add_message_with_indexopts(notmuch,
+							     filename,
+							     indexopts,
+							     readded ? NULL : &newmsg);
+	if (status == NOTMUCH_STATUS_SUCCESS ||
+	    status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
+	    if (!readded) {
+		/* re-add tags */
+		for (; notmuch_tags_valid (tags);
+		     notmuch_tags_move_to_next (tags)) {
+		    tag = notmuch_tags_get (tags);
+		    skip = FALSE;
+		    
+		    for (size_t i = 0; i < ARRAY_SIZE (autotags); i++)
+			if (strcmp (tag, autotags[i]) == 0)
+			    skip = TRUE;
+		    
+		    if (!skip) {
+			status = notmuch_message_add_tag (newmsg, tag);
+			if (status != NOTMUCH_STATUS_SUCCESS)
+			    ret = status;
+		    }
+		}
+		/* re-add properties */
+		for (; notmuch_message_properties_valid (properties);
+		     notmuch_message_properties_move_to_next (properties)) {
+		    propkey = notmuch_message_properties_key (properties);
+		    skip = FALSE;
+
+		    for (size_t i = 0; i < ARRAY_SIZE (autoproperties); i++)
+			if (strcmp (propkey, autoproperties[i]) == 0)
+			    skip = TRUE;
+
+		    if (!skip) {
+			status = notmuch_message_add_property (newmsg, propkey,
+							       notmuch_message_properties_value (properties));
+			if (status != NOTMUCH_STATUS_SUCCESS)
+			    ret = status;
+		    }
+		}
+		readded = TRUE;
+	    }
+	} else {
+	    /* if we failed to add this filename, go ahead and try the
+	     * next one as though it were first, but report the
+	     * error... */
+	    ret = status;
+	}
+    }
+    if (newmsg)
+	notmuch_message_destroy (newmsg);
+	    		
+    /* should we also destroy the incoming message object?  at the
+     * moment, we leave that to the caller */
+    return ret;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 66b3503..9076a9b 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1394,6 +1394,20 @@ notmuch_filenames_t *
 notmuch_message_get_filenames (notmuch_message_t *message);
 
 /**
+ * Re-index the e-mail corresponding to 'message' using the supplied index options
+ *
+ * Returns the status of the re-index operation.  (see the return
+ * codes documented in notmuch_database_add_message)
+ *
+ * After reindexing, the user should discard the message object passed
+ * in here by calling notmuch_message_destroy, since it refers to the
+ * original message, not to the reindexed message.
+ */
+notmuch_status_t
+notmuch_message_reindex (notmuch_message_t *message,
+			 notmuch_indexopts_t *indexopts);
+
+/**
  * Message flags.
  */
 typedef enum _notmuch_message_flag {
-- 
2.8.1

[PATCH] WIP: remove all non-prefixed-terms (and stemmed versions)

[David Bremner]

The testing here is not really suitable for production, since we export
a function just for testing.  It would be possible to modify the test
framework to test functions in notmuch-private.h, but this was the quick
and dirty solution.
---

dkg wrote:

> I could find no way to distinguish terms which were added during
>  indexing of the message body from other terms associated with the
>  document.

I think this does the trick. If it makes sense, I can polish it
up. I'd appreciate any ideas about the right way to manage the
testing.  We could either modify the test framework to test internal
functions, or continue on testing only exported functions and the CLI.

 lib/message.cc             | 33 ++++++++++++++++++++++
 lib/notmuch-private.h      |  2 ++
 lib/notmuch.h              |  4 +++
 test/T650-message-terms.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)
 create mode 100755 test/T650-message-terms.sh

diff --git a/lib/message.cc b/lib/message.cc
index 9d3e807..9a9845a 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -577,6 +577,39 @@ _notmuch_message_remove_terms (notmuch_message_t *message, const char *prefix)
     }
 }
 
+void notmuch_test_clear_terms(notmuch_message_t *message) {
+    _notmuch_message_remove_unprefixed_terms (message);
+    _notmuch_message_sync (message);
+}
+void
+_notmuch_message_remove_unprefixed_terms (notmuch_message_t *message)
+{
+    Xapian::TermIterator i;
+
+    for (i = message->doc.termlist_begin ();
+	 i != message->doc.termlist_end () &&
+	     ((*i).c_str ()[0] < 'A');
+	     i++) {
+	try {
+	    message->doc.remove_term ((*i));
+	    message->modified = TRUE;
+	} catch (const Xapian::InvalidArgumentError) {
+	    /* Ignore failure to remove non-existent term. */
+	}
+    }
+
+    /* We want to remove stemmed terms, but only those not from a
+       prefixed term */
+    for (i.skip_to ("Z["); i != message->doc.termlist_end (); i++) {
+	try {
+	    message->doc.remove_term ((*i));
+	    message->modified = TRUE;
+	} catch (const Xapian::InvalidArgumentError) {
+	    /* Ignore failure to remove non-existent term. */
+	}
+    }
+}
+
 /* Return true if p points at "new" or "cur". */
 static bool is_maildir (const char *p)
 {
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 65f7ead..646fc78 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -502,6 +502,8 @@ _notmuch_message_add_reply (notmuch_message_t *message,
 notmuch_database_t *
 _notmuch_message_database (notmuch_message_t *message);
 
+void
+_notmuch_message_remove_unprefixed_terms (notmuch_message_t *message);
 /* sha1.c */
 
 char *
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e03a05d..e964b1a 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1658,6 +1658,10 @@ notmuch_message_thaw (notmuch_message_t *message);
 void
 notmuch_message_destroy (notmuch_message_t *message);
 
+/* for testing */
+
+void
+notmuch_test_clear_terms(notmuch_message_t *message);
 /**
  * @name Message Properties
  *
diff --git a/test/T650-message-terms.sh b/test/T650-message-terms.sh
new file mode 100755
index 0000000..553e95b
--- /dev/null
+++ b/test/T650-message-terms.sh
@@ -0,0 +1,70 @@
+#!/usr/bin/env bash
+test_description="message API"
+
+. ./test-lib.sh || exit 1
+
+add_email_corpus
+
+cat <<EOF > c_head
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <talloc.h>
+#include <notmuch-test.h>
+
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_message_t *message = NULL;
+   const char *val;
+   notmuch_status_t stat;
+
+   EXPECT0(notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db));
+   EXPECT0(notmuch_database_find_message(db, "4EFC743A.3060609@april.org", &message));
+   if (message == NULL) {
+	fprintf (stderr, "unable to find message");
+	exit (1);
+   }
+EOF
+
+cat <<EOF > c_tail
+   EXPECT0(notmuch_database_destroy(db));
+}
+EOF
+
+add_email_corpus
+
+test_begin_subtest "check unique term"
+byid=$(notmuch count id:4EFC743A.3060609@april.org)
+byterm=$(notmuch count Boulogne)
+test_expect_equal "$byid" "$byterm"
+
+xapian-delve -1 -a ${MAIL_DIR}/.notmuch/xapian > BEFORE
+
+test_begin_subtest "clear non-prefixed terms from message"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+notmuch_test_clear_terms(message);
+}
+EOF
+byterm=$(notmuch count Boulogne)
+test_expect_equal 0 "$byterm"
+
+test_begin_subtest "check removed terms"
+xapian-delve -1 -a ${MAIL_DIR}/.notmuch/xapian > AFTER
+comm -2 -3 BEFORE AFTER | egrep '^Z?a' > REMOVED
+cat <<EOF > EXPECTED
+Zallan
+Zarch
+Zarch_packaging_standard
+Zarchlinux
+Zaur
+allan
+arch
+arch_packaging_standards
+archlinux
+aur
+EOF
+test_expect_equal_file EXPECTED REMOVED
+
+test_done
-- 
2.8.1

Re: [PATCH] WIP: remove all non-prefixed-terms (and stemmed versions)

[David Bremner]

David Bremner <david@tethera.net> writes:

> The testing here is not really suitable for production, since we export
> a function just for testing.  It would be possible to modify the test
> framework to test functions in notmuch-private.h, but this was the quick
> and dirty solution.

On looking at the problem a second time I think this should really drop
all of the non-(tag|property) terms, so including some prefixed terms as
well. I think that would be doable; it's probably worth having some
performance benchmark before introducing the extra complication versus
dkg's approach.

d

[PATCH v4 16/16] add "notmuch reindex" subcommand

[Daniel Kahn Gillmor]

This new subcommand takes a set of search terms, and re-indexes the
list of matching messages using the supplied options.

This can be used to index the cleartext of encrypted messages with
something like:

 notmuch reindex --try-decrypt \
    tag:encrypted and not has:index-decryption=success
---
 Makefile.local                    |   1 +
 doc/conf.py                       |   7 ++
 doc/index.rst                     |   1 +
 doc/man1/notmuch-reindex.rst      |  41 ++++++++++
 doc/man1/notmuch.rst              |   1 +
 doc/man7/notmuch-search-terms.rst |   7 +-
 notmuch-client.h                  |   3 +
 notmuch-reindex.c                 | 152 ++++++++++++++++++++++++++++++++++++++
 notmuch.c                         |   2 +
 test/T357-index-decryption.sh     |  67 +++++++++++++++++
 10 files changed, 280 insertions(+), 2 deletions(-)
 create mode 100644 doc/man1/notmuch-reindex.rst
 create mode 100644 notmuch-reindex.c

diff --git a/Makefile.local b/Makefile.local
index c13ba76..a916bd0 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -281,6 +281,7 @@ notmuch_client_srcs =		\
 	notmuch-dump.c		\
 	notmuch-insert.c	\
 	notmuch-new.c		\
+	notmuch-reindex.c       \
 	notmuch-reply.c		\
 	notmuch-restore.c	\
 	notmuch-search.c	\
diff --git a/doc/conf.py b/doc/conf.py
index 8b93296..8455a51 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -94,6 +94,10 @@ man_pages = [
         u'incorporate new mail into the notmuch database',
         [u'Carl Worth and many others'], 1),
 
+('man1/notmuch-reindex','notmuch-reindex',
+        u're-index matching messages',
+        [u'Carl Worth and many others'], 1),
+
 ('man1/notmuch-reply','notmuch-reply',
         u'constructs a reply template for a set of messages',
         [u'Carl Worth and many others'], 1),
@@ -162,6 +166,9 @@ texinfo_documents = [
 ('man1/notmuch-new','notmuch-new',u'notmuch Documentation',
       u'Carl Worth and many others', 'notmuch-new',
       'incorporate new mail into the notmuch database','Miscellaneous'),
+('man1/notmuch-reindex','notmuch-reindex',u'notmuch Documentation',
+      u'Carl Worth and many others', 'notmuch-reindex',
+      're-index matching messages','Miscellaneous'),
 ('man1/notmuch-reply','notmuch-reply',u'notmuch Documentation',
       u'Carl Worth and many others', 'notmuch-reply',
       'constructs a reply template for a set of messages','Miscellaneous'),
diff --git a/doc/index.rst b/doc/index.rst
index 344606d..aa6c9f4 100644
--- a/doc/index.rst
+++ b/doc/index.rst
@@ -18,6 +18,7 @@ Contents:
    man5/notmuch-hooks
    man1/notmuch-insert
    man1/notmuch-new
+   man1/notmuch-reindex
    man1/notmuch-reply
    man1/notmuch-restore
    man1/notmuch-search
diff --git a/doc/man1/notmuch-reindex.rst b/doc/man1/notmuch-reindex.rst
new file mode 100644
index 0000000..7ccc947
--- /dev/null
+++ b/doc/man1/notmuch-reindex.rst
@@ -0,0 +1,41 @@
+===========
+notmuch-reindex
+===========
+
+SYNOPSIS
+========
+
+**notmuch** **reindex** [*option* ...] <*search-term*> ...
+
+DESCRIPTION
+===========
+
+Re-index all messages matching the search terms.
+
+See **notmuch-search-terms(7)** for details of the supported syntax for
+<*search-term*\ >.
+
+The **reindex** command searches for all messages matching the
+supplied search terms, and re-creates the full-text index on these
+messages using the supplied options.
+
+Supported options for **reindex** include
+
+    ``--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
+========
+
+**notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
+**notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
+**notmuch-new(1)**,
+**notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
+**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst
index edd04ef..e6a14f6 100644
--- a/doc/man1/notmuch.rst
+++ b/doc/man1/notmuch.rst
@@ -140,6 +140,7 @@ SEE ALSO
 
 **notmuch-config(1)**, **notmuch-count(1)**, **notmuch-dump(1)**,
 **notmuch-hooks(5)**, **notmuch-insert(1)**, **notmuch-new(1)**,
+**notmuch-reindex(1)**,
 **notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
 **notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
 **notmuch-address(1)**
diff --git a/doc/man7/notmuch-search-terms.rst b/doc/man7/notmuch-search-terms.rst
index 075f88c..e5ea899 100644
--- a/doc/man7/notmuch-search-terms.rst
+++ b/doc/man7/notmuch-search-terms.rst
@@ -9,6 +9,8 @@ SYNOPSIS
 
 **notmuch** **dump** [--format=(batch-tag|sup)] [--] [--output=<*file*>] [--] [<*search-term*> ...]
 
+**notmuch** **reindex** [option ...] <*search-term*> ...
+
 **notmuch** **search** [option ...] <*search-term*> ...
 
 **notmuch** **show** [option ...] <*search-term*> ...
@@ -395,5 +397,6 @@ SEE ALSO
 
 **notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
 **notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
-**notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
-**notmuch-search(1)**, **notmuch-show(1)**, **notmuch-tag(1)**
+**notmuch-new(1)**, **notmuch-reindex(1)**, **notmuch-reply(1)**,
+**notmuch-restore(1)**, **notmuch-search(1)**, **notmuch-show(1)**,
+**notmuch-tag(1)**
diff --git a/notmuch-client.h b/notmuch-client.h
index a65bb6d..a32ef7a 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -169,6 +169,9 @@ int
 notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
+notmuch_reindex_command (notmuch_config_t *config, int argc, char *argv[]);
+
+int
 notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
diff --git a/notmuch-reindex.c b/notmuch-reindex.c
new file mode 100644
index 0000000..6fc88c5
--- /dev/null
+++ b/notmuch-reindex.c
@@ -0,0 +1,152 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2016 Daniel Kahn Gillmor
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
+ */
+
+#include "notmuch-client.h"
+#include "string-util.h"
+
+static volatile sig_atomic_t interrupted;
+
+static void
+handle_sigint (unused (int sig))
+{
+    static char msg[] = "Stopping...         \n";
+
+    /* This write is "opportunistic", so it's okay to ignore the
+     * result.  It is not required for correctness, and if it does
+     * fail or produce a short write, we want to get out of the signal
+     * handler as quickly as possible, not retry it. */
+    IGNORE_RESULT (write (2, msg, sizeof (msg) - 1));
+    interrupted = 1;
+}
+
+/* reindex all messages matching 'query_string' using the passed-in indexopts
+ */
+static int
+reindex_query (notmuch_database_t *notmuch, const char *query_string,
+	       notmuch_indexopts_t *indexopts)
+{
+    notmuch_query_t *query;
+    notmuch_messages_t *messages;
+    notmuch_message_t *message;
+    notmuch_status_t status;
+
+    int ret = NOTMUCH_STATUS_SUCCESS;
+
+    query = notmuch_query_create (notmuch, query_string);
+    if (query == NULL) {
+	fprintf (stderr, "Out of memory.\n");
+	return 1;
+    }
+
+    /* reindexing is not interested in any special sort order */
+    notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
+
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch reindex", query, status))
+	return status;
+
+    for (;
+	 notmuch_messages_valid (messages) && ! interrupted;
+	 notmuch_messages_move_to_next (messages)) {
+	message = notmuch_messages_get (messages);
+
+	notmuch_message_reindex(message, indexopts);
+	notmuch_message_destroy (message);
+	if (ret != NOTMUCH_STATUS_SUCCESS)
+	    break;
+    }
+
+    notmuch_query_destroy (query);
+
+    return ret || interrupted;
+}
+
+int
+notmuch_reindex_command (notmuch_config_t *config, int argc, char *argv[])
+{
+    char *query_string = NULL;
+    notmuch_database_t *notmuch;
+    struct sigaction action;
+    notmuch_bool_t try_decrypt = FALSE;
+    int opt_index;
+    int ret;
+    notmuch_status_t status;
+    notmuch_indexopts_t *indexopts = NULL;
+
+    /* Set up our handler for SIGINT */
+    memset (&action, 0, sizeof (struct sigaction));
+    action.sa_handler = handle_sigint;
+    sigemptyset (&action.sa_mask);
+    action.sa_flags = SA_RESTART;
+    sigaction (SIGINT, &action, NULL);
+
+    notmuch_opt_desc_t options[] = {
+	{ NOTMUCH_OPT_BOOLEAN, &try_decrypt, "try-decrypt", 0, 0 },
+	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
+	{ 0, 0, 0, 0, 0 }
+    };
+
+    opt_index = parse_arguments (argc, argv, options, 1);
+    if (opt_index < 0)
+	return EXIT_FAILURE;
+
+    notmuch_process_shared_options (argv[0]);
+
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
+	return EXIT_FAILURE;
+
+    notmuch_exit_if_unmatched_db_uuid (notmuch);
+
+    indexopts = notmuch_indexopts_create();
+    if (!indexopts)
+	return EXIT_FAILURE;
+
+    status = notmuch_indexopts_set_try_decrypt (indexopts, try_decrypt);
+    if (status)
+	fprintf (stderr, "Warning: failed to set --try-decrypt to %d (%s)\n",
+		 try_decrypt, notmuch_status_to_string (status));
+
+    if (try_decrypt) {
+	const char* gpg_path = notmuch_config_get_crypto_gpg_path (config);
+	status = notmuch_indexopts_set_gpg_path (indexopts, gpg_path);
+	if (status)
+	    fprintf (stderr, "Warning: failed to set gpg_path for reindexing to '%s' (%s)\n",
+		     gpg_path ? gpg_path : "(NULL)",
+		     notmuch_status_to_string (status));
+    }
+
+    query_string = query_string_from_args (config, argc-opt_index, argv+opt_index);
+    if (query_string == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return EXIT_FAILURE;
+    }
+
+    if (*query_string == '\0') {
+	fprintf (stderr, "Error: notmuch reindex requires at least one search term.\n");
+	return EXIT_FAILURE;
+    }
+    
+    ret = reindex_query (notmuch, query_string, indexopts);
+
+    notmuch_database_destroy (notmuch);
+
+    return ret || interrupted ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/notmuch.c b/notmuch.c
index 38b73c1..5aadea5 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -123,6 +123,8 @@ static command_t commands[] = {
       "Restore the tags from the given dump file (see 'dump')." },
     { "compact", notmuch_compact_command, FALSE,
       "Compact the notmuch database." },
+    { "reindex", notmuch_reindex_command, FALSE,
+      "Re-index all messages matching the search terms." },
     { "config", notmuch_config_command, FALSE,
       "Get or set settings in the notmuch configuration file." },
     { "help", notmuch_help_command, TRUE, /* create but don't save config */
diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 27d6885..9510c57 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -46,4 +46,71 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+# add a tag to all messages to ensure that it stays after reindexing
+test_expect_success 'tagging all messages' \
+                    'notmuch tag +blarney "encrypted message"'
+test_begin_subtest "verify that tags have not changed"
+output=$(notmuch search tag:blarney)
+expected='thread:0000000000000001   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)
+thread:0000000000000002   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox)'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+# see if first message shows up after reindexing with --try-decrypt
+test_expect_success 'reindex old messages' \
+                    'notmuch reindex --try-decrypt tag:encrypted and not has:index-decryption=success'
+test_begin_subtest "reindexed encrypted message, including cleartext"
+output=$(notmuch search wumpus)
+expected='thread:0000000000000002   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox)
+thread:0000000000000003   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+# and the same search, but by property ($expected is untouched):
+test_begin_subtest "emacs search by property for both messages"
+output=$(notmuch search has:index-decryption=success)
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+
+# try to remove cleartext indexing
+test_expect_success 'reindex without cleartext' \
+                    'notmuch reindex tag:encrypted and has:index-decryption=success'
+test_begin_subtest "reindexed encrypted messages, without cleartext"
+output=$(notmuch search wumpus)
+expected=''
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+# and the same search, but by property ($expected is untouched):
+test_begin_subtest "emacs search by property with both messages unindexed"
+output=$(notmuch search has:index-decryption=success)
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+# ensure that the tags remain even when we are dropping the cleartext.
+test_begin_subtest "verify that tags remain without cleartext"
+output=$(notmuch search tag:blarney)
+expected='thread:0000000000000004   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 002 (blarney encrypted inbox)
+thread:0000000000000005   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message for cleartext index 001 (blarney encrypted inbox)'
+test_expect_equal \
+    "$output" \
+    "$expected"
+
+
+# TODO: test removal of a message from the message store between
+# indexing and reindexing.
+
+# TODO: insert the same message into the message store twice, index,
+# remove one of them from the message store, and then reindex.
+# reindexing should return a failure but the message should still be
+# present? -- or what should the semantics be if you ask to reindex a
+# message whose underlying files have been renamed or moved or
+# removed?
+
 test_done
-- 
2.8.1

Re: [PATCH v4 16/16] add "notmuch reindex" subcommand

[David Bremner]

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


> +Supported options for **reindex** include
> +
> +    ``--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.

What can we say about re-indexing without the flag, when the user has
previously indexed cleartext? I guess this is at least partly a question
for Olly: if we delete terms from a xapian document, how recoverable are
those terms and  positions? I suppose it might depend on backend, but
does deleting terms provide at least same level of security as deleting
files in modern file systems (i.e. not much against determined state
level actors, but good enough to defeat most older brothers)

> +# TODO: test removal of a message from the message store between
> +# indexing and reindexing.
> +
> +# TODO: insert the same message into the message store twice, index,
> +# remove one of them from the message store, and then reindex.
> +# reindexing should return a failure but the message should still be
> +# present? -- or what should the semantics be if you ask to reindex a
> +# message whose underlying files have been renamed or moved or
> +# removed?

These tests don't seem hard to impliment, just a bit of drudge work?

TBH I'd have to read source to figure out the degree of robustness
promised by e.g. show/search for renames (without intervening new).
There is some argument for reindexing by path as being a useful use
case, if it could handle renames.

In any case, it would be nice (TM) to document what the current
behaviour is for users.

d

Re: [PATCH v4 16/16] add "notmuch reindex" subcommand

[Olly Betts]

On Mon, Aug 15, 2016 at 07:42:39AM +0900, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
> > +Supported options for **reindex** include
> > +
> > +    ``--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.
> 
> What can we say about re-indexing without the flag, when the user has
> previously indexed cleartext? I guess this is at least partly a question
> for Olly: if we delete terms from a xapian document, how recoverable are
> those terms and  positions? I suppose it might depend on backend, but
> does deleting terms provide at least same level of security as deleting
> files in modern file systems

That seems a fair assessment.  Probably the main extra security you'd
get is that there are less likely to be existing tools to get at the
data, and that it's spread over more places so it's harder to locate it
all so you can reconstruct the plain text (whereas if a deleted file
contained the plain text, it would be fairly easy to locate if you can
guess part of it, or at least write a bit of code to recognise likely
candidates).

> (i.e. not much against determined state level actors, but good enough
> to defeat most older brothers)

"Good enough against big brother, but not Big Brother"

Cheers,
    Olly