[PATCH v2 2/5] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

Daniel Kahn Gillmor dkg at fifthhorseman.net
Fri May 31 00:59:04 PDT 2019


Our _notmuch_message_crypto_potential_payload implementation could
only return a failure if bad arguments were passed to it.  It is an
internal function, so if that happens it's an entirely internal bug
for notmuch.

It will be more useful for this function to return whether or not the
part is in fact a cryptographic payload, so we dispense with the
status return.

If some future change suggests adding a status return back, there are
only a handful of call sites, and no pressure to retain a stable API,
so it could be changed easily. But for now, go with the simpler
function.

Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
---
 lib/index.cc  |  9 ++-------
 mime-node.c   |  6 +-----
 util/crypto.c | 27 ++++++++++++---------------
 util/crypto.h |  7 +++++--
 4 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 1fd9e67e..deb76f6f 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -405,7 +405,6 @@ _index_mime_part (notmuch_message_t *message,
 	  _notmuch_message_add_term (message, "tag", "encrypted");
 
 	for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
-	    notmuch_status_t status;
 	    GMimeObject *child;
 	    if (GMIME_IS_MULTIPART_SIGNED (multipart)) {
 		/* Don't index the signature, but index its content type. */
@@ -434,11 +433,7 @@ _index_mime_part (notmuch_message_t *message,
 		continue;
 	    }
 	    child = g_mime_multipart_get_part (multipart, i);
-	    status = _notmuch_message_crypto_potential_payload (msg_crypto, child, part, i);
-	    if (status)
-		_notmuch_database_log (notmuch_message_get_database (message),
-				       "Warning: failed to mark the potential cryptographic payload (%s).\n",
-				       notmuch_status_to_string (status));
+	    _notmuch_message_crypto_potential_payload (msg_crypto, child, part, i);
 	    _index_mime_part (message, indexopts, child, msg_crypto);
 	}
 	return;
@@ -577,7 +572,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
 	}
 	g_object_unref (decrypt_result);
     }
-    status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
+    _notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT);
     _index_mime_part (message, indexopts, clear, msg_crypto);
     g_object_unref (clear);
 
diff --git a/mime-node.c b/mime-node.c
index 0be03de7..33c51156 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -292,8 +292,6 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
 /* associate a MIME part with a node. */
 static bool
 _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild) {
-    notmuch_status_t status;
-
     /* Deal with the different types of parts */
     if (GMIME_IS_PART (part)) {
 	node->part = part;
@@ -334,9 +332,7 @@ _mime_node_set_up_part (mime_node_t *node, GMimeObject *part, int numchild) {
 	    node_verify (node, part);
 	}
     } else {
-	status = _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, numchild);
-	if (status)
-	    fprintf (stderr, "Warning: failed to record potential crypto payload (%s).\n", notmuch_status_to_string (status));
+	_notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, node->parent ? node->parent->part : NULL, numchild);
     }
 
     return true;
diff --git a/util/crypto.c b/util/crypto.c
index 9e185e03..743c751a 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -132,19 +132,16 @@ _notmuch_message_crypto_potential_sig_list (_notmuch_message_crypto_t *msg_crypt
 }
 
 
-notmuch_status_t
-_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum)
+bool
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto, GMimeObject *part, GMimeObject *parent, int childnum)
 {
     const char *protected_headers = NULL;
     const char *forwarded = NULL;
     const char *subject = NULL;
 
-    if (!msg_crypto || !payload)
-	return NOTMUCH_STATUS_NULL_POINTER;
-
     /* only fire on the first payload part encountered */
     if (msg_crypto->payload_encountered)
-	return NOTMUCH_STATUS_SUCCESS;
+	return false;
 
     /* the first child of multipart/encrypted that matches the
      * encryption protocol should be "control information" metadata,
@@ -152,11 +149,11 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
      * https://tools.ietf.org/html/rfc1847#page-8) */
     if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum == GMIME_MULTIPART_ENCRYPTED_VERSION) {
 	const char *enc_type = g_mime_object_get_content_type_parameter (parent, "protocol");
-	GMimeContentType *ct = g_mime_object_get_content_type (payload);
+	GMimeContentType *ct = g_mime_object_get_content_type (part);
 	if (ct && enc_type) {
 	    const char *part_type = g_mime_content_type_get_mime_type (ct);
 	    if (part_type && strcmp (part_type, enc_type) == 0)
-		return NOTMUCH_STATUS_SUCCESS;
+		return false;
 	}
     }
 
@@ -166,7 +163,7 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
      * envelope: */
     if ((msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_FULL) &&
 	(msg_crypto->sig_list == NULL))
-	return NOTMUCH_STATUS_SUCCESS;
+	return false;
 
     /* Verify that this payload has headers that are intended to be
      * exported to the larger message: */
@@ -174,16 +171,16 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
     /* Consider a payload that uses Alexei Melinkov's forwarded="no" for
      * message/global or message/rfc822:
      * https://tools.ietf.org/html/draft-melnikov-smime-header-signing-05#section-4 */
-    forwarded = g_mime_object_get_content_type_parameter (payload, "forwarded");
-    if (GMIME_IS_MESSAGE_PART (payload) && forwarded && strcmp (forwarded, "no") == 0) {
-	GMimeMessage *message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (payload));
+    forwarded = g_mime_object_get_content_type_parameter (part, "forwarded");
+    if (GMIME_IS_MESSAGE_PART (part) && forwarded && strcmp (forwarded, "no") == 0) {
+	GMimeMessage *message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (part));
 	subject = g_mime_message_get_subject (message);
 	/* FIXME: handle more than just Subject: at some point */
     } else {
 	/* Consider "memoryhole"-style protected headers as practiced by Enigmail and K-9 */
-	protected_headers = g_mime_object_get_content_type_parameter (payload, "protected-headers");
+	protected_headers = g_mime_object_get_content_type_parameter (part, "protected-headers");
 	if (protected_headers && strcasecmp("v1", protected_headers) == 0)
-	    subject = g_mime_object_get_header (payload, "Subject");
+	    subject = g_mime_object_get_header (part, "Subject");
 	/* FIXME: handle more than just Subject: at some point */
     }
 
@@ -193,7 +190,7 @@ _notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto
 	msg_crypto->payload_subject = talloc_strdup (msg_crypto, subject);
     }
 
-    return NOTMUCH_STATUS_SUCCESS;
+    return true;
 }
 
 
diff --git a/util/crypto.h b/util/crypto.h
index fdbb5da5..ba1bf905 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -90,9 +90,12 @@ _notmuch_message_crypto_successful_decryption (_notmuch_message_crypto_t *msg_cr
 
 /* call potential_payload during a depth-first-search on a message
  * when encountering a message part that is not part of the envelope.
+ *
+ * Returns true if part is the root of the cryptographic payload of
+ * this message.
  */
-notmuch_status_t
-_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum);
+bool
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t *msg_crypto, GMimeObject *part, GMimeObject *parent, int childnum);
 
 
 #ifdef __cplusplus
-- 
2.20.1



More information about the notmuch mailing list