Minor cleanup to mime-node.c

classic Classic list List threaded Threaded
5 messages Options
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

Minor cleanup to mime-node.c

This simple 2-patch series is a bit of cleanup that i noticed while
completing the work on handling S/MIME messages.

It's not strictly part of the S/MIME series, so breaking out this
minor cleanup separately should make it easier to review.

Regards,

        --dkg


_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] mime-node: rename decrypted_child to unwrapped_child

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 <[hidden email]>
---
 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.25.1

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by 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 <[hidden email]>
---
 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.25.1

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Tomi Ollila-2 Tomi Ollila-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] mime-node: Clean up unwrapped MIME parts correctly.

On Thu, Mar 19 2020, Daniel Kahn Gillmor wrote:

> 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 <[hidden email]>

Looks good to me.

Tomi

> ---
>  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.25.1
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] mime-node: rename decrypted_child to unwrapped_child

In reply to this post by Daniel Kahn Gillmor
Daniel Kahn Gillmor <[hidden email]> writes:

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

pushed

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch