Failing notmuch/SMIME test

Failing notmuch/SMIME test

[David Bremner]

One of the SMIME tests is failing for me.

T355-smime: Testing S/MIME signature verification and decryption
 FAIL   signature verification (notmuch CLI)
	--- T355-smime.4.expected	2022-03-18 10:31:31.877258855 +0000
	+++ T355-smime.4.output	2022-03-18 10:31:31.877258855 +0000
	@@ -24,7 +24,7 @@
	                         "sigstatus": [
	                             {
	                                 "created": 946728000,
	-                                "email": "<test_suite@notmuchmail.org>",
	+                                "email": "test_suite@notmuchmail.org",
	                                 "expires": 424242424,
	                                 "fingerprint": "616F46CD73834C63847756AF0DFB64A6E0972A47",
	                                 "status": "good",
	@@ -38,7 +38,7 @@
	                         "status": [
	                             {
	                                 "created": 946728000,
	-                                "email": "<test_suite@notmuchmail.org>",
	+                                "email": "test_suite@notmuchmail.org",
	                                 "expires": 424242424,
	                                 "fingerprint": "616F46CD73834C63847756AF0DFB64A6E0972A47",
	                                 "status": "good",


I'm running gpgsm 2.2.27, gpgme 1.16.0, and gmime 3.2.9. At guess the
change is due to the recent gmime upgrade, but that is pure speculation,
I could not find anything in the gmime git log to back it up. The change
looks innocuous enough, but of course it's enough to break the test, and
I'm not sure how to make this consistent between versions.

Re: Failing notmuch/SMIME test

[Daniel Kahn Gillmor]

[-- Attachment #1.1.1: Type: text/plain, Size: 2915 bytes --]

Hi Bremner, thanks for flagging this.

On Fri 2022-03-18 07:48:01 -0300, David Bremner wrote:
> One of the SMIME tests is failing for me.
>
> T355-smime: Testing S/MIME signature verification and decryption
>  FAIL   signature verification (notmuch CLI)
> 	--- T355-smime.4.expected	2022-03-18 10:31:31.877258855 +0000
> 	+++ T355-smime.4.output	2022-03-18 10:31:31.877258855 +0000
> 	@@ -24,7 +24,7 @@
> 	                         "sigstatus": [
> 	                             {
> 	                                 "created": 946728000,
> 	-                                "email": "<test_suite@notmuchmail.org>",
> 	+                                "email": "test_suite@notmuchmail.org",
> 	                                 "expires": 424242424,
> 	                                 "fingerprint": "616F46CD73834C63847756AF0DFB64A6E0972A47",
> 	                                 "status": "good",
> 	@@ -38,7 +38,7 @@
> 	                         "status": [
> 	                             {
> 	                                 "created": 946728000,
> 	-                                "email": "<test_suite@notmuchmail.org>",
> 	+                                "email": "test_suite@notmuchmail.org",
> 	                                 "expires": 424242424,
> 	                                 "fingerprint": "616F46CD73834C63847756AF0DFB64A6E0972A47",
> 	                                 "status": "good",
>
>
> I'm running gpgsm 2.2.27, gpgme 1.16.0, and gmime 3.2.9. At guess the
> change is due to the recent gmime upgrade, but that is pure speculation,
> I could not find anything in the gmime git log to back it up.

I think it is probably due to this squashed changeset in gmime:
https://github.com/jstedfast/gmime/commit/0ab298a0086c09c403b5d35effa73b59f271693d

(yes, this is my own proposed change, weirdly re-structured by github)

The root motivation for this change is some lack of clarity in the
underlying gpgme toolkit, which i've never been able to get resolved:
https://dev.gnupg.org/T5450

> The change looks innocuous enough, but of course it's enough to break
> the test, and I'm not sure how to make this consistent between
> versions.

I suppose the right way to fix this generically is a test which
abstracts out whether gmime reports an angle-addr or a addr-spec for
x.509 certs, and then adjust the tests to match.

I can try to send a patch for this, but it'll take me a while to swap
it all back in.

If anyone wants to propose a patch in the meantime, i'd also be happy to
review.

The simplest thing in the short term is probably to switch the test to
matching based on the bare e-mail address and assert a build-dep on
gmime 3.2.8 (see attached), but that seems a little bit extreme, since
gmime only released 3.2.9 recently (and 3.2.8 never made it out the door
via any formal channels, if i understand the history correctly).

I'll see whether i can make a better fix.

   --dkg


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-test-avoid-breakage-with-the-latest-gmime.patch --]
[-- Type: text/x-diff, Size: 2352 bytes --]

From df2b487c5db2af183a75bc32e3a3adf9b90c6316 Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Date: Sun, 20 Mar 2022 17:07:13 -0400
Subject: [PATCH] test: avoid breakage with the latest gmime

since gmime 3.2.7, it is now reporting the more stable "email" field from gpgme as the cert info:

https://github.com/jstedfast/gmime/commit/0ab298a0086c09c403b5d35effa73b59f271693d

However, this changes the minimum version of gmime quite dramatically.

A better fix would add a gmime test that normalizes the test results
appropriately, contingent on whether this change is made in gmime, but
i haven't written that test yet.
---
 configure          | 2 +-
 test/T355-smime.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 36f3f606..d529c358 100755
--- a/configure
+++ b/configure
@@ -481,7 +481,7 @@ if [ ${have_xapian} = "0" ]; then
     errors=$((errors + 1))
 fi
 
-GMIME_MINVER=3.0.3
+GMIME_MINVER=3.2.8
 
 printf "Checking for GMime development files (>= $GMIME_MINVER)... "
 if pkg-config --exists "gmime-3.0 >= $GMIME_MINVER"; then
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 31fa4b4e..c25f94f9 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -46,7 +46,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
- "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "<test_suite@notmuchmail.org>", "expires": 424242424, "created": 946728000}]}},
+ "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "test_suite@notmuchmail.org", "expires": 424242424, "created": 946728000}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -55,7 +55,7 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [{"fingerprint": "'$FINGERPRINT'",
  "status": "good",
  "userid": "CN=Notmuch Test Suite",
- "email": "<test_suite@notmuchmail.org>",
+ "email": "test_suite@notmuchmail.org",
  "expires": 424242424,
  "created": 946728000}],
  "content-type": "multipart/signed",
-- 
2.35.1


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Failing notmuch/SMIME test

[David Bremner]

Daniel Kahn Gillmor <dkg@debian.org> writes:

> I suppose the right way to fix this generically is a test which
> abstracts out whether gmime reports an angle-addr or a addr-spec for
> x.509 certs, and then adjust the tests to match.
>
> I can try to send a patch for this, but it'll take me a while to swap
> it all back in.
>
> If anyone wants to propose a patch in the meantime, i'd also be happy to
> review.
>
> The simplest thing in the short term is probably to switch the test to
> matching based on the bare e-mail address and assert a build-dep on
> gmime 3.2.8 (see attached), but that seems a little bit extreme, since
> gmime only released 3.2.9 recently (and 3.2.8 never made it out the door
> via any formal channels, if i understand the history correctly).

What do you think about accepting both with < > and without, via some
"sanitize" function? Or just marking the test broken for old gmime. I
think we do something like the latter in some places.

d

[PATCH 1/2] configure: restructure gmime cert validity checker code

[David Bremner]

The goal is to generalize this to also check the output format of
g_mime_certificate_get_email.
---
 configure | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 36f3f606..d6e1200e 100755
--- a/configure
+++ b/configure
@@ -552,11 +552,7 @@ EOF
 	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
+    cat > _check_gmime_cert.c <<EOF
 #include <stdio.h>
 #include <gmime/gmime.h>
 
@@ -589,16 +585,22 @@ int main () {
     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");
+#ifdef CHECK_VALIDITY
     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);
-
+#endif
     return 0;
 }
 EOF
+
+    # 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... "
+
     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 \
+    elif ${CC} -DCHECK_VALIDITY ${CFLAGS} ${gmime_cflags} _check_gmime_cert.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
-- 
2.35.1

[PATCH 2/2] test/smime: fix signature verification test with newer gmime.

[David Bremner]

The extra machinery to check for the actual output format is justified
by the possibility that distros may patch this newere output format
into older versions of gmime.
---
 configure          | 17 +++++++++++++++++
 test/T355-smime.sh |  7 +++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index d6e1200e..922de60d 100755
--- a/configure
+++ b/configure
@@ -588,6 +588,11 @@ int main () {
 #ifdef CHECK_VALIDITY
     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);
+#endif
+#ifdef CHECK_EMAIL
+    const char *email = g_mime_certificate_get_email (cert);
+    if (! email) return !! fprintf (stderr, "no email returned");
+    if (email[0] == '<') return 2;
 #endif
     return 0;
 }
@@ -622,6 +627,15 @@ EOF
 		errors=$((errors + 1))
 	    fi
 	fi
+	printf "Checking for GMime new email format... "
+	if ${CC} -DCHECK_EMAIL ${CFLAGS} ${gmime_cflags} _check_gmime_cert.c ${gmime_ldflags} -o _check_email &&
+		GNUPGHOME=${TEMP_GPG} ./_check_email; then
+	    gmime_new_email_format=1
+	    printf "Yes.\n"
+	else
+	    gmime_new_email_format=0
+	    printf "No (some tests will be skipped).\n"
+	fi
     else
 	printf 'No.\nFailed to set up gpgsm for testing X.509 certificate validity support.\n'
 	errors=$((errors + 1))
@@ -1559,6 +1573,9 @@ NOTMUCH_HAVE_XAPIAN_DB_RETRY_LOCK=${WITH_RETRY_LOCK}
 # Whether GMime can verify X.509 certificate validity
 NOTMUCH_GMIME_X509_CERT_VALIDITY=${gmime_x509_cert_validity}
 
+# Whether GMime returns bare emails (without <>)
+NOTMUCH_GMIME_NEW_EMAIL_FORMAT=${gmime_new_email_format}
+
 # Whether GMime can verify signatures when decrypting with a session key:
 NOTMUCH_GMIME_VERIFY_WITH_SESSION_KEY=${gmime_verify_with_session_key}
 
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 31fa4b4e..2905263b 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -35,6 +35,9 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "signature verification (notmuch CLI)"
+if [ $NOTMUCH_GMIME_NEW_EMAIL_FORMAT != 1 ]; then
+    test_subtest_known_broken
+fi
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
     | sed -e 's|"created": [-1234567890]*|"created": 946728000|g' \
@@ -46,7 +49,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
- "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "<test_suite@notmuchmail.org>", "expires": 424242424, "created": 946728000}]}},
+ "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "test_suite@notmuchmail.org", "expires": 424242424, "created": 946728000}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -55,7 +58,7 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [{"fingerprint": "'$FINGERPRINT'",
  "status": "good",
  "userid": "CN=Notmuch Test Suite",
- "email": "<test_suite@notmuchmail.org>",
+ "email": "test_suite@notmuchmail.org",
  "expires": 424242424,
  "created": 946728000}],
  "content-type": "multipart/signed",
-- 
2.35.1

Re: [PATCH 2/2] test/smime: fix signature verification test with newer gmime.

[Daniel Kahn Gillmor]

[-- Attachment #1.1: Type: text/plain, Size: 1247 bytes --]

Thanks, Bremner!

This series looks reasonable to me.  nice clever hack to reuse the
gmime embedded .c source for the test.

a bit of a tweak below:

On Sat 2022-04-09 09:34:53 -0300, David Bremner wrote:
> +	printf "Checking for GMime new email format... "
> +	if ${CC} -DCHECK_EMAIL ${CFLAGS} ${gmime_cflags} _check_gmime_cert.c ${gmime_ldflags} -o _check_email &&
> +		GNUPGHOME=${TEMP_GPG} ./_check_email; then
> +	    gmime_new_email_format=1
> +	    printf "Yes.\n"
> +	else
> +	    gmime_new_email_format=0
> +	    printf "No (some tests will be skipped).\n"
> +	fi
>      else
>  	printf 'No.\nFailed to set up gpgsm for testing X.509 certificate validity support.\n'
>  	errors=$((errors + 1))

Words like "new" have a tendency to get, well, old.

I'd say

   "Checking GMime emits email addresses from certs without angle brackets..."

And i'd name the variable gmime_cert_addresses_have_angle_brackets (so
"1" effectively means "probably a stale, deprecated version of GMime").

Then change the rest of the tests to match.

This is kind of an aesthetic choice -- i'd be fine with the original
patch too.  but it seems safer to just identify the out-of-date stuff
when it happens, rather than identifying the current stuff.

      --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] test/smime: fix signature verification test with newer gmime.

[Michael J Gruber]

[-- Attachment #1.1: Type: text/plain, Size: 2354 bytes --]

Am Mo., 11. Apr. 2022 um 02:36 Uhr schrieb Daniel Kahn Gillmor <
dkg@debian.org>:

> Thanks, Bremner!
>
> This series looks reasonable to me.  nice clever hack to reuse the
> gmime embedded .c source for the test.
>

Just so that others don't have to be wondering, too: notmuch does not embed
gmime sources and does not reuse them. (It would be very wrong to do so.)
notmuch has a file `_check_gmime_cert.c` which is used for configure checks
and which David cleverly amended to check for the return format of
signature checks (when compiled against the libgmime3).

If I read 2/2 correctly, though, then T355-smime does not adjust its
expected textual outcome to the results of the check, but rather marks the
test "known broken" if the signature check does not return the "new"
format. In other words: Unless you have a very new unpatched gmime,
T355-sime does not "really" do this subtest any more - it is happy as soon
as it fails for any reason.


> a bit of a tweak below:
>
> On Sat 2022-04-09 09:34:53 -0300, David Bremner wrote:
> > +     printf "Checking for GMime new email format... "
> > +     if ${CC} -DCHECK_EMAIL ${CFLAGS} ${gmime_cflags}
> _check_gmime_cert.c ${gmime_ldflags} -o _check_email &&
> > +             GNUPGHOME=${TEMP_GPG} ./_check_email; then
> > +         gmime_new_email_format=1
> > +         printf "Yes.\n"
> > +     else
> > +         gmime_new_email_format=0
> > +         printf "No (some tests will be skipped).\n"
> > +     fi
> >      else
> >       printf 'No.\nFailed to set up gpgsm for testing X.509 certificate
> validity support.\n'
> >       errors=$((errors + 1))
>
> Words like "new" have a tendency to get, well, old.
>
> I'd say
>
>    "Checking GMime emits email addresses from certs without angle
> brackets..."
>
> And i'd name the variable gmime_cert_addresses_have_angle_brackets (so
> "1" effectively means "probably a stale, deprecated version of GMime").
>
> Then change the rest of the tests to match.
>
> This is kind of an aesthetic choice -- i'd be fine with the original
> patch too.  but it seems safer to just identify the out-of-date stuff
> when it happens, rather than identifying the current stuff.
>
>       --dkg
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org
>

[-- Attachment #1.2: Type: text/html, Size: 3329 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] test/smime: fix signature verification test with newer gmime.

[Daniel Kahn Gillmor]

[-- Attachment #1.1: Type: text/plain, Size: 973 bytes --]

On Mon 2022-04-11 10:44:47 +0200, Michael J Gruber wrote:
> Just so that others don't have to be wondering, too: notmuch does not embed
> gmime sources and does not reuse them. (It would be very wrong to do so.)
> notmuch has a file `_check_gmime_cert.c` which is used for configure checks
> and which David cleverly amended to check for the return format of
> signature checks (when compiled against the libgmime3).

Yep, this is what i meant.

> If I read 2/2 correctly, though, then T355-smime does not adjust its
> expected textual outcome to the results of the check, but rather marks the
> test "known broken" if the signature check does not return the "new"
> format. In other words: Unless you have a very new unpatched gmime,
> T355-sime does not "really" do this subtest any more - it is happy as soon
> as it fails for any reason.

Exactly right.  Thanks for taking the time to explain it all in clearer
English than my rushed shorthand, Michael.

        --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2 2/2] test/smime: fix signature verification test with newer gmime.

[michaeljgruber+grubix+git]

From: David Bremner <david@tethera.net>

The extra machinery to check for the actual output format is justified
by the possibility that distros may patch this newer output format
into older versions of gmime.

Amended-by: Michael J Gruber <git@grubix.eu>
Signed-off-by: Michael J Gruber <git@grubix.eu>
---
Here is what I meant with my comments: We have everything in place to
adjust the expected test output to the detected gmime behaviour. This
also takes into account dkg's remarks on the variable names.

[And yes, I have list bounces again, please forgive my mess and multiple
subscriptions to work around it.]

 configure          | 17 +++++++++++++++++
 test/T355-smime.sh | 11 +++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index d6e1200e..056f9232 100755
--- a/configure
+++ b/configure
@@ -588,6 +588,11 @@ int main () {
 #ifdef CHECK_VALIDITY
     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);
+#endif
+#ifdef CHECK_EMAIL
+    const char *email = g_mime_certificate_get_email (cert);
+    if (! email) return !! fprintf (stderr, "no email returned");
+    if (email[0] == '<') return 2;
 #endif
     return 0;
 }
@@ -622,6 +627,15 @@ EOF
 		errors=$((errors + 1))
 	    fi
 	fi
+	printf "Checking whether GMime emits email addresses with angle brackets... "
+	if ${CC} -DCHECK_EMAIL ${CFLAGS} ${gmime_cflags} _check_gmime_cert.c ${gmime_ldflags} -o _check_email &&
+		GNUPGHOME=${TEMP_GPG} ./_check_email; then
+	    gmime_emits_angle_brackets=0
+	    printf "No.\n"
+	else
+	    gmime_emits_angle_brackets=1
+	    printf "Yes.\n"
+	fi
     else
 	printf 'No.\nFailed to set up gpgsm for testing X.509 certificate validity support.\n'
 	errors=$((errors + 1))
@@ -1559,6 +1573,9 @@ NOTMUCH_HAVE_XAPIAN_DB_RETRY_LOCK=${WITH_RETRY_LOCK}
 # Whether GMime can verify X.509 certificate validity
 NOTMUCH_GMIME_X509_CERT_VALIDITY=${gmime_x509_cert_validity}
 
+# Whether GMime emits addresses with angle brackets (with <>)
+NOTMUCH_GMIME_EMITS_ANGLE_BRACKETS=${gmime_emits_angle_brackets}
+
 # Whether GMime can verify signatures when decrypting with a session key:
 NOTMUCH_GMIME_VERIFY_WITH_SESSION_KEY=${gmime_verify_with_session_key}
 
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 31fa4b4e..b15169b7 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -35,6 +35,13 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "signature verification (notmuch CLI)"
+if [ $NOTMUCH_GMIME_EMITS_ANGLE_BRACKETS == 1 ]; then
+    LEFT_ANGLE='<'
+    RIGHT_ANGLE='>'
+else
+    LEFT_ANGLE=''
+    RIGHT_ANGLE=''
+fi
 output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | notmuch_json_show_sanitize \
     | sed -e 's|"created": [-1234567890]*|"created": 946728000|g' \
@@ -46,7 +53,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
- "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "<test_suite@notmuchmail.org>", "expires": 424242424, "created": 946728000}]}},
+ "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "'$LEFT_ANGLE'test_suite@notmuchmail.org'$RIGHT_ANGLE'", "expires": 424242424, "created": 946728000}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -55,7 +62,7 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [{"fingerprint": "'$FINGERPRINT'",
  "status": "good",
  "userid": "CN=Notmuch Test Suite",
- "email": "<test_suite@notmuchmail.org>",
+ "email": "'$LEFT_ANGLE'test_suite@notmuchmail.org'$RIGHT_ANGLE'",
  "expires": 424242424,
  "created": 946728000}],
  "content-type": "multipart/signed",
-- 
2.36.0.rc0.457.gf4fc0d8e4e

Re: [PATCH v2 2/2] test/smime: fix signature verification test with newer gmime.

[Daniel Kahn Gillmor]

[-- Attachment #1.1: Type: text/plain, Size: 5100 bytes --]

Thanks, Michael--

This LGTM.

It is more narrowly-targeted at permitting this specific variation than
Bremner's earlier version of the patch (and it doesn't have any tests
marked BROKEN), which is nice.

It might be marginally cleaner to swap out the LEFT_ANGLE RIGHT_ANGLE
variables for a single replacement variable like so:

if [ $NOTMUCH_GMIME_EMITS_ANGLE_BRACKETS == 1 ]; then
   EXPECTED_EMAIL_ADDR='<test_suite@notmuchmail.org>'
else
   EXPECTED_EMAIL_ADDR='test_suite@notmuchmail.org'
fi

This makes for only one variable substitution in the json comparison
tests, if i'm looking at it right.

Any of these approaches is fine with me.

    --dkg

On Tue 2022-04-12 22:15:56 +0200, michaeljgruber+grubix+git@gmail.com wrote:
> From: David Bremner <david@tethera.net>
>
> The extra machinery to check for the actual output format is justified
> by the possibility that distros may patch this newer output format
> into older versions of gmime.
>
> Amended-by: Michael J Gruber <git@grubix.eu>
> Signed-off-by: Michael J Gruber <git@grubix.eu>
> ---
> Here is what I meant with my comments: We have everything in place to
> adjust the expected test output to the detected gmime behaviour. This
> also takes into account dkg's remarks on the variable names.
>
> [And yes, I have list bounces again, please forgive my mess and multiple
> subscriptions to work around it.]
>
>  configure          | 17 +++++++++++++++++
>  test/T355-smime.sh | 11 +++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index d6e1200e..056f9232 100755
> --- a/configure
> +++ b/configure
> @@ -588,6 +588,11 @@ int main () {
>  #ifdef CHECK_VALIDITY
>      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);
> +#endif
> +#ifdef CHECK_EMAIL
> +    const char *email = g_mime_certificate_get_email (cert);
> +    if (! email) return !! fprintf (stderr, "no email returned");
> +    if (email[0] == '<') return 2;
>  #endif
>      return 0;
>  }
> @@ -622,6 +627,15 @@ EOF
>  		errors=$((errors + 1))
>  	    fi
>  	fi
> +	printf "Checking whether GMime emits email addresses with angle brackets... "
> +	if ${CC} -DCHECK_EMAIL ${CFLAGS} ${gmime_cflags} _check_gmime_cert.c ${gmime_ldflags} -o _check_email &&
> +		GNUPGHOME=${TEMP_GPG} ./_check_email; then
> +	    gmime_emits_angle_brackets=0
> +	    printf "No.\n"
> +	else
> +	    gmime_emits_angle_brackets=1
> +	    printf "Yes.\n"
> +	fi
>      else
>  	printf 'No.\nFailed to set up gpgsm for testing X.509 certificate validity support.\n'
>  	errors=$((errors + 1))
> @@ -1559,6 +1573,9 @@ NOTMUCH_HAVE_XAPIAN_DB_RETRY_LOCK=${WITH_RETRY_LOCK}
>  # Whether GMime can verify X.509 certificate validity
>  NOTMUCH_GMIME_X509_CERT_VALIDITY=${gmime_x509_cert_validity}
>  
> +# Whether GMime emits addresses with angle brackets (with <>)
> +NOTMUCH_GMIME_EMITS_ANGLE_BRACKETS=${gmime_emits_angle_brackets}
> +
>  # Whether GMime can verify signatures when decrypting with a session key:
>  NOTMUCH_GMIME_VERIFY_WITH_SESSION_KEY=${gmime_verify_with_session_key}
>  
> diff --git a/test/T355-smime.sh b/test/T355-smime.sh
> index 31fa4b4e..b15169b7 100755
> --- a/test/T355-smime.sh
> +++ b/test/T355-smime.sh
> @@ -35,6 +35,13 @@ EOF
>  test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest "signature verification (notmuch CLI)"
> +if [ $NOTMUCH_GMIME_EMITS_ANGLE_BRACKETS == 1 ]; then
> +    LEFT_ANGLE='<'
> +    RIGHT_ANGLE='>'
> +else
> +    LEFT_ANGLE=''
> +    RIGHT_ANGLE=''
> +fi
>  output=$(notmuch show --format=json --verify subject:"test signed message 001" \
>      | notmuch_json_show_sanitize \
>      | sed -e 's|"created": [-1234567890]*|"created": 946728000|g' \
> @@ -46,7 +53,7 @@ expected='[[[{"id": "XXXXX",
>   "timestamp": 946728000,
>   "date_relative": "2000-01-01",
>   "tags": ["inbox","signed"],
> - "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "<test_suite@notmuchmail.org>", "expires": 424242424, "created": 946728000}]}},
> + "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "'$LEFT_ANGLE'test_suite@notmuchmail.org'$RIGHT_ANGLE'", "expires": 424242424, "created": 946728000}]}},
>   "headers": {"Subject": "test signed message 001",
>   "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
>   "To": "test_suite@notmuchmail.org",
> @@ -55,7 +62,7 @@ expected='[[[{"id": "XXXXX",
>   "sigstatus": [{"fingerprint": "'$FINGERPRINT'",
>   "status": "good",
>   "userid": "CN=Notmuch Test Suite",
> - "email": "<test_suite@notmuchmail.org>",
> + "email": "'$LEFT_ANGLE'test_suite@notmuchmail.org'$RIGHT_ANGLE'",
>   "expires": 424242424,
>   "created": 946728000}],
>   "content-type": "multipart/signed",
> -- 
> 2.36.0.rc0.457.gf4fc0d8e4e

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 2/2] test/smime: fix signature verification test with newer gmime.

[David Bremner]

Daniel Kahn Gillmor <dkg@debian.org> writes:

> Thanks, Michael--
>
> This LGTM.
>
> It is more narrowly-targeted at permitting this specific variation than
> Bremner's earlier version of the patch (and it doesn't have any tests
> marked BROKEN), which is nice.
>
> It might be marginally cleaner to swap out the LEFT_ANGLE RIGHT_ANGLE
> variables for a single replacement variable like so:
>
> if [ $NOTMUCH_GMIME_EMITS_ANGLE_BRACKETS == 1 ]; then
>    EXPECTED_EMAIL_ADDR='<test_suite@notmuchmail.org>'
> else
>    EXPECTED_EMAIL_ADDR='test_suite@notmuchmail.org'
> fi

In the end I applied this version to master. Thanks to you both.

d