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 4BFE86DE0083 for ; Tue, 14 Nov 2017 05:02:18 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -0.001 X-Spam-Level: X-Spam-Status: No, score=-0.001 tagged_above=-999 required=5 tests=[AWL=0.010, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-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 e0p9nKveJmtl for ; Tue, 14 Nov 2017 05:02:13 -0800 (PST) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id AAD2B6DE0068 for ; Tue, 14 Nov 2017 05:02:13 -0800 (PST) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1eEaqw-0002ZP-4z; Tue, 14 Nov 2017 08:02:10 -0500 Received: (nullmailer pid 13111 invoked by uid 1000); Tue, 14 Nov 2017 13:02:08 -0000 From: David Bremner To: Daniel Kahn Gillmor , Notmuch Mail Subject: Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available In-Reply-To: <20171025065203.24403-4-dkg@fifthhorseman.net> References: <20171025065203.24403-1-dkg@fifthhorseman.net> <20171025065203.24403-4-dkg@fifthhorseman.net> Date: Tue, 14 Nov 2017 09:02:08 -0400 Message-ID: <87r2t19fov.fsf@tethera.net> 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: Tue, 14 Nov 2017 13:02:18 -0000 Daniel Kahn Gillmor writes: > Note that this only works for GMime 2.6.21 and later (the session key > interface wasn't available before then). I don't think we're ready > for this to be a minimum version requirement yet, so instead if you > build against a prior version of GMime, it simply won't try to use > stashed session keys. Since you wrote this, I've deprecated GMime 2.6. I'm not sure that changes anything here, but seems worth mentioning > --- > doc/man7/notmuch-properties.rst | 31 +++++++++++++++++++++++++++++++ > lib/index.cc | 2 +- > mime-node.c | 13 ++++++++++--- > util/crypto.c | 31 ++++++++++++++++++++++++++++++- > util/crypto.h | 3 ++- > 5 files changed, 74 insertions(+), 6 deletions(-) > > diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst > index 68121359..31d7a104 100644 > --- a/doc/man7/notmuch-properties.rst > +++ b/doc/man7/notmuch-properties.rst > @@ -74,6 +74,35 @@ of its normal activity. > **notmuch-config(1)**), then this property will not be set on that > message. > > +**session-key** > + > + When **notmuch-show(1)** or **nomtuch-reply** encounters a message > + with an encrypted part and ``--decrypt`` is set, if notmuch finds a > + ``session-key=`` property associated with the message, it will try > + that stashed session key for decryption. > + Its a nitpick, but I don't really understand/like including = with the property name. That will break, e.g. for anyone attempting to use it from the API. > - clear = _notmuch_crypto_decrypt (crypto_ctx, encrypted_data, NULL, &err); > + clear = _notmuch_crypto_decrypt (message, crypto_ctx, encrypted_data, NULL, &err); The way the diff works out, I was pretty confused by seeing several "wrong" calls to _notmuch_crypto_decrypt before the actual change. It would be nice to telegraph that somehow, perhaps in the commit message. > { > GMimeObject *ret = NULL; > > + /* the versions of notmuch that can support session key decryption */ > +#if (GMIME_MAJOR_VERSION >= 3 || (GMIME_MAJOR_VERSION == 2 && GMIME_MINOR_VERSION == 6 && GMIME_MICRO_VERSION >= 21)) Personally I would be fine with (and probably happier) only supporting new features when using gmime-3.0. Debugging crypto related stuff is always hard (see recent discussion about _mime_node_create, where we still don't know what's wrong, and are just papering over the problem), and it seems worth striving for simplicity as much as possible. I also don't know how motivated gmime upstream is to fix bugs in 2.6; I could certainly understand if the answer was "not very". There is, by the way, a function notmuch_built_with that can be used to introspect the library as to what optional features it is built with. It's used in notmuch_config to report back to the user about the presence of optional features.