unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Handle PKCS#7 S/MIME messages v2
@ 2020-05-12 22:29 Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 1/9] lib: index PKCS7 SignedData parts Daniel Kahn Gillmor
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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-dkg@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:20200512222010.371054-1-dkg@fifthhorseman.net must be applied
before this series can be applied.

Feedback and critiques welcome,

         --dkg



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/9] lib: index PKCS7 SignedData parts
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-23 11:57   ` David Bremner
  2020-05-12 22:29 ` [PATCH v2 2/9] smime: Identify encrypted S/MIME parts during indexing Daniel Kahn Gillmor
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/9] smime: Identify encrypted S/MIME parts during indexing
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 1/9] lib: index PKCS7 SignedData parts Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 3/9] cli: include wrapped part of PKCS#7 SignedData in the MIME tree Daniel Kahn Gillmor
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

We don't handle them correctly yet, but we can at least mark them as
being encrypted.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 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:0000000000000002   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message 001 (encrypted inbox)"
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/9] cli: include wrapped part of PKCS#7 SignedData in the MIME tree
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 1/9] lib: index PKCS7 SignedData parts Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 2/9] smime: Identify encrypted S/MIME parts during indexing Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 4/9] cli/show: If a leaf part has children, show them instead of omitting Daniel Kahn Gillmor
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/9] cli/show: If a leaf part has children, show them instead of omitting
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
                   ` (2 preceding siblings ...)
  2020-05-12 22:29 ` [PATCH v2 3/9] cli: include wrapped part of PKCS#7 SignedData in the MIME tree Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 5/9] cli/reply: Ignore PKCS#7 wrapper parts when replying Daniel Kahn Gillmor
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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 <dkg@fifthhorseman.net>
---
 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 <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"
 
 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/9] cli/reply: Ignore PKCS#7 wrapper parts when replying
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
                   ` (3 preceding siblings ...)
  2020-05-12 22:29 ` [PATCH v2 4/9] cli/show: If a leaf part has children, show them instead of omitting Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 6/9] crypto: Make _notmuch_crypto_decrypt take a GMimeObject Daniel Kahn Gillmor
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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 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 <test_suite@notmuchmail.org>
 Subject: Re: The FooCorp contract
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 6/9] crypto: Make _notmuch_crypto_decrypt take a GMimeObject
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
                   ` (4 preceding siblings ...)
  2020-05-12 22:29 ` [PATCH v2 5/9] cli/reply: Ignore PKCS#7 wrapper parts when replying Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 7/9] crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt Daniel Kahn Gillmor
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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 <dkg@fifthhorseman.net>
---
 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 (GMIME_MULTIPART_ENCRYPTED (part), flags, NULL,
 					      decrypt_result, err);
     return ret;
 }
diff --git a/util/crypto.h b/util/crypto.h
index f8bda0d1..4fa5599c 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -18,7 +18,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);
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 7/9] crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
                   ` (5 preceding siblings ...)
  2020-05-12 22:29 ` [PATCH v2 6/9] crypto: Make _notmuch_crypto_decrypt take a GMimeObject Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 8/9] smime: Pass PKCS#7 envelopedData to node_decrypt_and_verify Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 9/9] smime: Index cleartext of envelopedData when requested Daniel Kahn Gillmor
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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 <dkg@fifthhorseman.net>
---
 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 8/9] smime: Pass PKCS#7 envelopedData to node_decrypt_and_verify
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
                   ` (6 preceding siblings ...)
  2020-05-12 22:29 ` [PATCH v2 7/9] crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  2020-05-12 22:29 ` [PATCH v2 9/9] smime: Index cleartext of envelopedData when requested Daniel Kahn Gillmor
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

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 <dkg@fifthhorseman.net>
---
 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 <<EOF > 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 9/9] smime: Index cleartext of envelopedData when requested
  2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
                   ` (7 preceding siblings ...)
  2020-05-12 22:29 ` [PATCH v2 8/9] smime: Pass PKCS#7 envelopedData to node_decrypt_and_verify Daniel Kahn Gillmor
@ 2020-05-12 22:29 ` Daniel Kahn Gillmor
  8 siblings, 0 replies; 11+ messages in thread
From: Daniel Kahn Gillmor @ 2020-05-12 22:29 UTC (permalink / raw)
  To: Notmuch Mail

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 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:0000000000000002   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:0000000000000002   2000-01-01 [1/1] Notmuch Test Suite; test encrypted message 001 (encrypted inbox signed)"
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/9] lib: index PKCS7 SignedData parts
  2020-05-12 22:29 ` [PATCH v2 1/9] lib: index PKCS7 SignedData parts Daniel Kahn Gillmor
@ 2020-05-23 11:57   ` David Bremner
  0 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2020-05-23 11:57 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

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

series pushed

d

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-05-23 11:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-12 22:29 Handle PKCS#7 S/MIME messages v2 Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 1/9] lib: index PKCS7 SignedData parts Daniel Kahn Gillmor
2020-05-23 11:57   ` David Bremner
2020-05-12 22:29 ` [PATCH v2 2/9] smime: Identify encrypted S/MIME parts during indexing Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 3/9] cli: include wrapped part of PKCS#7 SignedData in the MIME tree Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 4/9] cli/show: If a leaf part has children, show them instead of omitting Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 5/9] cli/reply: Ignore PKCS#7 wrapper parts when replying Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 6/9] crypto: Make _notmuch_crypto_decrypt take a GMimeObject Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 7/9] crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 8/9] smime: Pass PKCS#7 envelopedData to node_decrypt_and_verify Daniel Kahn Gillmor
2020-05-12 22:29 ` [PATCH v2 9/9] smime: Index cleartext of envelopedData when requested Daniel Kahn Gillmor

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).