8d/5cac7847100a72685b9c26b6d8cd90a3592fe1

   1 Return-Path: <jani@nikula.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 76C66431FB6\r
   6         for <notmuch@notmuchmail.org>; Sat, 19 May 2012 04:27:07 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id o-s23igRXkHC for <notmuch@notmuchmail.org>;\r
  16         Sat, 19 May 2012 04:27:06 -0700 (PDT)\r
  17 Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com\r
  18         [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 2625B431FAE\r
  21         for <notmuch@notmuchmail.org>; Sat, 19 May 2012 04:27:05 -0700 (PDT)\r
  22 Received: by lbbgk8 with SMTP id gk8so3015279lbb.26\r
  23         for <notmuch@notmuchmail.org>; Sat, 19 May 2012 04:27:01 -0700 (PDT)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=google.com; s=20120113;\r
  26         h=from:to:subject:in-reply-to:references:user-agent:date:message-id\r
  27         :mime-version:content-type:x-gm-message-state;\r
  28         bh=W7lze5IZUYjrO0b3UOsDd8uVIWC4kGjaGpudWVzhAQ8=;\r
  29         b=TLvGysjBcU9jbIOvHVY3FauviSYzYLB8rGMreRFwfcpc7CQzHxsX8wt19Tf2hj+/fQ\r
  30         DyxVB4NC9lQwKSLQAutHpUQaVwkqcQZ3+H+CwGKm5A0URGV9oDhrjq3AdEbmFeNRXm85\r
  31         nPF4Q0AePddn8NGTvLpPL4QcQKBDnN0qwUKsoIUb6fLA6nUpkEjR4nPMO0wbq0ah1TE2\r
  32         IMG4X51L0FGK2y596X8uWIonOqeGHKmDbKnEJ1CXX2BhEI4zpLm6arTHYbaG0pDJ16Fe\r
  33         36f/u2n+tzXkr6N+3hEPui/P4mCzFkmVzyN8qRG8UvQr3gj3XnnZr+lFcl0hvPqdGMZ8\r
  34         7e7Q==\r
  35 Received: by 10.112.27.226 with SMTP id w2mr6171724lbg.57.1337426821869;\r
  36         Sat, 19 May 2012 04:27:01 -0700 (PDT)\r
  37 Received: from localhost (dsl-hkibrasgw4-fe50dc00-68.dhcp.inet.fi.\r
  38         [80.220.80.68])\r
  39         by mx.google.com with ESMTPS id pp2sm16668967lab.3.2012.05.19.04.26.59\r
  40         (version=SSLv3 cipher=OTHER); Sat, 19 May 2012 04:27:00 -0700 (PDT)\r
  41 From: Jani Nikula <jani@nikula.org>\r
  42 To: Jameson Graef Rollins <jrollins@finestructure.net>,\r
  43         Notmuch Mail <notmuch@notmuchmail.org>\r
  44 Subject: Re: [PATCH v2 4/5] cli: new crypto verify flag to handle verification\r
  45 In-Reply-To: <1337362357-31281-5-git-send-email-jrollins@finestructure.net>\r
  46 References: <1337362357-31281-1-git-send-email-jrollins@finestructure.net>\r
  47         <1337362357-31281-2-git-send-email-jrollins@finestructure.net>\r
  48         <1337362357-31281-3-git-send-email-jrollins@finestructure.net>\r
  49         <1337362357-31281-4-git-send-email-jrollins@finestructure.net>\r
  50         <1337362357-31281-5-git-send-email-jrollins@finestructure.net>\r
  51 User-Agent: Notmuch/0.13+13~gc259b9a (http://notmuchmail.org) Emacs/23.3.1\r
  52         (i686-pc-linux-gnu)\r
  53 Date: Sat, 19 May 2012 14:26:58 +0300\r
  54 Message-ID: <878vgoe7i5.fsf@nikula.org>\r
  55 MIME-Version: 1.0\r
  56 Content-Type: text/plain; charset=us-ascii\r
  57 X-Gm-Message-State:\r
  58  ALoCoQnSHx4/dhH530aGdeZ0bq49GQOByo9NOPWNkajMbJ+Wb3s2Rk+bGu1uDHX80GJJNjOntcE0\r
  59 X-BeenThere: notmuch@notmuchmail.org\r
  60 X-Mailman-Version: 2.1.13\r
  61 Precedence: list\r
  62 List-Id: "Use and development of the notmuch mail system."\r
  63         <notmuch.notmuchmail.org>\r
  64 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  65         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  66 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  67 List-Post: <mailto:notmuch@notmuchmail.org>\r
  68 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  69 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  70         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  71 X-List-Received-Date: Sat, 19 May 2012 11:27:07 -0000\r
  72 \r
  73 On Fri, 18 May 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:\r
  74 > Use this flag rather than depend on the existence of an initialized\r
  75 > gpgctx, to determine whether we should verify a multipart/signed.  We\r
  76 > will be moving to create the ctx lazily, so we don't want to depend on\r
  77 > it being previously initialized if it's not needed.\r
  78 > ---\r
  79 >  mime-node.c      |    5 ++---\r
  80 >  notmuch-client.h |    8 ++++----\r
  81 >  notmuch-reply.c  |    1 +\r
  82 >  notmuch-show.c   |   14 +++++++++++---\r
  83 >  4 files changed, 18 insertions(+), 10 deletions(-)\r
  84 >\r
  85 > diff --git a/mime-node.c b/mime-node.c\r
  86 > index 3dda900..3adbe5a 100644\r
  87 > --- a/mime-node.c\r
  88 > +++ b/mime-node.c\r
  89 > @@ -183,8 +183,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)\r
  90 >      }\r
  91 >  \r
  92 >      /* Handle PGP/MIME parts */\r
  93 > -    if (GMIME_IS_MULTIPART_ENCRYPTED (part)\r
  94 > -     && node->ctx->crypto->gpgctx && node->ctx->crypto->decrypt) {\r
  95 > +    if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt) {\r
  96 >       if (node->nchildren != 2) {\r
  97 >           /* this violates RFC 3156 section 4, so we won't bother with it. */\r
  98 >           fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "\r
  99 > @@ -218,7 +217,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)\r
 100 >                        (err ? err->message : "no error explanation given"));\r
 101 >           }\r
 102 >       }\r
 103 > -    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->gpgctx) {\r
 104 > +    } else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify) {\r
 105 >       if (node->nchildren != 2) {\r
 106 >           /* this violates RFC 3156 section 5, so we won't bother with it. */\r
 107 >           fprintf (stderr, "Error: %d part(s) for a multipart/signed message "\r
 108 > diff --git a/notmuch-client.h b/notmuch-client.h\r
 109 > index 9892968..c671c13 100644\r
 110 > --- a/notmuch-client.h\r
 111 > +++ b/notmuch-client.h\r
 112 > @@ -80,6 +80,7 @@ typedef struct notmuch_crypto {\r
 113 >  #else\r
 114 >      GMimeCipherContext* gpgctx;\r
 115 >  #endif\r
 116 > +    notmuch_bool_t verify;\r
 117 >      notmuch_bool_t decrypt;\r
 118 >  } notmuch_crypto_t;\r
 119 >  \r
 120 > @@ -345,10 +346,9 @@ struct mime_node {\r
 121 >  };\r
 122 >  \r
 123 >  /* Construct a new MIME node pointing to the root message part of\r
 124 > - * message.  If crypto->gpgctx is non-NULL, it will be used to verify\r
 125 > - * signatures on any child parts.  If crypto->decrypt is true, then\r
 126 > - * crypto.gpgctx will additionally be used to decrypt any encrypted\r
 127 > - * child parts.\r
 128 > + * message. If crypto->verify is true, signed child parts will be\r
 129 > + * verified. If crypto->decrypt is true, encrypted child parts will be\r
 130 > + * decrypted.\r
 131 >   *\r
 132 >   * Return value:\r
 133 >   *\r
 134 > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
 135 > index 34a906e..345be76 100644\r
 136 > --- a/notmuch-reply.c\r
 137 > +++ b/notmuch-reply.c\r
 138 > @@ -674,6 +674,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])\r
 139 >      int opt_index, ret = 0;\r
 140 >      int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);\r
 141 >      notmuch_crypto_t crypto = {\r
 142 > +     .verify = FALSE,\r
 143 >       .decrypt = FALSE\r
 144 >      };\r
 145 >      int format = FORMAT_DEFAULT;\r
 146 > diff --git a/notmuch-show.c b/notmuch-show.c\r
 147 > index 66c74e2..f4ee038 100644\r
 148 > --- a/notmuch-show.c\r
 149 > +++ b/notmuch-show.c\r
 150 > @@ -987,11 +987,11 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))\r
 151 >       .part = -1,\r
 152 >       .omit_excluded = TRUE,\r
 153 >       .crypto = {\r
 154 > +         .verify = FALSE,\r
 155 >           .decrypt = FALSE\r
 156 >       }\r
 157 >      };\r
 158 >      int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;\r
 159 > -    notmuch_bool_t verify = FALSE;\r
 160 >      int exclude = EXCLUDE_TRUE;\r
 161 >  \r
 162 >      notmuch_opt_desc_t options[] = {\r
 163 > @@ -1008,7 +1008,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))\r
 164 >       { NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },\r
 165 >       { NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },\r
 166 >       { NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },\r
 167 > -     { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },\r
 168 > +     { NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },\r
 169 >       { 0, 0, 0, 0, 0 }\r
 170 >      };\r
 171 >  \r
 172 > @@ -1018,6 +1018,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))\r
 173 >       return 1;\r
 174 >      }\r
 175 >  \r
 176 > +    /* decryption implies verification */\r
 177 > +    if (params.crypto.decrypt)\r
 178 > +     params.crypto.verify = TRUE;\r
 179 \r
 180 This does not change existing behaviour, only makes it more obvious\r
 181 (which is good), but this seems to be missing from the man page. I\r
 182 presume technically decryption doesn't have to imply verification, but\r
 183 it's probably a good thing. It should be documented, but does not have\r
 184 to be a part of this series.\r
 185 \r
 186 Thanks for working on this. The series looks good to me (apart from the\r
 187 comments already made by Austin), and the compromises after our debate\r
 188 reasonable.\r
 189 \r
 190 \r
 191 BR,\r
 192 Jani.\r
 193 \r
 194 \r
 195 > +\r
 196 >      if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) {\r
 197 >       /* if part was requested and format was not specified, use format=raw */\r
 198 >       if (params.part >= 0)\r
 199 > @@ -1052,7 +1056,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))\r
 200 >       break;\r
 201 >      }\r
 202 >  \r
 203 > -    if (params.crypto.decrypt || verify) {\r
 204 > +    if (params.crypto.decrypt || params.crypto.verify) {\r
 205 >  #ifdef GMIME_ATLEAST_26\r
 206 >       /* TODO: GMimePasswordRequestFunc */\r
 207 >       params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");\r
 208 > @@ -1063,6 +1067,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))\r
 209 >       if (params.crypto.gpgctx) {\r
 210 >           g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);\r
 211 >       } else {\r
 212 > +         /* If we fail to create the gpgctx set the verify and\r
 213 > +          * decrypt flags to FALSE so we don't try to do any\r
 214 > +          * further verification or decryption */\r
 215 > +         params.crypto.verify = FALSE;\r
 216 >           params.crypto.decrypt = FALSE;\r
 217 >           fprintf (stderr, "Failed to construct gpg context.\n");\r
 218 >       }\r
 219 > -- \r
 220 > 1.7.10\r
 221 >\r
 222 > _______________________________________________\r
 223 > notmuch mailing list\r
 224 > notmuch@notmuchmail.org\r
 225 > http://notmuchmail.org/mailman/listinfo/notmuch\r