From 67869efe47ac00d5b39b7ad3070a05181e2a4c21 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Sat, 19 May 2012 14:26:58 +0300 Subject: [PATCH] Re: [PATCH v2 4/5] cli: new crypto verify flag to handle verification --- 8d/5cac7847100a72685b9c26b6d8cd90a3592fe1 | 225 ++++++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 8d/5cac7847100a72685b9c26b6d8cd90a3592fe1 diff --git a/8d/5cac7847100a72685b9c26b6d8cd90a3592fe1 b/8d/5cac7847100a72685b9c26b6d8cd90a3592fe1 new file mode 100644 index 000000000..8240fa1de --- /dev/null +++ b/8d/5cac7847100a72685b9c26b6d8cd90a3592fe1 @@ -0,0 +1,225 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 76C66431FB6 + for ; Sat, 19 May 2012 04:27:07 -0700 (PDT) +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 o-s23igRXkHC for ; + Sat, 19 May 2012 04:27:06 -0700 (PDT) +Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com + [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 2625B431FAE + for ; Sat, 19 May 2012 04:27:05 -0700 (PDT) +Received: by lbbgk8 with SMTP id gk8so3015279lbb.26 + for ; Sat, 19 May 2012 04:27:01 -0700 (PDT) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=google.com; s=20120113; + h=from:to:subject:in-reply-to:references:user-agent:date:message-id + :mime-version:content-type:x-gm-message-state; + bh=W7lze5IZUYjrO0b3UOsDd8uVIWC4kGjaGpudWVzhAQ8=; + b=TLvGysjBcU9jbIOvHVY3FauviSYzYLB8rGMreRFwfcpc7CQzHxsX8wt19Tf2hj+/fQ + DyxVB4NC9lQwKSLQAutHpUQaVwkqcQZ3+H+CwGKm5A0URGV9oDhrjq3AdEbmFeNRXm85 + nPF4Q0AePddn8NGTvLpPL4QcQKBDnN0qwUKsoIUb6fLA6nUpkEjR4nPMO0wbq0ah1TE2 + IMG4X51L0FGK2y596X8uWIonOqeGHKmDbKnEJ1CXX2BhEI4zpLm6arTHYbaG0pDJ16Fe + 36f/u2n+tzXkr6N+3hEPui/P4mCzFkmVzyN8qRG8UvQr3gj3XnnZr+lFcl0hvPqdGMZ8 + 7e7Q== +Received: by 10.112.27.226 with SMTP id w2mr6171724lbg.57.1337426821869; + Sat, 19 May 2012 04:27:01 -0700 (PDT) +Received: from localhost (dsl-hkibrasgw4-fe50dc00-68.dhcp.inet.fi. + [80.220.80.68]) + by mx.google.com with ESMTPS id pp2sm16668967lab.3.2012.05.19.04.26.59 + (version=SSLv3 cipher=OTHER); Sat, 19 May 2012 04:27:00 -0700 (PDT) +From: Jani Nikula +To: Jameson Graef Rollins , + Notmuch Mail +Subject: Re: [PATCH v2 4/5] cli: new crypto verify flag to handle verification +In-Reply-To: <1337362357-31281-5-git-send-email-jrollins@finestructure.net> +References: <1337362357-31281-1-git-send-email-jrollins@finestructure.net> + <1337362357-31281-2-git-send-email-jrollins@finestructure.net> + <1337362357-31281-3-git-send-email-jrollins@finestructure.net> + <1337362357-31281-4-git-send-email-jrollins@finestructure.net> + <1337362357-31281-5-git-send-email-jrollins@finestructure.net> +User-Agent: Notmuch/0.13+13~gc259b9a (http://notmuchmail.org) Emacs/23.3.1 + (i686-pc-linux-gnu) +Date: Sat, 19 May 2012 14:26:58 +0300 +Message-ID: <878vgoe7i5.fsf@nikula.org> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Gm-Message-State: + ALoCoQnSHx4/dhH530aGdeZ0bq49GQOByo9NOPWNkajMbJ+Wb3s2Rk+bGu1uDHX80GJJNjOntcE0 +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: Sat, 19 May 2012 11:27:07 -0000 + +On Fri, 18 May 2012, Jameson Graef Rollins wrote: +> Use this flag rather than depend on the existence of an initialized +> gpgctx, to determine whether we should verify a multipart/signed. We +> will be moving to create the ctx lazily, so we don't want to depend on +> it being previously initialized if it's not needed. +> --- +> mime-node.c | 5 ++--- +> notmuch-client.h | 8 ++++---- +> notmuch-reply.c | 1 + +> notmuch-show.c | 14 +++++++++++--- +> 4 files changed, 18 insertions(+), 10 deletions(-) +> +> diff --git a/mime-node.c b/mime-node.c +> index 3dda900..3adbe5a 100644 +> --- a/mime-node.c +> +++ b/mime-node.c +> @@ -183,8 +183,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part) +> } +> +> /* Handle PGP/MIME parts */ +> - if (GMIME_IS_MULTIPART_ENCRYPTED (part) +> - && node->ctx->crypto->gpgctx && node->ctx->crypto->decrypt) { +> + if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt) { +> if (node->nchildren != 2) { +> /* this violates RFC 3156 section 4, so we won't bother with it. */ +> fprintf (stderr, "Error: %d part(s) for a multipart/encrypted " +> @@ -218,7 +217,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part) +> (err ? err->message : "no error explanation given")); +> } +> } +> - } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->gpgctx) { +> + } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify) { +> if (node->nchildren != 2) { +> /* this violates RFC 3156 section 5, so we won't bother with it. */ +> fprintf (stderr, "Error: %d part(s) for a multipart/signed message " +> diff --git a/notmuch-client.h b/notmuch-client.h +> index 9892968..c671c13 100644 +> --- a/notmuch-client.h +> +++ b/notmuch-client.h +> @@ -80,6 +80,7 @@ typedef struct notmuch_crypto { +> #else +> GMimeCipherContext* gpgctx; +> #endif +> + notmuch_bool_t verify; +> notmuch_bool_t decrypt; +> } notmuch_crypto_t; +> +> @@ -345,10 +346,9 @@ struct mime_node { +> }; +> +> /* Construct a new MIME node pointing to the root message part of +> - * message. If crypto->gpgctx is non-NULL, it will be used to verify +> - * signatures on any child parts. If crypto->decrypt is true, then +> - * crypto.gpgctx will additionally be used to decrypt any encrypted +> - * child parts. +> + * message. If crypto->verify is true, signed child parts will be +> + * verified. If crypto->decrypt is true, encrypted child parts will be +> + * decrypted. +> * +> * Return value: +> * +> diff --git a/notmuch-reply.c b/notmuch-reply.c +> index 34a906e..345be76 100644 +> --- a/notmuch-reply.c +> +++ b/notmuch-reply.c +> @@ -674,6 +674,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) +> int opt_index, ret = 0; +> int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all); +> notmuch_crypto_t crypto = { +> + .verify = FALSE, +> .decrypt = FALSE +> }; +> int format = FORMAT_DEFAULT; +> diff --git a/notmuch-show.c b/notmuch-show.c +> index 66c74e2..f4ee038 100644 +> --- a/notmuch-show.c +> +++ b/notmuch-show.c +> @@ -987,11 +987,11 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> .part = -1, +> .omit_excluded = TRUE, +> .crypto = { +> + .verify = FALSE, +> .decrypt = FALSE +> } +> }; +> int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +> - notmuch_bool_t verify = FALSE; +> int exclude = EXCLUDE_TRUE; +> +> notmuch_opt_desc_t options[] = { +> @@ -1008,7 +1008,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> { NOTMUCH_OPT_INT, ¶ms.part, "part", 'p', 0 }, +> { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 }, +> { NOTMUCH_OPT_BOOLEAN, ¶ms.crypto.decrypt, "decrypt", 'd', 0 }, +> - { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 }, +> + { NOTMUCH_OPT_BOOLEAN, ¶ms.crypto.verify, "verify", 'v', 0 }, +> { 0, 0, 0, 0, 0 } +> }; +> +> @@ -1018,6 +1018,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> return 1; +> } +> +> + /* decryption implies verification */ +> + if (params.crypto.decrypt) +> + params.crypto.verify = TRUE; + +This does not change existing behaviour, only makes it more obvious +(which is good), but this seems to be missing from the man page. I +presume technically decryption doesn't have to imply verification, but +it's probably a good thing. It should be documented, but does not have +to be a part of this series. + +Thanks for working on this. The series looks good to me (apart from the +comments already made by Austin), and the compromises after our debate +reasonable. + + +BR, +Jani. + + +> + +> if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { +> /* if part was requested and format was not specified, use format=raw */ +> if (params.part >= 0) +> @@ -1052,7 +1056,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> break; +> } +> +> - if (params.crypto.decrypt || verify) { +> + if (params.crypto.decrypt || params.crypto.verify) { +> #ifdef GMIME_ATLEAST_26 +> /* TODO: GMimePasswordRequestFunc */ +> params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg"); +> @@ -1063,6 +1067,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> if (params.crypto.gpgctx) { +> g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE); +> } else { +> + /* If we fail to create the gpgctx set the verify and +> + * decrypt flags to FALSE so we don't try to do any +> + * further verification or decryption */ +> + params.crypto.verify = FALSE; +> params.crypto.decrypt = FALSE; +> fprintf (stderr, "Failed to construct gpg context.\n"); +> } +> -- +> 1.7.10 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch -- 2.26.2