Re: [PATCH 2/2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7

[Tomi Ollila <tomi.ollila@iki.fi>]

On Wed, May 06 2020, Daniel Kahn Gillmor wrote:

> 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.

This works, on top of the previous series -- I skipped git am:ing 1/2,
so thos works without it -- it is good stuff just IMO requires some changes)

Tomi

>
> 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
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch