[PATCH v2 1/4] util/crypto: _notmuch_message_crypto: tracks message-wide crypto state

David Bremner david at tethera.net
Wed May 22 05:18:53 PDT 2019


Daniel Kahn Gillmor <dkg at fifthhorseman.net> writes:

> +static int
> +_notmuch_message_crypto_cleanup (_notmuch_message_crypto_t *msg_crypto)
> +{
> +    if (!msg_crypto)
> +	return 0;
> +    if (msg_crypto->sig_list)
> +	g_object_unref (msg_crypto->sig_list);
> +    return 0;
> +}

we currently call destructors

   - *_destroy
   - *_destructor (the most popular option)
   - *_free

Is there a good reason to introduce a fourth option?

> +notmuch_status_t
> +_notmuch_message_crypto_potential_sig_list (_notmuch_message_crypto_t *msg_crypto, GMimeSignatureList *sigs)
> +{
> +    if (!msg_crypto)
> +	return NOTMUCH_STATUS_NULL_POINTER;
> +
> +    /* Signatures that arrive after a payload part during DFS are not
> +     * part of the cryptographic envelope: */
> +    if (msg_crypto->payload_encountered)
> +	return NOTMUCH_STATUS_SUCCESS;
> +
> +    if (msg_crypto->sig_list)
> +	g_object_unref (msg_crypto->sig_list);
> +
> +    msg_crypto->sig_list = sigs;
> +    if (sigs)
> +	g_object_ref (sigs);

It might be worth a comment here to explain the interaction between
talloc and glib, i.e. why this is needed.

> +
> +
> +notmuch_status_t
> +_notmuch_message_crypto_successful_decryption (_notmuch_message_crypto_t *msg_crypto)
> +{
> +    if (!msg_crypto)
> +	return NOTMUCH_STATUS_NULL_POINTER;
> +
> +    if (!msg_crypto->payload_encountered)
> +	msg_crypto->decryption_status = NOTMUCH_MESSAGE_DECRYPTED_FULL;
> +    else if (msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_NONE)
> +	msg_crypto->decryption_status = NOTMUCH_MESSAGE_DECRYPTED_PARTIAL;
> +

The logic / purpose of this is not very clear without referring to the header.

> +/* The user probably wants to know if the entire message was in the
> + * clear.  When replying, the MUA probably wants to know whether there
> + * was any part decrypted in the message.  And when displaying to the
> + * user, we probably only want to display "encrypted message" if the
> + * entire message was covered by encryption. */

I know you've discussed this elsewhere, but do you have some sense that
most clients are generating encrypted messages that are "well formed" in
the sense of the entire message being covered by encryption? It might be
good to have some messages from enigmail etc... in the test suite when
we merge this change.


More information about the notmuch mailing list