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 B97446DE0289 for ; Thu, 3 May 2018 14:34:55 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[AWL=0.011, 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 siF59mM5UtNL for ; Thu, 3 May 2018 14:34:47 -0700 (PDT) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id 72B956DE0282 for ; Thu, 3 May 2018 14:34:47 -0700 (PDT) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1fELsC-0003y4-E9; Thu, 03 May 2018 17:34:44 -0400 Received: (nullmailer pid 5555 invoked by uid 1000); Thu, 03 May 2018 21:34:43 -0000 From: David Bremner To: Daniel Kahn Gillmor , Notmuch Mail Subject: Re: [PATCH 1/5] crypto: prepare for decryption of inline PGP encrypted messages In-Reply-To: <20171212071553.6440-2-dkg@fifthhorseman.net> References: <20171212071553.6440-1-dkg@fifthhorseman.net> <20171212071553.6440-2-dkg@fifthhorseman.net> Date: Thu, 03 May 2018 18:34:43 -0300 Message-ID: <87in84e83w.fsf@tethera.net> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.26 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: Thu, 03 May 2018 21:34:55 -0000 Daniel Kahn Gillmor writes: > Inline PGP encrypted messages are clearly worse than PGP/MIME > structured encrypted messages. There are no standards for how they > are formed, and they don't offer any structured metadata about how to > interpret the bytestream produced by decrypting them. > > However, some other MUAs and end-user workflows may make creation of > inline PGP encrypted messages the only available option for message > encryption, and when Notmuch encounters such a message, it should make > a reasonable best-effort to render the cleartext to the user. > > Due to ambiguities in interpretation of signatures on inline messages > (e.g. which parts of the message were actually signed? what character > encoding should the bytestream be interpreted as), we continue to > ignore inline-signed messages entirely, and we do not look at the > validity of any signatures that might be found when decrypting inline > PGP encrypted messages. > > We make use here of GMime's optimization function for detecting the > presence of inline PGP encrypted content, which is only found in GMime > 3.0 or later. I already objected to "here", since that doesn't happen in this commit. > > This change prepares the internal codebase for decrypting inline > encrypted messages, but does not yet actually use the capability. The ratio of backstory to "what is going on here" is a little high. Perhaps moving the last few lines to the top would help. > --- > + if (GMIME_IS_PART (part) || /* must be inline */ For some reason it wasn't obvious that you meant "inline PGP" where you wrote "inline" > #if (GMIME_MAJOR_VERSION < 3) > - ret = g_mime_multipart_encrypted_decrypt_session (part, > + ret = g_mime_multipart_encrypted_decrypt_session (GMIME_MULTIPART_ENCRYPTED (part), > crypto_ctx, > notmuch_message_properties_value (list), > decrypt_result, err); that lo > #else > - ret = g_mime_multipart_encrypted_decrypt (part, > - GMIME_DECRYPT_NONE, > - notmuch_message_properties_value (list), > - decrypt_result, err); > + if (GMIME_IS_MULTIPART_ENCRYPTED (part)) { > + ret = g_mime_multipart_encrypted_decrypt (GMIME_MULTIPART_ENCRYPTED (part), > + GMIME_DECRYPT_NONE, > + notmuch_message_properties_value (list), > + decrypt_result, err); > + } else if (GMIME_IS_PART (part) && > g_mime_part_get_openpgp_data (GMIME_PART (part)) == > GMIME_OPENPGP_DATA_ENCRYPTED) { Some of these lines are getting pretty long. devel/STYLE suggests 70 or 80 columns > break; > @@ -214,8 +225,16 @@ _notmuch_crypto_decrypt (bool *attempted, > GMimeDecryptFlags flags = GMIME_DECRYPT_NONE; > if (decrypt == NOTMUCH_DECRYPT_TRUE && decrypt_result) > flags |= GMIME_DECRYPT_EXPORT_SESSION_KEY; > - ret = g_mime_multipart_encrypted_decrypt(part, flags, NULL, > - decrypt_result, err); > + if (GMIME_IS_MULTIPART_ENCRYPTED (part)) { > + ret = g_mime_multipart_encrypted_decrypt(GMIME_MULTIPART_ENCRYPTED (part), flags, NULL, > + decrypt_result, err); > + } else if (GMIME_IS_PART (part) && g_mime_part_get_openpgp_data (GMIME_PART (part)) == GMIME_OPENPGP_DATA_ENCRYPTED) { > + *decrypt_result = g_mime_part_openpgp_decrypt (GMIME_PART (part), flags, NULL, err); > + if (decrypt_result) { > + ret = part; > + g_object_ref (ret); > + } > + } > #endif This looks like somewhat duplicated code. Did you try a little static function?