Handle PKCS#7 signedData (S/MIME single-part clearsigned)

Handle PKCS#7 signedData (S/MIME single-part clearsigned)

[Daniel Kahn Gillmor]

PKCS#7 is the binary format used by Cryptographic Message Syntax
(CMS).

S/MIME (RFC 8551) specifies a number of different "smime-type" PKCS#7
wrapping layers.

Until now, notmuch has only handled multipart/signed PKCS#7
clearsigned messages.

But S/MIME has another form of clearsigned message named "signedData"
(or as the smime-type parameter writes it "signed-data").  This type
of clearsigned message can transit through mangling MTAs slightly
better than multipart/signed, because an S/MIME-ignorant MTA will
treat it as an opaque blob.  This avoids the scenario where a legacy
MTA tampers with the body of a multipart/signed message in violation
of §3 of RFC 1847, breaking the signature.

However, PKCS#7 signedData objects are also opaque to legacy
(S/MIME-ignorant) MUAs, which can't read the contents of the
clearsigned message because they don't know how to unpack the PKCS#7.

notmuch is currently such a legacy MUA, but this series will change
that, giving notmuch the ability to read the contents of (and verify
the signatures on) PKCS#7 signedData parts.

Note that this series does not yet handle PKCS#7 envelopedData or
authEnvelopedData (the S/MIME encryption layers, which differ only in
ciphertext malleability), and it also does not handle compressedData.

The framework it puts in place should be useful in handling those
kinds of additional S/MIME layers, as long as GMime is capable of
handling them.  But the series is long enough without adding that
additional complexity, and I'd love to get some review of it before i
carry on working on the decryption parts.

The series starts with a small number of cleanup/framing patches, then
adds a sample S/MIME signedData message and a bunch of known-broken
tests, then proceeds to fix each of the tests.

Several of the patches should be simple to read and uncontroversial (I
hope!), and I welcome/encourage merging of those patches to reduce the
length of the remaining series.

As always, feedback and critique and questions are welcome :)

   --dkg

[PATCH 01/14] mime-node: Pass the correct flags to g_mime_multipart_signed_verify

[Daniel Kahn Gillmor]

GMIME_ENCRYPT_NONE and GMIME_VERIFY_NONE have the same value, but they
are different enumerated types.  So in C, this is a cosmetic change,
but it is technically correct if we only had stricter typing.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 mime-node.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index d4996a33..e531078c 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -201,7 +201,7 @@ node_verify (mime_node_t *node, GMimeObject *part)
 
     node->verify_attempted = true;
     node->sig_list = g_mime_multipart_signed_verify (
-	GMIME_MULTIPART_SIGNED (part), GMIME_ENCRYPT_NONE, &err);
+	GMIME_MULTIPART_SIGNED (part), GMIME_VERIFY_NONE, &err);
 
     if (node->sig_list)
 	set_signature_list_destructor (node);
-- 
2.24.0

[PATCH 02/14] tests/smime: Always use --batch with gpgsm

[Daniel Kahn Gillmor]

GnuPG's gpgsm, like gpg, should always be used with --batch when it is
invoked in a non-interactive environment.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T355-smime.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 336da917..c272533a 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -10,8 +10,8 @@ add_gpgsm_home ()
     _gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
     at_exit_function _gnupg_exit
     mkdir -m 0700 "$GNUPGHOME"
-    gpgsm --no-tty --no-common-certs-import --disable-dirmngr --import < $NOTMUCH_SRCDIR/test/smime/test.crt >"$GNUPGHOME"/import.log 2>&1
-    fpr=$(gpgsm  --list-key test_suite@notmuchmail.org | sed -n 's/.*fingerprint: //p')
+    gpgsm --batch --no-tty --no-common-certs-import --disable-dirmngr --import < $NOTMUCH_SRCDIR/test/smime/test.crt >"$GNUPGHOME"/import.log 2>&1
+    fpr=$(gpgsm --batch --list-key test_suite@notmuchmail.org | sed -n 's/.*fingerprint: //p')
     echo "$fpr S relax" >> $GNUPGHOME/trustlist.txt
     test_debug "cat $GNUPGHOME/import.log"
 }
-- 
2.24.0

[PATCH 03/14] tests/smime: Include the Sample LAMPS Certificate Authority

[Daniel Kahn Gillmor]

This CA is useful for test suites and the like, but is not an
actually-secure CA, because its secret key material is also published.

I plan to use it for its intended purpose in the notmuch test suite.

It was copied from this Internet Draft:

https://www.ietf.org/id/draft-dkg-lamps-samples-01.html#name-certificate-authority-certi

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T355-smime.sh |  2 ++
 test/smime/README  |  2 ++
 test/smime/ca.crt  | 20 ++++++++++++++++++++
 3 files changed, 24 insertions(+)
 create mode 100644 test/smime/ca.crt

diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index c272533a..dac9b1e5 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -13,6 +13,8 @@ add_gpgsm_home ()
     gpgsm --batch --no-tty --no-common-certs-import --disable-dirmngr --import < $NOTMUCH_SRCDIR/test/smime/test.crt >"$GNUPGHOME"/import.log 2>&1
     fpr=$(gpgsm --batch --list-key test_suite@notmuchmail.org | sed -n 's/.*fingerprint: //p')
     echo "$fpr S relax" >> $GNUPGHOME/trustlist.txt
+    gpgsm --quiet --batch --no-tty --no-common-certs-import --disable-dirmngr --import < $NOTMUCH_SRCDIR/test/smime/ca.crt
+    echo "4D:E0:FF:63:C0:E9:EC:01:29:11:C8:7A:EE:DA:3A:9A:7F:6E:C1:0D S" >> "$GNUPGHOME/trustlist.txt"
     test_debug "cat $GNUPGHOME/import.log"
 }
 
diff --git a/test/smime/README b/test/smime/README
index 92803c77..6b8d3f87 100644
--- a/test/smime/README
+++ b/test/smime/README
@@ -5,3 +5,5 @@ key+cert.pem: cert + unencryped private
     % gpsm --import test.crt
     % gpgsm --export-private-key-p12 -out foo.p12  (no passphrase)
     % openssl pkcs12 -in ns.p12 -clcerts -nodes > key+cert.pem
+
+ca.crt: from https://www.ietf.org/id/draft-dkg-lamps-samples-01.html#name-certificate-authority-certi
diff --git a/test/smime/ca.crt b/test/smime/ca.crt
new file mode 100644
index 00000000..b33d087f
--- /dev/null
+++ b/test/smime/ca.crt
@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDLTCCAhWgAwIBAgIULXcNXGI2bZp38sV7cF6VcQfnKDwwDQYJKoZIhvcNAQEN
+BQAwLTErMCkGA1UEAxMiU2FtcGxlIExBTVBTIENlcnRpZmljYXRlIEF1dGhvcml0
+eTAgFw0xOTExMjAwNjU0MThaGA8yMDUyMDkyNzA2NTQxOFowLTErMCkGA1UEAxMi
+U2FtcGxlIExBTVBTIENlcnRpZmljYXRlIEF1dGhvcml0eTCCASIwDQYJKoZIhvcN
+AQEBBQADggEPADCCAQoCggEBAMUfZ8+NYSh6h36zQcXBo5B6ficAcBJ1f3aLxyN8
+QXB83XuP8aDRWQ9uJvJpQkWVH4zx96/E/zI0t0lDMYtZNqra16h+gxbHJgoq2pRw
+RCOiyYu/p2vzvvZ1dtFTMc/mIigjA/73kokui62j1EFy//fNVIihkVS3rAweq+fI
+8qJHSMhdc2aYa9wOP0eGe/HTiDYgT4L4f2HTGMGGwQgj1vub0gpR4YHmNqr0GyEA
+63mHUQUZpnmN1FEl+nVFA5Ntu4uF++qf/tkTji89/eXYBdKX2yUdTeTIKoCI65IL
+EXxezjTc8aFjf/8E0aWGVZR/DtCsjWOh/s/mV7n/YPyb4+ECAwEAAaNDMEEwDwYD
+VR0TAQH/BAUwAwEB/zAPBgNVHQ8BAf8EBQMDBwYAMB0GA1UdDgQWBBS3Uk1zwIg9
+ssN6WgzzlPf3gKJ32zANBgkqhkiG9w0BAQ0FAAOCAQEALsU91Bmhc6EgCNr7inY2
+2gYPnosJ+kZ1eC0hvHIK9e0Tx74RmhTOe8M2C9YXQKehHpRaX+DLcjup6scoH/bT
+u0THbmzeOy29TTiFcyV9BK+SEKQWW4s98Fwdk9fPWcflHtYvqxjooAV3vHbt6Xmp
+KrKDz/jdg7t0ptI4zSqAf3wNppiJoswlOHBUnH2W1MIYkWQ4jYj5socblVlklHOr
+ykKUiEZAbjU+C1+0FhT4HgLjBB9R4H1H0JRKsggWiZBBJ6UpN0dTN4iD0mDVa0jy
+sJqqWnIViy/xaSDcNaWJmU3o2KmkMkdpinoJ5uLkAHQqXjFaujdU1PkufeA7v3uG
+Rw==
+-----END CERTIFICATE-----
-- 
2.24.0

[PATCH 04/14] tests/smime: consistently quote $GNUPGHOME

[Daniel Kahn Gillmor]

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T355-smime.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index dac9b1e5..cbd3e5a6 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -6,13 +6,13 @@ test_description='S/MIME signature verification and decryption'
 add_gpgsm_home ()
 {
     local fpr
-    [ -d ${GNUPGHOME} ] && return
+    [ -d "$GNUPGHOME" ] && return
     _gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
     at_exit_function _gnupg_exit
     mkdir -m 0700 "$GNUPGHOME"
     gpgsm --batch --no-tty --no-common-certs-import --disable-dirmngr --import < $NOTMUCH_SRCDIR/test/smime/test.crt >"$GNUPGHOME"/import.log 2>&1
     fpr=$(gpgsm --batch --list-key test_suite@notmuchmail.org | sed -n 's/.*fingerprint: //p')
-    echo "$fpr S relax" >> $GNUPGHOME/trustlist.txt
+    echo "$fpr S relax" >> "$GNUPGHOME/trustlist.txt"
     gpgsm --quiet --batch --no-tty --no-common-certs-import --disable-dirmngr --import < $NOTMUCH_SRCDIR/test/smime/ca.crt
     echo "4D:E0:FF:63:C0:E9:EC:01:29:11:C8:7A:EE:DA:3A:9A:7F:6E:C1:0D S" >> "$GNUPGHOME/trustlist.txt"
     test_debug "cat $GNUPGHOME/import.log"
-- 
2.24.0

[PATCH 05/14] test-lib.sh: add test_valid_json

[Daniel Kahn Gillmor]

This test does exactly what it says on the tin.  It expects JSON data
to be parseable by Python, at least.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/test-lib.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 7f8a3a4d..a54ae40f 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -503,6 +503,12 @@ test_expect_equal_json () {
     test_expect_equal "$output" "$expected" "$@"
 }
 
+# Ensure that the argument is valid JSON data.
+test_valid_json () {
+    PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON -c "import sys, json; json.load(sys.stdin)" <<<"$1"
+    test_expect_equal "$?" 0
+}
+
 # Sort the top-level list of JSON data from stdin.
 test_sort_json () {
     PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON -c \
-- 
2.24.0

[PATCH 06/14] tests/smime: add tests for S/MIME SignedData

[Daniel Kahn Gillmor]

Add a simple S/MIME SignedData message, taken from an upcoming draft
of
https://datatracker.ietf.org/doc/draft-autocrypt-lamps-protected-headers/

RFC 8551 describes a SignedData, a one-part clearsigned object that is
more resistant to common patterns of MTA message munging than
multipart/signed (but has the downside that it is only readable by
clients that implement S/MIME).

To make sure sure notmuch can handle this kind of object, we want to
know a few things:

Already working:

 - Is the content of the SignedData object indexed?  It actually is
   right now because of dumb luck -- i think we're indexing the raw
   CMS object and it happens to contain the cleartext of the message
   in a way that we can consume it before passing it on to Xapian.
 - Are we accidentally indexing the embedded PKCS#7 certificates? We
   don't want to, and for some reason I don't understand, our indexing
   is actually skipping the embedded certificates already.  That's
   good!

Still need fixing:
 - do we know the MIME type of the embedded part?
 - do we know that the message is signed?
 - can notmuch-show read its content?
 - can notmuch-show indicate the signature validity?
 - can notmuch-reply properly quote and attribute content?

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T355-smime.sh                          | 76 +++++++++++++++++++++
 test/corpora/pkcs7/smime-onepart-signed.eml | 51 ++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100644 test/corpora/pkcs7/smime-onepart-signed.eml

diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index cbd3e5a6..9055e589 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -96,4 +96,80 @@ Verification successful
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+add_email_corpus pkcs7
+
+test_begin_subtest "index PKCS#7 SignedData message"
+output=$(notmuch search --output=messages Thanks)
+expected=id:smime-onepart-signed@protected-headers.example
+test_expect_equal "$expected" "$output"
+
+test_begin_subtest "do not index embedded certificates from PKCS#7 SignedData"
+output=$(notmuch search --output=messages 'LAMPS Certificate')
+expected=''
+test_expect_equal "$expected" "$output"
+
+test_begin_subtest "know the MIME type of the embedded part in PKCS#7 SignedData"
+test_subtest_known_broken
+output=$(notmuch search --output=messages 'mimetype:text/plain')
+expected=id:smime-onepart-signed@protected-headers.example
+test_expect_equal "$expected" "$output"
+
+test_begin_subtest "PKCS#7 SignedData message is tagged 'signed'"
+test_subtest_known_broken
+output=$(notmuch dump id:smime-onepart-signed@protected-headers.example)
+expected='#notmuch-dump batch-tag:3 config,properties,tags
++inbox +signed +unread -- id:smime-onepart-signed@protected-headers.example'
+test_expect_equal "$expected" "$output"
+
+test_begin_subtest "show contents of PKCS#7 SignedData message"
+test_subtest_known_broken
+output=$(notmuch show --format=raw --part=2 id:smime-onepart-signed@protected-headers.example)
+whitespace=' '
+expected="Bob, we need to cancel this contract.
+
+Please start the necessary processes to make that happen today.
+
+Thanks, Alice
+--${whitespace}
+Alice Lovelace
+President
+OpenPGP Example Corp"
+test_expect_equal "$expected" "$output"
+
+test_begin_subtest "reply to PKCS#7 SignedData message with proper quoting and attribution"
+test_subtest_known_broken
+output=$(notmuch reply id:smime-onepart-signed@protected-headers.example)
+expected="From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: The FooCorp contract
+To: Alice Lovelace <alice@smime.example>, Bob Babbage <bob@smime.example>
+In-Reply-To: <smime-onepart-signed@protected-headers.example>
+References: <smime-onepart-signed@protected-headers.example>
+
+On Tue, 26 Nov 2019 20:11:29 -0400, Alice Lovelace <alice@smime.example> wrote:
+> Bob, we need to cancel this contract.
+>${whitespace}
+> Please start the necessary processes to make that happen today.
+>${whitespace}
+> Thanks, Alice
+> --${whitespace}
+> Alice Lovelace
+> President
+> OpenPGP Example Corp"
+test_expect_equal "$expected" "$output"
+
+test_begin_subtest "show PKCS#7 SignedData outputs valid JSON"
+output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
+test_valid_json "$output"
+
+test_begin_subtest "Verify signature on PKCS#7 SignedData message"
+test_subtest_known_broken
+output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
+test_json_nodes <<<"$output" \
+                'crypto:[0][0][0]["crypto"]["signed"]["status"][0]={
+                        "created" : 1574813489,
+                        "expires" : 2611032858,
+                        "fingerprint" : "702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB",
+                        "status" : "good"
+                     }'
+
 test_done
diff --git a/test/corpora/pkcs7/smime-onepart-signed.eml b/test/corpora/pkcs7/smime-onepart-signed.eml
new file mode 100644
index 00000000..070303b7
--- /dev/null
+++ b/test/corpora/pkcs7/smime-onepart-signed.eml
@@ -0,0 +1,51 @@
+Received: from localhost (localhost [127.0.0.1]); Tue, 26 Nov 2019
+ 20:11:46 -0400 (UTC-04:00)
+Content-Transfer-Encoding: base64
+Content-Type: application/pkcs7-mime; name="smime.p7m";
+ smime-type="signed-data"
+MIME-Version: 1.0
+From: Alice Lovelace <alice@smime.example>
+To: Bob Babbage <bob@smime.example>
+Date: Tue, 26 Nov 2019 20:11:29 -0400
+Subject: The FooCorp contract
+Message-ID: <smime-onepart-signed@protected-headers.example>
+
+MIIHRQYJKoZIhvcNAQcCoIIHNjCCBzICAQExDTALBglghkgBZQMEAgEwggHJBgkq
+hkiG9w0BBwGgggG6BIIBtkNvbnRlbnQtVHlwZTogdGV4dC9wbGFpbjsgY2hhcnNl
+dD0idXMtYXNjaWkiDQpGcm9tOiBBbGljZSBMb3ZlbGFjZSA8YWxpY2VAc21pbWUu
+ZXhhbXBsZT4NClRvOiBCb2IgQmFiYmFnZSA8Ym9iQHNtaW1lLmV4YW1wbGU+DQpE
+YXRlOiBUdWUsIDI2IE5vdiAyMDE5IDIwOjExOjI5IC0wNDAwDQpTdWJqZWN0OiBU
+aGUgRm9vQ29ycCBjb250cmFjdA0KTWVzc2FnZS1JRDogPHNtaW1lLW9uZXBhcnQt
+c2lnbmVkQHByb3RlY3RlZC1oZWFkZXJzLmV4YW1wbGU+DQoNCkJvYiwgd2UgbmVl
+ZCB0byBjYW5jZWwgdGhpcyBjb250cmFjdC4NCg0KUGxlYXNlIHN0YXJ0IHRoZSBu
+ZWNlc3NhcnkgcHJvY2Vzc2VzIHRvIG1ha2UgdGhhdCBoYXBwZW4gdG9kYXkuDQoN
+ClRoYW5rcywgQWxpY2UNCi0tIA0KQWxpY2UgTG92ZWxhY2UNClByZXNpZGVudA0K
+T3BlblBHUCBFeGFtcGxlIENvcnANCqCCA3IwggNuMIICVqADAgECAhRngrRZc1JL
+wfRxRxlq8P0RiqpMCzANBgkqhkiG9w0BAQ0FADAtMSswKQYDVQQDEyJTYW1wbGUg
+TEFNUFMgQ2VydGlmaWNhdGUgQXV0aG9yaXR5MCAXDTE5MTEyMDA2NTQxOFoYDzIw
+NTIwOTI3MDY1NDE4WjAZMRcwFQYDVQQDEw5BbGljZSBMb3ZlbGFjZTCCASIwDQYJ
+KoZIhvcNAQEBBQADggEPADCCAQoCggEBAMPurfll0bYkDPMkY1kNn2xXsAqHSGVF
++gWNNk3mbhF6BABhLJqDjei5aLXFE3Rq9/RRNivCMrTipF1XsbMIAKgQqr/GI1Q6
+yN8lfNsK5uU3d9kw5cOyEooGpOGUrvlKMD0LPGDt6MaiJj+KJ2TR73Wd4rfRIIJo
+FMmV9HZkOs+Tvcg8x6SzGhNq18X2HD10MD78eLXKm039obRD+z2JwWvGvrLbNBey
+O5A+aMxmCPXRoP1xrNZWBFgKB+WGYDRXW5CXXChthTwMBXFWf4aBpurKMZAyjK2E
+grQafn6h/DFddQz/NtT6Dr7UhJ2hfFFEW2rYbNsiqQAdllCb4FucWuECAwEAAaOB
+lzCBlDAMBgNVHRMBAf8EAjAAMB4GA1UdEQQXMBWBE2FsaWNlQHNtaW1lLmV4YW1w
+bGUwEwYDVR0lBAwwCgYIKwYBBQUHAwQwDwYDVR0PAQH/BAUDAwegADAdBgNVHQ4E
+FgQUrC5UWqT9VRivLuhmRDjRJdHXAHkwHwYDVR0jBBgwFoAUt1JNc8CIPbLDeloM
+85T394Cid9swDQYJKoZIhvcNAQENBQADggEBAHvqjhjPvKtVIVyleoutwa10jir3
+dooJcQIILM1AunjJ6yHpuuppkc0m3BhwnlOptTKb2EnvSIkTiMY037IBlHWW217Q
+cUpggEozgQm6Yb77aGptRovPi2XToEdpA8K//02I1jur1H1z8HqzVjMeHCqRaG3Z
+r4C2AngGSkb6D4yZkxBX8CjtHAsUon06UxYsGYRcVykgk3Qek9qxPScSX8yai1K7
+7xGcKUCLfIV/JMpv7ysPtXG7Jd62oNnp1T/3+KoP9JlLs5AiPLC13fjeYALPcHVG
+UXEwdIDp1AB/Zu0a6apHQqICncqRhEB4+hompiQHtlp3TqeAWXQbQUc437sxggHZ
+MIIB1QIBATBFMC0xKzApBgNVBAMTIlNhbXBsZSBMQU1QUyBDZXJ0aWZpY2F0ZSBB
+dXRob3JpdHkCFGeCtFlzUkvB9HFHGWrw/RGKqkwLMAsGCWCGSAFlAwQCAaBpMBgG
+CSqGSIb3DQEJAzELBgkqhkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTE5MTEyNzAw
+MTEyOVowLwYJKoZIhvcNAQkEMSIEILsI9kL3zfZiVOEDjAUWrbjHjGMLoGUwEqYH
+pOA9XZ+QMA0GCSqGSIb3DQEBAQUABIIBAGDat8UYN9MShlKEw3hYVVUk6HKO6Xjp
+rdgCBKpoyoWJy0VJis0xHxaT2gn/+TPu8a5l6RslgeALjMyflzyzAmrqnknQQG8K
+bvbt/MwpU/TxnmxT+2oP9TVmAx/IQOq4pQ35uK7peSPck2CcTvZjHTeVBWcsLVEk
+hELoSD8XFRBo34qdinBzW0/sMlyK1XnlN7khKry1g7uaXcurVqptRA1rWOvCOt72
+aElKG/Q7OoVgHxbUpdzV3Hqe9/UeTRDUqCs++on2pLlA0TA0Pq8RQ0hDHD/p0t41
+1RAT1/RbnGQiVfRilMan+VGT4shokb1RoANy/1rOO9ZKlyWToYdRl9E=
-- 
2.24.0

[PATCH 07/14] lib: index PKCS7 SignedData parts

[Daniel Kahn Gillmor]

When we are indexing, we should treat SignedData parts the same way
that we treat a multipart object, indexing the wrapped part as a
distinct MIME object.

Unfortunately, this means doing some sort of cryptographic
verification whose results we throw away, because GMime doesn't offer
us any way to unwrap without doing signature verification.

I've opened https://github.com/jstedfast/gmime/issues/67 to request
the capability from GMime but for now, we'll just accept the
additional performance hit.

As we do this indexing, we also apply the "signed" tag, by analogy
with how we handle multipart/signed messages.  These days, that kind
of change should probably be done with a property instead, but that's
a different set of changes.  This one is just for consistency.

Note that we are currently *only* handling signedData parts, which are
basically clearsigned messages.  PKCS#7 parts can also be
envelopedData and authEnvelopedData (which are effectively encryption
layers), and compressedData (which afaict isn't implemented anywhere,
i've never encountered it).  We're laying the groundwork for indexing
these other S/MIME types here, but we're only dealing with signedData
for now.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 lib/index.cc       | 57 ++++++++++++++++++++++++++++++++++++++++++++++
 test/T355-smime.sh |  2 --
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 158ba5cf..bbf13dc5 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -372,6 +372,12 @@ _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *ind
 			    GMimeMultipartEncrypted *part,
 			    _notmuch_message_crypto_t *msg_crypto);
 
+static void
+_index_pkcs7_part (notmuch_message_t *message,
+		   notmuch_indexopts_t *indexopts,
+		   GMimeObject *part,
+		   _notmuch_message_crypto_t *msg_crypto);
+
 /* Callback to generate terms for each mime part of a message. */
 static void
 _index_mime_part (notmuch_message_t *message,
@@ -466,6 +472,11 @@ _index_mime_part (notmuch_message_t *message,
 	goto DONE;
     }
 
+    if (GMIME_IS_APPLICATION_PKCS7_MIME (part)) {
+	_index_pkcs7_part (message, indexopts, part, msg_crypto);
+	goto DONE;
+    }
+
     if (! (GMIME_IS_PART (part))) {
 	_notmuch_database_log (notmuch_message_get_database (message),
 			       "Warning: Not indexing unknown mime part: %s.\n",
@@ -608,6 +619,52 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 
 }
 
+static void
+_index_pkcs7_part (notmuch_message_t *message,
+		   notmuch_indexopts_t *indexopts,
+		   GMimeObject *part,
+		   _notmuch_message_crypto_t *msg_crypto)
+{
+    GMimeApplicationPkcs7Mime *pkcs7;
+    GMimeSecureMimeType p7type;
+    GMimeObject *mimeobj = NULL;
+    GMimeSignatureList *sigs = NULL;
+    GError *err = NULL;
+    notmuch_database_t *notmuch = NULL;
+
+    pkcs7 = GMIME_APPLICATION_PKCS7_MIME (part);
+    p7type = g_mime_application_pkcs7_mime_get_smime_type (pkcs7);
+    notmuch = notmuch_message_get_database (message);
+    _index_content_type (message, part);
+
+    if (p7type == GMIME_SECURE_MIME_TYPE_SIGNED_DATA) {
+	sigs = g_mime_application_pkcs7_mime_verify (pkcs7, GMIME_VERIFY_NONE, &mimeobj, &err);
+	if (sigs == NULL) {
+	    _notmuch_database_log (notmuch, "Failed to verify PKCS#7 SignedData during indexing. (%d:%d) [%s]\n",
+				   err->domain, err->code, err->message);
+	    g_error_free (err);
+	    goto DONE;
+	}
+	_notmuch_message_add_term (message, "tag", "signed");
+	GMimeObject *toindex = mimeobj;
+	if (_notmuch_message_crypto_potential_payload (msg_crypto, mimeobj, part, 0) &&
+	    msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL) {
+	    toindex = _notmuch_repair_crypto_payload_skip_legacy_display (mimeobj);
+	    if (toindex != mimeobj)
+		notmuch_message_add_property (message, "index.repaired", "skip-protected-headers-legacy-display");
+	}
+	_index_mime_part (message, indexopts, toindex, msg_crypto);
+    } else {
+	_notmuch_database_log (notmuch, "Cannot currently handle PKCS#7 smime-type '%s'\n",
+			       g_mime_object_get_content_type_parameter (part, "smime-type"));
+    }
+ DONE:
+    if (mimeobj)
+	g_object_unref (mimeobj);
+    if (sigs)
+	g_object_unref (sigs);
+}
+
 static notmuch_status_t
 _notmuch_message_index_user_headers (notmuch_message_t *message, GMimeMessage *mime_message)
 {
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 9055e589..32fdd9dc 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -109,13 +109,11 @@ expected=''
 test_expect_equal "$expected" "$output"
 
 test_begin_subtest "know the MIME type of the embedded part in PKCS#7 SignedData"
-test_subtest_known_broken
 output=$(notmuch search --output=messages 'mimetype:text/plain')
 expected=id:smime-onepart-signed@protected-headers.example
 test_expect_equal "$expected" "$output"
 
 test_begin_subtest "PKCS#7 SignedData message is tagged 'signed'"
-test_subtest_known_broken
 output=$(notmuch dump id:smime-onepart-signed@protected-headers.example)
 expected='#notmuch-dump batch-tag:3 config,properties,tags
 +inbox +signed +unread -- id:smime-onepart-signed@protected-headers.example'
-- 
2.24.0

[PATCH 08/14] mime-node: rename decrypted_child to unwrapped_child

[Daniel Kahn Gillmor]

When walking the MIME tree, we might need to extract a new MIME
object.  Thus far, we've only done it when decrypting
multipart/encrypted messages, but PKCS#7 (RFC 8551, S/MIME) has
several other transformations that warrant a comparable form of
unwrapping.

Make this member re-usable for PKCS#7 unwrappings as well as
multipart/encrypted decryptions.

This change is just a naming change, it has no effect on function.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 mime-node.c      | 10 +++++-----
 notmuch-client.h |  6 ++++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index e531078c..2a823dfd 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -227,19 +227,19 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part)
     GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
     notmuch_message_t *message = NULL;
 
-    if (! node->decrypted_child) {
+    if (! node->unwrapped_child) {
 	for (mime_node_t *parent = node; parent; parent = parent->parent)
 	    if (parent->envelope_file) {
 		message = parent->envelope_file;
 		break;
 	    }
 
-	node->decrypted_child = _notmuch_crypto_decrypt (&node->decrypt_attempted,
+	node->unwrapped_child = _notmuch_crypto_decrypt (&node->decrypt_attempted,
 							 node->ctx->crypto->decrypt,
 							 message,
 							 encrypteddata, &decrypt_result, &err);
     }
-    if (! node->decrypted_child) {
+    if (! node->unwrapped_child) {
 	fprintf (stderr, "Failed to decrypt part: %s\n",
 		 err ? err->message : "no error explanation given");
 	goto DONE;
@@ -380,8 +380,8 @@ mime_node_child (mime_node_t *parent, int child)
 	return NULL;
 
     if (GMIME_IS_MULTIPART (parent->part)) {
-	if (child == GMIME_MULTIPART_ENCRYPTED_CONTENT && parent->decrypted_child)
-	    sub = parent->decrypted_child;
+	if (child == GMIME_MULTIPART_ENCRYPTED_CONTENT && parent->unwrapped_child)
+	    sub = parent->unwrapped_child;
 	else
 	    sub = g_mime_multipart_get_part (
 		GMIME_MULTIPART (parent->part), child);
diff --git a/notmuch-client.h b/notmuch-client.h
index 74690054..89e15ba6 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -395,8 +395,10 @@ struct mime_node {
     struct mime_node_context *ctx;
 
     /* Internal: For successfully decrypted multipart parts, the
-     * decrypted part to substitute for the second child. */
-    GMimeObject *decrypted_child;
+     * decrypted part to substitute for the second child; or, for
+     * PKCS#7 parts, the part returned after removing/processing the
+     * PKCS#7 transformation */
+    GMimeObject *unwrapped_child;
 
     /* Internal: The next child for depth-first traversal and the part
      * number to assign it (or -1 if unknown). */
-- 
2.24.0

[PATCH 09/14] mime-node: Clean up unwrapped MIME parts correctly.

[Daniel Kahn Gillmor]

Avoid a memory leak in the notmuch command line.

gmime_multipart_encrypted_decrypt returns a GMimeObject marked by
GMime as "transfer full", so we are supposed to clean up after it.

When parsing a message, notmuch would leak one GMimeObject part per
multipart/encrypted MIME layer.  We clean it up by analogy with
cleaning up the signature list associated with a MIME node.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 mime-node.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/mime-node.c b/mime-node.c
index 2a823dfd..ff6805bf 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -192,6 +192,26 @@ set_signature_list_destructor (mime_node_t *node)
     }
 }
 
+/* Unwrapped MIME part destructor */
+static int
+_unwrapped_child_free (GMimeObject **proxy)
+{
+    g_object_unref (*proxy);
+    return 0;
+}
+
+/* Set up unwrapped MIME part destructor */
+static void
+set_unwrapped_child_destructor (mime_node_t *node)
+{
+    GMimeObject **proxy = talloc (node, GMimeObject *);
+
+    if (proxy) {
+	*proxy = node->unwrapped_child;
+	talloc_set_destructor (proxy, _unwrapped_child_free);
+    }
+}
+
 /* Verify a signed mime node */
 static void
 node_verify (mime_node_t *node, GMimeObject *part)
@@ -238,6 +258,8 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part)
 							 node->ctx->crypto->decrypt,
 							 message,
 							 encrypteddata, &decrypt_result, &err);
+	if (node->unwrapped_child)
+	    set_unwrapped_child_destructor (node);
     }
     if (! node->unwrapped_child) {
 	fprintf (stderr, "Failed to decrypt part: %s\n",
-- 
2.24.0

[PATCH 10/14] cli: include wrapped part of PKCS#7 SignedData in the MIME tree

[Daniel Kahn Gillmor]

Unwrap a PKCS#7 SignedData part unconditionally when the cli is
traversing the MIME tree, and return it as a "child" of what would
otherwise be a leaf in the tree.

Unfortunately, this also breaks the JSON output.  We will fix that
next.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 mime-node.c        | 23 +++++++++++++++++++++--
 test/T355-smime.sh |  2 +-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index ff6805bf..b6431e3b 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -220,8 +220,17 @@ node_verify (mime_node_t *node, GMimeObject *part)
     notmuch_status_t status;
 
     node->verify_attempted = true;
-    node->sig_list = g_mime_multipart_signed_verify (
-	GMIME_MULTIPART_SIGNED (part), GMIME_VERIFY_NONE, &err);
+    if (GMIME_IS_APPLICATION_PKCS7_MIME (part))
+	node->sig_list = g_mime_application_pkcs7_mime_verify (
+	    GMIME_APPLICATION_PKCS7_MIME (part), GMIME_VERIFY_NONE, &node->unwrapped_child, &err);
+    else
+	node->sig_list = g_mime_multipart_signed_verify (
+	    GMIME_MULTIPART_SIGNED (part), GMIME_VERIFY_NONE, &err);
+
+    if (node->unwrapped_child) {
+	node->nchildren = 1;
+	set_unwrapped_child_destructor (node);
+    }
 
     if (node->sig_list)
 	set_signature_list_destructor (node);
@@ -376,6 +385,12 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild)
 	} else {
 	    node_verify (node, part);
 	}
+    } else if (GMIME_IS_APPLICATION_PKCS7_MIME (part) &&
+	       GMIME_SECURE_MIME_TYPE_SIGNED_DATA == g_mime_application_pkcs7_mime_get_smime_type (GMIME_APPLICATION_PKCS7_MIME (part))) {
+	/* If node->ctx->crypto->verify is false, it would be better
+	 * to just unwrap (instead of verifying), but
+	 * https://github.com/jstedfast/gmime/issues/67 */
+	node_verify (node, part);
     } else {
 	if (_notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, numchild) &&
 	    node->ctx->msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL) {
@@ -409,6 +424,10 @@ mime_node_child (mime_node_t *parent, int child)
 		GMIME_MULTIPART (parent->part), child);
     } else if (GMIME_IS_MESSAGE (parent->part)) {
 	sub = g_mime_message_get_mime_part (GMIME_MESSAGE (parent->part));
+    } else if (GMIME_IS_APPLICATION_PKCS7_MIME (parent->part) &&
+	       parent->unwrapped_child &&
+	       child == 0) {
+	sub = parent->unwrapped_child;
     } else {
 	/* This should have been caught by _mime_node_set_up_part */
 	INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 32fdd9dc..2b74918f 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -120,7 +120,6 @@ expected='#notmuch-dump batch-tag:3 config,properties,tags
 test_expect_equal "$expected" "$output"
 
 test_begin_subtest "show contents of PKCS#7 SignedData message"
-test_subtest_known_broken
 output=$(notmuch show --format=raw --part=2 id:smime-onepart-signed@protected-headers.example)
 whitespace=' '
 expected="Bob, we need to cancel this contract.
@@ -156,6 +155,7 @@ On Tue, 26 Nov 2019 20:11:29 -0400, Alice Lovelace <alice@smime.example> wrote:
 test_expect_equal "$expected" "$output"
 
 test_begin_subtest "show PKCS#7 SignedData outputs valid JSON"
+test_subtest_known_broken
 output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
 test_valid_json "$output"
 
-- 
2.24.0

[PATCH 11/14] cli/show: If a leaf part has children, show them instead of omitting

[Daniel Kahn Gillmor]

Until we did PKCS#7 unwrapping, no leaf MIME part could have a child.

Now, we treat the unwrapped MIME part as the child of the PKCS#7
SignedData object.  So in that case, we want to show it instead of
deliberately omitting the content.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 notmuch-show.c     | 11 ++++++++++-
 test/T355-smime.sh |  1 -
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 21792a57..c809f8e9 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -759,7 +759,16 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    sp->string_len (sp, (char *) part_content->data, part_content->len);
 	    g_object_unref (stream_memory);
 	} else {
-	    format_omitted_part_meta_sprinter (sp, meta, GMIME_PART (node->part));
+	    /* if we have a child part despite being a standard
+	     * (non-multipart) MIME part, that means there is
+	     * something to unwrap, which we will present in
+	     * content: */
+	    if (node->nchildren) {
+		sp->map_key (sp, "content");
+		sp->begin_list (sp);
+		nclose = 1;
+	    } else
+		format_omitted_part_meta_sprinter (sp, meta, GMIME_PART (node->part));
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
 	sp->map_key (sp, "content");
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 2b74918f..399bd91c 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -155,7 +155,6 @@ On Tue, 26 Nov 2019 20:11:29 -0400, Alice Lovelace <alice@smime.example> wrote:
 test_expect_equal "$expected" "$output"
 
 test_begin_subtest "show PKCS#7 SignedData outputs valid JSON"
-test_subtest_known_broken
 output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
 test_valid_json "$output"
 
-- 
2.24.0

[PATCH 12/14] cli/reply: Ignore PKCS#7 wrapper parts when replying

[Daniel Kahn Gillmor]

When composing a reply, no one wants to see this line in the proposed
message:

    Non-text part: application/pkcs7-mime

So we hide it, the same way we hide PGP/MIME cruft.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 notmuch-reply.c    | 5 +++--
 test/T355-smime.sh | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2c30f6f9..ceb4f39b 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -65,8 +65,9 @@ format_part_reply (GMimeStream *stream, mime_node_t *node)
 	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (node->part);
 
 	if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
-	    g_mime_content_type_is_type (content_type, "application", "pgp-signature")) {
-	    /* Ignore PGP/MIME cruft parts */
+	    g_mime_content_type_is_type (content_type, "application", "pgp-signature") ||
+	    g_mime_content_type_is_type (content_type, "application", "pkcs7-mime")) {
+	    /* Ignore PGP/MIME and S/MIME cruft parts */
 	} else if (g_mime_content_type_is_type (content_type, "text", "*") &&
 		   ! g_mime_content_type_is_type (content_type, "text", "html")) {
 	    show_text_part_content (node->part, stream, NOTMUCH_SHOW_TEXT_PART_REPLY);
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 399bd91c..ddc91a56 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -134,7 +134,6 @@ OpenPGP Example Corp"
 test_expect_equal "$expected" "$output"
 
 test_begin_subtest "reply to PKCS#7 SignedData message with proper quoting and attribution"
-test_subtest_known_broken
 output=$(notmuch reply id:smime-onepart-signed@protected-headers.example)
 expected="From: Notmuch Test Suite <test_suite@notmuchmail.org>
 Subject: Re: The FooCorp contract
-- 
2.24.0

[PATCH 13/14] cli: sprinter should be able to print an unsigned long

[Daniel Kahn Gillmor]

In some circumstances, we will encounter an unsigned long, and need to
emit it textually.

For example, in GPGME, the gpgme_signature_t object has members
(`created` and `expires`) that are unsigned long integers.  notmuch
doesn't use GPGME directly, but subsequent changes will make it clear
why we need to do have this capability.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 sprinter-json.c | 9 +++++++++
 sprinter-sexp.c | 9 +++++++++
 sprinter-text.c | 9 +++++++++
 sprinter.h      | 1 +
 4 files changed, 28 insertions(+)

diff --git a/sprinter-json.c b/sprinter-json.c
index c6ec8577..0fc734f7 100644
--- a/sprinter-json.c
+++ b/sprinter-json.c
@@ -131,6 +131,14 @@ json_integer (struct sprinter *sp, int val)
     fprintf (spj->stream, "%d", val);
 }
 
+static void
+json_ulong (struct sprinter *sp, unsigned long val)
+{
+    struct sprinter_json *spj = json_begin_value (sp);
+
+    fprintf (spj->stream, "%lu", val);
+}
+
 static void
 json_boolean (struct sprinter *sp, bool val)
 {
@@ -181,6 +189,7 @@ sprinter_json_create (const void *ctx, FILE *stream)
 	    .string = json_string,
 	    .string_len = json_string_len,
 	    .integer = json_integer,
+	    .ulong = json_ulong,
 	    .boolean = json_boolean,
 	    .null = json_null,
 	    .map_key = json_map_key,
diff --git a/sprinter-sexp.c b/sprinter-sexp.c
index 6891ea42..72f23f4d 100644
--- a/sprinter-sexp.c
+++ b/sprinter-sexp.c
@@ -168,6 +168,14 @@ sexp_integer (struct sprinter *sp, int val)
     fprintf (sps->stream, "%d", val);
 }
 
+static void
+sexp_ulong (struct sprinter *sp, unsigned long val)
+{
+    struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
+
+    fprintf (sps->stream, "%lu", val);
+}
+
 static void
 sexp_boolean (struct sprinter *sp, bool val)
 {
@@ -216,6 +224,7 @@ sprinter_sexp_create (const void *ctx, FILE *stream)
 	    .string = sexp_string,
 	    .string_len = sexp_string_len,
 	    .integer = sexp_integer,
+	    .ulong = sexp_ulong,
 	    .boolean = sexp_boolean,
 	    .null = sexp_null,
 	    .map_key = sexp_map_key,
diff --git a/sprinter-text.c b/sprinter-text.c
index 648b54b1..2c6f635e 100644
--- a/sprinter-text.c
+++ b/sprinter-text.c
@@ -51,6 +51,14 @@ text_integer (struct sprinter *sp, int val)
     fprintf (sptxt->stream, "%d", val);
 }
 
+static void
+text_ulong (struct sprinter *sp, unsigned long val)
+{
+    struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+    fprintf (sptxt->stream, "%lu", val);
+}
+
 static void
 text_boolean (struct sprinter *sp, bool val)
 {
@@ -123,6 +131,7 @@ sprinter_text_create (const void *ctx, FILE *stream)
 	    .string = text_string,
 	    .string_len = text_string_len,
 	    .integer = text_integer,
+	    .ulong = text_ulong,
 	    .boolean = text_boolean,
 	    .null = text_null,
 	    .map_key = text_map_key,
diff --git a/sprinter.h b/sprinter.h
index 182b1a8b..3c23d718 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -34,6 +34,7 @@ typedef struct sprinter {
     void (*string)(struct sprinter *, const char *);
     void (*string_len)(struct sprinter *, const char *, size_t);
     void (*integer)(struct sprinter *, int);
+    void (*ulong)(struct sprinter *, unsigned long);
     void (*boolean)(struct sprinter *, bool);
     void (*null)(struct sprinter *);
 
-- 
2.24.0

[PATCH 14/14] cli: Avoid bogus signature dates from GMime

[Daniel Kahn Gillmor]

As noted in https://github.com/jstedfast/gmime/issues/68, GMime
converts the unsigned longs returned from GPGME into time_t objects.

On architectures with a signed 32-bit time_t (like GNU/Linux x86_64),
this means that we're limited to 31 bits of data, which means the
clock wraps around in 2038.

Until GMime fixes this properly, we can regain 1 bit of space by
casting back from time_t to an unsigned long.  This gives us a window
of up until early 2106.

The example S/MIME SignedData message is signed by a certificate whose
expiration date is Fri Sep 27 06:54:18 UTC 2052 (2611032858 seconds
since the epoch).  GPGME reports the value faithfully as the
expiration date of the signature on this message, but GMime wraps it
back around to the negative.

Once GMime fixes #68, we should transition to their upstream fix
instead of maintaining this workaround forever.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 notmuch-show.c     | 4 ++--
 test/T355-smime.sh | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index c809f8e9..84839180 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -447,12 +447,12 @@ format_part_sigstatus_sprinter (sprinter_t *sp, GMimeSignatureList *siglist)
 	    time_t created = g_mime_signature_get_created (signature);
 	    if (created != -1) {
 		sp->map_key (sp, "created");
-		sp->integer (sp, created);
+		sp->ulong (sp, (unsigned long) created);
 	    }
 	    time_t expires = g_mime_signature_get_expires (signature);
 	    if (expires > 0) {
 		sp->map_key (sp, "expires");
-		sp->integer (sp, expires);
+		sp->ulong (sp, (unsigned long) expires);
 	    }
 	    if (certificate) {
 		const char *uid = g_mime_certificate_get_valid_userid (certificate);
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index ddc91a56..dedf5ab1 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -158,7 +158,6 @@ output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.ex
 test_valid_json "$output"
 
 test_begin_subtest "Verify signature on PKCS#7 SignedData message"
-test_subtest_known_broken
 output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
 test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]["signed"]["status"][0]={
-- 
2.24.0

Re: Handle PKCS#7 signedData (S/MIME single-part clearsigned)

[William Casarin]

Hey Daniel,

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
> Several of the patches should be simple to read and uncontroversial (I
> hope!), and I welcome/encourage merging of those patches to reduce the
> length of the remaining series.

I did a quick skim and it looks ok, although I'm not super familiar with
S/MIME. Any steps toward S/MIME support gets a Concept ACK from me!

Cheers,
Will

Re: Handle PKCS#7 signedData (S/MIME single-part clearsigned)

[Jameson Graef Rollins]

On Mon, Dec 09 2019, William Casarin <jb55@jb55.com> wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>> Several of the patches should be simple to read and uncontroversial (I
>> hope!), and I welcome/encourage merging of those patches to reduce the
>> length of the remaining series.
>
> I did a quick skim and it looks ok, although I'm not super familiar with
> S/MIME. Any steps toward S/MIME support gets a Concept ACK from me!

+1 on this.  Anything that pushes better crypto support is always good.
I gave the series a casual once over and it looks good to me, but I
haven't done an in-depth code review.

jamie.