[PATCH v3 0/6] cli: improved crypto internals

[PATCH v3 0/6] cli: improved crypto internals

[Jameson Graef Rollins]

Third version of this series [0].  I've made some changes in this
version:

* added a new crypto.c that holds two functions: one to return a
  context for a specified crypto protocol (and initialize it if
  needed), and another to cleanup a crypto struct.  This is introduced
  in the first patch, and makes the rest of the patches a little
  cleaner.

* Based on Jani's and Daniel's suggestion, I decided to leave the
  notmuch_show_params_t in notmuch-reply.c, since we might need it
  later and it's less changes here.

Otherwise everything else is the same.  I think the version is nicer
that the previous ones.

FWIW I also now have S/MIME support working based on this patch
series.  Daniel has kindly agreed to put together some tests for that,
so once this series is accepted and Daniel's tests are sent, I'll send
on the rest of the S/MIME support (which is actually just one small
patch on top of this series).

jamie.

[0] id:"1337205359-2444-1-git-send-email-jrollins@finestructure.net"
    id:"1337362357-31281-1-git-send-email-jrollins@finestructure.net"

[PATCH v3 1/6] cli: new crypto structure to store crypto contexts and parameters, and functions to support it

[Jameson Graef Rollins]

This new structure, notmuch_crypto_t, keeps all relevant crypto
contexts and parameters together, and will make it easier to pass the
stuff around and clean it up.  The name of the crypto context inside
this new struct will change, to reflect that it is actually a GPG
context, which is a sub type of Crypto context.  There are other types
of Crypto contexts (Pkcs7 in particular, which we hope to support) so
we want to be clear.

The new crypto.c contains functions to return the proper context from
the struct for a given protocol (and initialize it if needed), and to
cleanup a struct by releasing the crypto contexts.
---
 Makefile.local   |    1 +
 crypto.c         |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-client.h |   15 +++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 crypto.c

diff --git a/Makefile.local b/Makefile.local
index 53b4a0d..a890df2 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -292,6 +292,7 @@ notmuch_client_srcs =		\
 	notmuch-time.c		\
 	query-string.c		\
 	mime-node.c		\
+	crypto.c		\
 	json.c
 
 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
diff --git a/crypto.c b/crypto.c
new file mode 100644
index 0000000..c346999
--- /dev/null
+++ b/crypto.c
@@ -0,0 +1,65 @@
+/* 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 http://www.gnu.org/licenses/ .
+ *
+ * Authors: Jameson Rollins <jrollins@finestructure.net>
+ */
+
+#include "notmuch-client.h"
+
+/* for the specified protocol return the context pointer (initializing
+ * if needed) */
+GMimeCryptoContext *
+notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol)
+{
+    GMimeCryptoContext *cryptoctx = NULL;
+
+    if ((strcmp (protocol, "application/pgp-signature") == 0)
+	|| (strcmp (protocol, "application/pgp-encrypted") == 0)) {
+	if (!crypto->gpgctx) {
+#ifdef GMIME_ATLEAST_26
+	    /* TODO: GMimePasswordRequestFunc */
+	    crypto->gpgctx = g_mime_gpg_context_new (NULL, "gpg");
+#else
+	    GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
+	    crypto->gpgctx = g_mime_gpg_context_new (session, "gpg");
+	    g_object_unref (session);
+#endif
+	    if (crypto->gpgctx) {
+		g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) crypto->gpgctx, FALSE);
+	    } else {
+		fprintf (stderr, "Failed to construct gpg context.\n");
+	    }
+	}
+	cryptoctx = crypto->gpgctx;
+
+    } else {
+	fprintf (stderr, "Unknown or unsupported cryptographic protocol.\n");
+    }
+
+    return cryptoctx;
+}
+
+int
+notmuch_crypto_cleanup (notmuch_crypto_t *crypto)
+{
+    if (crypto->gpgctx) {
+	g_object_unref(crypto->gpgctx);
+	crypto->gpgctx = NULL;
+    }
+
+    return 0;
+}
diff --git a/notmuch-client.h b/notmuch-client.h
index 19b7f01..14d1e2f 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -74,6 +74,15 @@ typedef struct notmuch_show_format {
     const char *message_set_end;
 } notmuch_show_format_t;
 
+typedef struct notmuch_crypto {
+#ifdef GMIME_ATLEAST_26
+    GMimeCryptoContext* gpgctx;
+#else
+    GMimeCipherContext* gpgctx;
+#endif
+    notmuch_bool_t decrypt;
+} notmuch_crypto_t;
+
 typedef struct notmuch_show_params {
     notmuch_bool_t entire_thread;
     notmuch_bool_t omit_excluded;
@@ -113,6 +122,12 @@ chomp_newline (char *str)
 	str[strlen(str)-1] = '\0';
 }
 
+GMimeCryptoContext *
+notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol);
+
+int
+notmuch_crypto_cleanup (notmuch_crypto_t *crypto);
+
 int
 notmuch_count_command (void *ctx, int argc, char *argv[]);
 
-- 
1.7.10

[PATCH v3 2/6] cli: modify show and reply to use new crypto struct

[Jameson Graef Rollins]

notmuch_show_params_t is modified to use the new notmuch_crypto_t, and
notmuch-show and notmuch-reply are modified accordingly.
---
 notmuch-client.h |    7 +------
 notmuch-reply.c  |   29 ++++++++++++++++-------------
 notmuch-show.c   |   30 +++++++++++++++++-------------
 3 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 14d1e2f..86c9697 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -88,12 +88,7 @@ typedef struct notmuch_show_params {
     notmuch_bool_t omit_excluded;
     notmuch_bool_t raw;
     int part;
-#ifdef GMIME_ATLEAST_26
-    GMimeCryptoContext* cryptoctx;
-#else
-    GMimeCipherContext* cryptoctx;
-#endif
-    notmuch_bool_t decrypt;
+    notmuch_crypto_t crypto;
 } notmuch_show_params_t;
 
 /* There's no point in continuing when we've detected that we've done
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7184a5d..41b18e4 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -544,7 +544,7 @@ notmuch_reply_format_default(void *ctx,
 	g_object_unref (G_OBJECT (reply));
 	reply = NULL;
 
-	if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
+	if (mime_node_open (ctx, message, params->crypto.gpgctx, params->crypto.decrypt,
 			    &root) == NOTMUCH_STATUS_SUCCESS) {
 	    format_part_reply (root);
 	    talloc_free (root);
@@ -574,7 +574,7 @@ notmuch_reply_format_json(void *ctx,
 
     messages = notmuch_query_search_messages (query);
     message = notmuch_messages_get (messages);
-    if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
+    if (mime_node_open (ctx, message, params->crypto.gpgctx, params->crypto.decrypt,
 			&node) != NOTMUCH_STATUS_SUCCESS)
 	return 1;
 
@@ -675,7 +675,12 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     char *query_string;
     int opt_index, ret = 0;
     int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all);
-    notmuch_show_params_t params = { .part = -1 };
+    notmuch_show_params_t params = {
+	.part = -1,
+	.crypto = {
+	    .decrypt = FALSE
+	}
+    };
     int format = FORMAT_DEFAULT;
     int reply_all = TRUE;
 
@@ -689,7 +694,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "all", TRUE },
 				  { "sender", FALSE },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_BOOLEAN, &params.decrypt, "decrypt", 'd', 0 },
+	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -706,18 +711,18 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     else
 	reply_format_func = notmuch_reply_format_default;
 
-    if (params.decrypt) {
+    if (params.crypto.decrypt) {
 #ifdef GMIME_ATLEAST_26
 	/* TODO: GMimePasswordRequestFunc */
-	params.cryptoctx = g_mime_gpg_context_new (NULL, "gpg");
+	params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
 #else
 	GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
-	params.cryptoctx = g_mime_gpg_context_new (session, "gpg");
+	params.crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");
 #endif
-	if (params.cryptoctx) {
-	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.cryptoctx, FALSE);
+	if (params.crypto.gpgctx) {
+	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);
 	} else {
-	    params.decrypt = FALSE;
+	    params.crypto.decrypt = FALSE;
 	    fprintf (stderr, "Failed to construct gpg context.\n");
 	}
 #ifndef GMIME_ATLEAST_26
@@ -753,11 +758,9 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     if (reply_format_func (ctx, config, query, &params, reply_all) != 0)
 	return 1;
 
+    notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
-    if (params.cryptoctx)
-	g_object_unref(params.cryptoctx);
-
     return ret;
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index 95427d4..cc509a6 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -810,8 +810,8 @@ show_message (void *ctx,
     mime_node_t *root, *part;
     notmuch_status_t status;
 
-    status = mime_node_open (local, message, params->cryptoctx,
-			     params->decrypt, &root);
+    status = mime_node_open (local, message, params->crypto.gpgctx,
+			     params->crypto.decrypt, &root);
     if (status)
 	goto DONE;
     part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
@@ -984,7 +984,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     char *query_string;
     int opt_index, ret;
     const notmuch_show_format_t *format = &format_text;
-    notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
+    notmuch_show_params_t params = {
+	.part = -1,
+	.omit_excluded = TRUE,
+	.crypto = {
+	    .decrypt = FALSE
+	}
+    };
     int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
     notmuch_bool_t verify = FALSE;
     int exclude = EXCLUDE_TRUE;
@@ -1002,7 +1008,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
                                   { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &params.decrypt, "decrypt", 'd', 0 },
+	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
@@ -1047,18 +1053,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	break;
     }
 
-    if (params.decrypt || verify) {
+    if (params.crypto.decrypt || verify) {
 #ifdef GMIME_ATLEAST_26
 	/* TODO: GMimePasswordRequestFunc */
-	params.cryptoctx = g_mime_gpg_context_new (NULL, "gpg");
+	params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
 #else
 	GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
-	params.cryptoctx = g_mime_gpg_context_new (session, "gpg");
+	params.crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");
 #endif
-	if (params.cryptoctx) {
-	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.cryptoctx, FALSE);
+	if (params.crypto.gpgctx) {
+	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);
 	} else {
-	    params.decrypt = FALSE;
+	    params.crypto.decrypt = FALSE;
 	    fprintf (stderr, "Failed to construct gpg context.\n");
 	}
 #ifndef GMIME_ATLEAST_26
@@ -1115,11 +1121,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	ret = do_show (ctx, query, format, &params);
     }
 
+    notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
-    if (params.cryptoctx)
-	g_object_unref(params.cryptoctx);
-
     return ret;
 }
-- 
1.7.10

[PATCH v3 3/6] cli: modify mime_node_open to take new crypto struct as argument

[Jameson Graef Rollins]

This simplifies the interface considerably, getting rid of #ifdefs
along the way.
---
 mime-node.c      |   11 +++--------
 notmuch-client.h |   14 +++++---------
 notmuch-reply.c  |    6 ++----
 notmuch-show.c   |    3 +--
 4 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index a95bdab..183ff5d 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -61,12 +61,7 @@ _mime_node_context_free (mime_node_context_t *res)
 
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
-#ifdef GMIME_ATLEAST_26
-		GMimeCryptoContext *cryptoctx,
-#else
-		GMimeCipherContext *cryptoctx,
-#endif
-		notmuch_bool_t decrypt, 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;
@@ -118,8 +113,8 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 	goto DONE;
     }
 
-    mctx->cryptoctx = cryptoctx;
-    mctx->decrypt = decrypt;
+    mctx->cryptoctx = crypto->gpgctx;
+    mctx->decrypt = crypto->decrypt;
 
     /* Create the root node */
     root->part = GMIME_OBJECT (mctx->mime_message);
diff --git a/notmuch-client.h b/notmuch-client.h
index 86c9697..94af8f7 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -351,9 +351,10 @@ struct mime_node {
 };
 
 /* Construct a new MIME node pointing to the root message part of
- * message.  If cryptoctx is non-NULL, it will be used to verify
- * signatures on any child parts.  If decrypt is true, then cryptoctx
- * will additionally be used to decrypt any encrypted child parts.
+ * message.  If crypto->gpgctx is non-NULL, it will be used to verify
+ * signatures on any child parts.  If crypto->decrypt is true, then
+ * crypto.gpgctx will additionally be used to decrypt any encrypted
+ * child parts.
  *
  * Return value:
  *
@@ -365,12 +366,7 @@ struct mime_node {
  */
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
-#ifdef GMIME_ATLEAST_26
-		GMimeCryptoContext *cryptoctx,
-#else
-		GMimeCipherContext *cryptoctx,
-#endif
-		notmuch_bool_t decrypt, 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 41b18e4..148152c 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -544,8 +544,7 @@ notmuch_reply_format_default(void *ctx,
 	g_object_unref (G_OBJECT (reply));
 	reply = NULL;
 
-	if (mime_node_open (ctx, message, params->crypto.gpgctx, params->crypto.decrypt,
-			    &root) == NOTMUCH_STATUS_SUCCESS) {
+	if (mime_node_open (ctx, message, &(params->crypto), &root) == NOTMUCH_STATUS_SUCCESS) {
 	    format_part_reply (root);
 	    talloc_free (root);
 	}
@@ -574,8 +573,7 @@ notmuch_reply_format_json(void *ctx,
 
     messages = notmuch_query_search_messages (query);
     message = notmuch_messages_get (messages);
-    if (mime_node_open (ctx, message, params->crypto.gpgctx, params->crypto.decrypt,
-			&node) != NOTMUCH_STATUS_SUCCESS)
+    if (mime_node_open (ctx, message, &(params->crypto), &node) != NOTMUCH_STATUS_SUCCESS)
 	return 1;
 
     reply = create_reply_message (ctx, config, message, reply_all);
diff --git a/notmuch-show.c b/notmuch-show.c
index cc509a6..fb5e9b6 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -810,8 +810,7 @@ show_message (void *ctx,
     mime_node_t *root, *part;
     notmuch_status_t status;
 
-    status = mime_node_open (local, message, params->crypto.gpgctx,
-			     params->crypto.decrypt, &root);
+    status = mime_node_open (local, message, &(params->crypto), &root);
     if (status)
 	goto DONE;
     part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
-- 
1.7.10

[PATCH v3 4/6] cli: modify mime_node_context to use the new crypto struct

[Jameson Graef Rollins]

This simplifies some more interfaces and gets rid of another #ifdef.
---
 mime-node.c |   22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 183ff5d..a838224 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -33,12 +33,7 @@ typedef struct mime_node_context {
     GMimeMessage *mime_message;
 
     /* Context provided by the caller. */
-#ifdef GMIME_ATLEAST_26
-    GMimeCryptoContext *cryptoctx;
-#else
-    GMimeCipherContext *cryptoctx;
-#endif
-    notmuch_bool_t decrypt;
+    notmuch_crypto_t *crypto;
 } mime_node_context_t;
 
 static int
@@ -113,8 +108,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
 	goto DONE;
     }
 
-    mctx->cryptoctx = crypto->gpgctx;
-    mctx->decrypt = crypto->decrypt;
+    mctx->crypto = crypto;
 
     /* Create the root node */
     root->part = GMIME_OBJECT (mctx->mime_message);
@@ -190,7 +184,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 
     /* Handle PGP/MIME parts */
     if (GMIME_IS_MULTIPART_ENCRYPTED (part)
-	&& node->ctx->cryptoctx && node->ctx->decrypt) {
+	&& node->ctx->crypto->gpgctx && node->ctx->crypto->decrypt) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 4, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
@@ -203,10 +197,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 #ifdef GMIME_ATLEAST_26
 	    GMimeDecryptResult *decrypt_result = NULL;
 	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
-		(encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
+		(encrypteddata, node->ctx->crypto->gpgctx, &decrypt_result, &err);
 #else
 	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
-		(encrypteddata, node->ctx->cryptoctx, &err);
+		(encrypteddata, node->ctx->crypto->gpgctx, &err);
 #endif
 	    if (node->decrypted_child) {
 		node->decrypt_success = node->verify_attempted = TRUE;
@@ -224,7 +218,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 			 (err ? err->message : "no error explanation given"));
 	    }
 	}
-    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->cryptoctx) {
+    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->gpgctx) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 5, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/signed message "
@@ -233,7 +227,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	} else {
 #ifdef GMIME_ATLEAST_26
 	    node->sig_list = g_mime_multipart_signed_verify
-		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
+		(GMIME_MULTIPART_SIGNED (part), node->ctx->crypto->gpgctx, &err);
 	    node->verify_attempted = TRUE;
 
 	    if (!node->sig_list)
@@ -249,7 +243,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	     * In GMime 2.6, they're both non-const, so we'll be able
 	     * to clean up this asymmetry. */
 	    GMimeSignatureValidity *sig_validity = g_mime_multipart_signed_verify
-		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
+		(GMIME_MULTIPART_SIGNED (part), node->ctx->crypto->gpgctx, &err);
 	    node->verify_attempted = TRUE;
 	    node->sig_validity = sig_validity;
 	    if (sig_validity) {
-- 
1.7.10

[PATCH v3 5/6] cli: new crypto verify flag to handle verification

[Jameson Graef Rollins]

Use this flag rather than depend on the existence of an initialized
gpgctx, to determine whether we should verify a multipart/signed.  We
will be moving to create the ctx lazily, so we don't want to depend on
it being previously initialized if it's not needed.
---
 mime-node.c      |    5 ++---
 notmuch-client.h |    8 ++++----
 notmuch-reply.c  |    1 +
 notmuch-show.c   |   14 +++++++++++---
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index a838224..73e28c5 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -183,8 +183,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
     }
 
     /* Handle PGP/MIME parts */
-    if (GMIME_IS_MULTIPART_ENCRYPTED (part)
-	&& node->ctx->crypto->gpgctx && node->ctx->crypto->decrypt) {
+    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 4, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
@@ -218,7 +217,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 			 (err ? err->message : "no error explanation given"));
 	    }
 	}
-    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->gpgctx) {
+    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 5, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/signed message "
diff --git a/notmuch-client.h b/notmuch-client.h
index 94af8f7..db1c347 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -80,6 +80,7 @@ typedef struct notmuch_crypto {
 #else
     GMimeCipherContext* gpgctx;
 #endif
+    notmuch_bool_t verify;
     notmuch_bool_t decrypt;
 } notmuch_crypto_t;
 
@@ -351,10 +352,9 @@ struct mime_node {
 };
 
 /* Construct a new MIME node pointing to the root message part of
- * message.  If crypto->gpgctx is non-NULL, it will be used to verify
- * signatures on any child parts.  If crypto->decrypt is true, then
- * crypto.gpgctx will additionally be used to decrypt any encrypted
- * child parts.
+ * message. If crypto->verify is true, signed child parts will be
+ * verified. If crypto->decrypt is true, encrypted child parts will be
+ * decrypted.
  *
  * Return value:
  *
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 148152c..e4f293f 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -676,6 +676,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     notmuch_show_params_t params = {
 	.part = -1,
 	.crypto = {
+	    .verify = FALSE,
 	    .decrypt = FALSE
 	}
     };
diff --git a/notmuch-show.c b/notmuch-show.c
index fb5e9b6..3c06792 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -987,11 +987,11 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	.part = -1,
 	.omit_excluded = TRUE,
 	.crypto = {
+	    .verify = FALSE,
 	    .decrypt = FALSE
 	}
     };
     int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
-    notmuch_bool_t verify = FALSE;
     int exclude = EXCLUDE_TRUE;
 
     notmuch_opt_desc_t options[] = {
@@ -1008,7 +1008,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },
+	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -1018,6 +1018,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
+    /* decryption implies verification */
+    if (params.crypto.decrypt)
+	params.crypto.verify = TRUE;
+
     if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) {
 	/* if part was requested and format was not specified, use format=raw */
 	if (params.part >= 0)
@@ -1052,7 +1056,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	break;
     }
 
-    if (params.crypto.decrypt || verify) {
+    if (params.crypto.decrypt || params.crypto.verify) {
 #ifdef GMIME_ATLEAST_26
 	/* TODO: GMimePasswordRequestFunc */
 	params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
@@ -1063,6 +1067,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	if (params.crypto.gpgctx) {
 	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);
 	} else {
+	    /* If we fail to create the gpgctx set the verify and
+	     * decrypt flags to FALSE so we don't try to do any
+	     * further verification or decryption */
+	    params.crypto.verify = FALSE;
 	    params.crypto.decrypt = FALSE;
 	    fprintf (stderr, "Failed to construct gpg context.\n");
 	}
-- 
1.7.10

[PATCH v3 6/6] cli: use new notmuch_crypto_get_context in mime-node.c

[Jameson Graef Rollins]

This has the affect of lazily creating the crypto contexts only when
needed.  This removes code duplication from notmuch-show and
notmuch-reply, and should speed up these functions considerably if the
crypto flags are provided but the messages don't have any
cryptographic parts.
---
 mime-node.c      |   20 ++++++++++++++------
 notmuch-client.h |    3 ++-
 notmuch-reply.c  |   19 -------------------
 notmuch-show.c   |   23 -----------------------
 4 files changed, 16 insertions(+), 49 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 73e28c5..857c84c 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -150,6 +150,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 {
     mime_node_t *node = talloc_zero (parent, mime_node_t);
     GError *err = NULL;
+    GMimeCryptoContext *cryptoctx = NULL;
 
     /* Set basic node properties */
     node->part = part;
@@ -182,8 +183,15 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	return NULL;
     }
 
+    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt)
+	|| (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);
+    }
+
     /* Handle PGP/MIME parts */
-    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt) {
+    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt && cryptoctx) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 4, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
@@ -196,10 +204,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 #ifdef GMIME_ATLEAST_26
 	    GMimeDecryptResult *decrypt_result = NULL;
 	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
-		(encrypteddata, node->ctx->crypto->gpgctx, &decrypt_result, &err);
+		(encrypteddata, cryptoctx, &decrypt_result, &err);
 #else
 	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
-		(encrypteddata, node->ctx->crypto->gpgctx, &err);
+		(encrypteddata, cryptoctx, &err);
 #endif
 	    if (node->decrypted_child) {
 		node->decrypt_success = node->verify_attempted = TRUE;
@@ -217,7 +225,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 			 (err ? err->message : "no error explanation given"));
 	    }
 	}
-    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify) {
+    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify && cryptoctx) {
 	if (node->nchildren != 2) {
 	    /* this violates RFC 3156 section 5, so we won't bother with it. */
 	    fprintf (stderr, "Error: %d part(s) for a multipart/signed message "
@@ -226,7 +234,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	} else {
 #ifdef GMIME_ATLEAST_26
 	    node->sig_list = g_mime_multipart_signed_verify
-		(GMIME_MULTIPART_SIGNED (part), node->ctx->crypto->gpgctx, &err);
+		(GMIME_MULTIPART_SIGNED (part), cryptoctx, &err);
 	    node->verify_attempted = TRUE;
 
 	    if (!node->sig_list)
@@ -242,7 +250,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
 	     * In GMime 2.6, they're both non-const, so we'll be able
 	     * to clean up this asymmetry. */
 	    GMimeSignatureValidity *sig_validity = g_mime_multipart_signed_verify
-		(GMIME_MULTIPART_SIGNED (part), node->ctx->crypto->gpgctx, &err);
+		(GMIME_MULTIPART_SIGNED (part), cryptoctx, &err);
 	    node->verify_attempted = TRUE;
 	    node->sig_validity = sig_validity;
 	    if (sig_validity) {
diff --git a/notmuch-client.h b/notmuch-client.h
index db1c347..2434c4c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -354,7 +354,8 @@ struct mime_node {
 /* Construct a new MIME node pointing to the root message part of
  * message. If crypto->verify is true, signed child parts will be
  * verified. If crypto->decrypt is true, encrypted child parts will be
- * decrypted.
+ * decrypted.  If crypto->gpgctx is NULL, it will be lazily
+ * initialized.
  *
  * Return value:
  *
diff --git a/notmuch-reply.c b/notmuch-reply.c
index e4f293f..aecd173 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -710,25 +710,6 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     else
 	reply_format_func = notmuch_reply_format_default;
 
-    if (params.crypto.decrypt) {
-#ifdef GMIME_ATLEAST_26
-	/* TODO: GMimePasswordRequestFunc */
-	params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
-#else
-	GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
-	params.crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");
-#endif
-	if (params.crypto.gpgctx) {
-	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);
-	} else {
-	    params.crypto.decrypt = FALSE;
-	    fprintf (stderr, "Failed to construct gpg context.\n");
-	}
-#ifndef GMIME_ATLEAST_26
-	g_object_unref (session);
-#endif
-    }
-
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
 	return 1;
diff --git a/notmuch-show.c b/notmuch-show.c
index 3c06792..8247f1d 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1056,29 +1056,6 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	break;
     }
 
-    if (params.crypto.decrypt || params.crypto.verify) {
-#ifdef GMIME_ATLEAST_26
-	/* TODO: GMimePasswordRequestFunc */
-	params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
-#else
-	GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
-	params.crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");
-#endif
-	if (params.crypto.gpgctx) {
-	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);
-	} else {
-	    /* If we fail to create the gpgctx set the verify and
-	     * decrypt flags to FALSE so we don't try to do any
-	     * further verification or decryption */
-	    params.crypto.verify = FALSE;
-	    params.crypto.decrypt = FALSE;
-	    fprintf (stderr, "Failed to construct gpg context.\n");
-	}
-#ifndef GMIME_ATLEAST_26
-	g_object_unref (session);
-#endif
-    }
-
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
 	return 1;
-- 
1.7.10

Re: [PATCH v3 1/6] cli: new crypto structure to store crypto contexts and parameters, and functions to support it

[Jani Nikula]

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

On May 23, 2012 4:44 AM, "Jameson Graef Rollins" <jrollins@finestructure.net>
wrote:
>
> This new structure, notmuch_crypto_t, keeps all relevant crypto
> contexts and parameters together, and will make it easier to pass the
> stuff around and clean it up.  The name of the crypto context inside
> this new struct will change, to reflect that it is actually a GPG
> context, which is a sub type of Crypto context.  There are other types
> of Crypto contexts (Pkcs7 in particular, which we hope to support) so
> we want to be clear.
>
> The new crypto.c contains functions to return the proper context from
> the struct for a given protocol (and initialize it if needed), and to
> cleanup a struct by releasing the crypto contexts.
> ---
>  Makefile.local   |    1 +
>  crypto.c         |   65
++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  notmuch-client.h |   15 +++++++++++++
>  3 files changed, 81 insertions(+)
>  create mode 100644 crypto.c
>
> diff --git a/Makefile.local b/Makefile.local
> index 53b4a0d..a890df2 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -292,6 +292,7 @@ notmuch_client_srcs =               \
>        notmuch-time.c          \
>        query-string.c          \
>        mime-node.c             \
> +       crypto.c                \
>        json.c
>
>  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
> diff --git a/crypto.c b/crypto.c
> new file mode 100644
> index 0000000..c346999
> --- /dev/null
> +++ b/crypto.c
> @@ -0,0 +1,65 @@
> +/* 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 http://www.gnu.org/licenses/ .
> + *
> + * Authors: Jameson Rollins <jrollins@finestructure.net>
> + */
> +
> +#include "notmuch-client.h"
> +
> +/* for the specified protocol return the context pointer (initializing
> + * if needed) */
> +GMimeCryptoContext *
> +notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char
*protocol)
> +{
> +    GMimeCryptoContext *cryptoctx = NULL;

Isn't GMimeCryptoContext gmime 2.6 specific? It's #ifdeffed elsewhere at
least. Also affects the return type.

> +
> +    if ((strcmp (protocol, "application/pgp-signature") == 0)
> +       || (strcmp (protocol, "application/pgp-encrypted") == 0)) {

Is protocol guaranteed to be lower case?

> +       if (!crypto->gpgctx) {
> +#ifdef GMIME_ATLEAST_26
> +           /* TODO: GMimePasswordRequestFunc */
> +           crypto->gpgctx = g_mime_gpg_context_new (NULL, "gpg");
> +#else
> +           GMimeSession* session = g_object_new
(g_mime_session_get_type(), NULL);
> +           crypto->gpgctx = g_mime_gpg_context_new (session, "gpg");
> +           g_object_unref (session);
> +#endif
> +           if (crypto->gpgctx) {
> +               g_mime_gpg_context_set_always_trust ((GMimeGpgContext*)
crypto->gpgctx, FALSE);
> +           } else {
> +               fprintf (stderr, "Failed to construct gpg context.\n");
> +           }
> +       }
> +       cryptoctx = crypto->gpgctx;
> +
> +    } else {
> +       fprintf (stderr, "Unknown or unsupported cryptographic
protocol.\n");
> +    }
> +
> +    return cryptoctx;
> +}
> +
> +int
> +notmuch_crypto_cleanup (notmuch_crypto_t *crypto)
> +{
> +    if (crypto->gpgctx) {
> +       g_object_unref(crypto->gpgctx);
> +       crypto->gpgctx = NULL;
> +    }
> +
> +    return 0;
> +}
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..14d1e2f 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -74,6 +74,15 @@ typedef struct notmuch_show_format {
>     const char *message_set_end;
>  } notmuch_show_format_t;
>
> +typedef struct notmuch_crypto {
> +#ifdef GMIME_ATLEAST_26
> +    GMimeCryptoContext* gpgctx;
> +#else
> +    GMimeCipherContext* gpgctx;
> +#endif
> +    notmuch_bool_t decrypt;
> +} notmuch_crypto_t;
> +
>  typedef struct notmuch_show_params {
>     notmuch_bool_t entire_thread;
>     notmuch_bool_t omit_excluded;
> @@ -113,6 +122,12 @@ chomp_newline (char *str)
>        str[strlen(str)-1] = '\0';
>  }
>
> +GMimeCryptoContext *
> +notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char
*protocol);
> +
> +int
> +notmuch_crypto_cleanup (notmuch_crypto_t *crypto);
> +
>  int
>  notmuch_count_command (void *ctx, int argc, char *argv[]);
>
> --
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[-- Attachment #2: Type: text/html, Size: 6814 bytes --]

Re: [PATCH v3 1/6] cli: new crypto structure to store crypto contexts and parameters, and functions to support it

[Jameson Graef Rollins]

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

On Tue, May 22 2012, Jani Nikula <jani@nikula.org> wrote:
>> +/* for the specified protocol return the context pointer (initializing
>> + * if needed) */
>> +GMimeCryptoContext *
>> +notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char
> *protocol)
>> +{
>> +    GMimeCryptoContext *cryptoctx = NULL;
>
> Isn't GMimeCryptoContext gmime 2.6 specific? It's #ifdeffed elsewhere at
> least. Also affects the return type.

Yes, good catch, Jani.  Thank you.  Will fix.

>> +    if ((strcmp (protocol, "application/pgp-signature") == 0)
>> +       || (strcmp (protocol, "application/pgp-encrypted") == 0)) {
>
> Is protocol guaranteed to be lower case?

That's a good question.  I don't know.  I'll check and fix
appropriately.

jamie.

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