From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id EEE9F6DE00DB for ; Sat, 23 Sep 2017 09:05:52 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.036 X-Spam-Level: X-Spam-Status: No, score=0.036 tagged_above=-999 required=5 tests=[AWL=0.056, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id iep0G7cPrbJe for ; Sat, 23 Sep 2017 09:05:49 -0700 (PDT) Received: from mail-lf0-f52.google.com (mail-lf0-f52.google.com [209.85.215.52]) by arlo.cworth.org (Postfix) with ESMTPS id 9F1D76DE00C5 for ; Sat, 23 Sep 2017 09:05:48 -0700 (PDT) Received: by mail-lf0-f52.google.com with SMTP id l196so3450744lfl.1 for ; Sat, 23 Sep 2017 09:05:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nikula-org.20150623.gappssmtp.com; s=20150623; h=from:to:subject:in-reply-to:references:date:message-id:mime-version; bh=oOKjvRwkDu5IrORenKMcy6ECh4RamPwUP+4SEBxJSnU=; b=GmvYLSPsJPkAOa+8wx07L4UCrvAr0AuH3bLQEFj6i/uzbN89IfTPueJ3egX+Q2ix7g CWDlb6xP2dUzhNOBw6Bii7ZWBLK6MLJah+/bigNCW+rNNQXkSwPk+x56Nsj0Lpc0axWS F7FPgTWg51OFhO6aAT9dErTscIMSH3LV2N/7RxxXWAnNXKV4HE+Ae1MO4uRx2NVi4jgN WpkyHpc2/iMfI17GuxTDfwsppaypaBVijSCUu63zGzhmpA6LcBEd9isRew16DCiqBlKJ 2c3s61Y7UodOz4Hm+RZoI+7kg49o0qXjl8nS+xS2RJ+gF/6v6ChtUoZCrx0dPwc0T02U 1+eA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=oOKjvRwkDu5IrORenKMcy6ECh4RamPwUP+4SEBxJSnU=; b=g1qPCMY4YRpTMIwYHA2ocEIoUKj85AuhU4fmcqyg2HOYrRXSGiwBiZ8IVzKR5J1x/w oNdcCH+DXx/CZ+J9gcun4pOrRhxu3rdfbh5XuR4yaZa66jlQao+k+Uua7u5EVzYaV/fK XCJmzCMTzOcVLIheZgpUX/9yktEz4KI8kJO4GT139L5gF8Piu03l85SOsZgqLVrXwMQc +o4mcFkTpJawe0kSEjDU+D+3BCyVUkmFOC+dOS40N5wMq6zwjDaIt8A5FBWR2OuLsVmO wbtaRcze9p/uKFaWjUQyyxWyvz1Cc6mhgLSkcLFfCAM0JYviKFhHPjncNE42NE1BAUEB NH+w== X-Gm-Message-State: AHPjjUjJFwYlLahS/FNuDeYp+516FcIY/etNMCKXX2vn6NfX7v3yyDd+ vzi29xgQUvhJKcwuX0RezM5w4g== X-Google-Smtp-Source: AOwi7QAxTvl6JWzknfgLPviUObcsaAcWM/FRtbP/gu0oabCrfByDBN101MA0AEbmE9sp4tcXmaY16A== X-Received: by 10.25.205.13 with SMTP id d13mr861769lfg.0.1506182746521; Sat, 23 Sep 2017 09:05:46 -0700 (PDT) Received: from localhost (mobile-access-5d6a60-234.dhcp.inet.fi. [93.106.96.234]) by smtp.gmail.com with ESMTPSA id d63sm340430lfg.20.2017.09.23.09.05.43 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 23 Sep 2017 09:05:43 -0700 (PDT) From: Jani Nikula To: Daniel Kahn Gillmor , Notmuch Mail Subject: Re: [PATCH v2 05/10] crypto: index encrypted parts when indexopts try_decrypt is set. In-Reply-To: <20170915055359.24123-6-dkg@fifthhorseman.net> References: <20170912230153.4175-10-dkg@fifthhorseman.net> <20170915055359.24123-1-dkg@fifthhorseman.net> <20170915055359.24123-6-dkg@fifthhorseman.net> Date: Sat, 23 Sep 2017 19:05:40 +0300 Message-ID: <87shfdflij.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.23 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, 23 Sep 2017 16:05:53 -0000 On Fri, 15 Sep 2017, Daniel Kahn Gillmor wrote: > If we see index options that ask us to decrypt when indexing a > message, and we encounter an encrypted part, we'll try to descend into > it. > > If we can decrypt, we add the property index-decryption=success. > > If we can't decrypt (or recognize the encrypted type of mail), we add > the property index-decryption=failure. > > Note that a single message may have both values of the > "index-decryption" property: "success" and "failure". For example, > consider a message that includes multiple layers of encryption. If we > manage to decrypt the outer layer ("index-decryption=success"), but > fail on the inner layer ("index-decryption=failure"). > > Before re-indexing, we wipe this automatically-added property, so that > it will subsequently correspond to the actual semantics of the stored > index. > --- > lib/add-message.cc | 2 +- > lib/index.cc | 103 +++++++++++++++++++++++++++++++++++++++++++++----- > lib/message.cc | 14 ++++++- > lib/notmuch-private.h | 1 + > 4 files changed, 107 insertions(+), 13 deletions(-) > > diff --git a/lib/add-message.cc b/lib/add-message.cc > index 1fd91c14..66eb0a1f 100644 > --- a/lib/add-message.cc > +++ b/lib/add-message.cc > @@ -544,7 +544,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch, > if (!indexopts) > indexopts = def_indexopts = notmuch_database_get_default_indexopts (notmuch); > > - ret = _notmuch_message_index_file (message, message_file); > + ret = _notmuch_message_index_file (message, indexopts, message_file); > if (ret) > goto DONE; > > diff --git a/lib/index.cc b/lib/index.cc > index ceb444df..285928f7 100644 > --- a/lib/index.cc > +++ b/lib/index.cc > @@ -364,9 +364,17 @@ _index_content_type (notmuch_message_t *message, GMimeObject *part) > } > } > > +static void > +_index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, > +#if (GMIME_MAJOR_VERSION < 3) > + GMimeContentType *content_type, > +#endif I don't think adding this within ifdefs servers a useful purpose. It just makes the code harder to read all over the place. Please just pass it in unconditionally. > + GMimeMultipartEncrypted *part); > + > /* Callback to generate terms for each mime part of a message. */ > static void > _index_mime_part (notmuch_message_t *message, > + notmuch_indexopts_t *indexopts, > GMimeObject *part) > { > GMimeStream *stream, *filter; > @@ -409,17 +417,26 @@ _index_mime_part (notmuch_message_t *message, > } > } > if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) { > - /* Don't index encrypted parts, but index their content type. */ > - _index_content_type (message, > - g_mime_multipart_get_part (multipart, i)); > - if ((i != GMIME_MULTIPART_ENCRYPTED_VERSION) && > - (i != GMIME_MULTIPART_ENCRYPTED_CONTENT)) { > - _notmuch_database_log (_notmuch_message_database (message), > - "Warning: Unexpected extra parts of multipart/encrypted.\n"); > + if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) { > + _index_encrypted_mime_part(message, indexopts, > +#if (GMIME_MAJOR_VERSION < 3) > + content_type, > +#endif > + GMIME_MULTIPART_ENCRYPTED (part)); > + } else { > + /* Don't index the non-content parts of an > + * encrypted message, but do index their content > + * type. */ > + _index_content_type (message, > + g_mime_multipart_get_part (multipart, i)); > + if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) { > + _notmuch_database_log (_notmuch_message_database (message), > + "Warning: Unexpected extra parts of multipart/encrypted.\n"); > + } > } > continue; > } > - _index_mime_part (message, > + _index_mime_part (message, indexopts, > g_mime_multipart_get_part (multipart, i)); > } > return; > @@ -430,7 +447,7 @@ _index_mime_part (notmuch_message_t *message, > > mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (part)); > > - _index_mime_part (message, g_mime_message_get_mime_part (mime_message)); > + _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message)); > > return; > } > @@ -502,8 +519,74 @@ _index_mime_part (notmuch_message_t *message, > } > } > > +/* descend (if desired) into the cleartext part of an encrypted MIME > + * part while indexing. */ > +static void > +_index_encrypted_mime_part (notmuch_message_t *message, > + notmuch_indexopts_t *indexopts, > +#if (GMIME_MAJOR_VERSION < 3) > + GMimeContentType *content_type, > +#endif > + GMimeMultipartEncrypted *encrypted_data) > +{ > + notmuch_status_t status; > +#if (GMIME_MAJOR_VERSION < 3) > + GMimeCryptoContext* crypto_ctx = NULL; > + const char *protocol = NULL; > +#endif > + GError *err = NULL; > + notmuch_database_t * notmuch = NULL; > + GMimeObject *clear = NULL; > + > + if (!indexopts || !notmuch_indexopts_get_try_decrypt (indexopts)) > + return; > + > + notmuch = _notmuch_message_database (message); > + > +#if (GMIME_MAJOR_VERSION < 3) > + protocol = g_mime_content_type_get_parameter (content_type, "protocol"); > + status = _notmuch_crypto_get_gmime_ctx_for_protocol (&(indexopts->crypto), > + protocol, &crypto_ctx); > + if (status) { > + _notmuch_database_log (notmuch, "Warning: setup failed for decrypting " > + "during indexing. (%d)\n", status); > + status = notmuch_message_add_property (message, "index-decryption", "failure"); > + if (status) > + _notmuch_database_log (notmuch, "failed to add index-decryption " > + "property (%d)\n", status); > + return; > + } > +#endif I'd like this #if block to be abstracted to separate functions for gmime 2 vs. 3. Adding conditional compilation within functions is ugly and hard to track for both branches. > + /* we don't need the GMimeDecryptResult, because we're not looking > + * at validating signatures, and we don't care about indexing who > + * the message was ostensibly encrypted to. > + */ > + clear = g_mime_multipart_encrypted_decrypt(encrypted_data, crypto_ctx, > + NULL, &err); Speaking of which, where is crypto_ctx declared for gmime 3? > + if (err) { > + _notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n", > + err->domain, err->code, err->message); > + g_error_free(err); > + /* Indicate that we failed to decrypt during indexing */ > + status = notmuch_message_add_property (message, "index-decryption", "failure"); > + if (status) > + _notmuch_database_log (notmuch, "failed to add index-decryption " > + "property (%d)\n", status); > + return; > + } > + _index_mime_part (message, indexopts, clear); > + g_object_unref (clear); > + > + status = notmuch_message_add_property (message, "index-decryption", "success"); > + if (status) > + _notmuch_database_log (notmuch, "failed to add index-decryption " > + "property (%d)\n", status); > + > +} > + > notmuch_status_t > _notmuch_message_index_file (notmuch_message_t *message, > + notmuch_indexopts_t *indexopts, > notmuch_message_file_t *message_file) > { > GMimeMessage *mime_message; > @@ -531,7 +614,7 @@ _notmuch_message_index_file (notmuch_message_t *message, > subject = g_mime_message_get_subject (mime_message); > _notmuch_message_gen_terms (message, "subject", subject); > > - _index_mime_part (message, g_mime_message_get_mime_part (mime_message)); > + _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message)); > > return NOTMUCH_STATUS_SUCCESS; > } > diff --git a/lib/message.cc b/lib/message.cc > index 0e3b5a4f..2ffa25a3 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -1961,7 +1961,7 @@ _notmuch_message_frozen (notmuch_message_t *message) > > notmuch_status_t > notmuch_message_reindex (notmuch_message_t *message, > - notmuch_indexopts_t unused (*indexopts)) > + notmuch_indexopts_t *indexopts) > { > notmuch_database_t *notmuch = NULL; > notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS; > @@ -1969,6 +1969,7 @@ notmuch_message_reindex (notmuch_message_t *message, > notmuch_filenames_t *orig_filenames = NULL; > const char *orig_thread_id = NULL; > notmuch_message_file_t *message_file = NULL; > + const char *autoproperties[] = { "index-decryption" }; > > int found = 0; > > @@ -1999,6 +2000,15 @@ notmuch_message_reindex (notmuch_message_t *message, > goto DONE; > } > > + /* all automatically-added properties should be removed before re-indexing */ > + for (size_t i = 0; i < ARRAY_SIZE (autoproperties); i++) { > + ret = notmuch_message_remove_all_properties (message, autoproperties[i]); > + if (ret) { > + INTERNAL_ERROR ("failed to remove automatically-added property '%s'", autoproperties[i]); > + goto DONE; > + } > + } > + > /* re-add the filenames with the associated indexopts */ > for (; notmuch_filenames_valid (orig_filenames); > notmuch_filenames_move_to_next (orig_filenames)) { > @@ -2038,7 +2048,7 @@ notmuch_message_reindex (notmuch_message_t *message, > if (found == 0) > _notmuch_message_set_header_values (message, date, from, subject); > > - ret = _notmuch_message_index_file (message, message_file); > + ret = _notmuch_message_index_file (message, indexopts, message_file); > > if (ret == NOTMUCH_STATUS_FILE_ERROR) > continue; > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h > index 3168cf3c..362106c8 100644 > --- a/lib/notmuch-private.h > +++ b/lib/notmuch-private.h > @@ -447,6 +447,7 @@ _notmuch_database_link_message_to_parents (notmuch_database_t *notmuch, > > notmuch_status_t > _notmuch_message_index_file (notmuch_message_t *message, > + notmuch_indexopts_t *indexopts, > notmuch_message_file_t *message_file); > > /* messages.c */ > -- > 2.14.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch