[git] GpgOL - branch, master, updated. outlook-2007-removal-9-ga193ad3

by Andre Heinecke cvs at cvs.gnupg.org
Fri Jun 1 10:12:53 CEST 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  a193ad36f82ecf8e5a18b74dc9bc71fcf1ff2b38 (commit)
       via  b2ed39c55af20223d60a3c6673fb823e53c9a016 (commit)
      from  b6ddad7615a090d71c1fc00641280cf95af29e98 (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 a193ad36f82ecf8e5a18b74dc9bc71fcf1ff2b38
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jun 1 10:11:44 2018 +0200

    Show error status on syserror verify
    
    * src/mail.cpp (Mail::get_crypto_details): Show error.
    
    --
    This helps with diagnostics. E.g. and ed25519 signature
    in vs-nfd mode leads to a sys error with status
    Invalid pubkey algorithm.

diff --git a/src/mail.cpp b/src/mail.cpp
index c20dc71..fefe2f9 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -2349,6 +2349,11 @@ Mail::get_crypto_details()
                m_verify_result.numSignatures() < 1)
         {
           message += _("There was an error verifying the signature.\n");
+          const auto err = m_sig.status ();
+          if (err)
+            {
+              message += err.asString () + std::string ("\n");
+            }
         }
       else if (m_sig.summary() & Signature::Summary::SigExpired)
         {

commit b2ed39c55af20223d60a3c6673fb823e53c9a016
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jun 1 10:06:18 2018 +0200

    Block HTML for unsigned S/MIME messages
    
    * src/mail.cpp (Mail::set_block_status): New. Sets a MAPI prop
    to disable automatic HTML external references.
    (Mail::set_block_html): New. HTML content should be blocked.
    (Mail::parsing_done): check for block html.
    (Mail::update_body): Block HTML if necessary.
    * src/parsecontroller.cpp (ParseController::shouldBlockHtml): New.
    (is_valid_chksum): Check that the sig is valid even if it is
    untrusted.
    * src/mymapitags.h (PR_BLOCK_STATUS): New.
    * src/oomhelp.h (PR_BLOCK_STATUS_DASL): New.
    
    --
    This blocks HTML display in unsigned S/MIME Mails to avoid
    attacks that rely on HTML side channels. If there is
    no text/plain part it will show the unparsed HTML. Trying
    to parse it with Outlook and then inserting at as plain text
    left the references intact and appears to be too risky.
    
    GnuPG-Bug-Id: T3986

diff --git a/src/mail.cpp b/src/mail.cpp
index ca03066..c20dc71 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -1085,7 +1085,7 @@ Mail::update_body()
   /** Outlook does not show newlines if \r\r\n is a newline. We replace
     these as apparently some other buggy MUA sends this. */
   find_and_replace (html, "\r\r\n", "\r\n");
-  if (opt.prefer_html && !html.empty())
+  if (opt.prefer_html && !html.empty() && !m_block_html)
     {
       char *converted = ansi_charset_to_utf8 (m_parser->get_html_charset().c_str(),
                                               html.c_str(), html.size());
@@ -1096,9 +1096,71 @@ Mail::update_body()
           log_error ("%s:%s: Failed to modify html body of item.",
                      SRCNAME, __func__);
         }
+
       return;
     }
   auto body = m_parser->get_body ();
+
+  if (body.empty () && m_block_html && !html.empty())
+    {
+#if 0
+      Sadly the following code still offers to load external references
+      it might also be too dangerous if Outlook somehow autoloads the
+      references as soon as the Body is put into HTML
+
+
+      // Fallback to show HTML as plaintext if HTML display
+      // is blocked.
+      log_error ("%s:%s: No text body. Putting HTML into plaintext.",
+                 SRCNAME, __func__);
+
+      char *converted = ansi_charset_to_utf8 (m_parser->get_html_charset().c_str(),
+                                              html.c_str(), html.size());
+      int ret = put_oom_string (m_mailitem, "HTMLBody", converted ? converted : "");
+      xfree (converted);
+      if (ret)
+        {
+          log_error ("%s:%s: Failed to modify html body of item.",
+                     SRCNAME, __func__);
+          body = html;
+        }
+      else
+        {
+          char *plainBody = get_oom_string (m_mailitem, "Body");
+
+          if (!plainBody)
+            {
+              log_error ("%s:%s: Failed to obtain converted plain body.",
+                         SRCNAME, __func__);
+              body = html;
+            }
+          else
+            {
+              ret = put_oom_string (m_mailitem, "HTMLBody", plainBody);
+              xfree (plainBody);
+              if (ret)
+                {
+                  log_error ("%s:%s: Failed to put plain into html body of item.",
+                             SRCNAME, __func__);
+                  body = html;
+                }
+              else
+                {
+                  return;
+                }
+            }
+        }
+#endif
+      body = html;
+      std::string buf = _("HTML display disabled.");
+      buf += "\n\n";
+      buf += _("For security reasons HTML content in unsigned, encrypted\n"
+               "S/MIME mails cannot be displayed.\n\n"
+               "Please ask the sender to sign the message or to send it as plain text.");
+
+      gpgol_message_box (get_window(), buf.c_str() , _("GpgOL"), MB_OK);
+    }
+
   find_and_replace (body, "\r\r\n", "\r\n");
   char *converted = ansi_charset_to_utf8 (m_parser->get_body_charset().c_str(),
                                           body.c_str(), body.size());
@@ -1160,7 +1222,16 @@ Mail::parsing_done()
 
   TRACEPOINT;
   /* Set categories according to the result. */
-  update_categories();
+  update_categories ();
+
+  TRACEPOINT;
+  m_block_html = m_parser->shouldBlockHtml ();
+
+  if (m_block_html)
+    {
+      // Just to be careful.
+      set_block_status ();
+    }
 
   TRACEPOINT;
   /* Update the body */
@@ -2966,3 +3037,30 @@ Mail::get_verification_result_dump()
   ss << m_verify_result;
   return ss.str();
 }
+
+void
+Mail::set_block_status()
+{
+  SPropValue prop;
+
+  LPMESSAGE message = get_oom_base_message (m_mailitem);
+
+  prop.ulPropTag = PR_BLOCK_STATUS;
+  prop.Value.l = 1;
+  HRESULT hr = message->SetProps (1, &prop, NULL);
+
+  if (hr)
+    {
+      log_error ("%s:%s: can't set block value: hr=%#lx\n",
+                 SRCNAME, __func__, hr);
+    }
+
+  gpgol_release (message);
+  return;
+}
+
+void
+Mail::set_block_html(bool value)
+{
+  m_block_html = value;
+}
diff --git a/src/mail.h b/src/mail.h
index 677ac7b..20e737a 100644
--- a/src/mail.h
+++ b/src/mail.h
@@ -487,6 +487,13 @@ public:
 
   /* Gets the string dump of the verification result. */
   std::string get_verification_result_dump ();
+
+  /* Block loading HTML content */
+  void set_block_html (bool value);
+
+  /* Remove automatic loading of HTML references setting. */
+  void set_block_status ();
+
 private:
   void update_categories ();
   void update_sigstate ();
@@ -529,5 +536,6 @@ private:
   bool m_is_forwarded_crypto_mail; /* Is this a forward of a crypto mail */
   bool m_is_send_again; /* Is this a send again of a crypto mail */
   bool m_disable_att_remove_warning; /* Should not warn about attachment removal. */
+  bool m_block_html; /* Force blocking of html content. e.g for unsigned S/MIME mails. */
 };
 #endif // MAIL_H
diff --git a/src/mymapitags.h b/src/mymapitags.h
index 9232c37..1458102 100644
--- a/src/mymapitags.h
+++ b/src/mymapitags.h
@@ -846,6 +846,7 @@
 #define PR_SENT_REPRESENTING_SMTP_ADDRESS_A   PROP_TAG( PT_STRING8,     0x5d02)
 #define PR_SENT_REPRESENTING_SMTP_ADDRESS_W   PROP_TAG( PT_UNICODE,     0x5d02)
 #define PidTagSenderSmtpAddress_W             PROP_TAG( PT_UNICODE,     0x5d01)
+#define PR_BLOCK_STATUS                       PROP_TAG( PT_LONG,        0x1096)
 
 #define PROP_ID_SECURE_MIN                0x67F0
 #define PROP_ID_SECURE_MAX                0x67FF
diff --git a/src/oomhelp.h b/src/oomhelp.h
index ccc8236..c2afd4a 100644
--- a/src/oomhelp.h
+++ b/src/oomhelp.h
@@ -118,6 +118,8 @@ DEFINE_OLEGUID(IID_IOleWindow,                0x00000114, 0, 0);
   "http://schemas.microsoft.com/mapi/proptag/0x5D08001F"
 #define PR_PIDNameContentType_DASL \
   "http://schemas.microsoft.com/mapi/string/{00020386-0000-0000-C000-000000000046}/content-type/0x0000001F"
+#define PR_BLOCK_STATUS_DASL \
+  "http://schemas.microsoft.com/mapi/proptag/0x10960003"
 
 #ifdef __cplusplus
 extern "C" {
diff --git a/src/parsecontroller.cpp b/src/parsecontroller.cpp
index 3cd31a7..0db7734 100644
--- a/src/parsecontroller.cpp
+++ b/src/parsecontroller.cpp
@@ -77,7 +77,8 @@ ParseController::ParseController(LPSTREAM instream, msgtype_t type):
     m_inputprovider  (new MimeDataProvider(instream,
                           expect_no_headers(type))),
     m_outputprovider (new MimeDataProvider(expect_no_mime(type))),
-    m_type (type)
+    m_type (type),
+    m_block_html (false)
 {
   log_mime_parser ("%s:%s: Creating parser for stream: %p of type %i"
                    " expect no headers: %i expect no mime: %i",
@@ -90,7 +91,8 @@ ParseController::ParseController(FILE *instream, msgtype_t type):
     m_inputprovider  (new MimeDataProvider(instream,
                           expect_no_headers(type))),
     m_outputprovider (new MimeDataProvider(expect_no_mime(type))),
-    m_type (type)
+    m_type (type),
+    m_block_html (false)
 {
   log_mime_parser ("%s:%s: Creating parser for stream: %p of type %i",
                    SRCNAME, __func__, instream, type);
@@ -226,6 +228,25 @@ ParseController::setSender(const std::string &sender)
   m_sender = sender;
 }
 
+static bool
+is_valid_chksum(const GpgME::Signature &sig)
+{
+  switch (sig.summary())
+    {
+      case GpgME::Signature::Valid:
+      case GpgME::Signature::Green:
+      case GpgME::Signature::KeyRevoked:
+      case GpgME::Signature::KeyExpired:
+      case GpgME::Signature::SigExpired:
+      case GpgME::Signature::CrlMissing:
+      case GpgME::Signature::CrlTooOld:
+      case GpgME::Signature::TofuConflict:
+        return true;
+      default:
+        return false;
+    }
+}
+
 void
 ParseController::parse()
 {
@@ -358,9 +379,14 @@ ParseController::parse()
              m_verify_result.error().code());
 
   TRACEPOINT;
-  /* Ensure that the Keys for the signatures are available */
+
+  bool has_valid_encrypted_checksum = false;
+  /* Ensure that the Keys for the signatures are available
+     and if it has a valid encrypted checksum. */
   for (const auto sig: m_verify_result.signatures())
     {
+      has_valid_encrypted_checksum = is_valid_chksum (sig);
+
       sig.key(true, true);
       if (sig.validity() == Signature::Validity::Full ||
           sig.validity() == Signature::Validity::Ultimate)
@@ -371,6 +397,14 @@ ParseController::parse()
         }
     }
 
+  if (protocol == Protocol::CMS && decrypt && !m_decrypt_result.error() &&
+      !has_valid_encrypted_checksum)
+    {
+      log_debug ("%s:%s:%p Encrypted S/MIME without checksum. Block HTML.",
+                 SRCNAME, __func__, this);
+      m_block_html = true;
+    }
+
   if (opt.enable_debug)
     {
        std::stringstream ss;
diff --git a/src/parsecontroller.h b/src/parsecontroller.h
index d3c67c8..c318b9d 100644
--- a/src/parsecontroller.h
+++ b/src/parsecontroller.h
@@ -102,6 +102,9 @@ public:
 
   void setSender(const std::string &sender);
 
+  bool shouldBlockHtml() const
+  { return m_block_html; }
+
 private:
   /* State variables */
   MimeDataProvider *m_inputprovider;
@@ -111,6 +114,7 @@ private:
   GpgME::DecryptionResult m_decrypt_result;
   GpgME::VerificationResult m_verify_result;
   std::string m_sender;
+  bool m_block_html;
 };
 
 #endif /* PARSECONTROLLER_H */

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

Summary of changes:
 src/mail.cpp            | 107 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/mail.h              |   8 ++++
 src/mymapitags.h        |   1 +
 src/oomhelp.h           |   2 +
 src/parsecontroller.cpp |  40 ++++++++++++++++--
 src/parsecontroller.h   |   4 ++
 6 files changed, 157 insertions(+), 5 deletions(-)


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




More information about the Gnupg-commits mailing list