Re: [PATCH v5 2/4] util/repair: identify and repair "Mixed Up" mangled messages

2019-09-15 Thread Daniel Kahn Gillmor
On Sun 2019-09-15 23:26:59 +0300, Tomi Ollila wrote:
> On Sun, Sep 15 2019, Daniel Kahn Gillmor wrote:
>
>> This patch implements a functional identification and repair process
>
> If there is going to be more versions, then the above could be changed
> to either 
>
> 1) This commit implements...
>
> or just
>
> 2) Implement a functional ...
>
>> for "Mixed Up" MIME messages as described in
>> https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1
>>
>> The detection test is not entirely complete, in that it does not
>> verify the contents of the latter two message subparts, but this is
>> probably safe to skip, because those two parts are unlikely to be
>> readable anyway, and the only part we are effectively omitting (the
>> first subpart) is guaranteed to be empty anyway, so its removal can be
>> reversed if you want to do so.  I've left FIXMEs in the code so that
>> anyone excited about adding these additional checks can see where to
>> put them in.
>>
>> I'll use this functionality in the next two patches.
>
> If there is going to be more versions, then the above line could be removed...
>
> (as committed changes are not "patches" ...)

I concur with (and appreciate) your terminological pedantry.  the
revision on my gitlab branch has adopted your suggestions, but i don't
want to burden the list with another revision right now.  hopefully this
change to the commit message won't block adoption -- anyone merging the
series should feel free to adjust the commit message as Tomi recommends.

 --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v5 2/4] util/repair: identify and repair "Mixed Up" mangled messages

2019-09-15 Thread Tomi Ollila
On Sun, Sep 15 2019, Daniel Kahn Gillmor wrote:

> This patch implements a functional identification and repair process

If there is going to be more versions, then the above could be changed
to either 

1) This commit implements...

or just

2) Implement a functional ...

> for "Mixed Up" MIME messages as described in
> https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1
>
> The detection test is not entirely complete, in that it does not
> verify the contents of the latter two message subparts, but this is
> probably safe to skip, because those two parts are unlikely to be
> readable anyway, and the only part we are effectively omitting (the
> first subpart) is guaranteed to be empty anyway, so its removal can be
> reversed if you want to do so.  I've left FIXMEs in the code so that
> anyone excited about adding these additional checks can see where to
> put them in.
>
> I'll use this functionality in the next two patches.

If there is going to be more versions, then the above line could be removed...

(as committed changes are not "patches" ...)

Tomi

>
> Signed-off-by: Daniel Kahn Gillmor 
> ---
>  util/repair.c | 84 +++
>  util/repair.h | 10 ++
>  2 files changed, 94 insertions(+)
>
> diff --git a/util/repair.c b/util/repair.c
> index 629e6f23..9fba97b7 100644
> --- a/util/repair.c
> +++ b/util/repair.c
> @@ -120,3 +120,87 @@ _notmuch_repair_crypto_payload_skip_legacy_display 
> (GMimeObject *payload)
>   return payload;
>  }
>  }
> +
> +/* see
> + * 
> https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1.1
>  */
> +static bool
> +_notmuch_is_mixed_up_mangled (GMimeObject *part)
> +{
> +GMimeMultipart *mpart = NULL;
> +GMimeObject *parts[3] = {NULL, NULL, NULL};
> +GMimeContentType *type = NULL;
> +char *prelude_string = NULL;
> +bool prelude_is_empty;
> +
> +if (part == NULL)
> + return false;
> +type = g_mime_object_get_content_type (part);
> +if (type == NULL)
> + return false;
> +if (! g_mime_content_type_is_type (type, "multipart", "mixed"))
> + return false;
> +if (! GMIME_IS_MULTIPART (part)) /* probably impossible */
> + return false;
> +mpart = GMIME_MULTIPART (part);
> +if (mpart == NULL)
> + return false;
> +if (g_mime_multipart_get_count (mpart) != 3)
> + return false;
> +parts[0] = g_mime_multipart_get_part (mpart, 0);
> +if (! g_mime_content_type_is_type (g_mime_object_get_content_type 
> (parts[0]),
> +"text", "plain"))
> + return false;
> +if (! GMIME_IS_TEXT_PART (parts[0]))
> + return false;
> +parts[1] = g_mime_multipart_get_part (mpart, 1);
> +if (! g_mime_content_type_is_type (g_mime_object_get_content_type 
> (parts[1]),
> +"application", "pgp-encrypted"))
> + return false;
> +parts[2] = g_mime_multipart_get_part (mpart, 2);
> +if (! g_mime_content_type_is_type (g_mime_object_get_content_type 
> (parts[2]),
> +"application", "octet-stream"))
> + return false;
> +
> +/* Is parts[0] length 0? */
> +prelude_string = g_mime_text_part_get_text (GMIME_TEXT_PART (parts[0]));
> +prelude_is_empty = (prelude_string[0] == '\0');
> +g_free (prelude_string);
> +if (! prelude_is_empty)
> + return false;
> +
> +/* FIXME: after decoding and stripping whitespace, is parts[1]
> + * subpart just "Version: 1" ? */
> +
> +/* FIXME: can we determine that parts[2] subpart is *only* PGP
> + * encrypted data?  I tried g_mime_part_get_openpgp_data () but
> + * found https://github.com/jstedfast/gmime/issues/60 */
> +
> +return true;
> +}
> +
> +
> +/* see
> + * 
> https://tools.ietf.org/html/draft-dkg-openpgp-pgpmime-message-mangling-00#section-4.1.2
>  */
> +GMimeObject *
> +_notmuch_repair_mixed_up_mangled (GMimeObject *part)
> +{
> +GMimeMultipart *mpart = NULL, *mpart_ret = NULL;
> +GMimeObject *ret = NULL;
> +
> +if (! _notmuch_is_mixed_up_mangled (part))
> + return NULL;
> +mpart = GMIME_MULTIPART (part);
> +ret = GMIME_OBJECT (g_mime_multipart_encrypted_new ());
> +if (ret == NULL)
> + return NULL;
> +mpart_ret = GMIME_MULTIPART (ret);
> +if (mpart_ret == NULL) {
> + g_object_unref (ret);
> + return NULL;
> +}
> +g_mime_object_set_content_type_parameter (ret, "protocol", 
> "application/pgp-encrypted");
> +
> +g_mime_multipart_insert (mpart_ret, 0, g_mime_multipart_get_part (mpart, 
> 1));
> +g_mime_multipart_insert (mpart_ret, 1, g_mime_multipart_get_part (mpart, 
> 2));
> +return ret;
> +}
> diff --git a/util/repair.h b/util/repair.h
> index 9974d693..492f5a20 100644
> --- a/util/repair.h
> +++ b/util/repair.h
> @@ -25,9 +25,19 @@ extern "C" {
>   * returned object will only be released when the original part is
>   * disposed of.
>