[git] GpgOL - branch, master, updated. gpgol-2.0.6-94-gbf9098d

by Andre Heinecke cvs at cvs.gnupg.org
Thu Mar 15 09:30:43 CET 2018


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GnuPG extension for MS Outlook".

The branch, master has been updated
       via  bf9098d2d63de6feaf8fe9486299b7cae924022f (commit)
       via  7ba9dc3962fb3fb854c2b1cd9338618ffafbc5b4 (commit)
      from  39176388f3a1f55cfb05ae24e06851925c1d0032 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit bf9098d2d63de6feaf8fe9486299b7cae924022f
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Thu Mar 15 09:27:24 2018 +0100

    Fix unencrypted forward of crypto mail
    
    * src/mail.cpp (Mail::Mail): Add new marker for forwarded
    crypto mails.
    (Mail::remove_our_attachments): New. Removes gpgol attachments.
    (Mail::set_is_forwarded_crypto_mail, Mail::is_forwarded_crypto_mail):
    New accessors to marker.
    * src/mailitem-events.cpp (EVENT_SINK_INVOKE): Set marker on
    forward. Handle marker on write.
    
    --
    GnuPG-Bug-Id: T3836

diff --git a/src/mail.cpp b/src/mail.cpp
index b3c375c..7e8360f 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -92,7 +92,8 @@ Mail::Mail (LPDISPATCH mailitem) :
     m_is_gsuite(false),
     m_crypt_state(NoCryptMail),
     m_window(nullptr),
-    m_is_inline_response(false)
+    m_is_inline_response(false),
+    m_is_forwarded_crypto_mail(false)
 {
   if (get_mail_for_item (mailitem))
     {
@@ -2624,3 +2625,63 @@ Mail::locate_all_crypto_recipients()
         }
     }
 }
+
+int
+Mail::remove_our_attachments ()
+{
+  LPDISPATCH attachments = get_oom_object (m_mailitem, "Attachments");
+  if (!attachments)
+    {
+      TRACEPOINT;
+      return 0;
+    }
+  int count = get_oom_int (attachments, "Count");
+  LPDISPATCH to_delete[count];
+  int del_cnt = 0;
+  for (int i = 1; i <= count; i++)
+    {
+      auto item_str = std::string("Item(") + std::to_string (i) + ")";
+      LPDISPATCH attachment = get_oom_object (attachments, item_str.c_str());
+      if (!attachment)
+        {
+          TRACEPOINT;
+          continue;
+        }
+
+      attachtype_t att_type;
+      if (get_pa_int (attachment, GPGOL_ATTACHTYPE_DASL, (int*) &att_type))
+        {
+          /* Not our attachment. */
+          gpgol_release (attachment);
+          continue;
+        }
+
+      if (att_type == ATTACHTYPE_PGPBODY || att_type == ATTACHTYPE_MOSS ||
+          att_type == ATTACHTYPE_MOSSTEMPL)
+        {
+          /* One of ours to delete. */
+          to_delete[del_cnt++] = attachment;
+          /* Dont' release yet */
+          continue;
+        }
+      gpgol_release (attachment);
+    }
+  gpgol_release (attachments);
+
+  int ret = 0;
+
+  for (int i = 0; i < del_cnt; i++)
+    {
+      LPDISPATCH attachment = to_delete[i];
+
+      /* Delete the attachments that are marked to delete */
+      if (invoke_oom_method (attachment, "Delete", NULL))
+        {
+          log_error ("%s:%s: Error: deleting attachment %i",
+                     SRCNAME, __func__, i);
+          ret = -1;
+        }
+      gpgol_release (attachment);
+    }
+  return ret;
+}
diff --git a/src/mail.h b/src/mail.h
index 90f042e..7410f61 100644
--- a/src/mail.h
+++ b/src/mail.h
@@ -448,6 +448,15 @@ public:
   /** Get the mime data that should be used when sending. */
   std::string get_override_mime_data () const { return m_mime_data; }
 
+  /** Set if this is a forward of a crypto mail. */
+  void set_is_forwarded_crypto_mail (bool value) { m_is_forwarded_crypto_mail = value; }
+  bool is_forwarded_crypto_mail () { return m_is_forwarded_crypto_mail; }
+
+  /** Remove the hidden GpgOL attachments. This is needed when forwarding
+    without encryption so that our attachments are not included in the forward.
+    Returns 0 on success. Works in OOM. */
+  int remove_our_attachments ();
+
   void update_body ();
 private:
   void update_categories ();
@@ -488,5 +497,6 @@ private:
   HWND m_window;
   bool m_is_inline_response;
   std::string m_mime_data;
+  bool m_is_forwarded_crypto_mail; /* Is this a forward of a crypto mail */
 };
 #endif // MAIL_H
diff --git a/src/mailitem-events.cpp b/src/mailitem-events.cpp
index de4c274..273c842 100644
--- a/src/mailitem-events.cpp
+++ b/src/mailitem-events.cpp
@@ -494,10 +494,11 @@ EVENT_SINK_INVOKE(MailItemEvents)
 
                      Security consideration: Worst case we pass the write here but
                      an unload follows before we get the scheduled revert. This
-                     would leak plaintext.
+                     would leak plaintext. But does not happen in our tests.
 
                      Similarly if we crash or Outlook is closed before we see this
-                     revert. */
+                     revert. But as we immediately revert after the write this should
+                     also not happen. */
                   const std::string lastSubject = last_mail->get_subject ();
                   char *lastEntryID = get_oom_string (last_mail->item (), "EntryID");
                   int lastSize = get_oom_int (last_mail->item (), "Size");
@@ -542,6 +543,27 @@ EVENT_SINK_INVOKE(MailItemEvents)
               *(parms->rgvarg[0].pboolVal) = VARIANT_TRUE;
             }
 
+          if (!m_mail->is_crypto_mail () && m_mail->is_forwarded_crypto_mail () &&
+              !m_mail->needs_crypto () && m_mail->crypt_state () == Mail::NoCryptMail)
+            {
+              /* We are sure now that while this is a forward of an encrypted
+               * mail that the forward should not be signed or encrypted. So
+               * it's not constructed by us. We need to remove our attachments
+               * though so that they are not included in the forward. */
+              log_debug ("%s:%s: Writing unencrypted forward of crypt mail. "
+                         "Removing attachments. mail: %p item: %p",
+                         SRCNAME, __func__, m_mail, m_object);
+              if (m_mail->remove_our_attachments ())
+                {
+                  // Worst case we forward some encrypted data here not
+                  // a security problem, so let it pass.
+                  log_error ("%s:%s: Failed to remove our attachments.",
+                             SRCNAME, __func__);
+                }
+              /* Remove marker because we did this now. */
+              m_mail->set_is_forwarded_crypto_mail (false);
+            }
+
           log_debug ("%s:%s: Passing write event.",
                      SRCNAME, __func__);
           m_mail->set_needs_save (false);
@@ -616,6 +638,41 @@ EVENT_SINK_INVOKE(MailItemEvents)
           return S_OK;
         }
       case Forward:
+        {
+          log_oom_extra ("%s:%s: Forward: %p",
+                         SRCNAME, __func__, m_mail);
+          if (!m_mail->is_crypto_mail ())
+            {
+              /* Non crypto mails do not interest us.*/
+              break;
+            }
+          Mail *last_mail = Mail::get_last_mail ();
+          if (Mail::is_valid_ptr (last_mail))
+            {
+              /* We want to identify here if there was a mail created that
+                 should receive the contents of this mail. For this we check
+                 for a forward in the same loop as a mail creation.
+              */
+              char *lastEntryID = get_oom_string (last_mail->item (), "EntryID");
+              int lastSize = get_oom_int (last_mail->item (), "Size");
+              std::string lastEntryStr;
+              if (lastEntryID)
+                {
+                  lastEntryStr = lastEntryID;
+                  xfree (lastEntryID);
+                }
+
+              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 ());
+
+                  last_mail->set_is_forwarded_crypto_mail (true);
+                }
+            }
+        }
+      /* Fallthrough */
       case Reply:
       case ReplyAll:
         {

commit 7ba9dc3962fb3fb854c2b1cd9338618ffafbc5b4
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Thu Mar 15 08:26:46 2018 +0100

    Avoid double gpgme_data_identify
    
    * src/parsecontroller.cpp (ParseController::parse): Store first
    identify value and reuse it.

diff --git a/src/parsecontroller.cpp b/src/parsecontroller.cpp
index 860c215..484afc2 100644
--- a/src/parsecontroller.cpp
+++ b/src/parsecontroller.cpp
@@ -234,7 +234,9 @@ ParseController::parse()
 
   Data input (m_inputprovider);
 
-  if (input.type () == Data::Type::PGPSigned)
+  auto inputType = input.type ();
+
+  if (inputType == Data::Type::PGPSigned)
     {
       verify = true;
       decrypt = false;
@@ -289,7 +291,7 @@ ParseController::parse()
              decrypt, verify,
              protocol == OpenPGP ? "OpenPGP" :
              protocol == CMS ? "CMS" : "Unknown",
-             m_sender.empty() ? "none" : m_sender.c_str(), input.type ());
+             m_sender.empty() ? "none" : m_sender.c_str(), inputType);
   if (decrypt)
     {
       input.seek (0, SEEK_SET);

-----------------------------------------------------------------------

Summary of changes:
 src/mail.cpp            | 63 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/mail.h              | 10 ++++++++
 src/mailitem-events.cpp | 61 +++++++++++++++++++++++++++++++++++++++++++++--
 src/parsecontroller.cpp |  6 +++--
 4 files changed, 135 insertions(+), 5 deletions(-)


hooks/post-receive
-- 
GnuPG extension for MS Outlook
http://git.gnupg.org




More information about the Gnupg-commits mailing list