cope with inline PGP encrypted messages

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

cope with inline PGP encrypted messages

Inline PGP encrypted messages are clearly worse than PGP/MIME
structured encrypted messages.  There are no standards for how they
are formed, and they don't offer any structured metadata about how to
interpret the bytestream produced by decrypting them.

However, some other MUAs and end-user workflows may make creation of
inline PGP encrypted messages the only available option for message
encryption, and when Notmuch encounters such a message, it should make
a reasonable best-effort to render the cleartext to the user.

Due to ambiguities in interpretation of signatures on inline messages
(e.g. which parts of the message were actually signed?  what character
encoding should the bytestream be interpreted as), we continue to
ignore inline-signed messages entirely, and we do not look at the
validity of any signatures that might be found when decrypting inline
PGP encrypted messages.

We make use here of GMime's optimization function for detecting the
presence of inline PGP encrypted content, which is only found in GMime
3.0 or later.

This series is currently based n top of the "notmuch show
--decrypt=stash" series, which it needs to be able to apply cleanly.
If that series proves controversial, i could rebase this patch
manually against some earlier commit.

If you have applied this series, and you know you have some inline PGP
messages already in your message store, you can try to retroactively
reindex them with something like:

    notmuch reindex --decrypt=true BEGIN-PGP-MESSAGE and not tag:encrypted

I welcome review and feedback about this series.

  --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/5] crypto: prepare for decryption of inline PGP encrypted messages

Inline PGP encrypted messages are clearly worse than PGP/MIME
structured encrypted messages.  There are no standards for how they
are formed, and they don't offer any structured metadata about how to
interpret the bytestream produced by decrypting them.

However, some other MUAs and end-user workflows may make creation of
inline PGP encrypted messages the only available option for message
encryption, and when Notmuch encounters such a message, it should make
a reasonable best-effort to render the cleartext to the user.

Due to ambiguities in interpretation of signatures on inline messages
(e.g. which parts of the message were actually signed?  what character
encoding should the bytestream be interpreted as), we continue to
ignore inline-signed messages entirely, and we do not look at the
validity of any signatures that might be found when decrypting inline
PGP encrypted messages.

We make use here of GMime's optimization function for detecting the
presence of inline PGP encrypted content, which is only found in GMime
3.0 or later.

This change prepares the internal codebase for decrypting inline
encrypted messages, but does not yet actually use the capability.
---
 lib/index.cc  |  6 +++---
 mime-node.c   | 24 ++++++++++++++----------
 util/crypto.c | 35 +++++++++++++++++++++++++++--------
 util/crypto.h |  2 +-
 4 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index 22ca9ec1..f144b9fb 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -367,7 +367,7 @@ _index_content_type (notmuch_message_t *message, GMimeObject *part)
 static void
 _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts,
     GMimeContentType *content_type,
-    GMimeMultipartEncrypted *part);
+    GMimeObject *part);
 
 /* Callback to generate terms for each mime part of a message. */
 static void
@@ -421,7 +421,7 @@ _index_mime_part (notmuch_message_t *message,
  if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) {
     _index_encrypted_mime_part(message, indexopts,
        content_type,
-       GMIME_MULTIPART_ENCRYPTED (part));
+       part);
  } else {
     if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) {
  _notmuch_database_log (notmuch_message_get_database (message),
@@ -518,7 +518,7 @@ static void
 _index_encrypted_mime_part (notmuch_message_t *message,
     notmuch_indexopts_t *indexopts,
     g_mime_3_unused(GMimeContentType *content_type),
-    GMimeMultipartEncrypted *encrypted_data)
+    GMimeObject *encrypted_data)
 {
     notmuch_status_t status;
     GError *err = NULL;
diff --git a/mime-node.c b/mime-node.c
index 75b79f98..973133d9 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -196,10 +196,10 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
 {
     GError *err = NULL;
     GMimeDecryptResult *decrypt_result = NULL;
-    GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
     notmuch_message_t *message = NULL;
 
-    if (! node->decrypted_child) {
+    if (GMIME_IS_PART (part) || /* must be inline */
+ (GMIME_IS_MULTIPART_ENCRYPTED (part) && ! node->decrypted_child)) {
  for (mime_node_t *parent = node; parent; parent = parent->parent)
     if (parent->envelope_file) {
  message = parent->envelope_file;
@@ -209,7 +209,7 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
  node->decrypted_child = _notmuch_crypto_decrypt (&node->decrypt_attempted,
  node->ctx->crypto->decrypt,
  message,
- cryptoctx, encrypteddata, &decrypt_result, &err);
+ cryptoctx, part, &decrypt_result, &err);
     }
     if (! node->decrypted_child) {
  fprintf (stderr, "Failed to decrypt part: %s\n",
@@ -217,15 +217,19 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
  goto DONE;
     }
 
-    node->decrypt_success = true;
-    node->verify_attempted = true;
 
     if (decrypt_result) {
- /* This may be NULL if the part is not signed. */
- node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
- if (node->sig_list) {
-    g_object_ref (node->sig_list);
-    set_signature_list_destructor (node);
+ node->decrypt_success = true;
+ if (GMIME_IS_MULTIPART_ENCRYPTED (part)) {
+    /* Only check signatures on PGP/MIME messages, not inline
+       messages. To understand why, see
+       https://dkg.fifthhorseman.net/notes/inline-pgp-harmful/ */
+    node->verify_attempted = true;
+    node->sig_list = g_mime_decrypt_result_get_signatures (decrypt_result);
+    if (node->sig_list) {
+ g_object_ref (node->sig_list);
+ set_signature_list_destructor (node);
+    }
  }
 
 #if HAVE_GMIME_SESSION_KEYS
diff --git a/util/crypto.c b/util/crypto.c
index 9d3b6dad..7965ab8e 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -144,7 +144,7 @@ _notmuch_crypto_decrypt (bool *attempted,
  notmuch_decryption_policy_t decrypt,
  notmuch_message_t *message,
  g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
- GMimeMultipartEncrypted *part,
+ GMimeObject *part,
  GMimeDecryptResult **decrypt_result,
  GError **err)
 {
@@ -166,15 +166,26 @@ _notmuch_crypto_decrypt (bool *attempted,
     if (attempted)
  *attempted = true;
 #if (GMIME_MAJOR_VERSION < 3)
-    ret = g_mime_multipart_encrypted_decrypt_session (part,
+    ret = g_mime_multipart_encrypted_decrypt_session (GMIME_MULTIPART_ENCRYPTED (part),
       crypto_ctx,
       notmuch_message_properties_value (list),
       decrypt_result, err);
 #else
-    ret = g_mime_multipart_encrypted_decrypt (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_PART (part) && g_mime_part_get_openpgp_data (GMIME_PART (part)) == GMIME_OPENPGP_DATA_ENCRYPTED) {
+ *decrypt_result = g_mime_part_openpgp_decrypt (GMIME_PART (part),
+       GMIME_DECRYPT_NONE,
+       notmuch_message_properties_value (list),
+       err);
+ if (decrypt_result) {
+    ret = part;
+    g_object_ref (ret);
+ }
+    }
 #endif
     if (ret)
  break;
@@ -214,8 +225,16 @@ _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,
-     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_PART (part) && g_mime_part_get_openpgp_data (GMIME_PART (part)) == GMIME_OPENPGP_DATA_ENCRYPTED) {
+ *decrypt_result = g_mime_part_openpgp_decrypt (GMIME_PART (part), flags, NULL, err);
+ if (decrypt_result) {
+    ret = part;
+    g_object_ref (ret);
+ }
+    }
 #endif
     return ret;
 }
diff --git a/util/crypto.h b/util/crypto.h
index c384601c..d9e4a1c7 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -20,7 +20,7 @@ _notmuch_crypto_decrypt (bool *attempted,
  notmuch_decryption_policy_t decrypt,
  notmuch_message_t *message,
  GMimeCryptoContext* crypto_ctx,
- GMimeMultipartEncrypted *part,
+ GMimeObject *part,
  GMimeDecryptResult **decrypt_result,
  GError **err);
 
--
2.15.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/5] cli/{show, reply}: try to decrypt inline PGP encrypted messages

In reply to this post by Daniel Kahn Gillmor
We try this only for leaf parts that are explicitly marked as
Content-Type: text/*, since we don't want to accidentally match on any
other weird part that happens to contain the magic string, or on the
payload child of a multipart/encrypted part.

Of course, this only works for GMime 3.0 and later, because of how
we're detecting the presence of the OpenPGP inline encrypted blob.
---
 mime-node.c                        |  4 ++
 test/T359-inline-pgp-decryption.sh | 97 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100755 test/T359-inline-pgp-decryption.sh

diff --git a/mime-node.c b/mime-node.c
index 973133d9..3c94bb62 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -325,6 +325,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
  } else {
     node_verify (node, part, cryptoctx);
  }
+#if (GMIME_MAJOR_VERSION >= 3)
+    } else if (GMIME_IS_TEXT_PART (part) && g_mime_part_get_openpgp_data (GMIME_PART (part)) == GMIME_OPENPGP_DATA_ENCRYPTED) {
+ node_decrypt_and_verify (node, part, cryptoctx);
+#endif
     }
 
     return node;
diff --git a/test/T359-inline-pgp-decryption.sh b/test/T359-inline-pgp-decryption.sh
new file mode 100755
index 00000000..c0db8eaf
--- /dev/null
+++ b/test/T359-inline-pgp-decryption.sh
@@ -0,0 +1,97 @@
+#!/usr/bin/env bash
+
+test_description='Decryption of inline PGP messages'
+. $(dirname "$0")/test-lib.sh || exit 1
+
+##################################################
+
+add_gnupg_home
+
+test_begin_subtest "Adding inline PGP encrypted message"
+mkdir -p "$MAIL_DIR/cur"
+cat <<EOF > "$MAIL_DIR/cur/inline-pgp-encrypted.eml"
+Message-Id: [hidden email]
+Content-Type: text/plain
+Subject: inline PGP encrypted message
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+From: [hidden email]
+To: [hidden email]
+
+$(echo "this is the sekrit message" | gpg --no-tty --batch --quiet --trust-model=always --encrypt --armor --recipient [hidden email])
+EOF
+test_expect_success 'notmuch new'
+
+test_begin_subtest "inline PGP decryption, --format=json"
+test_subtest_broken_gmime_2
+output=$(notmuch show --format=json --decrypt=true id:[hidden email] \
+    | notmuch_json_show_sanitize)
+expected='
+ [[[{"body": [{
+ "content": "this is the sekrit message\n",
+ "content-type": "text/plain",
+ "encstatus": [{"status": "good" }],
+ "id": 1
+ }],
+ "date_relative": "2000-01-01",
+ "excluded": false,
+ "filename": ["YYYYY"],
+ "headers": {
+   "Date": "Sat, 01 Jan 2000 12:00:00 +0000",
+   "From": "[hidden email]",
+   "Subject": "inline PGP encrypted message",
+   "To": "[hidden email]"
+  },
+ "id": "XXXXX",
+ "match": true,
+ "tags": ["inbox", "unread"],
+ "timestamp": 946728000
+ },
+ []]]]'
+
+test_expect_equal_json \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "inline PGP decryption for reply"
+test_subtest_broken_gmime_2
+output=$(notmuch reply --format=json --decrypt=true id:[hidden email] \
+    | notmuch_json_show_sanitize)
+expected='
+ {"original": {"body": [{
+ "content": "this is the sekrit message\n",
+ "content-type": "text/plain",
+ "encstatus": [{"status": "good" }],
+ "id": 1
+ }],
+ "date_relative": "2000-01-01",
+ "excluded": false,
+ "filename": ["YYYYY"],
+ "headers": {
+   "Date": "Sat, 01 Jan 2000 12:00:00 +0000",
+   "From": "[hidden email]",
+   "Subject": "inline PGP encrypted message",
+   "To": "[hidden email]"
+  },
+ "id": "XXXXX",
+ "match": false,
+ "tags": ["inbox", "unread"],
+ "timestamp": 946728000
+ },
+ "reply-headers": {
+   "From": "Notmuch Test Suite <[hidden email]>",
+   "In-reply-to": "<[hidden email]>",
+   "References": "<[hidden email]>",
+   "Subject": "Re: inline PGP encrypted message"
+ }
+}'
+
+test_expect_equal_json \
+    "$output" \
+    "$expected"
+
+test_begin_subtest "searching for cleartext of inline PGP encrypted message should fail"
+output=$(notmuch search 'sekrit')
+expected=''
+test_expect_equal "$output" "$expected"
+
+test_done
--
2.15.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 3/5] index: tag text parts with inline PGP encryption as "encrypted"

In reply to this post by Daniel Kahn Gillmor
Assuming we have GMime 3.0 (which has efficient detection of inline
PGP encrypted blobs) we should be able to mark those messages with the
same tag that we mark PGP/MIME and S/MIME encrypted messages.
---
 lib/index.cc                       | 6 ++++++
 test/T359-inline-pgp-decryption.sh | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index f144b9fb..e03f5230 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -468,6 +468,12 @@ _index_mime_part (notmuch_message_t *message,
  return;
     }
 
+#if (GMIME_MAJOR_VERSION >= 3)
+    if (GMIME_IS_TEXT_PART (part) && g_mime_part_get_openpgp_data (GMIME_PART (part)) == GMIME_OPENPGP_DATA_ENCRYPTED) {
+ _notmuch_message_add_term (message, "tag", "encrypted");
+    }
+#endif
+
     byte_array = g_byte_array_new ();
 
     stream = g_mime_stream_mem_new_with_byte_array (byte_array);
diff --git a/test/T359-inline-pgp-decryption.sh b/test/T359-inline-pgp-decryption.sh
index c0db8eaf..314ca786 100755
--- a/test/T359-inline-pgp-decryption.sh
+++ b/test/T359-inline-pgp-decryption.sh
@@ -43,7 +43,7 @@ expected='
   },
  "id": "XXXXX",
  "match": true,
- "tags": ["inbox", "unread"],
+ "tags": ["encrypted", "inbox", "unread"],
  "timestamp": 946728000
  },
  []]]]'
@@ -74,7 +74,7 @@ expected='
   },
  "id": "XXXXX",
  "match": false,
- "tags": ["inbox", "unread"],
+ "tags": ["encrypted", "inbox", "unread"],
  "timestamp": 946728000
  },
  "reply-headers": {
--
2.15.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 4/5] index: _index_encrypted_mime_part returns success or failure

In reply to this post by Daniel Kahn Gillmor
This change prepares us to know whether or not
_index_encrypted_mime_part succeeded or not on a given MIME part.

We don't currently make use of the information, but we will in
subsequent changes.
---
 lib/index.cc | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/index.cc b/lib/index.cc
index e03f5230..29ede685 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -364,7 +364,7 @@ _index_content_type (notmuch_message_t *message, GMimeObject *part)
     }
 }
 
-static void
+static bool
 _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts,
     GMimeContentType *content_type,
     GMimeObject *part);
@@ -419,6 +419,8 @@ _index_mime_part (notmuch_message_t *message,
  _index_content_type (message,
      g_mime_multipart_get_part (multipart, i));
  if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) {
+    /* deliberately ignore return value here: if it fails to decrypt,
+       we have nothing else to try */
     _index_encrypted_mime_part(message, indexopts,
        content_type,
        part);
@@ -519,8 +521,9 @@ _index_mime_part (notmuch_message_t *message,
 }
 
 /* descend (if desired) into the cleartext part of an encrypted MIME
- * part while indexing. */
-static void
+ * part while indexing.  Returns true if there was a successful
+ * decryption, false if there was not.*/
+static bool
 _index_encrypted_mime_part (notmuch_message_t *message,
     notmuch_indexopts_t *indexopts,
     g_mime_3_unused(GMimeContentType *content_type),
@@ -532,7 +535,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     GMimeObject *clear = NULL;
 
     if (!indexopts || (notmuch_indexopts_get_decrypt_policy (indexopts) == NOTMUCH_DECRYPT_FALSE))
- return;
+ return false;
 
     notmuch = notmuch_message_get_database (message);
 
@@ -550,7 +553,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     if (status)
  _notmuch_database_log_append (notmuch, "failed to add index.decryption "
       "property (%d)\n", status);
-    return;
+    return false;
  }
     }
 #endif
@@ -560,7 +563,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     clear = _notmuch_crypto_decrypt (&attempted, notmuch_indexopts_get_decrypt_policy (indexopts),
      message, crypto_ctx, encrypted_data, get_sk ? &decrypt_result : NULL, &err);
     if (!attempted)
- return;
+ return false;
     if (err || !clear) {
  if (decrypt_result)
     g_object_unref (decrypt_result);
@@ -576,7 +579,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
  if (status)
     _notmuch_database_log_append (notmuch, "failed to add index.decryption "
   "property (%d)\n", status);
- return;
+ return false;
     }
     if (decrypt_result) {
 #if HAVE_GMIME_SESSION_KEYS
@@ -597,7 +600,7 @@ _index_encrypted_mime_part (notmuch_message_t *message,
     if (status)
  _notmuch_database_log (notmuch, "failed to add index.decryption "
        "property (%d)\n", status);
-
+    return true;
 }
 
 notmuch_status_t
--
2.15.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 5/5] index: try indexing the cleartext of inline PGP encrypted text parts

In reply to this post by Daniel Kahn Gillmor
Assuming that we're using GMime 3.0 or later, and the user has asked
for decryption of some sort, we should go ahead and index the
cleartext.
---
 lib/index.cc                       | 7 +++++++
 test/T359-inline-pgp-decryption.sh | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/lib/index.cc b/lib/index.cc
index 29ede685..49f7cfbf 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -473,6 +473,13 @@ _index_mime_part (notmuch_message_t *message,
 #if (GMIME_MAJOR_VERSION >= 3)
     if (GMIME_IS_TEXT_PART (part) && g_mime_part_get_openpgp_data (GMIME_PART (part)) == GMIME_OPENPGP_DATA_ENCRYPTED) {
  _notmuch_message_add_term (message, "tag", "encrypted");
+ if (_index_encrypted_mime_part(message, indexopts,
+       content_type,
+       part))
+    return;
+ /* if decryption on inline PGP encrypted message fails, we
+ * should still fall through and try indexing the MIME part
+ * anyway (this is what we did before inline PGP decryption) */
     }
 #endif
 
diff --git a/test/T359-inline-pgp-decryption.sh b/test/T359-inline-pgp-decryption.sh
index 314ca786..66b85d5b 100755
--- a/test/T359-inline-pgp-decryption.sh
+++ b/test/T359-inline-pgp-decryption.sh
@@ -94,4 +94,11 @@ output=$(notmuch search 'sekrit')
 expected=''
 test_expect_equal "$output" "$expected"
 
+test_begin_subtest "reindexing cleartext of inline PGP encrypted message should succeed"
+test_subtest_broken_gmime_2
+notmuch reindex --decrypt=true id:[hidden email]
+output=$(notmuch search 'sekrit')
+expected='thread:0000000000000001   2000-01-01 [1/1] [hidden email]; inline PGP encrypted message (encrypted inbox unread)'
+test_expect_equal "$output" "$expected"
+
 test_done
--
2.15.1

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