unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
To: Notmuch Mail <notmuch@notmuchmail.org>
Subject: [PATCH 2/2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7
Date: Wed,  6 May 2020 19:54:38 -0400	[thread overview]
Message-ID: <20200506235438.100518-2-dkg@fifthhorseman.net> (raw)
In-Reply-To: <20200506235438.100518-1-dkg@fifthhorseman.net>

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 <dkg@fifthhorseman.net>
---
 configure                      | 79 ++++++++++++++++++++++++++++++++++
 test/T355-smime.sh             | 19 +++++---
 test/T356-protected-headers.sh | 15 ++++++-
 3 files changed, 104 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 <<EOF
+#include <stdio.h>
+#include <gmime/gmime.h>
+
+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");
+
+    sig_list = g_mime_application_pkcs7_mime_verify (body, GMIME_VERIFY_NONE, &output, &error);
+    if (error || output == NULL) return !! fprintf (stderr, "verify failed\n");
+
+    if (sig_list == NULL) return !! fprintf (stderr, "no GMimeSignatureList found\n");
+    len = g_mime_signature_list_length (sig_list);
+    if (len != 1) return !! fprintf (stderr, "expected 1 signature, got %d\n", len);
+    sig = g_mime_signature_list_get_signature (sig_list, 0);
+    if (sig == NULL) return !! fprintf (stderr, "no GMimeSignature found at position 0\n");
+    cert = g_mime_signature_get_certificate (sig);
+    if (cert == NULL) return !! fprintf (stderr, "no GMimeCertificate found\n");
+    validity = g_mime_certificate_get_id_validity (cert);
+    if (validity != GMIME_VALIDITY_FULL) return !! fprintf (stderr, "Got validity %d, expected %d\n", validity, GMIME_VALIDITY_FULL);
+
+    return 0;
+}
+EOF
+    if ! TEMP_GPG=$(mktemp -d "${TMPDIR:-/tmp}/notmuch.XXXXXX"); then
+        printf 'No.\nCould not make tempdir for testing X.509 certificate validity support.\n'
+        errors=$((errors + 1))
+    elif ${CC} ${CFLAGS} ${gmime_cflags} _check_x509_validity.c ${gmime_ldflags} -o _check_x509_validity \
+            && echo disable-crl-checks > "$TEMP_GPG/gpgsm.conf" \
+            && echo "4D:E0:FF:63:C0:E9:EC:01:29:11:C8:7A:EE:DA:3A:9A:7F:6E:C1:0D S" >> "$TEMP_GPG/trustlist.txt" \
+            && GNUPGHOME=${TEMP_GPG} gpgsm --batch --quiet --import < "$srcdir"/test/smime/ca.crt
+    then
+        if GNUPGHOME=${TEMP_GPG} ./_check_x509_validity; then
+            gmime_x509_cert_validity=1
+            printf "Yes.\n"
+        else
+            gmime_x509_cert_validity=0
+            printf "No.\n"
+            if pkg-config --exists "gmime-3.0 >= 3.2.7"; then
+                cat <<EOF
+*** Error: GMime fails to calculate X.509 certificate validity, and
+is later than 3.2.7, which should have fixed this issue.
+
+Please follow up on https://github.com/jstedfast/gmime/pull/90 with
+more details.
+EOF
+                errors=$((errors + 1))
+            fi
+        fi
+    else
+        printf 'No.\nFailed to set up gpgsm for testing X.509 certificate validity support.\n'
+        errors=$((errors + 1))
+    fi
+    if [ -n "$TEMP_GPG" -a -d "$TEMP_GPG" ]; then
+        rm -rf "$TEMP_GPG"
+    fi
 else
     have_gmime=0
     printf "No.\n"
@@ -1334,6 +1410,9 @@ NOTMUCH_HAVE_XAPIAN_DB_RETRY_LOCK=${WITH_RETRY_LOCK}
 # Which backend will Xapian use by default?
 NOTMUCH_DEFAULT_XAPIAN_BACKEND=${default_xapian_backend}
 
+# Whether GMime can verify X.509 certificate validity
+NOTMUCH_GMIME_X509_CERT_VALIDITY=${gmime_x509_cert_validity}
+
 # do we have man pages?
 NOTMUCH_HAVE_MAN=$((have_sphinx))
 
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 3573b5ee..170f8649 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -177,13 +177,18 @@ test_valid_json "$output"
 
 test_begin_subtest "Verify signature on PKCS#7 SignedData message"
 output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
+
+test_json_nodes <<<"$output" \
+                'created:[0][0][0]["crypto"]["signed"]["status"][0]["created"]=1574813489' \
+                'expires:[0][0][0]["crypto"]["signed"]["status"][0]["expires"]=2611032858' \
+                'fingerprint:[0][0][0]["crypto"]["signed"]["status"][0]["fingerprint"]="702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB"' \
+                'status:[0][0][0]["crypto"]["signed"]["status"][0]["status"]="good"'
+
+test_begin_subtest "Verify signature on PKCS#7 SignedData message signer User ID"
+if [ $NOTMUCH_GMIME_X509_CERT_VALIDITY -ne 1 ]; then
+    test_subtest_known_broken
+fi
 test_json_nodes <<<"$output" \
-                'crypto:[0][0][0]["crypto"]["signed"]["status"][0]={
-                        "created" : 1574813489,
-                        "expires" : 2611032858,
-                        "fingerprint" : "702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB",
-                        "userid" : "CN=Alice Lovelace",
-                        "status" : "good"
-                     }'
+                'userid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Alice Lovelace"'
 
 test_done
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 547b0c9a..074a2345 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -162,8 +162,13 @@ for variant in multipart-signed onepart-signed; do
                     'signed_subject:[0][0][0]["crypto"]["signed"]["headers"]=["Subject"]' \
                     'sig_good:[0][0][0]["crypto"]["signed"]["status"][0]["status"]="good"' \
                     'sig_fpr:[0][0][0]["crypto"]["signed"]["status"][0]["fingerprint"]="702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB"' \
-                    'sig_uid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Alice Lovelace"' \
                     'not_encrypted:[0][0][0]["crypto"]!"decrypted"'
+    test_begin_subtest "verify signed PKCS#7 subject ($variant) signer User ID"
+    if [ $NOTMUCH_GMIME_X509_CERT_VALIDITY -ne 1 ]; then
+        test_subtest_known_broken
+    fi
+    test_json_nodes <<<"$output" \
+                    'sig_uid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Alice Lovelace"'
 done
 
 for variant in sign+enc sign+enc+legacy-disp; do
@@ -173,8 +178,14 @@ for variant in sign+enc sign+enc+legacy-disp; do
                     'signed_subject:[0][0][0]["crypto"]["signed"]["headers"]=["Subject"]' \
                     'sig_good:[0][0][0]["crypto"]["signed"]["status"][0]["status"]="good"' \
                     'sig_fpr:[0][0][0]["crypto"]["signed"]["status"][0]["fingerprint"]="702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB"' \
-                    'sig_uid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Alice Lovelace"' \
                     'encrypted:[0][0][0]["crypto"]["decrypted"]={"status":"full","header-mask":{"Subject":"..."}}'
+    test_begin_subtest "confirm signed and encrypted PKCS#7 subject ($variant) signer User ID"
+    if [ $NOTMUCH_GMIME_X509_CERT_VALIDITY -ne 1 ]; then
+        test_subtest_known_broken
+    fi
+    test_json_nodes <<<"$output" \
+                    'sig_uid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Alice Lovelace"'
+
 done
 
 test_begin_subtest "confirm encryption-protected PKCS#7 subject (enc+legacy-disp)"
-- 
2.26.2

  reply	other threads:[~2020-05-06 23:55 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30 20:13 Handle PKCS#7 S/MIME messages Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 1/9] lib: index PKCS7 SignedData parts Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 2/9] smime: Identify encrypted S/MIME parts during indexing Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 3/9] cli: include wrapped part of PKCS#7 SignedData in the MIME tree Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 4/9] cli/show: If a leaf part has children, show them instead of omitting Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 5/9] cli/reply: Ignore PKCS#7 wrapper parts when replying Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 6/9] crypto: Make _notmuch_crypto_decrypt take a GMimeObject Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 7/9] crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 8/9] smime: Pass PKCS#7 envelopedData to node_decrypt_and_verify Daniel Kahn Gillmor
2020-04-30 20:13 ` [PATCH 9/9] smime: Index cleartext of envelopedData when requested Daniel Kahn Gillmor
2020-05-01 21:15 ` Handle PKCS#7 S/MIME messages Tomi Ollila
2020-05-04 19:16   ` Daniel Kahn Gillmor
2020-05-05  8:32     ` Tomi Ollila
2020-05-05 18:07       ` David Bremner
2020-05-06 23:54       ` [PATCH 1/2] test-lib: mark function variables as local Daniel Kahn Gillmor
2020-05-06 23:54         ` Daniel Kahn Gillmor [this message]
2020-05-07 20:54           ` [PATCH 2/2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7 Tomi Ollila
2020-05-12 22:20           ` [PATCH 2/2 v2] " Daniel Kahn Gillmor
2020-05-21 23:29             ` David Bremner
2020-05-22  0:41               ` Daniel Kahn Gillmor
2020-05-22  0:42               ` [PATCH 2/2 v3] " Daniel Kahn Gillmor
2020-05-23 11:56                 ` David Bremner
2020-05-07  7:31         ` [PATCH 1/2] test-lib: mark function variables as local Tomi Ollila
2020-05-08 20:04           ` Daniel Kahn Gillmor
2020-05-08 23:24         ` [PATCH 1/2 v2] " Daniel Kahn Gillmor
2020-05-09  7:09           ` Tomi Ollila
2020-05-09 11:47           ` David Bremner
2020-05-10 18:03             ` Daniel Kahn Gillmor
2020-05-10 19:02               ` David Bremner
2020-05-12 22:14             ` Daniel Kahn Gillmor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200506235438.100518-2-dkg@fifthhorseman.net \
    --to=dkg@fifthhorseman.net \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).