Re: test suite: FIXED messages are misordered with tests
Daniel Kahn Gillmor writes: > > Clearly, that FIXED should come *after* the "T356-protected-headers:" > separator. > Can you reproduce this without running the tests in parallel? d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/9] cli: include wrapped part of PKCS#7 SignedData in the MIME tree
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 --- 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 7c28282a..4de0fbef 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -142,7 +142,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. @@ -178,6 +177,7 @@ On Tue, 26 Nov 2019 20:11:29 -0400, Alice Lovelace 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.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Handle PKCS#7 S/MIME messages v2
This revision of the PKCS#7 S/MIME handling series is based on (and very similar to) the series sent on the thread starting at id:20200430201328.725651-1-...@fifthhorseman.net However, it is rebased after more gracefully handling the subtle errors in X.509 certificate validity when built against older versions of GMime. In particular, the patch found at id:2020051010.371054-1-...@fifthhorseman.net must be applied before this series can be applied. Feedback and critiques welcome, --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/9] lib: index PKCS7 SignedData parts
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 --- 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 f8e8e396..a7eecedf 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -132,13 +132,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_
[PATCH v2 2/9] smime: Identify encrypted S/MIME parts during indexing
We don't handle them correctly yet, but we can at least mark them as being encrypted. Signed-off-by: Daniel Kahn Gillmor --- lib/index.cc | 4 test/T355-smime.sh | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/index.cc b/lib/index.cc index bbf13dc5..f029b334 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -654,6 +654,10 @@ _index_pkcs7_part (notmuch_message_t *message, notmuch_message_add_property (message, "index.repaired", "skip-protected-headers-legacy-display"); } _index_mime_part (message, indexopts, toindex, msg_crypto); +} else if (p7type == GMIME_SECURE_MIME_TYPE_ENVELOPED_DATA) { + _notmuch_message_add_term (message, "tag", "encrypted"); + if (notmuch_indexopts_get_decrypt_policy (indexopts) != NOTMUCH_DECRYPT_FALSE) + _notmuch_database_log (notmuch, "Cannot decrypt PKCS#7 envelopedData (S/MIME encrypted messages)\n"); } else { _notmuch_database_log (notmuch, "Cannot currently handle PKCS#7 smime-type '%s'\n", g_mime_object_get_content_type_parameter (part, "smime-type")); diff --git a/test/T355-smime.sh b/test/T355-smime.sh index a7eecedf..7c28282a 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -98,7 +98,6 @@ test_json_nodes <<<"$output" \ 'crypto_uid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Notmuch Test Suite"' test_begin_subtest "encrypted+signed message is known to be encrypted, but signature is unknown" -test_subtest_known_broken output=$(notmuch search subject:"test encrypted message 001") test_expect_equal "$output" "thread:0002 2000-01-01 [1/1] Notmuch Test Suite; test encrypted message 001 (encrypted inbox)" -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 4/9] cli/show: If a leaf part has children, show them instead of omitting
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. This fixes the test of the protected subject in id:smime-onepart-signed@protected-headers.example. Signed-off-by: Daniel Kahn Gillmor --- notmuch-show.c | 11 ++- test/T355-smime.sh | 6 +++--- test/T356-protected-headers.sh | 3 +-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index ab1cd144..36265043 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 4de0fbef..03aada20 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -177,12 +177,10 @@ On Tue, 26 Nov 2019 20:11:29 -0400, Alice Lovelace 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" 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" \ @@ -192,7 +190,9 @@ test_json_nodes <<<"$output" \ 'status:[0][0][0]["crypto"]["signed"]["status"][0]["status"]="good"' test_begin_subtest "Verify signature on PKCS#7 SignedData message signer User ID" -test_subtest_known_broken +if [ $NOTMUCH_GMIME_X509_CERT_VALIDITY -ne 1 ]; then +test_subtest_known_broken +fi test_json_nodes <<<"$output" \ 'userid:[0][0][0]["crypto"]["signed"]["status"][0]["userid"]="CN=Alice Lovelace"' diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh index 5fd27434..5beffaf0 100755 --- a/test/T356-protected-headers.sh +++ b/test/T356-protected-headers.sh @@ -157,7 +157,6 @@ test_expect_equal "$output" id:protected-with-legacy-display@crypto.notmuchmail. for variant in multipart-signed onepart-signed; do test_begin_subtest "verify signed PKCS#7 subject ($variant)" -[ "$variant" = multipart-signed ] || test_subtest_known_broken output=$(notmuch show --verify --format=json "id:smime-${variant}@protected-headers.example") test_json_nodes <<<"$output" \ 'signed_subject:[0][0][0]["crypto"]["signed"]["headers"]=["Subject"]' \ @@ -165,7 +164,7 @@ for variant in multipart-signed onepart-signed; do 'sig_fpr:[0][0][0]["crypto"]["signed"]["status"][0]["fingerprint"]="702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB"' \ '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 ] || [ "$variant" != multipart-signed ]; then +if [ $NOTMUCH_GMIME_X509_CERT_VALIDITY -ne 1 ]; then test_subtest_known_broken fi test_json_nodes <<<"$output" \ -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 5/9] cli/reply: Ignore PKCS#7 wrapper parts when replying
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 --- 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 03aada20..8d225bc1 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -156,7 +156,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 Subject: Re: The FooCorp contract -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 7/9] crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt
In the two places where _notmuch_crypto_decrypt handles multipart/encrypted messages (PGP/MIME), we should also handle PKCS#7 envelopedData (S/MIME). This is insufficient for fully handling S/MIME encrypted data because _notmuch_crypto_decrypt isn't yet actually invoked for envelopedData parts, but that will happen in the following changes. Signed-off-by: Daniel Kahn Gillmor --- util/crypto.c | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/util/crypto.c b/util/crypto.c index fbd5f011..c09f467b 100644 --- a/util/crypto.c +++ b/util/crypto.c @@ -55,10 +55,21 @@ _notmuch_crypto_decrypt (bool *attempted, } if (attempted) *attempted = true; - ret = g_mime_multipart_encrypted_decrypt (GMIME_MULTIPART_ENCRYPTED (part), - GMIME_DECRYPT_NONE, - notmuch_message_properties_value (list), - decrypt_result, err); + if (GMIME_IS_MULTIPART_ENCRYPTED (part)) { + ret = g_mime_multipart_encrypted_decrypt (GMIME_MULTIPART_ENCRYPTED (part), + GMIME_DECRYPT_NONE, + notmuch_message_properties_value (list), + decrypt_result, err); + } else if (GMIME_IS_APPLICATION_PKCS7_MIME (part)) { + GMimeApplicationPkcs7Mime *pkcs7 = GMIME_APPLICATION_PKCS7_MIME (part); + GMimeSecureMimeType type = g_mime_application_pkcs7_mime_get_smime_type (pkcs7); + if (type == GMIME_SECURE_MIME_TYPE_ENVELOPED_DATA) { + ret = g_mime_application_pkcs7_mime_decrypt (pkcs7, + GMIME_DECRYPT_NONE, + notmuch_message_properties_value (list), + decrypt_result, err); + } + } if (ret) break; } @@ -81,8 +92,17 @@ _notmuch_crypto_decrypt (bool *attempted, GMimeDecryptFlags flags = GMIME_DECRYPT_NONE; if (decrypt == NOTMUCH_DECRYPT_TRUE && decrypt_result) flags |= GMIME_DECRYPT_EXPORT_SESSION_KEY; -ret = g_mime_multipart_encrypted_decrypt (GMIME_MULTIPART_ENCRYPTED (part), flags, NULL, - decrypt_result, err); +if (GMIME_IS_MULTIPART_ENCRYPTED (part)) { + ret = g_mime_multipart_encrypted_decrypt (GMIME_MULTIPART_ENCRYPTED (part), flags, NULL, + decrypt_result, err); +} else if (GMIME_IS_APPLICATION_PKCS7_MIME (part)) { + GMimeApplicationPkcs7Mime *pkcs7 = GMIME_APPLICATION_PKCS7_MIME (part); + GMimeSecureMimeType p7type = g_mime_application_pkcs7_mime_get_smime_type (pkcs7); + if (p7type == GMIME_SECURE_MIME_TYPE_ENVELOPED_DATA) { + ret = g_mime_application_pkcs7_mime_decrypt (pkcs7, flags, NULL, +decrypt_result, err); + } +} return ret; } -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 6/9] crypto: Make _notmuch_crypto_decrypt take a GMimeObject
As we prepare to handle S/MIME-encrypted PKCS#7 EnvelopedData (which is not multipart), we don't want to be limited to passing only GMimeMultipartEncrypted MIME parts to _notmuch_crypto_decrypt. There is no functional change here, just a matter of adjusting how we pass arguments internally. Signed-off-by: Daniel Kahn Gillmor --- lib/index.cc | 8 mime-node.c | 3 +-- util/crypto.c | 6 +++--- util/crypto.h | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/index.cc b/lib/index.cc index f029b334..da9a3abe 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -369,7 +369,7 @@ _index_content_type (notmuch_message_t *message, GMimeObject *part) static void _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, - GMimeMultipartEncrypted *part, + GMimeObject *part, _notmuch_message_crypto_t *msg_crypto); static void @@ -439,7 +439,7 @@ _index_mime_part (notmuch_message_t *message, g_mime_multipart_get_part (multipart, i)); if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) { _index_encrypted_mime_part (message, indexopts, - GMIME_MULTIPART_ENCRYPTED (part), + part, msg_crypto); } else { if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) { @@ -551,7 +551,7 @@ _index_mime_part (notmuch_message_t *message, static void _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, - GMimeMultipartEncrypted *encrypted_data, + GMimeObject *encrypted_data, _notmuch_message_crypto_t *msg_crypto) { notmuch_status_t status; @@ -603,7 +603,7 @@ _index_encrypted_mime_part (notmuch_message_t *message, g_object_unref (decrypt_result); } GMimeObject *toindex = clear; -if (_notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT) && +if (_notmuch_message_crypto_potential_payload (msg_crypto, clear, encrypted_data, GMIME_MULTIPART_ENCRYPTED_CONTENT) && msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL) { toindex = _notmuch_repair_crypto_payload_skip_legacy_display (clear); if (toindex != clear) diff --git a/mime-node.c b/mime-node.c index b6431e3b..c2ee858d 100644 --- a/mime-node.c +++ b/mime-node.c @@ -253,7 +253,6 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part) GError *err = NULL; GMimeDecryptResult *decrypt_result = NULL; notmuch_status_t status; -GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part); notmuch_message_t *message = NULL; if (! node->unwrapped_child) { @@ -266,7 +265,7 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part) node->unwrapped_child = _notmuch_crypto_decrypt (&node->decrypt_attempted, node->ctx->crypto->decrypt, message, -encrypteddata, &decrypt_result, &err); +part, &decrypt_result, &err); if (node->unwrapped_child) set_unwrapped_child_destructor (node); } diff --git a/util/crypto.c b/util/crypto.c index 0bb6f526..fbd5f011 100644 --- a/util/crypto.c +++ b/util/crypto.c @@ -34,7 +34,7 @@ GMimeObject * _notmuch_crypto_decrypt (bool *attempted, notmuch_decryption_policy_t decrypt, notmuch_message_t *message, -GMimeMultipartEncrypted *part, +GMimeObject *part, GMimeDecryptResult **decrypt_result, GError **err) { @@ -55,7 +55,7 @@ _notmuch_crypto_decrypt (bool *attempted, } if (attempted) *attempted = true; - ret = g_mime_multipart_encrypted_decrypt (part, + ret = g_mime_multipart_encrypted_decrypt (GMIME_MULTIPART_ENCRYPTED (part), GMIME_DECRYPT_NONE, notmuch_message_properties_value (list), decrypt_result, err); @@ -81,7 +81,7 @@ _notmuch_crypto_decrypt (bool *attempted, GMimeDecryptFlags flags = GMIME_DECRYPT_NONE; if (decrypt == NOTMUCH_DECRYPT_TRUE && decrypt_result) flags |= GMIME_DECRYPT_EXPORT_SESSION_KEY; -ret = g_mime_multipart_encrypted_decrypt (part, flags, NULL, +ret = g_mime_multipart_encrypted_decrypt
[PATCH v2 9/9] smime: Index cleartext of envelopedData when requested
Signed-off-by: Daniel Kahn Gillmor --- lib/index.cc | 5 +++-- test/T355-smime.sh | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/index.cc b/lib/index.cc index da9a3abe..826aa341 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -656,8 +656,9 @@ _index_pkcs7_part (notmuch_message_t *message, _index_mime_part (message, indexopts, toindex, msg_crypto); } else if (p7type == GMIME_SECURE_MIME_TYPE_ENVELOPED_DATA) { _notmuch_message_add_term (message, "tag", "encrypted"); - if (notmuch_indexopts_get_decrypt_policy (indexopts) != NOTMUCH_DECRYPT_FALSE) - _notmuch_database_log (notmuch, "Cannot decrypt PKCS#7 envelopedData (S/MIME encrypted messages)\n"); + _index_encrypted_mime_part (message, indexopts, + part, + 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")); diff --git a/test/T355-smime.sh b/test/T355-smime.sh index 1f11725f..170f8649 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -107,12 +107,10 @@ test_begin_subtest "Reindex cleartext" test_expect_success "notmuch reindex --decrypt=true subject:'test encrypted message 001'" test_begin_subtest "signature is now known" -test_subtest_known_broken output=$(notmuch search subject:"test encrypted message 001") test_expect_equal "$output" "thread:0002 2000-01-01 [1/1] Notmuch Test Suite; test encrypted message 001 (encrypted inbox signed)" test_begin_subtest "Encrypted body is indexed" -test_subtest_known_broken output=$(notmuch search 'this is a test encrypted message') test_expect_equal "$output" "thread:0002 2000-01-01 [1/1] Notmuch Test Suite; test encrypted message 001 (encrypted inbox signed)" -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 8/9] smime: Pass PKCS#7 envelopedData to node_decrypt_and_verify
This change means we can support "notmuch show --decrypt=true" for S/MIME encrypted messages, resolving several outstanding broken tests, including all the remaining S/MIME protected header examples. We do not yet handle indexing the cleartext of S/MIME encrypted messages, though. Signed-off-by: Daniel Kahn Gillmor --- mime-node.c| 6 ++ test/T355-smime.sh | 2 -- test/T356-protected-headers.sh | 6 +++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/mime-node.c b/mime-node.c index c2ee858d..f552e03a 100644 --- a/mime-node.c +++ b/mime-node.c @@ -390,6 +390,12 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild) * to just unwrap (instead of verifying), but * https://github.com/jstedfast/gmime/issues/67 */ node_verify (node, part); +} else if (GMIME_IS_APPLICATION_PKCS7_MIME (part) && + GMIME_SECURE_MIME_TYPE_ENVELOPED_DATA == g_mime_application_pkcs7_mime_get_smime_type (GMIME_APPLICATION_PKCS7_MIME (part)) && + (node->ctx->crypto->decrypt != NOTMUCH_DECRYPT_FALSE)) { + node_decrypt_and_verify (node, part); + if (node->unwrapped_child && node->nchildren == 0) + node->nchildren = 1; } 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) { diff --git a/test/T355-smime.sh b/test/T355-smime.sh index 8d225bc1..1f11725f 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -80,7 +80,6 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "Decryption (notmuch CLI)" -test_subtest_known_broken notmuch show --decrypt=true subject:"test encrypted message 001" |\ grep "^This is a" > OUTPUT cat < EXPECTED @@ -89,7 +88,6 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "Cryptographic message status (encrypted+signed)" -test_subtest_known_broken output=$(notmuch show --format=json --decrypt=true subject:"test encrypted message 001") test_json_nodes <<<"$output" \ 'crypto_encrypted:[0][0][0]["crypto"]["decrypted"]["status"]="full"' \ diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh index 5beffaf0..074a2345 100755 --- a/test/T356-protected-headers.sh +++ b/test/T356-protected-headers.sh @@ -173,7 +173,6 @@ done for variant in sign+enc sign+enc+legacy-disp; do test_begin_subtest "confirm signed and encrypted PKCS#7 subject ($variant)" -test_subtest_known_broken output=$(notmuch show --decrypt=true --format=json "id:smime-${variant}@protected-headers.example") test_json_nodes <<<"$output" \ 'signed_subject:[0][0][0]["crypto"]["signed"]["headers"]=["Subject"]' \ @@ -181,14 +180,15 @@ for variant in sign+enc sign+enc+legacy-disp; do 'sig_fpr:[0][0][0]["crypto"]["signed"]["status"][0]["fingerprint"]="702BA4B157F1E2B7D16B0C6A5FFC8A7DE2057DEB"' \ '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" -test_subtest_known_broken +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)" -test_subtest_known_broken output=$(notmuch show --decrypt=true --format=json "id:smime-enc+legacy-disp@protected-headers.example") test_json_nodes <<<"$output" \ 'encrypted:[0][0][0]["crypto"]["decrypted"]={"status":"full","header-mask":{"Subject":"..."}}' \ -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2 v2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7
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 | 17 +--- test/T356-protected-headers.sh | 13 +- 3 files changed, 100 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", &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.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
Re: [PATCH 1/2 v2] test-lib: mark function variables as local
On Sat 2020-05-09 08:47:10 -0300, David Bremner wrote: > I'm confused about where to apply 2/2. If I apply it on top of (updated) > master, it causes test failures. If I apply after the rest of the > patches in this thread then presumably there is some interval where the > build is broken (if only for certain GMime versions). fwiw, i believe that the current master ( 627460d76b95a07084c2b6fc7f647a5547e1 ) *is* broken on systems with older gmime. In particular, on debian buster, i see: T356-protected-headers: Testing Message decryption with protected headers FAIL verify signed PKCS#7 subject (multipart-signed) sig_uid: object not found: data[0][0][0]["crypto"]["signed"]["status"][0]["userid"] My [PATCH v2/2 v2] in this thread (sent shortly) should resolve this situation. Then, i'll send a rebased version of the full S/MIME series in a new thread. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
test suite: FIXED messages are misordered with tests
I'm debugging/diagnosing/trying to clean up some "FIXED" known-broken tests right now. Sometimes, depending on circumstances i can't predict (race conditions?), I see funny output like: ~~~ Use "make V=1" to see the details for passing and known broken tests. INFO: using 2m timeout for tests INFO: running tests with moreutils parallel FIXED verify signed PKCS#7 subject (onepart-signed) signer User ID T356-protected-headers: Testing Message decryption with protected headers BROKEN confirm signed and encrypted PKCS#7 subject (sign+enc) BROKEN confirm signed and encrypted PKCS#7 subject (sign+enc) signer User ID BROKEN confirm signed and encrypted PKCS#7 subject (sign+enc+legacy-disp) BROKEN confirm signed and encrypted PKCS#7 subject (sign+enc+legacy-disp) signer User ID BROKEN confirm encryption-protected PKCS#7 subject (enc+legacy-disp) ~~~ Clearly, that FIXED should come *after* the "T356-protected-headers:" separator. This is a minor bug, i suppose, but i confess i don't understand the maze of shell functions in test-lib.sh well enough to see why this is happening, let alone to fix it. Anyone interested in fixing it should be able to do so by marking a good test "known broken" and then re-running the test suite. The above output is taken from: make -j4 check NOTMUCH_TESTS=T356-protected-headers.sh Sorry to send a bug report with no fixes! --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] configure: report GMime minimum version in ./configure output
We already report the minimum version for Glib, zlib, and Xapian development libraries. For consistency, report it for GMime as well. Signed-off-by: Daniel Kahn Gillmor --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 92e5bd1b..a67b4384 100755 --- a/configure +++ b/configure @@ -468,7 +468,7 @@ fi GMIME_MINVER=3.0.3 -printf "Checking for GMime development files... " +printf "Checking for GMime development files (>= $GMIME_MINVER)... " if pkg-config --exists "gmime-3.0 >= $GMIME_MINVER"; then printf "Yes.\n" have_gmime=1 -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: feature request: parameters for reply templates
On Tue, May 12 2020, Jörg Volbers wrote: >> My gut reaction is that doing more formatting in notmuch reply >> is probably a mistake; it's hard enough to get two users to >> agree on these kind of customizations, never mind two different >> MUAs. Probably what we need to do is make sure the structured >> (json/s-expr) output has enough information for clients to do >> what they need/want to do. > > I agree, but why not add some switches which allow to turn some > aspects of the reply output on or off? I am thinking here in > particular of the parts which are not plain text. Since not every > mail has an attachment, it makes perfect sense to let the user > decide whether a reply includes a text reference to the > attachment. A simple on-off-switch would be enough. Did you have spec in mind ? =D > > JV > Tomi ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: feature request: parameters for reply templates
My gut reaction is that doing more formatting in notmuch reply is probably a mistake; it's hard enough to get two users to agree on these kind of customizations, never mind two different MUAs. Probably what we need to do is make sure the structured (json/s-expr) output has enough information for clients to do what they need/want to do. I agree, but why not add some switches which allow to turn some aspects of the reply output on or off? I am thinking here in particular of the parts which are not plain text. Since not every mail has an attachment, it makes perfect sense to let the user decide whether a reply includes a text reference to the attachment. A simple on-off-switch would be enough. JV -- http://www.joergvolbers.de https://fu-berlin.academia.edu/joergvolbers https://www.geisteswissenschaften.fu-berlin.de/we01/institut/mitarbeiter/koop_gaeste/volbers/index.html ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: feature request: parameters for reply templates
Emmanuel Beffara writes: > > I use `notmuch reply` with the default format indirectly, because I use > bower an it delegates the task of preparing replies to this command. I > feel it would make sense to define new settings to handle all this, but > maybe there are good reasons not to? > My gut reaction is that doing more formatting in notmuch reply is probably a mistake; it's hard enough to get two users to agree on these kind of customizations, never mind two different MUAs. Probably what we need to do is make sure the structured (json/s-expr) output has enough information for clients to do what they need/want to do. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch