[PATCH v3 2/2] Add compatibility with gmime 2.6
Austin Clements
amdragon at MIT.EDU
Thu Jan 19 20:10:44 PST 2012
Nearly there. A few more small comments.
Quoth Thomas Jost on Jan 20 at 1:06 am:
> There are lots of API changes in gmime 2.6 crypto handling. By adding
> preprocessor directives, it is however possible to add gmime 2.6 compatibility
> while preserving compatibility with gmime 2.4 too.
>
> This is mostly based on id:"8762i8hrb9.fsf at bookbinder.fernseed.info".
>
> This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
> crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test is
> currently broken (signature verification with signer key unavailable), most
> likely because of a bug in gmime which will hopefully be fixed in a future
> version.
> ---
> mime-node.c | 60 ++++++++++++++++++++++++++++++++--
> notmuch-client.h | 30 ++++++++++++++++-
> notmuch-reply.c | 7 ++++
> notmuch-show.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> show-message.c | 4 ++
> test/crypto | 2 +
> 6 files changed, 192 insertions(+), 6 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index d26bb44..ad19f5e 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -33,7 +33,11 @@ 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;
>
> @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)
>
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> - GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
> - mime_node_t **root_out)
> +#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);
> mime_node_context_t *mctx;
> @@ -112,12 +120,21 @@ DONE:
> return status;
> }
>
> +#ifdef GMIME_ATLEAST_26
> +static int
> +_signature_list_free (GMimeSignatureList **proxy)
> +{
> + g_object_unref (*proxy);
> + return 0;
> +}
> +#else
> static int
> _signature_validity_free (GMimeSignatureValidity **proxy)
> {
> g_mime_signature_validity_free (*proxy);
> return 0;
> }
> +#endif
>
> static mime_node_t *
> _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> @@ -165,11 +182,24 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> GMimeMultipartEncrypted *encrypteddata =
> GMIME_MULTIPART_ENCRYPTED (part);
> node->decrypt_attempted = TRUE;
> +#ifdef GMIME_ATLEAST_26
> + GMimeDecryptResult *decrypt_result = NULL;
> + node->decrypted_child = g_mime_multipart_encrypted_decrypt
> + (encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
> +#else
> node->decrypted_child = g_mime_multipart_encrypted_decrypt
> (encrypteddata, node->ctx->cryptoctx, &err);
> +#endif
> if (node->decrypted_child) {
> node->decrypt_success = node->verify_attempted = TRUE;
> +#ifdef GMIME_ATLEAST_26
> + /* This may be NULL if the part is not signed. */
> + node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
> + g_object_ref (node->sig_list);
Apparently you can't g_object_ref NULL, so there should be a
conditional around this. (Does g_mime_decrypt_result_get_signatures
*really* return NULL for an empty list? I feel like various tests
should have failed in various versions of this patch if it did.)
> + g_object_unref (decrypt_result);
> +#else
> node->sig_validity = g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
> +#endif
> } else {
> fprintf (stderr, "Failed to decrypt part: %s\n",
> (err ? err->message : "no error explanation given"));
> @@ -182,6 +212,15 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> "(must be exactly 2)\n",
> node->nchildren);
> } else {
> +#ifdef GMIME_ATLEAST_26
> + node->sig_list = g_mime_multipart_signed_verify
> + (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> + node->verify_attempted = TRUE;
> +
> + if (!node->sig_list)
> + fprintf (stderr, "Failed to verify signed part: %s\n",
> + (err ? err->message : "no error explanation given"));
> +#else
> /* For some reason the GMimeSignatureValidity returned
> * here is not a const (inconsistent with that
> * returned by
> @@ -195,17 +234,30 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> node->verify_attempted = TRUE;
> node->sig_validity = sig_validity;
> if (sig_validity) {
> - GMimeSignatureValidity **proxy =
> - talloc (node, GMimeSignatureValidity *);
> + GMimeSignatureValidity **proxy = talloc (node, GMimeSignatureValidity *);
(See below)
> *proxy = sig_validity;
> talloc_set_destructor (proxy, _signature_validity_free);
> }
> +#endif
> }
> }
>
> +#ifdef GMIME_ATLEAST_26
> + /* sig_list may be created in both above cases, so we need to
> + * cleanly handle it here. */
> + if (node->sig_list) {
> + GMimeSignatureList **proxy =
> + talloc (node, GMimeSignatureList *);
Oops. I think you un-split the wrong line. The one up above is now
too long and this one is still unnecessarily split.
> + *proxy = node->sig_list;
> + talloc_set_destructor (proxy, _signature_list_free);
> + }
> +#endif
> +
> +#ifndef GMIME_ATLEAST_26
> if (node->verify_attempted && !node->sig_validity)
> fprintf (stderr, "Failed to verify signed part: %s\n",
> (err ? err->message : "no error explanation given"));
> +#endif
>
> if (err)
> g_error_free (err);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 62ede28..9c1d383 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -30,6 +30,14 @@
>
> #include <gmime/gmime.h>
>
> +/* GMIME_CHECK_VERSION in gmime 2.4 is not usable from the
> + * preprocessor (it calls a runtime function). But since
> + * GMIME_MAJOR_VERSION and friends were added in gmime 2.6, we can use
> + * these to check the version number. */
> +#ifdef GMIME_MAJOR_VERSION
> +#define GMIME_ATLEAST_26
> +#endif
> +
> #include "notmuch.h"
>
> /* This is separate from notmuch-private.h because we're trying to
> @@ -69,7 +77,11 @@ typedef struct notmuch_show_format {
> void (*part_start) (GMimeObject *part,
> int *part_count);
> void (*part_encstatus) (int status);
> +#ifdef GMIME_ATLEAST_26
> + void (*part_sigstatus) (GMimeSignatureList* siglist);
> +#else
> void (*part_sigstatus) (const GMimeSignatureValidity* validity);
> +#endif
> void (*part_content) (GMimeObject *part);
> void (*part_end) (GMimeObject *part);
> const char *part_sep;
> @@ -83,7 +95,11 @@ typedef struct notmuch_show_params {
> int entire_thread;
> int raw;
> int part;
> +#ifdef GMIME_ATLEAST_26
> + GMimeCryptoContext* cryptoctx;
> +#else
> GMimeCipherContext* cryptoctx;
> +#endif
> int decrypt;
> } notmuch_show_params_t;
>
> @@ -290,11 +306,17 @@ typedef struct mime_node {
>
> /* True if signature verification on this part was attempted. */
> notmuch_bool_t verify_attempted;
> +#ifdef GMIME_ATLEAST_26
> + /* The list of signatures for signed or encrypted containers. If
> + * there are no signatures, this will be NULL. */
> + GMimeSignatureList* sig_list;
> +#else
> /* For signed or encrypted containers, the validity of the
> * signature. May be NULL if signature verification failed. If
> * there are simply no signatures, this will be non-NULL with an
> * empty signers list. */
> const GMimeSignatureValidity *sig_validity;
> +#endif
>
> /* Internal: Context inherited from the root iterator. */
> struct mime_node_context *ctx;
> @@ -319,8 +341,12 @@ typedef struct mime_node {
> */
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> - GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
> - mime_node_t **node_out);
> +#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.
> * parent will be used as the talloc context for the returned child
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 0f682db..bf67960 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -688,15 +688,22 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> reply_format_func = notmuch_reply_format_default;
>
> if (decrypt) {
> +#ifdef GMIME_ATLEAST_26
> + /* TODO: GMimePasswordRequestFunc */
> + params.cryptoctx = 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");
> +#endif
> if (params.cryptoctx) {
> g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.cryptoctx, FALSE);
> params.decrypt = TRUE;
> } else {
> fprintf (stderr, "Failed to construct gpg context.\n");
> }
> +#ifndef GMIME_ATLEAST_26
> g_object_unref (session);
> +#endif
> }
>
> config = notmuch_config_open (ctx, NULL, NULL);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 91f566c..190210c 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -76,7 +76,11 @@ static void
> format_part_encstatus_json (int status);
>
> static void
> +#ifdef GMIME_ATLEAST_26
> +format_part_sigstatus_json (GMimeSignatureList* siglist);
> +#else
> format_part_sigstatus_json (const GMimeSignatureValidity* validity);
> +#endif
>
> static void
> format_part_content_json (GMimeObject *part);
> @@ -486,6 +490,21 @@ show_text_part_content (GMimeObject *part, GMimeStream *stream_out)
> g_object_unref(stream_filter);
> }
>
> +#ifdef GMIME_ATLEAST_26
> +static const char*
> +signature_status_to_string (GMimeSignatureStatus x)
> +{
> + switch (x) {
> + case GMIME_SIGNATURE_STATUS_GOOD:
> + return "good";
> + case GMIME_SIGNATURE_STATUS_BAD:
> + return "bad";
> + case GMIME_SIGNATURE_STATUS_ERROR:
> + return "error";
> + }
> + return "unknown";
> +}
> +#else
> static const char*
> signer_status_to_string (GMimeSignerStatus x)
> {
> @@ -501,6 +520,7 @@ signer_status_to_string (GMimeSignerStatus x)
> }
> return "unknown";
> }
> +#endif
>
> static void
> format_part_start_text (GMimeObject *part, int *part_count)
> @@ -592,6 +612,73 @@ format_part_encstatus_json (int status)
> printf ("}]");
> }
>
> +#ifdef GMIME_ATLEAST_26
> +static void
> +format_part_sigstatus_json (GMimeSignatureList *siglist)
> +{
> + printf (", \"sigstatus\": [");
> +
> + if (!siglist) {
> + printf ("]");
> + return;
> + }
> +
> + void *ctx_quote = talloc_new (NULL);
> + int i;
> + for (i = 0; i < g_mime_signature_list_length (siglist); i++) {
> + GMimeSignature *signature = g_mime_signature_list_get_signature (siglist, i);
> +
> + if (i > 0)
> + printf (", ");
> +
> + printf ("{");
> +
> + /* status */
> + GMimeSignatureStatus status = g_mime_signature_get_status (signature);
> + printf ("\"status\": %s",
> + json_quote_str (ctx_quote,
> + signature_status_to_string (status)));
> +
> + GMimeCertificate *certificate = g_mime_signature_get_certificate (signature);
> + if (status == GMIME_SIGNATURE_STATUS_GOOD) {
> + if (certificate)
> + printf (", \"fingerprint\": %s", json_quote_str (ctx_quote, g_mime_certificate_get_fingerprint (certificate)));
> + /* these dates are seconds since the epoch; should we
> + * provide a more human-readable format string? */
> + time_t created = g_mime_signature_get_created (signature);
> + if (created != -1)
> + printf (", \"created\": %d", (int) created);
> + time_t expires = g_mime_signature_get_expires (signature);
> + if (expires > 0)
> + printf (", \"expires\": %d", (int) expires);
> + /* output user id only if validity is FULL or ULTIMATE. */
> + /* note that gmime is using the term "trust" here, which
> + * is WRONG. It's actually user id "validity". */
> + if (certificate) {
> + const char *name = g_mime_certificate_get_name (certificate);
> + GMimeCertificateTrust trust = g_mime_certificate_get_trust (certificate);
> + if (name && (trust == GMIME_CERTIFICATE_TRUST_FULLY || trust == GMIME_CERTIFICATE_TRUST_ULTIMATE))
> + printf (", \"userid\": %s", json_quote_str (ctx_quote, name));
> + }
> + } else if (certificate) {
> + const char *key_id = g_mime_certificate_get_key_id (certificate);
> + if (key_id)
> + printf (", \"keyid\": %s", json_quote_str (ctx_quote, key_id));
> + }
> +
> + GMimeSignatureError errors = g_mime_signature_get_errors (signature);
> + if (errors != GMIME_SIGNATURE_ERROR_NONE) {
> + printf (", \"errors\": %d", errors);
> + }
> +
> + printf ("}");
> + }
> +
> + printf ("]");
> +
> + talloc_free (ctx_quote);
> +}
> +#else
> static void
> format_part_sigstatus_json (const GMimeSignatureValidity* validity)
> {
> @@ -652,6 +739,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
>
> talloc_free (ctx_quote);
> }
> +#endif
>
> static void
> format_part_content_json (GMimeObject *part)
> @@ -990,13 +1078,20 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
> (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
> if (params.cryptoctx == NULL) {
> +#ifdef GMIME_ATLEAST_26
> + /* TODO: GMimePasswordRequestFunc */
> + if (NULL == (params.cryptoctx = g_mime_gpg_context_new(NULL, "gpg")))
> +#else
> GMimeSession* session = g_object_new(g_mime_session_get_type(), NULL);
> if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, "gpg")))
> +#endif
> fprintf (stderr, "Failed to construct gpg context.\n");
> else
> g_mime_gpg_context_set_always_trust((GMimeGpgContext*)params.cryptoctx, FALSE);
> +#ifndef GMIME_ATLEAST_26
> g_object_unref (session);
> session = NULL;
> +#endif
> }
> if (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)
> params.decrypt = 1;
> diff --git a/show-message.c b/show-message.c
> index 8768889..83ecf81 100644
> --- a/show-message.c
> +++ b/show-message.c
> @@ -48,7 +48,11 @@ show_message_part (mime_node_t *node,
> format->part_encstatus (node->decrypt_success);
>
> if (node->verify_attempted && format->part_sigstatus)
> +#ifdef GMIME_ATLEAST_26
> + format->part_sigstatus (node->sig_list);
> +#else
> format->part_sigstatus (node->sig_validity);
> +#endif
>
> format->part_content (part);
>
> diff --git a/test/crypto b/test/crypto
> index 0af4aa8..3779abc 100755
> --- a/test/crypto
> +++ b/test/crypto
> @@ -104,6 +104,8 @@ test_expect_equal \
> "$expected"
>
> test_begin_subtest "signature verification with signer key unavailable"
> +# this is broken with current versions of gmime-2.6
> +(ldd $(which notmuch) | grep -q gmime-2.6) && test_subtest_known_broken
Just to be nitpicky, you should either escape the . in the regexp or
pass -F to grep. Otherwise I think this hack is fine (though it might
have to get a little fancier once GMime fixes this bug).
> # move the gnupghome temporarily out of the way
> mv "${GNUPGHOME}"{,.bak}
> output=$(notmuch show --format=json --verify subject:"test signed message 001" \
More information about the notmuch
mailing list