From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id 4OeOOs8Ox17jBwAA0tVLHw (envelope-from ) for ; Thu, 21 May 2020 23:29:19 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id qHZuNs8Ox16KFAAA1q6Kng (envelope-from ) for ; Thu, 21 May 2020 23:29:19 +0000 Received: from arlo.cworth.org (arlo.cworth.org [50.126.95.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 8C721940365 for ; Thu, 21 May 2020 23:29:18 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 7EF896DE10AD; Thu, 21 May 2020 16:29:14 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org 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 kLzOWXQm4l6C; Thu, 21 May 2020 16:29:13 -0700 (PDT) Received: from arlo.cworth.org (localhost [IPv6:::1]) by arlo.cworth.org (Postfix) with ESMTP id 545F36DE1028; Thu, 21 May 2020 16:29:13 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 9EADE6DE1028 for ; Thu, 21 May 2020 16:29:11 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org 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 7y282lqjxdGQ for ; Thu, 21 May 2020 16:29:10 -0700 (PDT) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id 7D7DC6DE0F54 for ; Thu, 21 May 2020 16:29:10 -0700 (PDT) Received: from remotemail by fethera.tethera.net with local (Exim 4.92) (envelope-from ) id 1jbucd-0008FL-L1; Thu, 21 May 2020 19:29:07 -0400 Received: (nullmailer pid 1493259 invoked by uid 1000); Thu, 21 May 2020 23:29:05 -0000 From: David Bremner To: Daniel Kahn Gillmor , Notmuch Mail Subject: Re: [PATCH 2/2 v2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7 In-Reply-To: <20200512222010.371054-1-dkg@fifthhorseman.net> References: <20200506235438.100518-2-dkg@fifthhorseman.net> <20200512222010.371054-1-dkg@fifthhorseman.net> Date: Thu, 21 May 2020 20:29:05 -0300 Message-ID: <875zcovpdq.fsf@tethera.net> MIME-Version: 1.0 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: notmuch-bounces@notmuchmail.org Sender: "notmuch" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 50.126.95.6 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Spam-Score: 0.49 X-TUID: Bk9ZjZNVNS+j Daniel Kahn Gillmor writes: > When checking cryptographic signatures, Notmuch relies on GMime to > tell it whether the certificate that signs a message has a valid User > ID or not. > > If the User ID is not valid, then notmuch does not report the signer's > User ID to the user. This means that the consumer of notmuch's > cryptographic summary of a message (or of its protected headers) can > be confident in relaying the reported identity to the user. > > However, some versions of GMime before 3.2.7 cannot report Certificate > validity for X.509 certificates. This is resolved upstream in GMime > at https://github.com/jstedfast/gmime/pull/90. > > We adapt to this by marking tests of reported User IDs for > S/MIME-signed messages as known-broken if GMime is older than 3.2.7 > and has not been patched. > > If GMime >= 3.2.7 and certificate validity still doesn't work for > X.509 certs, then there has likely been a regression in GMime and we > should fail early, during ./configure. > > To break out these specific User ID checks from other checks, i had to > split some tests into two parts, and reuse $output across the two > subtests. > > Signed-off-by: Daniel Kahn Gillmor > --- > configure | 79 ++++++++++++++++++++++++++++++++++ > test/T355-smime.sh | 17 +++++--- > test/T356-protected-headers.sh | 13 +++++- > 3 files changed, 100 insertions(+), 9 deletions(-) > > diff --git a/configure b/configure > index 0cfdaa6f..92e5bd1b 100755 > --- a/configure > +++ b/configure > @@ -536,6 +536,82 @@ EOF > if [ -n "$TEMP_GPG" -a -d "$TEMP_GPG" ]; then > rm -rf "$TEMP_GPG" > fi > + > + # see https://github.com/jstedfast/gmime/pull/90 > + # should be fixed in GMime in 3.2.7, but some distros might patch > + printf "Checking for GMime X.509 certificate validity... " > + > + cat > _check_x509_validity.c < +#include > +#include > + > +int main () { > + GError *error = NULL; > + GMimeParser *parser = NULL; > + GMimeApplicationPkcs7Mime *body = NULL; > + GMimeSignatureList *sig_list = NULL; > + GMimeSignature *sig = NULL; > + GMimeCertificate *cert = NULL; > + GMimeObject *output = NULL; > + GMimeValidity validity = GMIME_VALIDITY_UNKNOWN; > + int len; > + > + g_mime_init (); > + parser = g_mime_parser_new (); > + g_mime_parser_init_with_stream (parser, g_mime_stream_file_open("test/corpora/pkcs7/smime-onepart-signed.eml", "r", &error)); > + if (error) return !! fprintf (stderr, "failed to instantiate parser with test/corpora/pkcs7/smime-onepart-signed.eml\n"); > + > + body = GMIME_APPLICATION_PKCS7_MIME(g_mime_message_get_mime_part (g_mime_parser_construct_message (parser, NULL))); > + if (body == NULL) return !! fprintf (stderr, "did not find a application/pkcs7 message\n"); I find these long lines with !! in the middle pretty surprising. Is there some reason for this style? It doesn't seem to fit with the usual conventions. This line in particular has a tab in the middle. > + elif ${CC} ${CFLAGS} ${gmime_cflags} _check_x509_validity.c ${gmime_ldflags} -o _check_x509_validity \ The other test files are cleaned up in configure (source and binary) once we are done with them. As far as I could follow, the changes to the tests themselves look reasonable.