[git] GpgOL - branch, master, updated. gpgol-2.1.0-10-g7eed3c4
by Andre Heinecke
cvs at cvs.gnupg.org
Tue Apr 24 08:41:20 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 7eed3c4c5e9f84bed0e412213cf404a18cd54358 (commit)
via 4f8e54c7db31df8f9ed4d222e4f0c73cc0c87445 (commit)
via 0b4e0418077b78baadec540573e837c5498e0ce0 (commit)
via f6135cfc36625cbe558c6bc5fa2410526c66ed6b (commit)
via 3dcea486dcdc5647dd618b255d6c76155dcf6fd8 (commit)
from e8451f067808125b79b36eda0828fb53bbe32e4f (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 7eed3c4c5e9f84bed0e412213cf404a18cd54358
Author: Andre Heinecke <aheinecke at intevation.de>
Date: Tue Apr 24 08:37:48 2018 +0200
Fix strange crash in recipient lookup
* src/mail.cpp (Mail::locate_keys): Add reentrancy guard.
--
The strangest thing seems to happen here:
In get_recipients the lookup for "AddressEntry" on
an unresolved address might cause network traffic.
So Outlook somehow "detaches" this call and keeps
processing window messages while the call is running.
So our do_delayed_locate might trigger a second locate.
If we access the OOM in this call while we access the
same object in the blocked "detached" call we crash.
(T3931)
After the window message is handled outlook retunrs
in the original lookup.
A better fix here might be a non recursive lock
of the OOM. But I expect that if we lock the handling
of the Windowmessage we might deadlock.
GnuPG-Bug-Id: T3931
This also might be a cause for the crashes which caused
GnuPG-Bug-Id: T3838
diff --git a/src/mail.cpp b/src/mail.cpp
index cad8eb6..10f7344 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -2467,11 +2467,44 @@ Mail::get_sig_fpr() const
void
Mail::locate_keys()
{
- char ** recipients = get_recipients ();
- KeyCache::instance()->startLocate (recipients);
- KeyCache::instance()->startLocate (get_sender ().c_str ());
+ static bool locate_in_progress;
+
+ if (locate_in_progress)
+ {
+ /** XXX
+ The strangest thing seems to happen here:
+ In get_recipients the lookup for "AddressEntry" on
+ an unresolved address might cause network traffic.
+
+ So Outlook somehow "detaches" this call and keeps
+ processing window messages while the call is running.
+
+ So our do_delayed_locate might trigger a second locate.
+ If we access the OOM in this call while we access the
+ same object in the blocked "detached" call we crash.
+ (T3931)
+
+ After the window message is handled outlook retunrs
+ in the original lookup.
+
+ A better fix here might be a non recursive lock
+ of the OOM. But I expect that if we lock the handling
+ of the Windowmessage we might deadlock.
+ */
+ log_debug ("%s:%s: Locate for %p already in progress.",
+ SRCNAME, __func__, this);
+ return;
+ }
+ locate_in_progress = true;
+
+ // First update oom data to have recipients and sender updated.
+ update_oom_data ();
+ char ** recipients = take_cached_recipients ();
KeyCache::instance()->startLocateSecret (get_sender ().c_str ());
+ KeyCache::instance()->startLocate (get_sender ().c_str ());
+ KeyCache::instance()->startLocate (recipients);
release_cArray (recipients);
+ locate_in_progress = false;
}
bool
commit 4f8e54c7db31df8f9ed4d222e4f0c73cc0c87445
Author: Andre Heinecke <aheinecke at intevation.de>
Date: Tue Apr 24 08:36:32 2018 +0200
Fix a possible (unlikely) use after free
* src/oomhelp.cpp (get_oom_recipients): Fix a possible
use after release of recipient object.
(get_recipient_addr_fallbacks): Add usual null guard.
diff --git a/src/oomhelp.cpp b/src/oomhelp.cpp
index 4ba8715..bfb84b6 100644
--- a/src/oomhelp.cpp
+++ b/src/oomhelp.cpp
@@ -1110,6 +1110,10 @@ get_pa_int (LPDISPATCH pDisp, const char *property, int *rInt)
static char *
get_recipient_addr_fallbacks (LPDISPATCH recipient)
{
+ if (!recipient)
+ {
+ return nullptr;
+ }
LPDISPATCH addr_entry = get_oom_object (recipient, "AddressEntry");
if (!addr_entry)
@@ -1204,14 +1208,15 @@ get_oom_recipients (LPDISPATCH recipients)
}
/* No PR_SMTP_ADDRESS first fallback */
resolved = get_recipient_addr_fallbacks (recipient);
- gpgol_release (recipient);
if (resolved)
{
recipientAddrs[i-1] = resolved;
+ gpgol_release (recipient);
continue;
}
char *address = get_oom_string (recipient, "Address");
+ gpgol_release (recipient);
log_debug ("%s:%s: Failed to look up Address probably EX addr is returned",
SRCNAME, __func__);
recipientAddrs[i-1] = address;
commit 0b4e0418077b78baadec540573e837c5498e0ce0
Author: Andre Heinecke <aheinecke at intevation.de>
Date: Mon Apr 23 15:06:54 2018 +0200
Format error if decryption was canceled
* src/parsecontroller.cpp (ParseController::parse): Format
error if decryption was canceled.
--
Otherwise it would show an empty body in that case.
diff --git a/src/parsecontroller.cpp b/src/parsecontroller.cpp
index 15dea50..3cd31a7 100644
--- a/src/parsecontroller.cpp
+++ b/src/parsecontroller.cpp
@@ -321,7 +321,8 @@ ParseController::parse()
{
verify = false;
}
- if (m_decrypt_result.error () || m_decrypt_result.isNull ())
+ if (m_decrypt_result.error () || m_decrypt_result.isNull () ||
+ m_decrypt_result.error ().isCanceled ())
{
m_error = format_error (m_decrypt_result, protocol);
}
commit f6135cfc36625cbe558c6bc5fa2410526c66ed6b
Author: Andre Heinecke <aheinecke at intevation.de>
Date: Mon Apr 23 15:03:37 2018 +0200
Fix display of No Seckey errors
* src/parsecontroller.cpp (format_error): At some point
decrpyt failed was no longer set but instead it changed
to no seckey. This is better but was not handled.
diff --git a/src/parsecontroller.cpp b/src/parsecontroller.cpp
index 454ac4c..15dea50 100644
--- a/src/parsecontroller.cpp
+++ b/src/parsecontroller.cpp
@@ -177,7 +177,8 @@ format_error(GpgME::DecryptionResult result, Protocol protocol)
msg = _("Decryption canceled or timed out.");
}
- if (result.error ().code () == GPG_ERR_DECRYPT_FAILED)
+ if (result.error ().code () == GPG_ERR_DECRYPT_FAILED ||
+ result.error ().code () == GPG_ERR_NO_SECKEY)
{
no_sec = true;
for (const auto &recipient: result.recipients ()) {
commit 3dcea486dcdc5647dd618b255d6c76155dcf6fd8
Author: Andre Heinecke <aheinecke at intevation.de>
Date: Mon Apr 23 14:34:12 2018 +0200
Improve error handling on encryption
* src/cryptcontroller.cpp (CryptController::parse_output): Handle
the case the the user selects no encryption recipients.
(CryptController::do_crypto): Accept error as argument and
set it / use it.
* src/mail.cpp (do_crypt): Handle error.
--
We still get General Error and Unknown Error way too much
but maybe it helps a bit. At least it won't give us
a little bit more information and don't show the bug
message.
GnuPG-Bug-Id: T3897
diff --git a/src/cryptcontroller.cpp b/src/cryptcontroller.cpp
index bc308cb..2e9829b 100644
--- a/src/cryptcontroller.cpp
+++ b/src/cryptcontroller.cpp
@@ -294,7 +294,9 @@ CryptController::parse_output (GpgME::Data &resolverOutput)
{
log_error ("%s:%s: Encrypt requested but no recipient fingerprints",
SRCNAME, __func__);
- return -1;
+ gpgol_message_box (m_mail->get_window(), _("No recipients for encryption selected."),
+ _("GpgOL"), MB_OK);
+ return -2;
}
return lookup_fingerprints (sigFpr, recpFprs);
@@ -511,7 +513,7 @@ CryptController::resolve_keys ()
}
int
-CryptController::do_crypto ()
+CryptController::do_crypto (GpgME::Error &err)
{
log_debug ("%s:%s",
SRCNAME, __func__);
@@ -569,17 +571,21 @@ CryptController::do_crypto ()
do_inline ? m_bodyInput : m_input,
m_output,
GpgME::Context::AlwaysTrust);
+ const auto err1 = result_pair.first.error();
+ const auto err2 = result_pair.second.error();
- if (result_pair.first.error() || result_pair.second.error())
+ if (err1 || err2)
{
log_error ("%s:%s: Encrypt / Sign error %s %s.",
SRCNAME, __func__, result_pair.first.error().asString(),
result_pair.second.error().asString());
+ err = err1 ? err1 : err2;
return -1;
}
- if (result_pair.first.error().isCanceled() || result_pair.second.error().isCanceled())
+ if (err1.isCanceled() || err2.isCanceled())
{
+ err = err1.isCanceled() ? err1 : err2;
log_debug ("%s:%s: User cancled",
SRCNAME, __func__);
return -2;
@@ -590,13 +596,14 @@ CryptController::do_crypto ()
// First sign then encrypt
const auto sigResult = ctx->sign (m_input, m_output,
GpgME::Detached);
- if (sigResult.error())
+ err = sigResult.error();
+ if (err)
{
log_error ("%s:%s: Signing error %s.",
SRCNAME, __func__, sigResult.error().asString());
return -1;
}
- if (sigResult.error().isCanceled())
+ if (err.isCanceled())
{
log_debug ("%s:%s: User cancled",
SRCNAME, __func__);
@@ -631,13 +638,14 @@ CryptController::do_crypto ()
const auto encResult = ctx->encrypt (m_recipients, multipart,
m_output,
GpgME::Context::AlwaysTrust);
- if (encResult.error())
+ err = encResult.error();
+ if (err)
{
log_error ("%s:%s: Encryption error %s.",
- SRCNAME, __func__, encResult.error().asString());
+ SRCNAME, __func__, err.asString());
return -1;
}
- if (encResult.error().isCanceled())
+ if (err.isCanceled())
{
log_debug ("%s:%s: User cancled",
SRCNAME, __func__);
@@ -650,13 +658,14 @@ CryptController::do_crypto ()
const auto result = ctx->encrypt (m_recipients, do_inline ? m_bodyInput : m_input,
m_output,
GpgME::Context::AlwaysTrust);
- if (result.error())
+ err = result.error();
+ if (err)
{
log_error ("%s:%s: Encryption error %s.",
- SRCNAME, __func__, result.error().asString());
+ SRCNAME, __func__, err.asString());
return -1;
}
- if (result.error().isCanceled())
+ if (err.isCanceled())
{
log_debug ("%s:%s: User cancled",
SRCNAME, __func__);
@@ -668,13 +677,14 @@ CryptController::do_crypto ()
const auto result = ctx->sign (do_inline ? m_bodyInput : m_input, m_output,
do_inline ? GpgME::Clearsigned :
GpgME::Detached);
- if (result.error())
+ err = result.error();
+ if (err)
{
log_error ("%s:%s: Signing error %s.",
- SRCNAME, __func__, result.error().asString());
+ SRCNAME, __func__, err.asString());
return -1;
}
- if (result.error().isCanceled())
+ if (err.isCanceled())
{
log_debug ("%s:%s: User cancled",
SRCNAME, __func__);
diff --git a/src/cryptcontroller.h b/src/cryptcontroller.h
index 8bcf1d4..4ba57d9 100644
--- a/src/cryptcontroller.h
+++ b/src/cryptcontroller.h
@@ -33,6 +33,7 @@ class Overlay;
namespace GpgME
{
class SigningResult;
+ class Error;
} // namespace GpgME
class CryptController
@@ -53,9 +54,12 @@ public:
/** @brief Does the actual crypto work according to op.
Can be called in a different thread then the UI Thread.
+ An operational error is returned in the passed err
+ variable.
+
@returns 0 on success.
*/
- int do_crypto ();
+ int do_crypto (GpgME::Error &err);
/** @brief Update the MAPI structure of the mail with
the result. */
diff --git a/src/mail.cpp b/src/mail.cpp
index a0e5727..cad8eb6 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -812,7 +812,8 @@ do_crypt (LPVOID arg)
return -1;
}
- int rc = crypter->do_crypto();
+ GpgME::Error err;
+ int rc = crypter->do_crypto(err);
gpgrt_lock_lock (&dtor_lock);
if (!Mail::is_valid_ptr (mail))
@@ -823,20 +824,31 @@ do_crypt (LPVOID arg)
return 0;
}
- if (rc == -1)
+ mail->set_window_enabled (true);
+
+ if (rc == -1 || err)
{
mail->reset_crypter ();
crypter = nullptr;
- gpgol_bug (mail->get_window (),
- ERR_CRYPT_RESOLVER_FAILED);
+ if (err)
+ {
+ char *buf = nullptr;
+ gpgrt_asprintf (&buf, _("Crypto operation failed:\n%s"),
+ err.asString());
+ gpgol_message_box (mail->get_window(), buf, _("GpgOL"), MB_OK);
+ xfree (buf);
+ }
+ else
+ {
+ gpgol_bug (mail->get_window (),
+ ERR_CRYPT_RESOLVER_FAILED);
+ }
}
- mail->set_window_enabled (true);
-
- if (rc)
+ if (rc || err.isCanceled())
{
- log_debug ("%s:%s: crypto failed for: %p with: %i",
- SRCNAME, __func__, arg, rc);
+ log_debug ("%s:%s: crypto failed for: %p with: %i err: %i",
+ SRCNAME, __func__, arg, rc, err.code());
mail->set_crypt_state (Mail::NoCryptMail);
mail->reset_crypter ();
crypter = nullptr;
-----------------------------------------------------------------------
Summary of changes:
src/cryptcontroller.cpp | 40 +++++++++++++++++-----------
src/cryptcontroller.h | 6 ++++-
src/mail.cpp | 69 ++++++++++++++++++++++++++++++++++++++++---------
src/oomhelp.cpp | 7 ++++-
src/parsecontroller.cpp | 6 +++--
5 files changed, 97 insertions(+), 31 deletions(-)
hooks/post-receive
--
GnuPG extension for MS Outlook
http://git.gnupg.org
More information about the Gnupg-commits
mailing list