Clean reply / forwards of unsigned S/MIME Mails
authorAndre Heinecke <aheinecke@intevation.de>
Mon, 11 Jun 2018 16:19:48 +0000 (18:19 +0200)
committerAndre Heinecke <aheinecke@intevation.de>
Mon, 11 Jun 2018 16:23:20 +0000 (18:23 +0200)
* src/mailitem-events.cpp (EVENT_SINK_INVOKE): Improve
reply / forward handling and check for HTML Blocked Mails to
clean them.
* src/windowmessages.cpp, src/windowmessages.h
(do_in_ui_thread_async): Extend to take delay as opt. param.
(CLEAR_REPLY_FORWARD): New message.
(gpgol_window_proc): Handle it.

--
This is ugly ugly ugly. As we don't have a reliable
way to detect when a reply / forward is filled with data
we just wait a second before wiping the reply if necessary.

The change in the write event to not invalidate the
last mail also poses a regression risk.

GnuPG-Bug-Id: T3986

src/mailitem-events.cpp
src/windowmessages.cpp
src/windowmessages.h

index e4dd961..9effeb1 100644 (file)
@@ -135,6 +135,8 @@ EVENT_SINK_INVOKE(MailItemEvents)
           return S_OK;
         }
     }
+
+  bool is_reply = false;
   switch(dispid)
     {
       case Open:
@@ -592,7 +594,10 @@ EVENT_SINK_INVOKE(MailItemEvents)
                                  " Pass but schedule revert.",
                                  SRCNAME, __func__);
 
-                      Mail::invalidate_last_mail ();
+                      /* This might be a forward. So don't invalidate yet. */
+
+                      // Mail::invalidate_last_mail ();
+
                       do_in_ui_thread_async (REVERT_MAIL, m_mail);
                       return S_OK;
                     }
@@ -719,10 +724,16 @@ EVENT_SINK_INVOKE(MailItemEvents)
                          SRCNAME, __func__);
           return S_OK;
         }
+      /* Fallthrough */
+      case ReplyAll:
+      case Reply:
+        {
+          is_reply = true;
+        }
       case Forward:
         {
-          log_oom_extra ("%s:%s: Forward: %p",
-                         SRCNAME, __func__, m_mail);
+          log_oom_extra ("%s:%s: %s : %p",
+                         SRCNAME, __func__, is_reply ? "reply" : "forward", m_mail);
           if (!m_mail->is_crypto_mail ())
             {
               /* Non crypto mails do not interest us.*/
@@ -746,18 +757,52 @@ EVENT_SINK_INVOKE(MailItemEvents)
 
               if (!lastSize && !lastEntryStr.size ())
                 {
-                  log_debug ("%s:%s: Forward in the same loop as empty load."
-                             " Marking %p (item %p) as forwarded.",
-                             SRCNAME, __func__, last_mail, last_mail->item ());
+                  if (!is_reply)
+                    {
+                      log_debug ("%s:%s: Forward in the same loop as empty "
+                                 "load Marking %p (item %p) as forwarded.",
+                                 SRCNAME, __func__, last_mail,
+                                 last_mail->item ());
 
-                  last_mail->set_is_forwarded_crypto_mail (true);
+                      last_mail->set_is_forwarded_crypto_mail (true);
+                    }
+                  else
+                    {
+                      log_debug ("%s:%s: Reply in the same loop as empty "
+                                 "load Marking %p (item %p) as reply.",
+                                 SRCNAME, __func__, last_mail,
+                                 last_mail->item ());
+                    }
+                  if (m_mail->is_block_html())
+                    {
+                      std::string caption = _("GpgOL") + std::string (": ");
+                      caption += is_reply ? _("Dangerous reply") :
+                                            _("Dangerous forward");
+                      std::string buf = _("Unsigned S/MIME mails are not integrity "
+                                          "protected.");
+                      buf += "\n\n";
+
+                      if (is_reply)
+                        {
+                          buf += _("For security reasons no decrypted contents"
+                                   " are included in this reply.");
+                        }
+                      else
+                        {
+                          buf += _("For security reasons no decrypted contents"
+                                   " are included in the forwarded mail.");
+                        }
+
+                      gpgol_message_box (get_active_hwnd (), buf.c_str(),
+                                         _("GpgOL"), MB_OK);
+
+                      do_in_ui_thread_async (CLEAR_REPLY_FORWARD, last_mail, 1000);
+                    }
                 }
+              // We can now invalidate the last mail
+              Mail::invalidate_last_mail ();
             }
-        }
-      /* Fallthrough */
-      case Reply:
-      case ReplyAll:
-        {
+
           log_oom_extra ("%s:%s: Reply Forward ReplyAll: %p",
                          SRCNAME, __func__, m_mail);
           if (!opt.reply_crypt)
index 73292d1..36c0631 100644 (file)
@@ -182,6 +182,19 @@ gpgol_window_proc (HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
               xfree (ctx->data);
               break;
             }
+          case (CLEAR_REPLY_FORWARD):
+            {
+              auto mail = (Mail*) ctx->data;
+              if (!Mail::is_valid_ptr (mail))
+                {
+                  log_debug ("%s:%s: Clear reply forward for mail which is gone.",
+                             SRCNAME, __func__);
+                  break;
+                }
+              mail->wipe (true);
+              mail->remove_all_attachments ();
+              break;
+            }
           default:
             log_debug ("%s:%s: Unknown msg %x",
                        SRCNAME, __func__, ctx->wmsg_type);
@@ -242,7 +255,7 @@ send_msg_to_ui_thread (wm_ctx_t *ctx)
 int
 do_in_ui_thread (gpgol_wmsg_type type, void *data)
 {
-  wm_ctx_t ctx = {NULL, UNKNOWN, 0};
+  wm_ctx_t ctx = {NULL, UNKNOWN, 0, 0};
   ctx.wmsg_type = type;
   ctx.data = data;
   if (send_msg_to_ui_thread (&ctx))
@@ -256,19 +269,25 @@ static DWORD WINAPI
 do_async (LPVOID arg)
 {
   wm_ctx_t *ctx = (wm_ctx_t*) arg;
-  log_debug ("%s:%s: Do async with type %i",
-             SRCNAME, __func__, ctx ? ctx->wmsg_type : -1);
+  log_debug ("%s:%s: Do async with type %i after %i ms",
+             SRCNAME, __func__, ctx ? ctx->wmsg_type : -1,
+             ctx->delay);
+  if (ctx->delay)
+    {
+      Sleep (ctx->delay);
+    }
   send_msg_to_ui_thread (ctx);
   xfree (ctx);
   return 0;
 }
 
 void
-do_in_ui_thread_async (gpgol_wmsg_type type, void *data)
+do_in_ui_thread_async (gpgol_wmsg_type type, void *data, int delay)
 {
   wm_ctx_t *ctx = (wm_ctx_t *) calloc (1, sizeof (wm_ctx_t));
   ctx->wmsg_type = type;
   ctx->data = data;
+  ctx->delay = delay;
 
   CloseHandle (CreateThread (NULL, 0, do_async, (LPVOID) ctx, 0, NULL));
 }
index 56d9db8..dcc372c 100644 (file)
@@ -52,6 +52,7 @@ typedef enum _gpgol_wmsg_type
   BRING_TO_FRONT, /* Bring the active Outlook window to the front. */
   INVALIDATE_LAST_MAIL,
   REVERT_MAIL,
+  CLEAR_REPLY_FORWARD,
 } gpgol_wmsg_type;
 
 typedef struct
@@ -59,6 +60,7 @@ typedef struct
   void *data; /* Pointer to arbitrary data depending on msg type */
   gpgol_wmsg_type wmsg_type; /* Type of the msg. */
   int err; /* Set to true on error */
+  int delay;
 } wm_ctx_t;
 
 /** Create and register the responder window.
@@ -77,9 +79,12 @@ int
 do_in_ui_thread (gpgol_wmsg_type type, void *data);
 
 /** Send a message to the UI thread but returns
-    immediately without waiting for the execution. */
+    immediately without waiting for the execution.
+
+    The delay is used in the detached thread to delay
+    the sending of the actual message. */
 void
-do_in_ui_thread_async (gpgol_wmsg_type type, void *data);
+do_in_ui_thread_async (gpgol_wmsg_type type, void *data, int delay = 0);
 
 /** Create our filter before outlook Window Messages. */
 HHOOK