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

2020-05-07 Thread Tomi Ollila
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 
> ---
>  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 < +#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", 
> ));
> +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, , );
> +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.XX"); 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 < +*** Error: GMime fails to calculate X.509 certificate validity, and
> +is later 

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

2020-05-06 Thread Daniel Kahn Gillmor
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 | 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 <
+#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", 
));
+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, 
, );
+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.XX"); 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