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 1EF666DE0C51 for ; Sun, 15 Sep 2019 00:38:56 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -1.348 X-Spam-Level: X-Spam-Status: No, score=-1.348 tagged_above=-999 required=5 tests=[AWL=1.153, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001] 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 EpIicDl8W5Nx for ; Sun, 15 Sep 2019 00:38:54 -0700 (PDT) Received: from che.mayfirst.org (che.mayfirst.org [162.247.75.118]) by arlo.cworth.org (Postfix) with ESMTPS id 985F26DE0C3B for ; Sun, 15 Sep 2019 00:38:54 -0700 (PDT) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/simple; d=fifthhorseman.net; i=@fifthhorseman.net; q=dns/txt; s=2019; t=1568533131; h=from : to : subject : in-reply-to : references : date : message-id : mime-version : content-type : from; bh=FvtBIuywa7QlglVK+71Jz+vdqR7sEkqHi4PjHW3AOdY=; b=HkDlGVc6p5095E590GanN3EkYeh7FpqIY8NzTwF8UFcSm3PYQCbPqRYm USZY2RnN8Aptg8LrfKxDfIWSMiN1Dw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fifthhorseman.net; i=@fifthhorseman.net; q=dns/txt; s=2019rsa; t=1568533131; h=from : to : subject : in-reply-to : references : date : message-id : mime-version : content-type : from; bh=FvtBIuywa7QlglVK+71Jz+vdqR7sEkqHi4PjHW3AOdY=; b=D0nc4Ayf6izxQWbGoErzJmpnuW9zcjB9E5SI9U/Q4KHiaTvgv6PNstY5 F+wZD6UZXuswi7aE3yluW2UIYK+bd0y0ZnzYPiV5XtER9bVrRPyH7J9Jkl yVq/99dBv4i3cdTUl3b48Y2t0SCV0xgSPfNlRXn1ILbUDGPl8e9vUecddc GDjKaXAnQZoPlQcZTQ2kRFqe06g6lxN8RGgUS2vC5jVciQF6d2yhtnDzTI l7b+m01Gg+p2NKgdPI/r91QIygWB+iDJ6HYX4NjmFFOo9rSjl/8x1VrkLy 4aQOGY8BAe1ayQopb9nZ8c8TTKcl18t2CvsrNfyym241bUlYcPGrvA== Received: from fifthhorseman.net (ool-6c3a0662.static.optonline.net [108.58.6.98]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by che.mayfirst.org (Postfix) with ESMTPSA id 778E0F9A6; Sun, 15 Sep 2019 03:38:51 -0400 (EDT) Received: by fifthhorseman.net (Postfix, from userid 1000) id D7B222048F; Sun, 15 Sep 2019 03:37:09 -0400 (EDT) From: Daniel Kahn Gillmor To: David Bremner , Notmuch Mail Subject: Re: [PATCH v4 2/4] util/repair: identify and repair "Mixed Up" mangled messages In-Reply-To: <871rwj1vwc.fsf@tethera.net> References: <20190909032726.8931-1-dkg@fifthhorseman.net> <20190909032726.8931-3-dkg@fifthhorseman.net> <871rwj1vwc.fsf@tethera.net> Autocrypt: addr=dkg@fifthhorseman.net; prefer-encrypt=mutual; keydata= mDMEXEK/AhYJKwYBBAHaRw8BAQdAr/gSROcn+6m8ijTN0DV9AahoHGafy52RRkhCZVwxhEe0K0Rh bmllbCBLYWhuIEdpbGxtb3IgPGRrZ0BmaWZ0aGhvcnNlbWFuLm5ldD6ImQQTFggAQQIbAQUJA8Jn AAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgBYhBMS8Lds4zOlkhevpwvIGkReQOOXGBQJcQsbzAhkB AAoJEPIGkReQOOXG4fkBAO1joRxqAZY57PjdzGieXLpluk9RkWa3ufkt3YUVEpH/AP9c+pgIxtyW +FwMQRjlqljuj8amdN4zuEqaCy4hhz/1DbgzBFxCv4sWCSsGAQQB2kcPAQEHQERSZxSPmgtdw6nN u7uxY7bzb9TnPrGAOp9kClBLRwGfiPUEGBYIACYWIQTEvC3bOMzpZIXr6cLyBpEXkDjlxgUCXEK/ iwIbAgUJAeEzgACBCRDyBpEXkDjlxnYgBBkWCAAdFiEEyQ5tNiAKG5IqFQnndhgZZSmuX/gFAlxC v4sACgkQdhgZZSmuX/iVWgD/fCU4ONzgy8w8UCHGmrmIZfDvdhg512NIBfx+Mz9ls5kA/Rq97vz4 z48MFuBdCuu0W/fVqVjnY7LN5n+CQJwGC0MIA7QA/RyY7Sz2gFIOcrns0RpoHr+3WI+won3xCD8+ sVXSHZvCAP98HCjDnw/b0lGuCR7coTXKLIM44/LFWgXAdZjm1wjODbg4BFxCv50SCisGAQQBl1UB BQEBB0BG4iXnHX/fs35NWKMWQTQoRI7oiAUt0wJHFFJbomxXbAMBCAeIfgQYFggAJhYhBMS8Lds4 zOlkhevpwvIGkReQOOXGBQJcQr+dAhsMBQkB4TOAAAoJEPIGkReQOOXGe/cBAPlek5d9xzcXUn/D kY6jKmxe26CTws3ZkbK6Aa5Ey/qKAP0VuPQSCRxA7RKfcB/XrEphfUFkraL06Xn/xGwJ+D0hCw== Date: Sun, 15 Sep 2019 03:37:09 -0400 Message-ID: <87ftky104a.fsf@fifthhorseman.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.29 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: Sun, 15 Sep 2019 07:38:56 -0000 --=-=-= Content-Type: text/plain Thanks for the review, David. On Fri 2019-09-13 22:58:27 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> +/* 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 *first, *second, *third = NULL; >> + char *prelude_string = NULL; >> + bool prelude_is_empty; >> + >> + if (! g_mime_content_type_is_type (g_mime_object_get_content_type (part), >> + "multipart", "mixed")) >> + return false; > > Can g_mime_object_get_content_type plausibly fail (and return NULL) here? hm, yes, it can -- if it's passed NULL, for example. It's not an external function, but I can program this more defensively. i'll do that in the next revision. >> + if (! GMIME_IS_MULTIPART (part)) >> + return false; > > I guess this happens if the mime structure does not match the content > type declaration? Not sure if this needs a comment or if it's clear > enough. yeah, this is unlikely, maybe even impossible given the current gmime implementation, but it's a defensive step. >> + mpart = GMIME_MULTIPART (part); >> + if (mpart == NULL) >> + return false; >> + if (g_mime_multipart_get_count (mpart) != 3) >> + return false; >> + first = g_mime_multipart_get_part (mpart, 0); > > there's a slight cognitive dissonance for me between the zero and one > based indexing schemes here. part0, part1, and part2? or maybe an > GMimeObject *part[3] sure, i'll make it *parts[3] to avoid the dissonance. ("part" is already taken -- it's the argument for the function. >> + if (! g_mime_content_type_is_type (g_mime_object_get_content_type (first), >> + "text", "plain")) >> + return false; >> + if (! GMIME_IS_TEXT_PART (first)) >> + return false; >> + second = g_mime_multipart_get_part (mpart, 1); >> + if (! g_mime_content_type_is_type (g_mime_object_get_content_type (second), >> + "application", "pgp-encrypted")) >> + return false; >> + third = g_mime_multipart_get_part (mpart, 2); >> + if (! g_mime_content_type_is_type (g_mime_object_get_content_type (third), >> + "application", "octet-stream")) >> + return false; >> + >> + /* Is first subpart length 0? */ >> + prelude_string = g_mime_text_part_get_text (GMIME_TEXT_PART (first)); >> + prelude_is_empty = ! (strcmp ("", prelude_string)); >> + g_free (prelude_string); > > It might make sense to use the EMPTY_STRING macro here, although > currently it's only accesible via notmuch-private.h hm, notmuch-private.h is in lib/, which seems inappropriate for use outside the library. however, i agree that this seems like a superfluous call to strcmp -- i'll change it for a simple test. I've updated this 2/4 patch on my mixed-up-mangling branch (visible at https://gitlab.com/dkg/notmuch), and i'll send it to this thread shortly. --dkg --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQTJDm02IAobkioVCed2GBllKa5f+AUCXX3qJQAKCRB2GBllKa5f +J2nAQCFXtP4Q1qW6PJIyNkH38LEgYNj3QmK6yuSV2FDC41BggD+PK6IciFsQBgp ZEwT/8j2Qd8QMp70eh94IFJi+IZliwc= =uUB9 -----END PGP SIGNATURE----- --=-=-=--