[PATCH] Add pseudo-compatibility with gmime 2.6

[PATCH] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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@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
fails (signature verification with signer key unavailable) but this will be hard
to fix since the new API does not report the reason why a signature verification
fails (other than the human-readable error message).
---
 mime-node.c      |   50 ++++++++++++++++++++++++++-
 notmuch-client.h |   27 ++++++++++++++-
 notmuch-reply.c  |    7 ++++
 notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 show-message.c   |    4 ++
 5 files changed, 181 insertions(+), 4 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d26bb44..ae2473d 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_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_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_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,23 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 	    GMimeMultipartEncrypted *encrypteddata =
 		GMIME_MULTIPART_ENCRYPTED (part);
 	    node->decrypt_attempted = TRUE;
+#ifdef GMIME_26
+	    GMimeDecryptResult *decrypt_result = g_mime_decrypt_result_new ();
+	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
+		(encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
+	    if (node->decrypted_child) {
+		node->decrypt_success = node->verify_attempted = TRUE;
+		node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
+		if (!node->sig_list)
+		    fprintf (stderr, "Failed to get signatures: %s\n",
+			     (err ? err->message : "no error explanation given"));
+#else
 	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
 		(encrypteddata, node->ctx->cryptoctx, &err);
 	    if (node->decrypted_child) {
 		node->decrypt_success = node->verify_attempted = TRUE;
 		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 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 		     "(must be exactly 2)\n",
 		     node->nchildren);
 	} else {
+#ifdef GMIME_26
+	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
+		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
+	    node->verify_attempted = TRUE;
+	    node->sig_list = sig_list;
+	    if (sig_list) {
+		GMimeSignatureList **proxy =
+		    talloc (node, GMimeSignatureList *);
+		*proxy = sig_list;
+		talloc_set_destructor (proxy, _signature_list_free);
+	    }
+#else
 	    /* For some reason the GMimeSignatureValidity returned
 	     * here is not a const (inconsistent with that
 	     * returned by
@@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 		*proxy = sig_validity;
 		talloc_set_destructor (proxy, _signature_validity_free);
 	    }
+#endif
 	}
     }
 
+#ifdef GMIME_26
+    if (node->verify_attempted && !node->sig_list)
+#else
     if (node->verify_attempted && !node->sig_validity)
+#endif
 	fprintf (stderr, "Failed to verify signed part: %s\n",
 		 (err ? err->message : "no error explanation given"));
 
diff --git a/notmuch-client.h b/notmuch-client.h
index 517c010..e85f882 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -30,6 +30,12 @@
 
 #include <gmime/gmime.h>
 
+/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
+ * GMIME_MAJOR_VERSION and friends. */
+#ifdef GMIME_MAJOR_VERSION
+#define GMIME_26
+#endif
+
 #include "notmuch.h"
 
 /* This is separate from notmuch-private.h because we're trying to
@@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
     void (*part_start) (GMimeObject *part,
 			int *part_count);
     void (*part_encstatus) (int status);
+#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
     int entire_thread;
     int raw;
     int part;
+#ifdef GMIME_26
+    GMimeCryptoContext* cryptoctx;
+#else
     GMimeCipherContext* cryptoctx;
+#endif
     int decrypt;
 } notmuch_show_params_t;
 
@@ -286,7 +300,12 @@ typedef struct mime_node {
      * signature.  May be NULL if signature verification failed.  If
      * there are simply no signatures, this will be non-NULL with an
      * empty signers list. */
+#ifdef GMIME_26
+    /* TODO: update the above comment... */
+    GMimeSignatureList* sig_list;
+#else
     const GMimeSignatureValidity *sig_validity;
+#endif
 
     /* Internal: Context inherited from the root iterator. */
     struct mime_node_context *ctx;
@@ -311,8 +330,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_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 da3acce..dc37c51 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_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_26
 	g_object_unref (session);
+#endif
     }
 
     config = notmuch_config_open (ctx, NULL, NULL);
diff --git a/notmuch-show.c b/notmuch-show.c
index d14dac9..263ab72 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_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_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,75 @@ format_part_encstatus_json (int status)
     printf ("}]");
 }
 
+#ifdef GMIME_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\": %x", errors);
+	}
+
+	printf ("}");
+     }
+
+    printf ("]");
+
+    talloc_free (ctx_quote);
+}
+#else
 static void
 format_part_sigstatus_json (const GMimeSignatureValidity* validity)
 {
@@ -652,6 +741,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
 
     talloc_free (ctx_quote);
 }
+#endif
 
 static void
 format_part_content_json (GMimeObject *part)
@@ -990,13 +1080,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_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_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..65269fd 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_26
+	format->part_sigstatus (node->sig_list);
+#else
 	format->part_sigstatus (node->sig_validity);
+#endif
 
     format->part_content (part);
 
-- 
1.7.8.3

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Tim Stoakes]

Thomas Jost(schnouki@schnouki.net)@170112-00:56:
> 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@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
> fails (signature verification with signer key unavailable) but this will be hard
> to fix since the new API does not report the reason why a signature verification
> fails (other than the human-readable error message).

+1 from me.

I literally just coded up a similar patch, and the first email I see
with the newly compiled notmuch is this one. I've now tried Thomas's
(better) patch with gmime-2.6.4 and notmuch
7ddd849015759a329bf8fef8c8b5a93359408962, and can confirm it works fine
for me. This build breakage is otherwise quite painful.

Thanks Thomas.

Tim

-- 
Tim Stoakes

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Kazuo Teramoto]

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

On Tue, Jan 17, 2012 at 12:56:39AM +0100, Thomas Jost wrote:
> 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.

I tested this with gmime 2.4.31 and 2.6.4. Works for me.

In a related note: Arch Linux started to ship gmime 2.6.4.

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

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Austin Clements]

Quoth Thomas Jost on Jan 17 at 12:56 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.

Awesome.  Comments inline below.

> This is mostly based on id:"8762i8hrb9.fsf@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
> fails (signature verification with signer key unavailable) but this will be hard
> to fix since the new API does not report the reason why a signature verification
> fails (other than the human-readable error message).

What is the result of this failing test?

> ---
>  mime-node.c      |   50 ++++++++++++++++++++++++++-
>  notmuch-client.h |   27 ++++++++++++++-
>  notmuch-reply.c  |    7 ++++
>  notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  show-message.c   |    4 ++
>  5 files changed, 181 insertions(+), 4 deletions(-)
> 
> diff --git a/mime-node.c b/mime-node.c
> index d26bb44..ae2473d 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_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_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_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,23 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
>  	    GMimeMultipartEncrypted *encrypteddata =
>  		GMIME_MULTIPART_ENCRYPTED (part);
>  	    node->decrypt_attempted = TRUE;
> +#ifdef GMIME_26
> +	    GMimeDecryptResult *decrypt_result = g_mime_decrypt_result_new ();

I think g_mime_multipart_encrypted_decrypt allocates the
GMimeDecryptResult for you, so this will just leak memory.

> +	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
> +		(encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
> +	    if (node->decrypted_child) {
> +		node->decrypt_success = node->verify_attempted = TRUE;
> +		node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
> +		if (!node->sig_list)
> +		    fprintf (stderr, "Failed to get signatures: %s\n",
> +			     (err ? err->message : "no error explanation given"));

My understanding is that g_mime_decrypt_result_get_signatures returns
NULL if there are no signatures and that this isn't an error.  This
differs from 2.4, which would return an empty but non-NULL list.

Also, I believe you have to free the sig_list in both branches now,
which means the talloc_set_destructor can be moved to common logic
outside of the if decrypted/signed.

> +#else
>  	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
>  		(encrypteddata, node->ctx->cryptoctx, &err);
>  	    if (node->decrypted_child) {
>  		node->decrypt_success = node->verify_attempted = TRUE;
>  		node->sig_validity = g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
> +#endif

It's confusing to have the open braces in the #ifdef'd region with a
matching close brace outside of it (and I imagine this confuses
editors and uncrustify, too).  You could either copy the else part in
both branches of the #ifdef or avoid duplicated code with something
like

#ifdef GMIME_26
  .. node->decrypted_child = ..
#else
  .. node->decrypted_child = ..
#endif
  if (node->decrypted_child) {
    node->decrypt_success = node->verify_attempted = TRUE;
#ifdef GMIME_26
    node->sig_list = ..
#else
    node->sig_validity = ..
#endif
  } else {
    fprintf (stderr, ..);
  }

>  	    } else {
>  		fprintf (stderr, "Failed to decrypt part: %s\n",
>  			 (err ? err->message : "no error explanation given"));
> @@ -182,6 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
>  		     "(must be exactly 2)\n",
>  		     node->nchildren);
>  	} else {
> +#ifdef GMIME_26
> +	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> +		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> +	    node->verify_attempted = TRUE;
> +	    node->sig_list = sig_list;
> +	    if (sig_list) {
> +		GMimeSignatureList **proxy =
> +		    talloc (node, GMimeSignatureList *);
> +		*proxy = sig_list;
> +		talloc_set_destructor (proxy, _signature_list_free);
> +	    }
> +#else
>  	    /* For some reason the GMimeSignatureValidity returned
>  	     * here is not a const (inconsistent with that
>  	     * returned by
> @@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
>  		*proxy = sig_validity;
>  		talloc_set_destructor (proxy, _signature_validity_free);
>  	    }
> +#endif
>  	}
>      }
>  
> +#ifdef GMIME_26
> +    if (node->verify_attempted && !node->sig_list)

Hmm.  This is correct for signed parts, but will incorrectly trigger
for an encrypted part with no signatures.  For 2.6, I think this error
checking may have to move into the branches of the if encrypted/signed
since for encrypted parts you have to check if
g_mime_multipart_encrypted_decrypt returned NULL.

> +#else
>      if (node->verify_attempted && !node->sig_validity)
> +#endif
>  	fprintf (stderr, "Failed to verify signed part: %s\n",
>  		 (err ? err->message : "no error explanation given"));
>  
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 517c010..e85f882 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -30,6 +30,12 @@
>  
>  #include <gmime/gmime.h>
>  
> +/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
> + * GMIME_MAJOR_VERSION and friends. */

Hah.

> +#ifdef GMIME_MAJOR_VERSION
> +#define GMIME_26
> +#endif
> +
>  #include "notmuch.h"
>  
>  /* This is separate from notmuch-private.h because we're trying to
> @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
>      void (*part_start) (GMimeObject *part,
>  			int *part_count);
>      void (*part_encstatus) (int status);
> +#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
>      int entire_thread;
>      int raw;
>      int part;
> +#ifdef GMIME_26
> +    GMimeCryptoContext* cryptoctx;
> +#else
>      GMimeCipherContext* cryptoctx;
> +#endif
>      int decrypt;
>  } notmuch_show_params_t;
>  
> @@ -286,7 +300,12 @@ typedef struct mime_node {
>       * signature.  May be NULL if signature verification failed.  If
>       * there are simply no signatures, this will be non-NULL with an
>       * empty signers list. */
> +#ifdef GMIME_26
> +    /* TODO: update the above comment... */

Since this behaves very differently in 2.6, I think documenting it is
important (and being very careful about the differences).  Maybe

/* The list of signatures for signed or encrypted containers.  If
 * there are no signatures, this will be NULL. */

> +    GMimeSignatureList* sig_list;
> +#else
>      const GMimeSignatureValidity *sig_validity;
> +#endif
>  
>      /* Internal: Context inherited from the root iterator. */
>      struct mime_node_context *ctx;
> @@ -311,8 +330,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_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 da3acce..dc37c51 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_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_26
>  	g_object_unref (session);
> +#endif
>      }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d14dac9..263ab72 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_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_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,75 @@ format_part_encstatus_json (int status)
>      printf ("}]");
>  }
>  
> +#ifdef GMIME_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) {

Style nit: notmuch uses 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)
> +	{

Style nit: break after brace.

(Presumably this is copied from the existing
format_part_sigstatus_json, but since it's technically new code
there's no reason not to fix these up.)

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

Is it intentional that the two above checks are different?  I would
think the second should be expires != -1.

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

Break after brace.

> +		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\": %x", errors);

This should be %d (I would say 0x%x, but JSON doesn't support hex
literals).  I see this bug came from the original
format_part_sigstatus_json.  Maybe there should be a quick patch
before this one that fixes the source of the bug?

> +	}
> +
> +	printf ("}");
> +     }
> +
> +    printf ("]");
> +
> +    talloc_free (ctx_quote);
> +}
> +#else
>  static void
>  format_part_sigstatus_json (const GMimeSignatureValidity* validity)
>  {
> @@ -652,6 +741,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
>  
>      talloc_free (ctx_quote);
>  }
> +#endif
>  
>  static void
>  format_part_content_json (GMimeObject *part)
> @@ -990,13 +1080,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_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_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..65269fd 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_26
> +	format->part_sigstatus (node->sig_list);
> +#else
>  	format->part_sigstatus (node->sig_validity);
> +#endif
>  
>      format->part_content (part);
>  

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Adrian Perez]

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


Hi,

On Tue, 17 Jan 2012 00:56:39 +0100, Thomas Jost <schnouki@schnouki.net> wrote:

> 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 approach is better than just dropping support for 2.4 (as my patch
does), so I would favor integrating this one instead of mine :D

Br.

-- 
Adrian Perez <aperez@igalia.com> - Sent from my toaster
Igalia - Free Software Engineering

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

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Adrian Perez]

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


Hi, 

Just a couple of comments inline :)

On Mon, 16 Jan 2012 22:47:14 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Thomas Jost on Jan 17 at 12:56 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.
> 
> Awesome.  Comments inline below.
> 
> > This is mostly based on id:"8762i8hrb9.fsf@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
> > fails (signature verification with signer key unavailable) but this will be hard
> > to fix since the new API does not report the reason why a signature verification
> > fails (other than the human-readable error message).
> 
> What is the result of this failing test?
> 
> > ---
> >  mime-node.c      |   50 ++++++++++++++++++++++++++-
> >  notmuch-client.h |   27 ++++++++++++++-
> >  notmuch-reply.c  |    7 ++++
> >  notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  show-message.c   |    4 ++
> >  5 files changed, 181 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mime-node.c b/mime-node.c
> > index d26bb44..ae2473d 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_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_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_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,23 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  	    GMimeMultipartEncrypted *encrypteddata =
> >  		GMIME_MULTIPART_ENCRYPTED (part);
> >  	    node->decrypt_attempted = TRUE;
> > +#ifdef GMIME_26
> > +	    GMimeDecryptResult *decrypt_result = g_mime_decrypt_result_new ();
> 
> I think g_mime_multipart_encrypted_decrypt allocates the
> GMimeDecryptResult for you, so this will just leak memory.

Yes, this is a leak. The documentation says that the function is
responsible of allocating the GMimeDecryptResult. The g_object_unref()
is still needed in notmuch's code to free it, though.

See http://git.gnome.org/browse/gmime/tree/gmime/gmime-multipart-encrypted.c#n282

> > +	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
> > +		(encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
> > +	    if (node->decrypted_child) {
> > +		node->decrypt_success = node->verify_attempted = TRUE;
> > +		node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
> > +		if (!node->sig_list)
> > +		    fprintf (stderr, "Failed to get signatures: %s\n",
> > +			     (err ? err->message : "no error explanation given"));
> 
> My understanding is that g_mime_decrypt_result_get_signatures returns
> NULL if there are no signatures and that this isn't an error.  This
> differs from 2.4, which would return an empty but non-NULL list.
> 
> Also, I believe you have to free the sig_list in both branches now,
> which means the talloc_set_destructor can be moved to common logic
> outside of the if decrypted/signed.
> 
> > +#else
> >  	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
> >  		(encrypteddata, node->ctx->cryptoctx, &err);
> >  	    if (node->decrypted_child) {
> >  		node->decrypt_success = node->verify_attempted = TRUE;
> >  		node->sig_validity = g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
> > +#endif
> 
> It's confusing to have the open braces in the #ifdef'd region with a
> matching close brace outside of it (and I imagine this confuses
> editors and uncrustify, too).  You could either copy the else part in
> both branches of the #ifdef or avoid duplicated code with something
> like
> 
> #ifdef GMIME_26
>   .. node->decrypted_child = ..
> #else
>   .. node->decrypted_child = ..
> #endif
>   if (node->decrypted_child) {
>     node->decrypt_success = node->verify_attempted = TRUE;
> #ifdef GMIME_26
>     node->sig_list = ..
> #else
>     node->sig_validity = ..
> #endif
>   } else {
>     fprintf (stderr, ..);
>   }
> 
> >  	    } else {
> >  		fprintf (stderr, "Failed to decrypt part: %s\n",
> >  			 (err ? err->message : "no error explanation given"));
> > @@ -182,6 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  		     "(must be exactly 2)\n",
> >  		     node->nchildren);
> >  	} else {
> > +#ifdef GMIME_26
> > +	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> > +		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> > +	    node->verify_attempted = TRUE;
> > +	    node->sig_list = sig_list;
> > +	    if (sig_list) {
> > +		GMimeSignatureList **proxy =
> > +		    talloc (node, GMimeSignatureList *);
> > +		*proxy = sig_list;
> > +		talloc_set_destructor (proxy, _signature_list_free);
> > +	    }
> > +#else
> >  	    /* For some reason the GMimeSignatureValidity returned
> >  	     * here is not a const (inconsistent with that
> >  	     * returned by
> > @@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  		*proxy = sig_validity;
> >  		talloc_set_destructor (proxy, _signature_validity_free);
> >  	    }
> > +#endif
> >  	}
> >      }
> >  
> > +#ifdef GMIME_26
> > +    if (node->verify_attempted && !node->sig_list)
> 
> Hmm.  This is correct for signed parts, but will incorrectly trigger
> for an encrypted part with no signatures.  For 2.6, I think this error
> checking may have to move into the branches of the if encrypted/signed
> since for encrypted parts you have to check if
> g_mime_multipart_encrypted_decrypt returned NULL.
> 
> > +#else
> >      if (node->verify_attempted && !node->sig_validity)
> > +#endif
> >  	fprintf (stderr, "Failed to verify signed part: %s\n",
> >  		 (err ? err->message : "no error explanation given"));
> >  
> > diff --git a/notmuch-client.h b/notmuch-client.h
> > index 517c010..e85f882 100644
> > --- a/notmuch-client.h
> > +++ b/notmuch-client.h
> > @@ -30,6 +30,12 @@
> >  
> >  #include <gmime/gmime.h>
> >  
> > +/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
> > + * GMIME_MAJOR_VERSION and friends. */
> 
> Hah.
> 
> > +#ifdef GMIME_MAJOR_VERSION
> > +#define GMIME_26
> > +#endif
> > +
> >  #include "notmuch.h"
> >  
> >  /* This is separate from notmuch-private.h because we're trying to
> > @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
> >      void (*part_start) (GMimeObject *part,
> >  			int *part_count);
> >      void (*part_encstatus) (int status);
> > +#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
> >      int entire_thread;
> >      int raw;
> >      int part;
> > +#ifdef GMIME_26
> > +    GMimeCryptoContext* cryptoctx;
> > +#else
> >      GMimeCipherContext* cryptoctx;
> > +#endif
> >      int decrypt;
> >  } notmuch_show_params_t;
> >  
> > @@ -286,7 +300,12 @@ typedef struct mime_node {
> >       * signature.  May be NULL if signature verification failed.  If
> >       * there are simply no signatures, this will be non-NULL with an
> >       * empty signers list. */
> > +#ifdef GMIME_26
> > +    /* TODO: update the above comment... */
> 
> Since this behaves very differently in 2.6, I think documenting it is
> important (and being very careful about the differences).  Maybe
> 
> /* The list of signatures for signed or encrypted containers.  If
>  * there are no signatures, this will be NULL. */
> 
> > +    GMimeSignatureList* sig_list;
> > +#else
> >      const GMimeSignatureValidity *sig_validity;
> > +#endif
> >  
> >      /* Internal: Context inherited from the root iterator. */
> >      struct mime_node_context *ctx;
> > @@ -311,8 +330,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_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 da3acce..dc37c51 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_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_26
> >  	g_object_unref (session);
> > +#endif
> >      }
> >  
> >      config = notmuch_config_open (ctx, NULL, NULL);
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index d14dac9..263ab72 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_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_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,75 @@ format_part_encstatus_json (int status)
> >      printf ("}]");
> >  }
> >  
> > +#ifdef GMIME_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) {
> 
> Style nit: notmuch uses 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)
> > +	{
> 
> Style nit: break after brace.
> 
> (Presumably this is copied from the existing
> format_part_sigstatus_json, but since it's technically new code
> there's no reason not to fix these up.)
> 
> > +	    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);
> 
> Is it intentional that the two above checks are different?  I would
> think the second should be expires != -1.

Yes, it should check for “expires != -1”. Also, wouldn't it be needed to
cast do “expires != (time_t) -1” to avoid compiler warnings? (time_t may
be different from int.)
 
> > +	    /* 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)
> > +	    {
> 
> Break after brace.
> 
> > +		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\": %x", errors);
> 
> This should be %d (I would say 0x%x, but JSON doesn't support hex
> literals).  I see this bug came from the original
> format_part_sigstatus_json.  Maybe there should be a quick patch
> before this one that fixes the source of the bug?
> 
> > +	}
> > +
> > +	printf ("}");
> > +     }
> > +
> > +    printf ("]");
> > +
> > +    talloc_free (ctx_quote);
> > +}
> > +#else
> >  static void
> >  format_part_sigstatus_json (const GMimeSignatureValidity* validity)
> >  {
> > @@ -652,6 +741,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
> >  
> >      talloc_free (ctx_quote);
> >  }
> > +#endif
> >  
> >  static void
> >  format_part_content_json (GMimeObject *part)
> > @@ -990,13 +1080,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_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_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..65269fd 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_26
> > +	format->part_sigstatus (node->sig_list);
> > +#else
> >  	format->part_sigstatus (node->sig_validity);
> > +#endif
> >  
> >      format->part_content (part);
> >  

-- 
Adrian Perez <aperez@igalia.com> - Sent from my toaster
Igalia - Free Software Engineering

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

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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

On Mon, 16 Jan 2012 22:47:14 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Thomas Jost on Jan 17 at 12:56 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.
> 
> Awesome.  Comments inline below.

Thanks for reviewing this so quickly. Replies inline, and new patches
incoming soon.


> > This is mostly based on id:"8762i8hrb9.fsf@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
> > fails (signature verification with signer key unavailable) but this will be hard
> > to fix since the new API does not report the reason why a signature verification
> > fails (other than the human-readable error message).
> 
> What is the result of this failing test?

The test looks like that:

        FAIL   signature verification with signer key unavailable
           --- crypto.4.expected   2012-01-16 23:05:06.765651183 +0000
           +++ crypto.4.output     2012-01-16 23:05:06.765651183 +0000
           @@ -12,9 +12,7 @@
             "Bcc": "",
             "Date": "01 Jan 2000 12:00:00 -0000"},
             "body": [{"id": 1,
           - "sigstatus": [{"status": "error",
           - "keyid": "6D92612D94E46381",
           - "errors": 2}],
           + "sigstatus": [],
             "content-type": "multipart/signed",
             "content": [{"id": 2,
             "content-type": "text/plain",
       Failed to verify signed part: gpg: keyblock resource `/home/schnouki/dev/notmuch/test/tmp.crypto/gnupg/pubring.gpg': file open error
       gpg: armor header: Version: GnuPG v1.4.11 (GNU/Linux)
       gpg: Signature made Mon Jan 16 23:05:06 2012 UTC using RSA key ID 94E46381
       gpg: Can't check signature: public key not found

So basically if a signer public key is missing, we can't get the status
signature: empty "sigstatus" field in the JSON output, "Unknown
signature status" in Emacs.

IMHO this is a bug in gmime 2.6, and I think I know what causes it. I'll
file a bug in gmime and hopefully they'll find a cleaner way to fix it
than the one I came up with :)

> 
> > ---
> >  mime-node.c      |   50 ++++++++++++++++++++++++++-
> >  notmuch-client.h |   27 ++++++++++++++-
> >  notmuch-reply.c  |    7 ++++
> >  notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  show-message.c   |    4 ++
> >  5 files changed, 181 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mime-node.c b/mime-node.c
> > index d26bb44..ae2473d 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_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_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_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,23 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  	    GMimeMultipartEncrypted *encrypteddata =
> >  		GMIME_MULTIPART_ENCRYPTED (part);
> >  	    node->decrypt_attempted = TRUE;
> > +#ifdef GMIME_26
> > +	    GMimeDecryptResult *decrypt_result = g_mime_decrypt_result_new ();
> 
> I think g_mime_multipart_encrypted_decrypt allocates the
> GMimeDecryptResult for you, so this will just leak memory.

You're right, fixed. Will it be freed along with node->decrypted_child,
or do we need to handle this ourself?

> 
> > +	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
> > +		(encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
> > +	    if (node->decrypted_child) {
> > +		node->decrypt_success = node->verify_attempted = TRUE;
> > +		node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
> > +		if (!node->sig_list)
> > +		    fprintf (stderr, "Failed to get signatures: %s\n",
> > +			     (err ? err->message : "no error explanation given"));
> 
> My understanding is that g_mime_decrypt_result_get_signatures returns
> NULL if there are no signatures and that this isn't an error.  This
> differs from 2.4, which would return an empty but non-NULL list.
>
> Also, I believe you have to free the sig_list in both branches now,
> which means the talloc_set_destructor can be moved to common logic
> outside of the if decrypted/signed.

Hmmm, you're right about g_mime_decrypt_result_get_signatures. I should
have RTFM.
Also moved talloc_set_destructor outside the if/else, thanks.

> 
> > +#else
> >  	    node->decrypted_child = g_mime_multipart_encrypted_decrypt
> >  		(encrypteddata, node->ctx->cryptoctx, &err);
> >  	    if (node->decrypted_child) {
> >  		node->decrypt_success = node->verify_attempted = TRUE;
> >  		node->sig_validity = g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
> > +#endif
> 
> It's confusing to have the open braces in the #ifdef'd region with a
> matching close brace outside of it (and I imagine this confuses
> editors and uncrustify, too).  You could either copy the else part in
> both branches of the #ifdef or avoid duplicated code with something
> like
> 
> #ifdef GMIME_26
>   .. node->decrypted_child = ..
> #else
>   .. node->decrypted_child = ..
> #endif
>   if (node->decrypted_child) {
>     node->decrypt_success = node->verify_attempted = TRUE;
> #ifdef GMIME_26
>     node->sig_list = ..
> #else
>     node->sig_validity = ..
> #endif
>   } else {
>     fprintf (stderr, ..);
>   }

Right. Avoids duplication and makes it easier to read.

> 
> >  	    } else {
> >  		fprintf (stderr, "Failed to decrypt part: %s\n",
> >  			 (err ? err->message : "no error explanation given"));
> > @@ -182,6 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  		     "(must be exactly 2)\n",
> >  		     node->nchildren);
> >  	} else {
> > +#ifdef GMIME_26
> > +	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> > +		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> > +	    node->verify_attempted = TRUE;
> > +	    node->sig_list = sig_list;
> > +	    if (sig_list) {
> > +		GMimeSignatureList **proxy =
> > +		    talloc (node, GMimeSignatureList *);
> > +		*proxy = sig_list;
> > +		talloc_set_destructor (proxy, _signature_list_free);
> > +	    }
> > +#else
> >  	    /* For some reason the GMimeSignatureValidity returned
> >  	     * here is not a const (inconsistent with that
> >  	     * returned by
> > @@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  		*proxy = sig_validity;
> >  		talloc_set_destructor (proxy, _signature_validity_free);
> >  	    }
> > +#endif
> >  	}
> >      }
> >  
> > +#ifdef GMIME_26
> > +    if (node->verify_attempted && !node->sig_list)
> 
> Hmm.  This is correct for signed parts, but will incorrectly trigger
> for an encrypted part with no signatures.  For 2.6, I think this error
> checking may have to move into the branches of the if encrypted/signed
> since for encrypted parts you have to check if
> g_mime_multipart_encrypted_decrypt returned NULL.

That sound right. The weird part is that it did not cause anything to
fail in the test suite...

Anyway, for 2.6 I moved this test into the "if signed" branch since it's
perfectly acceptable to have sig_list == NULL for an encrypted part.

> 
> > +#else
> >      if (node->verify_attempted && !node->sig_validity)
> > +#endif
> >  	fprintf (stderr, "Failed to verify signed part: %s\n",
> >  		 (err ? err->message : "no error explanation given"));
> >  
> > diff --git a/notmuch-client.h b/notmuch-client.h
> > index 517c010..e85f882 100644
> > --- a/notmuch-client.h
> > +++ b/notmuch-client.h
> > @@ -30,6 +30,12 @@
> >  
> >  #include <gmime/gmime.h>
> >  
> > +/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
> > + * GMIME_MAJOR_VERSION and friends. */
> 
> Hah.
> 
> > +#ifdef GMIME_MAJOR_VERSION
> > +#define GMIME_26
> > +#endif
> > +
> >  #include "notmuch.h"
> >  
> >  /* This is separate from notmuch-private.h because we're trying to
> > @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
> >      void (*part_start) (GMimeObject *part,
> >  			int *part_count);
> >      void (*part_encstatus) (int status);
> > +#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
> >      int entire_thread;
> >      int raw;
> >      int part;
> > +#ifdef GMIME_26
> > +    GMimeCryptoContext* cryptoctx;
> > +#else
> >      GMimeCipherContext* cryptoctx;
> > +#endif
> >      int decrypt;
> >  } notmuch_show_params_t;
> >  
> > @@ -286,7 +300,12 @@ typedef struct mime_node {
> >       * signature.  May be NULL if signature verification failed.  If
> >       * there are simply no signatures, this will be non-NULL with an
> >       * empty signers list. */
> > +#ifdef GMIME_26
> > +    /* TODO: update the above comment... */
> 
> Since this behaves very differently in 2.6, I think documenting it is
> important (and being very careful about the differences).  Maybe
> 
> /* The list of signatures for signed or encrypted containers.  If
>  * there are no signatures, this will be NULL. */

Added, thanks.

> 
> > +    GMimeSignatureList* sig_list;
> > +#else
> >      const GMimeSignatureValidity *sig_validity;
> > +#endif
> >  
> >      /* Internal: Context inherited from the root iterator. */
> >      struct mime_node_context *ctx;
> > @@ -311,8 +330,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_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 da3acce..dc37c51 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_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_26
> >  	g_object_unref (session);
> > +#endif
> >      }
> >  
> >      config = notmuch_config_open (ctx, NULL, NULL);
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index d14dac9..263ab72 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_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_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,75 @@ format_part_encstatus_json (int status)
> >      printf ("}]");
> >  }
> >  
> > +#ifdef GMIME_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) {
> 
> Style nit: notmuch uses i++.

Ack.

> 
> > +	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)
> > +	{
> 
> Style nit: break after brace.
> 
> (Presumably this is copied from the existing
> format_part_sigstatus_json, but since it's technically new code
> there's no reason not to fix these up.)

Ack too. (Copied from the original patch, which was probably copied from
the existing code...)

> 
> > +	    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);
> 
> Is it intentional that the two above checks are different?  I would
> think the second should be expires != -1.

It was so in the original patch, which caused one of the tests to fail.
-1 means "unknown", and AFAICT 0 means "never expires". The current
behaviour is to add the "expires" field only if there is an expiration
date, so we need to check if expires > 0.

> 
> > +	    /* 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)
> > +	    {
> 
> Break after brace.
> 
> > +		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\": %x", errors);
> 
> This should be %d (I would say 0x%x, but JSON doesn't support hex
> literals).  I see this bug came from the original
> format_part_sigstatus_json.  Maybe there should be a quick patch
> before this one that fixes the source of the bug?

Right. I'll add a first patch to change this in the original format_part_sigstatus_json.

> 
> > +	}
> > +
> > +	printf ("}");
> > +     }
> > +
> > +    printf ("]");
> > +
> > +    talloc_free (ctx_quote);
> > +}
> > +#else
> >  static void
> >  format_part_sigstatus_json (const GMimeSignatureValidity* validity)
> >  {
> > @@ -652,6 +741,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
> >  
> >      talloc_free (ctx_quote);
> >  }
> > +#endif
> >  
> >  static void
> >  format_part_content_json (GMimeObject *part)
> > @@ -990,13 +1080,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_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_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..65269fd 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_26
> > +	format->part_sigstatus (node->sig_list);
> > +#else
> >  	format->part_sigstatus (node->sig_validity);
> > +#endif
> >  
> >      format->part_content (part);
> >  

-- 
Thomas/Schnouki

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

[PATCH v2 1/2] show: don't use hex literals in JSON output

[Thomas Jost]

JSON does not support hex literals (0x..) so numbers must be formatted as %d
instead of %x.
---
 notmuch-show.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index d14dac9..91f566c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -641,7 +641,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
                printf (", \"keyid\": %s", json_quote_str (ctx_quote, signer->keyid));
        }
        if (signer->errors != GMIME_SIGNER_ERROR_NONE) {
-           printf (", \"errors\": %x", signer->errors);
+           printf (", \"errors\": %d", signer->errors);
        }
 
        printf ("}");
-- 
1.7.8.3

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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@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
fails (signature verification with signer key unavailable) but this will be hard
to fix since the new API does not report the reason why a signature verification
fails (other than the human-readable error message).
---
 mime-node.c      |   56 ++++++++++++++++++++++++++++++--
 notmuch-client.h |   28 +++++++++++++++-
 notmuch-reply.c  |    7 ++++
 notmuch-show.c   |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 show-message.c   |    4 ++
 5 files changed, 185 insertions(+), 5 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d26bb44..e575e1c 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_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_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_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,22 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 	    GMimeMultipartEncrypted *encrypteddata =
 		GMIME_MULTIPART_ENCRYPTED (part);
 	    node->decrypt_attempted = TRUE;
+#ifdef GMIME_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;
+		node->decrypt_success = node->verify_attempted =TRUE;
+#ifdef GMIME_26
+		/* This may be NULL if the part is not signed. */
+		node->sig_list = g_mime_decrypt_result_get_signatures (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 +210,16 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 		     "(must be exactly 2)\n",
 		     node->nchildren);
 	} else {
+#ifdef GMIME_26
+	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
+		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
+	    node->verify_attempted = TRUE;
+	    node->sig_list = sig_list;
+
+	    if (!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
@@ -200,12 +238,24 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 		*proxy = sig_validity;
 		talloc_set_destructor (proxy, _signature_validity_free);
 	    }
+#endif
 	}
     }
 
+#ifdef GMIME_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 *);
+	*proxy = node->sig_list;
+	talloc_set_destructor (proxy, _signature_list_free);
+    }
+#else
     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..9167042 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -30,6 +30,12 @@
 
 #include <gmime/gmime.h>
 
+/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
+ * GMIME_MAJOR_VERSION and friends. */
+#ifdef GMIME_MAJOR_VERSION
+#define GMIME_26
+#endif
+
 #include "notmuch.h"
 
 /* This is separate from notmuch-private.h because we're trying to
@@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
     void (*part_start) (GMimeObject *part,
 			int *part_count);
     void (*part_encstatus) (int status);
+#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
     int entire_thread;
     int raw;
     int part;
+#ifdef GMIME_26
+    GMimeCryptoContext* cryptoctx;
+#else
     GMimeCipherContext* cryptoctx;
+#endif
     int decrypt;
 } notmuch_show_params_t;
 
@@ -290,11 +304,17 @@ typedef struct mime_node {
 
     /* True if signature verification on this part was attempted. */
     notmuch_bool_t verify_attempted;
+#ifdef GMIME_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 +339,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_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..b3d7127 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_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_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..10223e0 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_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_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_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_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_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..65269fd 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_26
+	format->part_sigstatus (node->sig_list);
+#else
 	format->part_sigstatus (node->sig_validity);
+#endif
 
     format->part_content (part);
 
-- 
1.7.8.3

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Tomi Ollila]

On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> 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@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
> fails (signature verification with signer key unavailable) but this will be hard
> to fix since the new API does not report the reason why a signature verification
> fails (other than the human-readable error message).
> ---

LGTM. Some whitespace things but most of those were there already;
I'd have one uncrustify round to be applied to the source and after
that be more strict about those...

actually gmime 2.4 has GMIME_CHECK_VERSION defined as
g_mime_check_version (major, minor, micro) so the comment about it
is not entirely accurate (it is unusable in our context, though)


Tomi

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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

On Tue, 17 Jan 2012 14:48:34 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> > 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@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
> > fails (signature verification with signer key unavailable) but this will be hard
> > to fix since the new API does not report the reason why a signature verification
> > fails (other than the human-readable error message).
> > ---
> 
> LGTM. Some whitespace things but most of those were there already;
> I'd have one uncrustify round to be applied to the source and after
> that be more strict about those...

Thanks! I'll fix the whitespace issues in these patches when the source
in Git is uncrustified then.

> actually gmime 2.4 has GMIME_CHECK_VERSION defined as
> g_mime_check_version (major, minor, micro) so the comment about it
> is not entirely accurate (it is unusable in our context, though)

Oops, yes. What I really meant is that GMIME_MAJOR_VERSION is not
available as a preprocessor constant in 2.4, and GMIME_CHECK_VERSION is
unusable in our context since it just calls a runtime function.

By the way, how do you guys feel about setting GMIME_26 in
notmuch-client.h? Is that good enough, or should it be done in configure
so that gcc is called with "-DGMIME_26"?

I filed a bug about gmime 2.6 incorrect handling of signatures with
missing public keys: https://bugzilla.gnome.org/show_bug.cgi?id=668085.
A patch is attached there, feel free to test and comment.

(Arch Linux users: I made a little PKGBUILD that includes this patch if
you want to build your own gmime 2.6.4:
http://fichiers.schnouki.net/tmp/gmime-2.6.4-1.src.tar.gz)

Best regards,

-- 
Thomas/Schnouki

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

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Austin Clements]

Quoth Thomas Jost on Jan 17 at 11:48 am:
> On Mon, 16 Jan 2012 22:47:14 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Thomas Jost on Jan 17 at 12:56 am:
> > > This is mostly based on id:"8762i8hrb9.fsf@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
> > > fails (signature verification with signer key unavailable) but this will be hard
> > > to fix since the new API does not report the reason why a signature verification
> > > fails (other than the human-readable error message).
> > 
> > What is the result of this failing test?
> 
> The test looks like that:
> 
>         FAIL   signature verification with signer key unavailable
>            --- crypto.4.expected   2012-01-16 23:05:06.765651183 +0000
>            +++ crypto.4.output     2012-01-16 23:05:06.765651183 +0000
>            @@ -12,9 +12,7 @@
>              "Bcc": "",
>              "Date": "01 Jan 2000 12:00:00 -0000"},
>              "body": [{"id": 1,
>            - "sigstatus": [{"status": "error",
>            - "keyid": "6D92612D94E46381",
>            - "errors": 2}],
>            + "sigstatus": [],
>              "content-type": "multipart/signed",
>              "content": [{"id": 2,
>              "content-type": "text/plain",
>        Failed to verify signed part: gpg: keyblock resource `/home/schnouki/dev/notmuch/test/tmp.crypto/gnupg/pubring.gpg': file open error
>        gpg: armor header: Version: GnuPG v1.4.11 (GNU/Linux)
>        gpg: Signature made Mon Jan 16 23:05:06 2012 UTC using RSA key ID 94E46381
>        gpg: Can't check signature: public key not found
> 
> So basically if a signer public key is missing, we can't get the status
> signature: empty "sigstatus" field in the JSON output, "Unknown
> signature status" in Emacs.
> 
> IMHO this is a bug in gmime 2.6, and I think I know what causes it. I'll
> file a bug in gmime and hopefully they'll find a cleaner way to fix it
> than the one I came up with :)

Oh, okay.  That does seem like a bug in GMime.  Do you think it would
be possible to mark this test as "broken" if notmuch is using GMime
2.6?  Off the top of my head, I can't think of an easy way to plumb
that information through to the test suite.  I don't think we should
push any patches that knowingly break a test, even if it's in just one
configuration.

> > 
> > > ---
> > >  mime-node.c      |   50 ++++++++++++++++++++++++++-
> > >  notmuch-client.h |   27 ++++++++++++++-
> > >  notmuch-reply.c  |    7 ++++
> > >  notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  show-message.c   |    4 ++
> > >  5 files changed, 181 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/mime-node.c b/mime-node.c
> > > index d26bb44..ae2473d 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_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_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_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,23 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> > >  	    GMimeMultipartEncrypted *encrypteddata =
> > >  		GMIME_MULTIPART_ENCRYPTED (part);
> > >  	    node->decrypt_attempted = TRUE;
> > > +#ifdef GMIME_26
> > > +	    GMimeDecryptResult *decrypt_result = g_mime_decrypt_result_new ();
> > 
> > I think g_mime_multipart_encrypted_decrypt allocates the
> > GMimeDecryptResult for you, so this will just leak memory.
> 
> You're right, fixed. Will it be freed along with node->decrypted_child,
> or do we need to handle this ourself?

That would be nice to know.  My guess would be that we have to free it
ourselves (or dereference it, at any rate), but the documentation
doesn't say.

> > >  	    } else {
> > >  		fprintf (stderr, "Failed to decrypt part: %s\n",
> > >  			 (err ? err->message : "no error explanation given"));
> > > @@ -182,6 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> > >  		     "(must be exactly 2)\n",
> > >  		     node->nchildren);
> > >  	} else {
> > > +#ifdef GMIME_26
> > > +	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> > > +		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> > > +	    node->verify_attempted = TRUE;
> > > +	    node->sig_list = sig_list;
> > > +	    if (sig_list) {
> > > +		GMimeSignatureList **proxy =
> > > +		    talloc (node, GMimeSignatureList *);
> > > +		*proxy = sig_list;
> > > +		talloc_set_destructor (proxy, _signature_list_free);
> > > +	    }
> > > +#else
> > >  	    /* For some reason the GMimeSignatureValidity returned
> > >  	     * here is not a const (inconsistent with that
> > >  	     * returned by
> > > @@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> > >  		*proxy = sig_validity;
> > >  		talloc_set_destructor (proxy, _signature_validity_free);
> > >  	    }
> > > +#endif
> > >  	}
> > >      }
> > >  
> > > +#ifdef GMIME_26
> > > +    if (node->verify_attempted && !node->sig_list)
> > 
> > Hmm.  This is correct for signed parts, but will incorrectly trigger
> > for an encrypted part with no signatures.  For 2.6, I think this error
> > checking may have to move into the branches of the if encrypted/signed
> > since for encrypted parts you have to check if
> > g_mime_multipart_encrypted_decrypt returned NULL.
> 
> That sound right. The weird part is that it did not cause anything to
> fail in the test suite...

It would be worth adding a test with an encrypted but unsigned part.
I don't know enough GPG myself to do that.

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Austin Clements]

Quoth Thomas Jost on Jan 17 at 11:50 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@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
> fails (signature verification with signer key unavailable) but this will be hard
> to fix since the new API does not report the reason why a signature verification
> fails (other than the human-readable error message).
> ---
>  mime-node.c      |   56 ++++++++++++++++++++++++++++++--
>  notmuch-client.h |   28 +++++++++++++++-
>  notmuch-reply.c  |    7 ++++
>  notmuch-show.c   |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  show-message.c   |    4 ++
>  5 files changed, 185 insertions(+), 5 deletions(-)
> 
> diff --git a/mime-node.c b/mime-node.c
> index d26bb44..e575e1c 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_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_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_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,22 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
>  	    GMimeMultipartEncrypted *encrypteddata =
>  		GMIME_MULTIPART_ENCRYPTED (part);
>  	    node->decrypt_attempted = TRUE;
> +#ifdef GMIME_26
> +	    GMimeDecryptResult *decrypt_result = NULL;

Reading through the GMime code, it looks like we do have to unref
decrypt_result.  I think this is easy, though.  Right after you call
g_mime_decrypt_result_get_signatures below, do:

  g_object_ref (node->sig_list);
  g_object_unref (decrypt_result);

> +	    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;
> +		node->decrypt_success = node->verify_attempted =TRUE;

Whitespace.  (There should be no diff on the above line)

> +#ifdef GMIME_26
> +		/* This may be NULL if the part is not signed. */
> +		node->sig_list = g_mime_decrypt_result_get_signatures (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 +210,16 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
>  		     "(must be exactly 2)\n",
>  		     node->nchildren);
>  	} else {
> +#ifdef GMIME_26
> +	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> +		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> +	    node->verify_attempted = TRUE;
> +	    node->sig_list = sig_list;

Just a nit.  This could be
  node->sig_list = g_mime_multipart_signed_verify ( ... )
To me, the local variable just makes this code more verbose without
adding anything.  Up to you.

> +
> +	    if (!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
> @@ -200,12 +238,24 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
>  		*proxy = sig_validity;
>  		talloc_set_destructor (proxy, _signature_validity_free);
>  	    }
> +#endif
>  	}
>      }
>  
> +#ifdef GMIME_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 *);

This doesn't need to be split into two lines.

> +	*proxy = node->sig_list;
> +	talloc_set_destructor (proxy, _signature_list_free);
> +    }
> +#else
>      if (node->verify_attempted && !node->sig_validity)
>  	fprintf (stderr, "Failed to verify signed part: %s\n",
>  		 (err ? err->message : "no error explanation given"));
> +#endif

I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
GMIME_26, since they aren't logical alternates of each other.

>  
>      if (err)
>  	g_error_free (err);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 62ede28..9167042 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -30,6 +30,12 @@
>  
>  #include <gmime/gmime.h>
>  
> +/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
> + * GMIME_MAJOR_VERSION and friends. */
> +#ifdef GMIME_MAJOR_VERSION
> +#define GMIME_26
> +#endif
> +
>  #include "notmuch.h"
>  
>  /* This is separate from notmuch-private.h because we're trying to
> @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
>      void (*part_start) (GMimeObject *part,
>  			int *part_count);
>      void (*part_encstatus) (int status);
> +#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
>      int entire_thread;
>      int raw;
>      int part;
> +#ifdef GMIME_26
> +    GMimeCryptoContext* cryptoctx;
> +#else
>      GMimeCipherContext* cryptoctx;
> +#endif
>      int decrypt;
>  } notmuch_show_params_t;
>  
> @@ -290,11 +304,17 @@ typedef struct mime_node {
>  
>      /* True if signature verification on this part was attempted. */
>      notmuch_bool_t verify_attempted;
> +#ifdef GMIME_26
> +    /* The list of signatures for signed or encrypted containers. If
> +      * there are no signatures, this will be NULL. */

Spacing.

> +    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 +339,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_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..b3d7127 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_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_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..10223e0 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_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_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_26
> +static void
> +format_part_sigstatus_json (GMimeSignatureList *siglist)
> +{
> +    printf (", \"sigstatus\": [");
> +
> +    if (!siglist) {
> +	printf ("]");
> +	return;
> +     }

Indentation.

> +
> +    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_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_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..65269fd 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_26
> +	format->part_sigstatus (node->sig_list);
> +#else
>  	format->part_sigstatus (node->sig_validity);
> +#endif
>  
>      format->part_content (part);
>  

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Tomi Ollila]

On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Thomas Jost on Jan 17 at 11:50 am:
> >  
> > +#ifdef GMIME_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 *);
> 
> This doesn't need to be split into two lines.
> 
> > +	*proxy = node->sig_list;
> > +	talloc_set_destructor (proxy, _signature_list_free);
> > +    }
> > +#else
> >      if (node->verify_attempted && !node->sig_validity)
> >  	fprintf (stderr, "Failed to verify signed part: %s\n",
> >  		 (err ? err->message : "no error explanation given"));
> > +#endif
> 
> I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
> GMIME_26, since they aren't logical alternates of each other.

That reminds me that it should then be like GMIME_ATLEAST_26, so
that this might be useful when GMIME > 2.6 is available...

Tomi

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Tom Prince]

On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> 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
> fails (signature verification with signer key unavailable) but this will be hard
> to fix since the new API does not report the reason why a signature verification
> fails (other than the human-readable error message).

How did you test against multiple versions? Using different machines? If
there was a way for configure (or something to pick the version, I would
setup the buildbot to test against both, so we don't regress either.

  Tom

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Austin Clements]

Quoth Tomi Ollila on Jan 18 at 10:15 am:
> On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Thomas Jost on Jan 17 at 11:50 am:
> > >  
> > > +#ifdef GMIME_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 *);
> > 
> > This doesn't need to be split into two lines.
> > 
> > > +	*proxy = node->sig_list;
> > > +	talloc_set_destructor (proxy, _signature_list_free);
> > > +    }
> > > +#else
> > >      if (node->verify_attempted && !node->sig_validity)
> > >  	fprintf (stderr, "Failed to verify signed part: %s\n",
> > >  		 (err ? err->message : "no error explanation given"));
> > > +#endif
> > 
> > I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
> > GMIME_26, since they aren't logical alternates of each other.
> 
> That reminds me that it should then be like GMIME_ATLEAST_26, so
> that this might be useful when GMIME > 2.6 is available...

Hopefully before GMIME 2.8 comes out, we'll be able to remove all of
the GMIME 2.4 compatibility.  But GMIME_ATLEAST_26 would be fine, too.

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Tomi Ollila]

On Wed, 18 Jan 2012 12:19:45 -0500, Tom Prince <tom.prince@ualberta.net> wrote:
> 
> How did you test against multiple versions? Using different machines? If
> there was a way for configure (or something to pick the version, I would
> setup the buildbot to test against both, so we don't regress either.

I currently compile notmuch on Fedora 15 so that I have 

LIBRARY_PATH=/my/own/path/to/x86_64-linux/lib
and
CPATH=/my/own/path/to/x86_64-linux/include

and gmime 2.4 development files are located in these
directories. I'll be hiding gmime 2.4 stuff from these
soon in order to build against 2.6 stuff.

Tomi

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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

Thanks for this review Austin. New version coming soon.

On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Thomas Jost on Jan 17 at 11:50 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@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
> > fails (signature verification with signer key unavailable) but this will be hard
> > to fix since the new API does not report the reason why a signature verification
> > fails (other than the human-readable error message).
> > ---
> >  mime-node.c      |   56 ++++++++++++++++++++++++++++++--
> >  notmuch-client.h |   28 +++++++++++++++-
> >  notmuch-reply.c  |    7 ++++
> >  notmuch-show.c   |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  show-message.c   |    4 ++
> >  5 files changed, 185 insertions(+), 5 deletions(-)
> > 
> > diff --git a/mime-node.c b/mime-node.c
> > index d26bb44..e575e1c 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_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_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_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,22 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  	    GMimeMultipartEncrypted *encrypteddata =
> >  		GMIME_MULTIPART_ENCRYPTED (part);
> >  	    node->decrypt_attempted = TRUE;
> > +#ifdef GMIME_26
> > +	    GMimeDecryptResult *decrypt_result = NULL;
> 
> Reading through the GMime code, it looks like we do have to unref
> decrypt_result.  I think this is easy, though.  Right after you call
> g_mime_decrypt_result_get_signatures below, do:
> 
>   g_object_ref (node->sig_list);
>   g_object_unref (decrypt_result);

Added, thanks!

> 
> > +	    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;
> > +		node->decrypt_success = node->verify_attempted =TRUE;
> 
> Whitespace.  (There should be no diff on the above line)

Oops, my bad.

> 
> > +#ifdef GMIME_26
> > +		/* This may be NULL if the part is not signed. */
> > +		node->sig_list = g_mime_decrypt_result_get_signatures (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 +210,16 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  		     "(must be exactly 2)\n",
> >  		     node->nchildren);
> >  	} else {
> > +#ifdef GMIME_26
> > +	    GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> > +		(GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> > +	    node->verify_attempted = TRUE;
> > +	    node->sig_list = sig_list;
> 
> Just a nit.  This could be
>   node->sig_list = g_mime_multipart_signed_verify ( ... )
> To me, the local variable just makes this code more verbose without
> adding anything.  Up to you.

Yep, the local variable is useless. Removed it.

> 
> > +
> > +	    if (!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
> > @@ -200,12 +238,24 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> >  		*proxy = sig_validity;
> >  		talloc_set_destructor (proxy, _signature_validity_free);
> >  	    }
> > +#endif
> >  	}
> >      }
> >  
> > +#ifdef GMIME_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 *);
> 
> This doesn't need to be split into two lines.

You're right. It was more readable when that piece of code was more
indented, but now one line is fine :)

> 
> > +	*proxy = node->sig_list;
> > +	talloc_set_destructor (proxy, _signature_list_free);
> > +    }
> > +#else
> >      if (node->verify_attempted && !node->sig_validity)
> >  	fprintf (stderr, "Failed to verify signed part: %s\n",
> >  		 (err ? err->message : "no error explanation given"));
> > +#endif
> 
> I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
> GMIME_26, since they aren't logical alternates of each other.

Agreed.

> 
> >  
> >      if (err)
> >  	g_error_free (err);
> > diff --git a/notmuch-client.h b/notmuch-client.h
> > index 62ede28..9167042 100644
> > --- a/notmuch-client.h
> > +++ b/notmuch-client.h
> > @@ -30,6 +30,12 @@
> >  
> >  #include <gmime/gmime.h>
> >  
> > +/* GMIME_CHECK_VERSION is only available in gmime >= 2.5. But so are
> > + * GMIME_MAJOR_VERSION and friends. */
> > +#ifdef GMIME_MAJOR_VERSION
> > +#define GMIME_26
> > +#endif
> > +
> >  #include "notmuch.h"
> >  
> >  /* This is separate from notmuch-private.h because we're trying to
> > @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {
> >      void (*part_start) (GMimeObject *part,
> >  			int *part_count);
> >      void (*part_encstatus) (int status);
> > +#ifdef GMIME_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 +93,11 @@ typedef struct notmuch_show_params {
> >      int entire_thread;
> >      int raw;
> >      int part;
> > +#ifdef GMIME_26
> > +    GMimeCryptoContext* cryptoctx;
> > +#else
> >      GMimeCipherContext* cryptoctx;
> > +#endif
> >      int decrypt;
> >  } notmuch_show_params_t;
> >  
> > @@ -290,11 +304,17 @@ typedef struct mime_node {
> >  
> >      /* True if signature verification on this part was attempted. */
> >      notmuch_bool_t verify_attempted;
> > +#ifdef GMIME_26
> > +    /* The list of signatures for signed or encrypted containers. If
> > +      * there are no signatures, this will be NULL. */
> 
> Spacing.
> 
> > +    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 +339,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_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..b3d7127 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_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_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..10223e0 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_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_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_26
> > +static void
> > +format_part_sigstatus_json (GMimeSignatureList *siglist)
> > +{
> > +    printf (", \"sigstatus\": [");
> > +
> > +    if (!siglist) {
> > +	printf ("]");
> > +	return;
> > +     }
> 
> Indentation.
> 
> > +
> > +    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_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_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..65269fd 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_26
> > +	format->part_sigstatus (node->sig_list);
> > +#else
> >  	format->part_sigstatus (node->sig_validity);
> > +#endif
> >  
> >      format->part_content (part);
> >  

-- 
Thomas/Schnouki

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

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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

On Wed, 18 Jan 2012 12:35:34 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Tomi Ollila on Jan 18 at 10:15 am:
> > On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > Quoth Thomas Jost on Jan 17 at 11:50 am:
> > > >  
> > > > +#ifdef GMIME_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 *);
> > > 
> > > This doesn't need to be split into two lines.
> > > 
> > > > +	*proxy = node->sig_list;
> > > > +	talloc_set_destructor (proxy, _signature_list_free);
> > > > +    }
> > > > +#else
> > > >      if (node->verify_attempted && !node->sig_validity)
> > > >  	fprintf (stderr, "Failed to verify signed part: %s\n",
> > > >  		 (err ? err->message : "no error explanation given"));
> > > > +#endif
> > > 
> > > I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
> > > GMIME_26, since they aren't logical alternates of each other.
> > 
> > That reminds me that it should then be like GMIME_ATLEAST_26, so
> > that this might be useful when GMIME > 2.6 is available...
> 
> Hopefully before GMIME 2.8 comes out, we'll be able to remove all of
> the GMIME 2.4 compatibility.  But GMIME_ATLEAST_26 would be fine, too.

Heh, maybe things will change again in 2.8 and "ATLEAST_26" will become
"ONLY_26"... But changed to GMIME_ATLEAST_26 anyway, thanks for the
suggestion :)

-- 
Thomas/Schnouki

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

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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

On Wed, 18 Jan 2012 20:00:12 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, 18 Jan 2012 12:19:45 -0500, Tom Prince <tom.prince@ualberta.net> wrote:
> > 
> > How did you test against multiple versions? Using different machines? If
> > there was a way for configure (or something to pick the version, I would
> > setup the buildbot to test against both, so we don't regress either.
> 
> I currently compile notmuch on Fedora 15 so that I have 
> 
> LIBRARY_PATH=/my/own/path/to/x86_64-linux/lib
> and
> CPATH=/my/own/path/to/x86_64-linux/include
> 
> and gmime 2.4 development files are located in these
> directories. I'll be hiding gmime 2.4 stuff from these
> soon in order to build against 2.6 stuff.
> 
> Tomi

For testing gmime 2.4 and 2.6, I just uninstalled 2.6 and reinstalled
2.4 (kept the binary package on purpose -- not a problem since notmuch
is the only package using gmime on my system).

When hacking the gmime git in order to fix
https://bugzilla.gnome.org/show_bug.cgi?id=668085, I set some
environment variables before calling "./configure" and building notmuch:

  LDFLAGS="-Wl,-rpath,/home/schnouki/dev/gmime/gmime/.libs" ./configure --prefix=/usr --sysconfdir=/etc
  make
  ldd ./notmuch

Regards,

-- 
Thomas/Schnouki

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

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Thomas Jost]

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

On Tue, 17 Jan 2012 15:38:36 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Thomas Jost on Jan 17 at 11:48 am:
> > On Mon, 16 Jan 2012 22:47:14 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > Quoth Thomas Jost on Jan 17 at 12:56 am:
> > > > This is mostly based on id:"8762i8hrb9.fsf@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
> > > > fails (signature verification with signer key unavailable) but this will be hard
> > > > to fix since the new API does not report the reason why a signature verification
> > > > fails (other than the human-readable error message).
> > > 
> > > What is the result of this failing test?
> > 
> > The test looks like that:
> > 
> >         FAIL   signature verification with signer key unavailable
> >            --- crypto.4.expected   2012-01-16 23:05:06.765651183 +0000
> >            +++ crypto.4.output     2012-01-16 23:05:06.765651183 +0000
> >            @@ -12,9 +12,7 @@
> >              "Bcc": "",
> >              "Date": "01 Jan 2000 12:00:00 -0000"},
> >              "body": [{"id": 1,
> >            - "sigstatus": [{"status": "error",
> >            - "keyid": "6D92612D94E46381",
> >            - "errors": 2}],
> >            + "sigstatus": [],
> >              "content-type": "multipart/signed",
> >              "content": [{"id": 2,
> >              "content-type": "text/plain",
> >        Failed to verify signed part: gpg: keyblock resource `/home/schnouki/dev/notmuch/test/tmp.crypto/gnupg/pubring.gpg': file open error
> >        gpg: armor header: Version: GnuPG v1.4.11 (GNU/Linux)
> >        gpg: Signature made Mon Jan 16 23:05:06 2012 UTC using RSA key ID 94E46381
> >        gpg: Can't check signature: public key not found
> > 
> > So basically if a signer public key is missing, we can't get the status
> > signature: empty "sigstatus" field in the JSON output, "Unknown
> > signature status" in Emacs.
> > 
> > IMHO this is a bug in gmime 2.6, and I think I know what causes it. I'll
> > file a bug in gmime and hopefully they'll find a cleaner way to fix it
> > than the one I came up with :)
> 
> Oh, okay.  That does seem like a bug in GMime.  Do you think it would
> be possible to mark this test as "broken" if notmuch is using GMime
> 2.6?  Off the top of my head, I can't think of an easy way to plumb
> that information through to the test suite.  I don't think we should
> push any patches that knowingly break a test, even if it's in just one
> configuration.

Here is how I did:

  (ldd notmuch | grep -q gmime-2.6) && test_subtest_known_broken

ldd notmuch will show "/path/to/libgmime-2.4.so.*" or
"libgmime-2.6.so.*" so we can easily check this in the test suite.
It's a little hacky but it seems to work. AFAIK ldd is a pretty standard
tool so it should be available (almost) everywhere. However if you have
a better idea I'll be glad to hear it.

> > > > +#ifdef GMIME_26
> > > > +    if (node->verify_attempted && !node->sig_list)
> > > 
> > > Hmm.  This is correct for signed parts, but will incorrectly trigger
> > > for an encrypted part with no signatures.  For 2.6, I think this error
> > > checking may have to move into the branches of the if encrypted/signed
> > > since for encrypted parts you have to check if
> > > g_mime_multipart_encrypted_decrypt returned NULL.
> > 
> > That sound right. The weird part is that it did not cause anything to
> > fail in the test suite...
> 
> It would be worth adding a test with an encrypted but unsigned part.
> I don't know enough GPG myself to do that.

It looks like there's already one: "emacs delivery of encrypted message
with attachment" + following decryptions.

Regards,

-- 
Thomas/Schnouki

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

[PATCH v3 0/2] gmime 2.6 compatibilty, 3rd iteration

[Thomas Jost]

Hi list,

Here's another update of the patches to add gmime 2.6 compatibilty
while still preserving compatibility with gmime 2.4.

Any comments or review will be much appreciated.

The changes compared to the previous version ([1] and [2]) are pretty
minor:
- space and indentation fixes
- correctly dereference the instance of GMimeDecryptResult allocated
  by g_mime_decrypt_result_get_signatures()
- remove a useless local variable
- rename the preprocessor constant GMIME_26 to GMIME_ATLEAST_26
- mark one crypto test as broken when using gmime 2.6 (because of a
  bug in gmime [3])

The first patch is really not specific to gmime so it could probably
be pushed right away.

Thanks to Austin Clements, Tomi Ollila and Adrian Perez for their
reviews of the previous patches!

Best regards,
Thomas

[1] id:"1326797453-9405-1-git-send-email-schnouki@schnouki.net"
[2] id:"1326797453-9405-2-git-send-email-schnouki@schnouki.net"
[3] https://bugzilla.gnome.org/show_bug.cgi?id=668085

Thomas Jost (2):
  show: don't use hex literals in JSON output
  Add compatibility with gmime 2.6

 mime-node.c      |   60 +++++++++++++++++++++++++++++++--
 notmuch-client.h |   30 +++++++++++++++-
 notmuch-reply.c  |    7 ++++
 notmuch-show.c   |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 show-message.c   |    4 ++
 test/crypto      |    2 +
 6 files changed, 193 insertions(+), 7 deletions(-)

-- 
1.7.8.4

[PATCH v3 1/2] show: don't use hex literals in JSON output

[Thomas Jost]

JSON does not support hex literals (0x..) so numbers must be formatted as %d
instead of %x.
---
 notmuch-show.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index d14dac9..91f566c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -641,7 +641,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
                printf (", \"keyid\": %s", json_quote_str (ctx_quote, signer->keyid));
        }
        if (signer->errors != GMIME_SIGNER_ERROR_NONE) {
-           printf (", \"errors\": %x", signer->errors);
+           printf (", \"errors\": %d", signer->errors);
        }
 
        printf ("}");
-- 
1.7.8.4

[PATCH v3 2/2] Add compatibility with gmime 2.6

[Thomas Jost]

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@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);
+		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 *);
 		*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 *);
+	*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
 # move the gnupghome temporarily out of the way
 mv "${GNUPGHOME}"{,.bak}
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
-- 
1.7.8.4

Re: [PATCH v3 2/2] Add compatibility with gmime 2.6

[Austin Clements]

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@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" \

Re: [PATCH v3 2/2] Add compatibility with gmime 2.6

[Thomas Jost]

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

On Thu, 19 Jan 2012 23:10:44 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Nearly there.  A few more small comments.

Thanks again :) Will post new version soon, including a new patch to
update NEWS and INSTALL.

> 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@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.

Right, added. Thanks.

> (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.)

Yes it does. I added some fprintf() in gmime (in gpg_decrypt() and
g_mime_decrypt_result_get_signatures()). Decryption tests (after "emacs
delivery of encrypted message with attachment") clearly show that
g_mime_decrypt_result_get_signatures returns NULL when there's no
signatures in an encrypted part.

I guess the reason why tests did not fail is that
format_part_sigstatus_json just displays an empty JSON array if sig_list
is NULL. And that's also what's expected when an encrypted part has no
signature.

I had a quick look at the GObject code; apparently if you pass NULL to
g_object_ref (or anything that is not a GObject) it will just return
NULL. So that's why this one did not fail either.

> 
> > +		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.

Argh. Sorry about that. (I guess hacking while watching DS9 is not a
good idea: too distracting. Won't do it again until next time.)

> 
> > +	*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).

Added -F :)

I guess we could also use pkg-config to test if gmime 2.6 is present. It
would also be simpler to test the version number once gmime fixes this
bug:

  pkg-config --atleast-version=2.6.x gmime-2.6 || test_subtest_known_broken

But this would mean assuming that notmuch is built against a system-wide
gmime. Or it would require setting PKG_CONFIG_PATH before tests...
Complicated.

So IMHO once gmime fixes this bug we should just remove the
test_subtest_known_broken and maybe add something like this in
notmuch-client.h:

  #ifdef GMIME_MAJOR_VERSION
  #define GMIME_ATLEAST_26
  #if !GMIME_CHECK_VERSION(2, 6, x)
  #warning "Building against an old and buggy version of gmime. Please update to 2.6.x."
  #endif
  #endif

> 
> >  # move the gnupghome temporarily out of the way
> >  mv "${GNUPGHOME}"{,.bak}
> >  output=$(notmuch show --format=json --verify subject:"test signed message 001" \

-- 
Thomas/Schnouki

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

[PATCH v4 1/3] show: don't use hex literals in JSON output

[Thomas Jost]

JSON does not support hex literals (0x..) so numbers must be formatted as %d
instead of %x.
---
 notmuch-show.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index d14dac9..91f566c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -641,7 +641,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
                printf (", \"keyid\": %s", json_quote_str (ctx_quote, signer->keyid));
        }
        if (signer->errors != GMIME_SIGNER_ERROR_NONE) {
-           printf (", \"errors\": %x", signer->errors);
+           printf (", \"errors\": %d", signer->errors);
        }
 
        printf ("}");
-- 
1.7.8.4

[PATCH v4 2/3] Add compatibility with gmime 2.6

[Thomas Jost]

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@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      |   57 +++++++++++++++++++++++++++++++-
 notmuch-client.h |   30 ++++++++++++++++-
 notmuch-reply.c  |    7 ++++
 notmuch-show.c   |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 show-message.c   |    4 ++
 test/crypto      |    2 +
 6 files changed, 191 insertions(+), 4 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d26bb44..27077f7 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,25 @@ _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);
+		if (node->sig_list)
+		    g_object_ref (node->sig_list);
+		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 +213,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
@@ -200,12 +240,25 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 		*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 *);
+	*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..446a58b 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 -Fq gmime-2.6) && test_subtest_known_broken
 # move the gnupghome temporarily out of the way
 mv "${GNUPGHOME}"{,.bak}
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
-- 
1.7.8.4

[PATCH v4 3/3] Update NEWS and INSTALL about gmime 2.6

[Thomas Jost]

---
 INSTALL |   12 ++++++------
 NEWS    |    9 +++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/INSTALL b/INSTALL
index e51b397..bc98f1d 100644
--- a/INSTALL
+++ b/INSTALL
@@ -20,8 +20,8 @@ configure stage.
 
 Dependencies
 ------------
-Notmuch depends on three libraries: Xapian, GMime 2.4, and Talloc
-which are each described below:
+Notmuch depends on three libraries: Xapian, GMime 2.4 or 2.6, and
+Talloc which are each described below:
 
 	Xapian
 	------
@@ -39,14 +39,14 @@ which are each described below:
 	reading mail while notmuch would wait for Xapian when removing
 	the "inbox" and "unread" tags from messages in a thread.
 
-	GMime 2.4
-	---------
-	GMime 2.4 provides decoding of MIME email messages for Notmuch.
+	GMime 2.4 or 2.6
+	----------------
+	GMime provides decoding of MIME email messages for Notmuch.
 
 	Without GMime, Notmuch would not be able to extract and index
 	the actual text from email message encoded as BASE64, etc.
 
-	GMime 2.4 is available from http://spruce.sourceforge.net/gmime/
+	GMime is available from http://spruce.sourceforge.net/gmime/
 
 	Talloc
 	------
diff --git a/NEWS b/NEWS
index 6afa912..e78472c 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,15 @@ New functions
   notmuch_query_add_tag_exclude supports the new tag exclusion
   feature.
 
+Build fixes
+-----------
+
+Compatibility with GMime 2.6
+
+  It is now possible to build notmuch against both GMime 2.4 and 2.6.
+  However they may be some issues in PGP signature verification
+  because of a bug in current versions of GMime 2.6.
+
 Notmuch 0.11 (2012-01-13)
 =========================
 
-- 
1.7.8.4

Re: [PATCH v4 1/3] show: don't use hex literals in JSON output

[David Bremner]

On Fri, 20 Jan 2012 10:39:23 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> JSON does not support hex literals (0x..) so numbers must be formatted as %d
> instead of %x.
> ---
>  notmuch-show.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
 
Probably I'm just being lazy here, but can you explain why this change
does not require a corresponding change on the emacs side?

d

Re: [PATCH v4 1/3] show: don't use hex literals in JSON output

[Thomas Jost]

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

On Fri, 20 Jan 2012 07:55:03 -0400, David Bremner <david@tethera.net> wrote:
> On Fri, 20 Jan 2012 10:39:23 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> > JSON does not support hex literals (0x..) so numbers must be formatted as %d
> > instead of %x.
> > ---
> >  notmuch-show.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
>  
> Probably I'm just being lazy here, but can you explain why this change
> does not require a corresponding change on the emacs side?

Because Emacs already does the right thing. JSON numbers are supposed to
be decimal only (see http://json.org/: digits are 0-9 only), but the
current code could result in displaying a hexadecimal number instead
("c" instead of "12"). This would then trigger an error in Emacs, or in
any other correct JSON parser.

However we are quite lucky: because of the possible values of the gmime
error codes, such an error cannot happen.

The most common gmime error codes are 1 (expired signature), 2 (no
public key), 4 (expired key) and 8 (revoked key). The other possible
value is 16 (unsupported algorithm) but obviously it is much more rare.
If this happens, the current code will add '"errors": 10' (hex for
16...). This is valid JSON (it looks like a decimal number) but it is
incorrect (should be 16, not 10).

With this patch, notmuch will correctly display '"errors": 16' if such a
case happens.

(By the way, this issue was spotted by Austin Clements in
id:"20120117034714.GG16740@mit.edu", so he deserves the credits :))

Regards,

-- 
Thomas/Schnouki

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

Re: [PATCH v4 1/3] show: don't use hex literals in JSON output

[David Bremner]

On Fri, 20 Jan 2012 13:33:58 +0100, Thomas Jost <schnouki@schnouki.net> wrote:

> The most common gmime error codes are 1 (expired signature), 2 (no
> public key), 4 (expired key) and 8 (revoked key). The other possible
> value is 16 (unsupported algorithm) but obviously it is much more rare.
> If this happens, the current code will add '"errors": 10' (hex for
> 16...). This is valid JSON (it looks like a decimal number) but it is
> incorrect (should be 16, not 10).

Maybe something like this for the commit message?

d

Re: [PATCH] Add pseudo-compatibility with gmime 2.6

[Tomi Ollila]

On Fri, 20 Jan 2012 00:52:47 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> 
> Here is how I did:
> 
>   (ldd notmuch | grep -q gmime-2.6) && test_subtest_known_broken
> 
> ldd notmuch will show "/path/to/libgmime-2.4.so.*" or
> "libgmime-2.6.so.*" so we can easily check this in the test suite.
> It's a little hacky but it seems to work. AFAIK ldd is a pretty standard
> tool so it should be available (almost) everywhere. However if you have
> a better idea I'll be glad to hear it.

The "hack" is good in a sense that if that check fails in any case
the test_subtest_known_broken is not executed and we get FAIL instead of
BROKEN.
The subshell is unneeded:

	ldd notmuch | grep -q gmime-2.6 && test_subtest_known_broken

does the trick (potentially less forks)... ok now I have to test ;)

haha:
 $ rm xfoo.* xbar.*
 
 $ strace -ff -o xfoo sh -c '(ldd /bin/ls | grep -q libc) && echo foo'
 foo
 $ ls xfoo.*
 xfoo.14277  xfoo.14279  xfoo.14281  xfoo.14283  xfoo.14285
 xfoo.14278  xfoo.14280  xfoo.14282  xfoo.14284

 $ strace -ff -o xbar sh -c 'ldd /bin/ls | grep -q libc && echo foo'
 foo
 $ ls xbar.*
 xbar.14292  xbar.14294  xbar.14296  xbar.14298
 xbar.14293  xbar.14295  xbar.14297  xbar.14299


Tomi

Re: [PATCH v4 2/3] Add compatibility with gmime 2.6

[Austin Clements]

LGTM!

Quoth Thomas Jost on Jan 20 at 10:39 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@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.

Re: [PATCH v4 3/3] Update NEWS and INSTALL about gmime 2.6

[Austin Clements]

Quoth Thomas Jost on Jan 20 at 10:39 am:
> ---
>  INSTALL |   12 ++++++------
>  NEWS    |    9 +++++++++
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/INSTALL b/INSTALL
> index e51b397..bc98f1d 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -20,8 +20,8 @@ configure stage.
>  
>  Dependencies
>  ------------
> -Notmuch depends on three libraries: Xapian, GMime 2.4, and Talloc
> -which are each described below:
> +Notmuch depends on three libraries: Xapian, GMime 2.4 or 2.6, and
> +Talloc which are each described below:
>  
>  	Xapian
>  	------
> @@ -39,14 +39,14 @@ which are each described below:
>  	reading mail while notmuch would wait for Xapian when removing
>  	the "inbox" and "unread" tags from messages in a thread.
>  
> -	GMime 2.4
> -	---------
> -	GMime 2.4 provides decoding of MIME email messages for Notmuch.
> +	GMime 2.4 or 2.6
> +	----------------
> +	GMime provides decoding of MIME email messages for Notmuch.
>  
>  	Without GMime, Notmuch would not be able to extract and index
>  	the actual text from email message encoded as BASE64, etc.
>  
> -	GMime 2.4 is available from http://spruce.sourceforge.net/gmime/
> +	GMime is available from http://spruce.sourceforge.net/gmime/
>  
>  	Talloc
>  	------
> diff --git a/NEWS b/NEWS
> index 6afa912..e78472c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -36,6 +36,15 @@ New functions
>    notmuch_query_add_tag_exclude supports the new tag exclusion
>    feature.
>  
> +Build fixes
> +-----------
> +
> +Compatibility with GMime 2.6
> +
> +  It is now possible to build notmuch against both GMime 2.4 and 2.6.
> +  However they may be some issues in PGP signature verification

Typo and comma: "However, there".  It would be good to describe the
bug, lest people hold off on upgrading because they think it's
something more dire.  Maybe

"However, a bug in current GMime 2.6 causes notmuch not to report
signatures where the signer key is unavailable (GNOME bug 668085)."

> +  because of a bug in current versions of GMime 2.6.
> +
>  Notmuch 0.11 (2012-01-13)
>  =========================
>  

Re: [PATCH v4 2/3] Add compatibility with gmime 2.6

[Tomi Ollila]

On Fri, 20 Jan 2012 10:39:24 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> 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@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.
> ---

+1

Tomi

Re: [PATCH v4 2/3] Add compatibility with gmime 2.6

[David Bremner]

On Fri, 20 Jan 2012 10:39:24 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> 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.
> 

pushed. 

d

Re: [PATCH v4 3/3] Update NEWS and INSTALL about gmime 2.6

[David Bremner]

On Fri, 20 Jan 2012 10:39:25 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> ---
>  INSTALL |   12 ++++++------
>  NEWS    |    9 +++++++++
>  2 files changed, 15 insertions(+), 6 deletions(-)

Pushed. 

Note, I'm still waiting for an update of 1/3, or to convinced otherwise.

d

[PATCH] show: don't use hex literals in JSON output

[Thomas Jost]

JSON does not support hex literals (0x..) so numbers must be formatted
as %d instead of %x.

Currently, the possible values for the gmime error code are 1 (expired
signature), 2 (no public key), 4 (expired key) and 8 (revoked key).
The other possible value is 16 (unsupported algorithm) but obviously
it is much more rare. If this happens, the current code will add
'"errors": 10'. This is valid JSON (it looks like a decimal number)
but it is incorrect (should be 16, not 10).

Since this is just an issue in the JSON encoder, no changes are needed
on the Emacs side (or in other UIs using the JSON output).
---

 notmuch-show.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 43ee211..7b40568 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -728,7 +728,7 @@ format_part_sigstatus_json (const GMimeSignatureValidity* validity)
                printf (", \"keyid\": %s", json_quote_str (ctx_quote, signer->keyid));
        }
        if (signer->errors != GMIME_SIGNER_ERROR_NONE) {
-           printf (", \"errors\": %x", signer->errors);
+           printf (", \"errors\": %d", signer->errors);
        }
 
        printf ("}");
-- 
1.7.8.4

[PATCH] Fix NEWS about gmime 2.6

[Thomas Jost]

Previous version had a typo ("they may be" instead of "there may be")
and was lacking a proper description of the gmime bug.
---
Argh. Did not know how to tell it in proper English, ended with a huge
typo. Sorry about that, and thanks for noticing and fixing it :)

 NEWS |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index e78472c..2c2d9e9 100644
--- a/NEWS
+++ b/NEWS
@@ -42,8 +42,8 @@ Build fixes
 Compatibility with GMime 2.6
 
   It is now possible to build notmuch against both GMime 2.4 and 2.6.
-  However they may be some issues in PGP signature verification
-  because of a bug in current versions of GMime 2.6.
+  However, a bug in current GMime 2.6 causes notmuch not to report
+  signatures where the signer key is unavailable (GNOME bug 668085).
 
 Notmuch 0.11 (2012-01-13)
 =========================
-- 
1.7.8.4

Re: [PATCH] Fix NEWS about gmime 2.6

[David Bremner]

On Sun, 22 Jan 2012 01:29:41 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> Previous version had a typo ("they may be" instead of "there may be")
> and was lacking a proper description of the gmime bug.
> ---

pushed, 

d

Re: [PATCH] show: don't use hex literals in JSON output

[David Bremner]

On Sun, 22 Jan 2012 01:20:57 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> JSON does not support hex literals (0x..) so numbers must be formatted
> as %d instead of %x.

pushed, 

d