* [PATCH v4 0/7] cli: improved crypto internals @ 2012-05-23 22:40 Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Jameson Graef Rollins ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail Ok, hopefully last version. This addresses Jani's latest comments. The first patch uses a typedef to handle the incompatibility between the crypto context between GMIME 2.4 and 2.6. We probably should have done this originally, as it gets rid of a bunch of #ifdefs, and should help reduce confusion. I also move to use strcasecmp for the crypto protocol, with a comment that points to the relevant RFCs for justification. jamie. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility 2012-05-23 22:40 [PATCH v4 0/7] cli: improved crypto internals Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Jameson Graef Rollins 2012-05-25 14:41 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Austin Clements 2012-05-25 13:15 ` [PATCH v4 0/7] cli: improved crypto internals Jani Nikula 2012-05-25 14:44 ` Austin Clements 2 siblings, 2 replies; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail gmime 2.4 defines GMimeCipherContext, while 2.6 defines GMimeCryptoContext. We can use a typedef to cover this discrepancy and remove a bunch of #ifdefs. --- mime-node.c | 8 -------- notmuch-client.h | 10 ++-------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/mime-node.c b/mime-node.c index a95bdab..06fdb70 100644 --- a/mime-node.c +++ b/mime-node.c @@ -33,11 +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; } mime_node_context_t; @@ -61,11 +57,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) { const char *filename = notmuch_message_get_filename (message); diff --git a/notmuch-client.h b/notmuch-client.h index 19b7f01..337409f 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -36,6 +36,8 @@ * these to check the version number. */ #ifdef GMIME_MAJOR_VERSION #define GMIME_ATLEAST_26 +#else +typedef GMimeCipherContext GMimeCryptoContext; #endif #include "notmuch.h" @@ -79,11 +81,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_show_params_t; @@ -355,11 +353,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); /* Return a new MIME node for the requested child part of parent. -- 1.7.10 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it 2012-05-23 22:40 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 3/7] cli: modify show and reply to use new crypto struct Jameson Graef Rollins 2012-05-25 14:42 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Austin Clements 2012-05-25 14:41 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Austin Clements 1 sibling, 2 replies; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail 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 | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ notmuch-client.h | 11 +++++++++ 3 files changed, 83 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..25c2d10 --- /dev/null +++ b/crypto.c @@ -0,0 +1,71 @@ +/* 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; + + /* As per RFC 1847 section 2.1: "the [protocol] value token is + * comprised of the type and sub-type tokens of the Content-Type". + * As per RFC 1521 section 2: "Content-Type values, subtypes, and + * parameter names as defined in this document are + * case-insensitive." Thus, we use strcasecmp for the protocol. + */ + if ((strcasecmp (protocol, "application/pgp-signature") == 0) + || (strcasecmp (protocol, "application/pgp-encrypted") == 0)) { + if (!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 337409f..a8b00ab 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -76,6 +76,11 @@ typedef struct notmuch_show_format { const char *message_set_end; } notmuch_show_format_t; +typedef struct notmuch_crypto { + GMimeCryptoContext* gpgctx; + notmuch_bool_t decrypt; +} notmuch_crypto_t; + typedef struct notmuch_show_params { notmuch_bool_t entire_thread; notmuch_bool_t omit_excluded; @@ -111,6 +116,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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 3/7] cli: modify show and reply to use new crypto struct 2012-05-23 22:40 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 4/7] cli: modify mime_node_open to take new crypto struct as argument Jameson Graef Rollins 2012-05-25 14:42 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Austin Clements 1 sibling, 1 reply; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail 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 | 3 +-- notmuch-reply.c | 29 ++++++++++++++++------------- notmuch-show.c | 30 +++++++++++++++++------------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/notmuch-client.h b/notmuch-client.h index a8b00ab..9f44dc9 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -86,8 +86,7 @@ typedef struct notmuch_show_params { notmuch_bool_t omit_excluded; notmuch_bool_t raw; int part; - GMimeCryptoContext* cryptoctx; - 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, ¶ms.decrypt, "decrypt", 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, ¶ms.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, ¶ms, reply_all) != 0) return 1; + notmuch_crypto_cleanup (¶ms.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, ¶ms.part, "part", 'p', 0 }, { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 }, - { NOTMUCH_OPT_BOOLEAN, ¶ms.decrypt, "decrypt", 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, ¶ms.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, ¶ms); } + notmuch_crypto_cleanup (¶ms.crypto); notmuch_query_destroy (query); notmuch_database_destroy (notmuch); - if (params.cryptoctx) - g_object_unref(params.cryptoctx); - return ret; } -- 1.7.10 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 4/7] cli: modify mime_node_open to take new crypto struct as argument 2012-05-23 22:40 ` [PATCH v4 3/7] cli: modify show and reply to use new crypto struct Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 5/7] cli: modify mime_node_context to use the new crypto struct Jameson Graef Rollins 0 siblings, 1 reply; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail This simplifies the interface considerably, getting rid of #ifdefs along the way. --- mime-node.c | 7 +++---- notmuch-client.h | 10 +++++----- notmuch-reply.c | 6 ++---- notmuch-show.c | 3 +-- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/mime-node.c b/mime-node.c index 06fdb70..ba3a01b 100644 --- a/mime-node.c +++ b/mime-node.c @@ -57,8 +57,7 @@ _mime_node_context_free (mime_node_context_t *res) notmuch_status_t mime_node_open (const void *ctx, notmuch_message_t *message, - GMimeCryptoContext *cryptoctx, - 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; @@ -110,8 +109,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 9f44dc9..8d92d41 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -349,9 +349,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: * @@ -363,8 +364,7 @@ struct mime_node { */ notmuch_status_t mime_node_open (const void *ctx, notmuch_message_t *message, - GMimeCryptoContext *cryptoctx, - 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 5/7] cli: modify mime_node_context to use the new crypto struct 2012-05-23 22:40 ` [PATCH v4 4/7] cli: modify mime_node_open to take new crypto struct as argument Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 6/7] cli: new crypto verify flag to handle verification Jameson Graef Rollins 0 siblings, 1 reply; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail This simplifies some more interfaces and gets rid of another #ifdef. --- mime-node.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/mime-node.c b/mime-node.c index ba3a01b..a838224 100644 --- a/mime-node.c +++ b/mime-node.c @@ -33,8 +33,7 @@ typedef struct mime_node_context { GMimeMessage *mime_message; /* Context provided by the caller. */ - GMimeCryptoContext *cryptoctx; - notmuch_bool_t decrypt; + notmuch_crypto_t *crypto; } mime_node_context_t; static int @@ -109,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); @@ -186,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 " @@ -199,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; @@ -220,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 " @@ -229,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) @@ -245,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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 6/7] cli: new crypto verify flag to handle verification 2012-05-23 22:40 ` [PATCH v4 5/7] cli: modify mime_node_context to use the new crypto struct Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 7/7] cli: use new notmuch_crypto_get_context in mime-node.c Jameson Graef Rollins 0 siblings, 1 reply; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail 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 8d92d41..c501186 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -78,6 +78,7 @@ typedef struct notmuch_show_format { typedef struct notmuch_crypto { GMimeCryptoContext* gpgctx; + notmuch_bool_t verify; notmuch_bool_t decrypt; } notmuch_crypto_t; @@ -349,10 +350,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, ¶ms.part, "part", 'p', 0 }, { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 }, { NOTMUCH_OPT_BOOLEAN, ¶ms.crypto.decrypt, "decrypt", 'd', 0 }, - { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 }, + { NOTMUCH_OPT_BOOLEAN, ¶ms.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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 7/7] cli: use new notmuch_crypto_get_context in mime-node.c 2012-05-23 22:40 ` [PATCH v4 6/7] cli: new crypto verify flag to handle verification Jameson Graef Rollins @ 2012-05-23 22:40 ` Jameson Graef Rollins 0 siblings, 0 replies; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-23 22:40 UTC (permalink / raw) To: Notmuch Mail 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 c501186..243b49c 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -352,7 +352,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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it 2012-05-23 22:40 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 3/7] cli: modify show and reply to use new crypto struct Jameson Graef Rollins @ 2012-05-25 14:42 ` Austin Clements 1 sibling, 0 replies; 16+ messages in thread From: Austin Clements @ 2012-05-25 14:42 UTC (permalink / raw) To: Jameson Graef Rollins; +Cc: Notmuch Mail Quoth Jameson Graef Rollins on May 23 at 3:40 pm: > 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 | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > notmuch-client.h | 11 +++++++++ > 3 files changed, 83 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..25c2d10 > --- /dev/null > +++ b/crypto.c > @@ -0,0 +1,71 @@ > +/* 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; > + > + /* As per RFC 1847 section 2.1: "the [protocol] value token is > + * comprised of the type and sub-type tokens of the Content-Type". > + * As per RFC 1521 section 2: "Content-Type values, subtypes, and > + * parameter names as defined in this document are > + * case-insensitive." Thus, we use strcasecmp for the protocol. > + */ > + if ((strcasecmp (protocol, "application/pgp-signature") == 0) > + || (strcasecmp (protocol, "application/pgp-encrypted") == 0)) { > + if (!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); Missing space. > + crypto->gpgctx = NULL; > + } > + > + return 0; > +} > diff --git a/notmuch-client.h b/notmuch-client.h > index 337409f..a8b00ab 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -76,6 +76,11 @@ typedef struct notmuch_show_format { > const char *message_set_end; > } notmuch_show_format_t; > > +typedef struct notmuch_crypto { > + GMimeCryptoContext* gpgctx; > + notmuch_bool_t decrypt; > +} notmuch_crypto_t; > + > typedef struct notmuch_show_params { > notmuch_bool_t entire_thread; > notmuch_bool_t omit_excluded; > @@ -111,6 +116,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[]); > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility 2012-05-23 22:40 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Jameson Graef Rollins @ 2012-05-25 14:41 ` Austin Clements 2012-05-25 15:47 ` Jameson Graef Rollins 1 sibling, 1 reply; 16+ messages in thread From: Austin Clements @ 2012-05-25 14:41 UTC (permalink / raw) To: Jameson Graef Rollins; +Cc: Notmuch Mail Quoth Jameson Graef Rollins on May 23 at 3:40 pm: > gmime 2.4 defines GMimeCipherContext, while 2.6 defines > GMimeCryptoContext. We can use a typedef to cover this discrepancy > and remove a bunch of #ifdefs. > --- > mime-node.c | 8 -------- > notmuch-client.h | 10 ++-------- > 2 files changed, 2 insertions(+), 16 deletions(-) > > diff --git a/mime-node.c b/mime-node.c > index a95bdab..06fdb70 100644 > --- a/mime-node.c > +++ b/mime-node.c > @@ -33,11 +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; > } mime_node_context_t; > > @@ -61,11 +57,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) > { > const char *filename = notmuch_message_get_filename (message); > diff --git a/notmuch-client.h b/notmuch-client.h > index 19b7f01..337409f 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -36,6 +36,8 @@ > * these to check the version number. */ > #ifdef GMIME_MAJOR_VERSION > #define GMIME_ATLEAST_26 > +#else > +typedef GMimeCipherContext GMimeCryptoContext; I like the typedef idea, but I don't think we should overload GMimeCryptoContext like this. If someone is reading through the GMime 2.4 code and sees this, they're going to assume that it's a GMime structure, go looking for it, find that it's only in 2.6 and be baffled. Instead, how about providing a typedef to abstract *both* cases? Something like #ifdef GMIME_MAJOR_VERSION #define GMIME_ATLEAST_26 typedef notmuch_crypto_context_t GMimeCipherContext; #else typedef notmuch_crypto_context_t GMimeCryptoContext; #endif > #endif > > #include "notmuch.h" > @@ -79,11 +81,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_show_params_t; > > @@ -355,11 +353,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); > > /* Return a new MIME node for the requested child part of parent. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility 2012-05-25 14:41 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Austin Clements @ 2012-05-25 15:47 ` Jameson Graef Rollins 2012-05-25 16:23 ` Austin Clements 2012-05-25 18:14 ` Tomi Ollila 0 siblings, 2 replies; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-25 15:47 UTC (permalink / raw) To: Austin Clements; +Cc: Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 1471 bytes --] On Fri, May 25 2012, Austin Clements <amdragon@MIT.EDU> wrote: >> diff --git a/notmuch-client.h b/notmuch-client.h >> index 19b7f01..337409f 100644 >> --- a/notmuch-client.h >> +++ b/notmuch-client.h >> @@ -36,6 +36,8 @@ >> * these to check the version number. */ >> #ifdef GMIME_MAJOR_VERSION >> #define GMIME_ATLEAST_26 >> +#else >> +typedef GMimeCipherContext GMimeCryptoContext; > > I like the typedef idea, but I don't think we should overload > GMimeCryptoContext like this. If someone is reading through the GMime > 2.4 code and sees this, they're going to assume that it's a GMime > structure, go looking for it, find that it's only in 2.6 and be > baffled. Instead, how about providing a typedef to abstract *both* > cases? Something like > > #ifdef GMIME_MAJOR_VERSION > #define GMIME_ATLEAST_26 > typedef notmuch_crypto_context_t GMimeCipherContext; > #else > typedef notmuch_crypto_context_t GMimeCryptoContext; > #endif Hey, Austin. I briefly thought about this, but it seemed kind of heavy handed given that I hope these ifdefs will go away in the not-too-distant future. Do we really have a lot of gmime 2.4 readers that would not have access to gmime 2.6 documentation? I'm pretty sure that I would personally end up looking at documentation for both versions. But anyway, if this really is a concern, I guess it's not *that* much effort to support a new typedef indefinitely to alleviate any potential confusion. Any other opinions? jamie. [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility 2012-05-25 15:47 ` Jameson Graef Rollins @ 2012-05-25 16:23 ` Austin Clements 2012-05-25 18:14 ` Tomi Ollila 1 sibling, 0 replies; 16+ messages in thread From: Austin Clements @ 2012-05-25 16:23 UTC (permalink / raw) To: Jameson Graef Rollins; +Cc: Notmuch Mail Quoth Jameson Graef Rollins on May 25 at 8:47 am: > On Fri, May 25 2012, Austin Clements <amdragon@MIT.EDU> wrote: > >> diff --git a/notmuch-client.h b/notmuch-client.h > >> index 19b7f01..337409f 100644 > >> --- a/notmuch-client.h > >> +++ b/notmuch-client.h > >> @@ -36,6 +36,8 @@ > >> * these to check the version number. */ > >> #ifdef GMIME_MAJOR_VERSION > >> #define GMIME_ATLEAST_26 > >> +#else > >> +typedef GMimeCipherContext GMimeCryptoContext; > > > > I like the typedef idea, but I don't think we should overload > > GMimeCryptoContext like this. If someone is reading through the GMime > > 2.4 code and sees this, they're going to assume that it's a GMime > > structure, go looking for it, find that it's only in 2.6 and be > > baffled. Instead, how about providing a typedef to abstract *both* > > cases? Something like > > > > #ifdef GMIME_MAJOR_VERSION > > #define GMIME_ATLEAST_26 > > typedef notmuch_crypto_context_t GMimeCipherContext; > > #else > > typedef notmuch_crypto_context_t GMimeCryptoContext; > > #endif > > Hey, Austin. I briefly thought about this, but it seemed kind of heavy > handed given that I hope these ifdefs will go away in the > not-too-distant future. Do we really have a lot of gmime 2.4 readers > that would not have access to gmime 2.6 documentation? I'm pretty sure > that I would personally end up looking at documentation for both > versions. > > But anyway, if this really is a concern, I guess it's not *that* much > effort to support a new typedef indefinitely to alleviate any potential > confusion. > > Any other opinions? The same thought had crossed my mind, but I figured we could remove the typedefs once we do drop support for 2.6 and simply replace notmuch_crypto_context_t with GMimeCipherContext at that point. I don't think that would require many changes at that point (GMimeCryptoContext and GMimeCipherContext together appear only eight times in the current master, and I think your series reduces that even further), and most of the places it would change would probably also require stripping out other compatibility code. You probably have a much better sense for the extent of this than I do, though. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility 2012-05-25 15:47 ` Jameson Graef Rollins 2012-05-25 16:23 ` Austin Clements @ 2012-05-25 18:14 ` Tomi Ollila 1 sibling, 0 replies; 16+ messages in thread From: Tomi Ollila @ 2012-05-25 18:14 UTC (permalink / raw) To: Jameson Graef Rollins, Austin Clements; +Cc: Notmuch Mail On Fri, May 25 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote: > On Fri, May 25 2012, Austin Clements <amdragon@MIT.EDU> wrote: >>> diff --git a/notmuch-client.h b/notmuch-client.h >>> index 19b7f01..337409f 100644 >>> --- a/notmuch-client.h >>> +++ b/notmuch-client.h >>> @@ -36,6 +36,8 @@ >>> * these to check the version number. */ >>> #ifdef GMIME_MAJOR_VERSION >>> #define GMIME_ATLEAST_26 >>> +#else >>> +typedef GMimeCipherContext GMimeCryptoContext; >> >> I like the typedef idea, but I don't think we should overload >> GMimeCryptoContext like this. If someone is reading through the GMime >> 2.4 code and sees this, they're going to assume that it's a GMime >> structure, go looking for it, find that it's only in 2.6 and be >> baffled. Instead, how about providing a typedef to abstract *both* >> cases? Something like >> >> #ifdef GMIME_MAJOR_VERSION >> #define GMIME_ATLEAST_26 >> typedef notmuch_crypto_context_t GMimeCipherContext; >> #else >> typedef notmuch_crypto_context_t GMimeCryptoContext; >> #endif > > Hey, Austin. I briefly thought about this, but it seemed kind of heavy > handed given that I hope these ifdefs will go away in the > not-too-distant future. Do we really have a lot of gmime 2.4 readers > that would not have access to gmime 2.6 documentation? I'm pretty sure > that I would personally end up looking at documentation for both > versions. > > But anyway, if this really is a concern, I guess it's not *that* much > effort to support a new typedef indefinitely to alleviate any potential > confusion. > > Any other opinions? I like Austin's suggestion; as long as gmime 2.4 is supported it is clearer that the type name is not just a little different -- someone browsing gmime 2.4 code may experience some WTF:s (or was it FTW, cannot remember ;) when they try to locate GMimeCipherContext there... and, in if in some day gmime 2.4 support is dropped, M-x tags-query-replace can be used to drop the notmuch_crypto_context_t type. I personally am using gmime 2.4 as it has much less other requirements (i.e. ./configure --prefix=... && make install does it -- gmime 2.6 requires some other "nondefault" libraries also) and I don't plan to update that (and not the least reason is that I keep testing 2.4 compatibility all the time when new commits appear to my git tree) > jamie. Tomi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 0/7] cli: improved crypto internals 2012-05-23 22:40 [PATCH v4 0/7] cli: improved crypto internals Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Jameson Graef Rollins @ 2012-05-25 13:15 ` Jani Nikula 2012-05-25 15:48 ` Jameson Graef Rollins 2012-05-25 14:44 ` Austin Clements 2 siblings, 1 reply; 16+ messages in thread From: Jani Nikula @ 2012-05-25 13:15 UTC (permalink / raw) To: Jameson Graef Rollins, Notmuch Mail On Wed, 23 May 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote: > Ok, hopefully last version. This addresses Jani's latest comments. I don't have any real concerns left regarding this series. Just a nitpick, commit messages for patches 4/7 and 5/7 have leftover references to removed #ifdefs, but no big deal IMO. Jani. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 0/7] cli: improved crypto internals 2012-05-25 13:15 ` [PATCH v4 0/7] cli: improved crypto internals Jani Nikula @ 2012-05-25 15:48 ` Jameson Graef Rollins 0 siblings, 0 replies; 16+ messages in thread From: Jameson Graef Rollins @ 2012-05-25 15:48 UTC (permalink / raw) To: Jani Nikula, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 473 bytes --] On Fri, May 25 2012, Jani Nikula <jani@nikula.org> wrote: > On Wed, 23 May 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote: >> Ok, hopefully last version. This addresses Jani's latest comments. > > I don't have any real concerns left regarding this series. Just a > nitpick, commit messages for patches 4/7 and 5/7 have leftover > references to removed #ifdefs, but no big deal IMO. No, that was sloppy on my part. Sorry about that. I'll fix it. jamie. [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 0/7] cli: improved crypto internals 2012-05-23 22:40 [PATCH v4 0/7] cli: improved crypto internals Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Jameson Graef Rollins 2012-05-25 13:15 ` [PATCH v4 0/7] cli: improved crypto internals Jani Nikula @ 2012-05-25 14:44 ` Austin Clements 2 siblings, 0 replies; 16+ messages in thread From: Austin Clements @ 2012-05-25 14:44 UTC (permalink / raw) To: Jameson Graef Rollins; +Cc: Notmuch Mail Quoth Jameson Graef Rollins on May 23 at 3:40 pm: > Ok, hopefully last version. This addresses Jani's latest comments. > > The first patch uses a typedef to handle the incompatibility between > the crypto context between GMIME 2.4 and 2.6. We probably should have > done this originally, as it gets rid of a bunch of #ifdefs, and should > help reduce confusion. > > I also move to use strcasecmp for the crypto protocol, with a comment > that points to the relevant RFCs for justification. This series LGTM other than the one and a half comments I just sent. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-05-25 18:14 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-23 22:40 [PATCH v4 0/7] cli: improved crypto internals Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 3/7] cli: modify show and reply to use new crypto struct Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 4/7] cli: modify mime_node_open to take new crypto struct as argument Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 5/7] cli: modify mime_node_context to use the new crypto struct Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 6/7] cli: new crypto verify flag to handle verification Jameson Graef Rollins 2012-05-23 22:40 ` [PATCH v4 7/7] cli: use new notmuch_crypto_get_context in mime-node.c Jameson Graef Rollins 2012-05-25 14:42 ` [PATCH v4 2/7] cli: new crypto structure to store crypto contexts and parameters, and functions to support it Austin Clements 2012-05-25 14:41 ` [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility Austin Clements 2012-05-25 15:47 ` Jameson Graef Rollins 2012-05-25 16:23 ` Austin Clements 2012-05-25 18:14 ` Tomi Ollila 2012-05-25 13:15 ` [PATCH v4 0/7] cli: improved crypto internals Jani Nikula 2012-05-25 15:48 ` Jameson Graef Rollins 2012-05-25 14:44 ` Austin Clements
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).