From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id D7593431FAF for ; Thu, 19 Jan 2012 20:11:08 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZTcra30qH50D for ; Thu, 19 Jan 2012 20:11:07 -0800 (PST) Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU [18.7.68.34]) by olra.theworths.org (Postfix) with ESMTP id 1CDD9431FAE for ; Thu, 19 Jan 2012 20:11:07 -0800 (PST) X-AuditID: 12074422-b7fd66d0000008f9-88-4f18e95aa2a9 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39]) by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP id 0F.BB.02297.A59E81F4; Thu, 19 Jan 2012 23:11:06 -0500 (EST) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id q0K4B5Vh018166; Thu, 19 Jan 2012 23:11:05 -0500 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) (authenticated bits=0) (User authenticated as amdragon@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q0K4B3Fn010435 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Thu, 19 Jan 2012 23:11:04 -0500 (EST) Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) (envelope-from ) id 1Ro5oK-0004c6-S6; Thu, 19 Jan 2012 23:10:44 -0500 Date: Thu, 19 Jan 2012 23:10:44 -0500 From: Austin Clements To: Thomas Jost Subject: Re: [PATCH v3 2/2] Add compatibility with gmime 2.6 Message-ID: <20120120041044.GU16740@mit.edu> References: <87ty3r2rqt.fsf@schnouki.net> <1327017987-3361-1-git-send-email-schnouki@schnouki.net> <1327017987-3361-3-git-send-email-schnouki@schnouki.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1327017987-3361-3-git-send-email-schnouki@schnouki.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IRYrdT1416KeFvsPGTvMX1mzOZLfb1+zsw eTxbdYvZY8qsuewBTFFcNimpOZllqUX6dglcGWvnPWEt2FVbsePeR+YGxrdJXYycHBICJhK3 7j5ghLDFJC7cW8/WxcjFISSwj1Hi2sT/LBDOBkaJ9/c6oTInmSSuHrzIDOEsYZT4enYZUxcj BweLgKrEv52OIKPYBDQktu1fDjZWREBdomH+bjYQm1lAWuLb72YmEFtYwEZi5t8VzCA2r4CO RH/3C6iZ8xklVl7ZxwqREJQ4OfMJC0SzlsSNfy/BdoEMWv6PA8TkFHCVeHFCFaRCVEBFYsrJ bWwTGIVmIWmehaR5FkLzAkbmVYyyKblVurmJmTnFqcm6xcmJeXmpRbqmermZJXqpKaWbGMFB 7aK0g/HnQaVDjAIcjEo8vFyuEv5CrIllxZW5hxglOZiURHlVHgOF+JLyUyozEosz4otKc1KL DzFKcDArifA6PAHK8aYkVlalFuXDpKQ5WJTEedW13vkJCaQnlqRmp6YWpBbBZGU4OJQkeG+9 AGoULEpNT61Iy8wpQUgzcXCCDOcBGt4DUsNbXJCYW5yZDpE/xagoJc67CyQhAJLIKM2D64Ul nVeM4kCvCPOeA6niASYsuO5XQIOZgAZ7NImBDC5JREhJNTAGM2XKWr5evv2GQmls/ItkweN5 35VYP04WUc6+nbdThVNZZLJjh7/le+9eRwXm2dVCLNOlVXUcyjO6/+XJpPauTq6b+2qT7UbT vbOeXU1McWV3NPmiHRj3I2h+t1WXULfqKo/ykEWm9wtj2eQEhbVFWg7VMS7b3bzlU1BX6Z2f HGr7FH9WKrEUZyQaajEXFScCAFOMfXgVAwAA Cc: notmuch@notmuchmail.org X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jan 2012 04:11:09 -0000 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_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" \