Re: [PATCH 1/5] crypto: prepare for decryption of inline PGP encrypted messages

[David Bremner <david@tethera.net>]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> 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?