Re: [PATCH v2 4/5] cli: new crypto verify flag to handle verification

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