Commit 27020080 authored by Nathan Owens's avatar Nathan Owens
Browse files

user/trojita: CVE patches (#137)

parent f7628676
......@@ -2,7 +2,7 @@
# Maintainer: Kiyoshi Aman <adelie@aerdan.vulpine.house>
pkgname=trojita
pkgver=0.7
pkgrel=0
pkgrel=1
pkgdesc="Qt-based IMAP email client"
url="http://trojita.flaska.net/"
arch="all"
......@@ -15,7 +15,15 @@ makedepends="cmake extra-cmake-modules zlib-dev qt5-qtbase-dev qt5-qtwebkit-dev
qtkeychain-dev"
source="https://sourceforge.net/projects/trojita/files/src/trojita-$pkgver.tar.xz
use-qgpgme.patch
fix-gpg.patch"
fix-gpg.patch
CVE-2019-10734.patch
CVE-2020-15047.patch
"
# secfixes:
# 0.7-r1:
# - CVE-2019-10734
# - CVE-2020-15047
build() {
if [ "$CBUILD" != "$CHOST" ]; then
......@@ -34,7 +42,8 @@ build() {
}
check() {
CTEST_OUTPUT_ON_FAILURE=TRUE ctest
# test_Html_formatting: requires X11
CTEST_OUTPUT_ON_FAILURE=TRUE ctest -E test_Html_formatting
}
package() {
......@@ -43,4 +52,6 @@ package() {
sha512sums="fe4d9316f97d913619f27d24a5023c3d8dd4a6b9fb058651be12c67188f394aa8cbb60c7593e5eb28fc12fc883b76deeeb5f4f631edd255fdec4c5862c9a91c8 trojita-0.7.tar.xz
740c2410d7236d722482f05dd1d2c681e35543620823cb7c1396710081f9de4f6ae530c5b5442ecf5d08a8e552f0697f3a35bf51e07a3b4336dec7021b665706 use-qgpgme.patch
9d0fbf7c0b0f6975990a7705f9d43043e5807401cee179d7a07f9514856332d6bb1aa8448e84d0083003c34a3bb181080b973e8c1f77d1b5a8930d07d57702da fix-gpg.patch"
9d0fbf7c0b0f6975990a7705f9d43043e5807401cee179d7a07f9514856332d6bb1aa8448e84d0083003c34a3bb181080b973e8c1f77d1b5a8930d07d57702da fix-gpg.patch
db96a566924b5d7b80787ab624af3726d5dd3459653192436a377d6482ab73801a7dcca1df1b1d937cf0d0798b827e04f8ef2c1124f91dc9da3e8036ef61e28a CVE-2019-10734.patch
2477612aca1e558fa3ba2b434a701cc255c573ac7e2001e7b5921c9b991f7c95720f53b70b49824e36bafb53ab53477950cb8d436e637fda4d59c7ec5883ce5f CVE-2020-15047.patch"
From 8db7f450d52539b4c72ee968384911b6813ad1e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jkt@kde.org>
Date: Thu, 25 Jun 2020 21:39:34 +0200
Subject: [PATCH] Prevent a possible decryption oracle attack
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Thanks to Jens Mueller (Ruhr-Uni Bochum and FH Münster) for reporting
this. The gist is that an attacker can embed arbitrary ciphertext into
their messages. Trojita decrypts that, and when we hit reply, the
original *cleartext* gets quoted and put into a reply for the attacker
to see.
Fix this by not quoting any plaintext which originated in an encrypted
message. That's pretty draconian, but hey, it works and we never came up
with any better patch. Also, given that Trojita does not encrypt
outgoing messages yet, this is probably also a conservative thing to do.
Change-Id: I84c45b9e707eb7c99eb7183c6ef59ef41cd62c43
CVE: CVE-2019-10734
BUG: 404697
---
src/Cryptography/GpgMe++.cpp | 2 ++
src/Gui/MessageView.cpp | 9 ++++++++-
src/Gui/PartWidget.cpp | 8 ++++++++
src/Imap/Model/ItemRoles.h | 2 +-
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/Cryptography/GpgMe++.cpp b/src/Cryptography/GpgMe++.cpp
index e012f603..716b8aff 100644
--- a/src/Cryptography/GpgMe++.cpp
+++ b/src/Cryptography/GpgMe++.cpp
@@ -267,6 +267,8 @@ QVariant GpgMePart::data(int role) const
switch (role) {
case Imap::Mailbox::RolePartSignatureVerifySupported:
return m_wasSigned;
+ case RolePartDecryptionSupported:
+ return m_isAllegedlyEncrypted;
case RolePartCryptoNotFinishedYet:
return m_waitingForData ||
(m_crypto.valid() &&
diff --git a/src/Gui/MessageView.cpp b/src/Gui/MessageView.cpp
index 7d649308..c95e0878 100644
--- a/src/Gui/MessageView.cpp
+++ b/src/Gui/MessageView.cpp
@@ -354,7 +354,6 @@ bool MessageView::eventFilter(QObject *object, QEvent *event)
QString MessageView::quoteText() const
{
if (auto w = bodyWidget()) {
- QStringList quote = Composer::quoteText(w->quoteMe().split(QLatin1Char('\n')));
const Imap::Message::Envelope &e = message.data(Imap::Mailbox::RoleMessageEnvelope).value<Imap::Message::Envelope>();
QString sender;
if (!e.from.isEmpty())
@@ -362,6 +361,14 @@ QString MessageView::quoteText() const
if (e.from.isEmpty())
sender = tr("you");
+ if (messageModel->index(0, 0) /* fake message root */.child(0, 0) /* first MIME part */.data(Imap::Mailbox::RolePartDecryptionSupported).toBool()) {
+ // This is just an UX improvement shortcut: real filtering for CVE-2019-10734 is in
+ // MultipartSignedEncryptedWidget::quoteMe().
+ // That is required because the encrypted part might not be the root part of the message.
+ return tr("On %1, %2 sent an encrypted message:\n> ...\n\n").arg(e.date.toLocalTime().toString(Qt::SystemLocaleLongDate), sender);
+ }
+
+ QStringList quote = Composer::quoteText(w->quoteMe().split(QLatin1Char('\n')));
// One extra newline at the end of the quoted text to separate the response
quote << QString();
diff --git a/src/Gui/PartWidget.cpp b/src/Gui/PartWidget.cpp
index bb27604d..96eff338 100644
--- a/src/Gui/PartWidget.cpp
+++ b/src/Gui/PartWidget.cpp
@@ -378,6 +378,14 @@ void MultipartSignedEncryptedWidget::updateStatusIndicator()
QString MultipartSignedEncryptedWidget::quoteMe() const
{
+ if (m_partIndex.data(Imap::Mailbox::RolePartDecryptionSupported).toBool()) {
+ // See CVE-2019-10734, the point is not to leak cleartext from encrypted content. Even when Trojita starts supporting
+ // encryption of outgoing mail, we will have to check whether the encrypted cleartext is from the same sender, whether
+ // it matches the list of recipients (which is dynamic and can be set later on), etc etc.
+ // TL;DR, this is a can of worms.
+ return tr("[Encrypted message]");
+ }
+
return quoteMeHelper(children());
}
diff --git a/src/Imap/Model/ItemRoles.h b/src/Imap/Model/ItemRoles.h
index 4588d4d0..00adb3bb 100644
--- a/src/Imap/Model/ItemRoles.h
+++ b/src/Imap/Model/ItemRoles.h
@@ -193,7 +193,7 @@ enum {
RolePartSignatureVerifySupported,
/** @short Is the format of this particular multipart/encrypted supported and recognized?
- See RolePartSignatureVerifySupported, this is an equivalent.
+ If true, this message part represents content of an encrypted message that Trojita can attempt to decrypt.
*/
RolePartDecryptionSupported,
/** @short Is there any point in waiting longer?
--
GitLab
From 77ddd5d44f2bf4155d0c9b6f7d05f01713b32d5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jkt@kde.org>
Date: Thu, 25 Jun 2020 11:30:51 +0200
Subject: [PATCH] SMTP: Do not ignore TLS errors
This fixes a CVE-2020-15047 (category: CWE-295). Since commit 0083eea5ed
which added initial, experimental support for SMTP message submission,
we have apparently never implemented proper SSL/TLS error handling, and
the code has ever since just kept silently ignoring any certificate
verification errors. As a result, Trojita was susceptible to a MITM
attack when sending e-mails. The information leaked include user's
authentication details, including the password, and the content of sent
messages.
Sorry for this :(.
Now, this patch re-enabes proper TLS error handling. It was not possible
to directly re-use our code for TLS key pinning which we are using for
IMAP connections. In the Qt TLS code, the decision to accept or not
accept a TLS connection is a blocking one, so the IMAP code relies upon
the protocol state machine (i.e., another layer) for deciding whether to
use or not to use the just-established TLS connection. Implementing an
equivalent code in the SMTP library would be nice, but this hot-fix has
a priority. As a result, SMTP connections to hosts with, e.g.,
self-signed TLS certs, are no longer possible. Let's hope that this is
not a practical problem with Lets Encrypt anymore.
Thanks to Damian Poddebniak for reporting this bug.
Change-Id: Icd6bbb2b0fb3e45159fc9699ebd07ab84262fe37
CVE: CVE-2020-15047
BUG: 423453
---
src/MSA/SMTP.cpp | 11 +++++++++--
src/MSA/SMTP.h | 1 +
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/MSA/SMTP.cpp b/src/MSA/SMTP.cpp
index 3a054516..ac1eefc5 100644
--- a/src/MSA/SMTP.cpp
+++ b/src/MSA/SMTP.cpp
@@ -21,6 +21,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "SMTP.h"
+#include "UiUtils/Formatting.h"
namespace MSA
{
@@ -32,8 +33,8 @@ SMTP::SMTP(QObject *parent, const QString &host, quint16 port, bool encryptedCon
user(user), failed(false), isWaitingForPassword(false), sendingMode(MODE_SMTP_INVALID)
{
qwwSmtp = new QwwSmtpClient(this);
- // FIXME: handle SSL errors properly
- connect(qwwSmtp, &QwwSmtpClient::sslErrors, qwwSmtp, &QwwSmtpClient::ignoreSslErrors);
+ // FIXME: handle SSL errors in the same way as we handle IMAP TLS errors, with key pinning, etc.
+ connect(qwwSmtp, &QwwSmtpClient::sslErrors, this, &SMTP::handleSslErrors);
connect(qwwSmtp, &QwwSmtpClient::connected, this, &AbstractMSA::sending);
connect(qwwSmtp, &QwwSmtpClient::done, this, &SMTP::handleDone);
connect(qwwSmtp, &QwwSmtpClient::socketError, this, &SMTP::handleError);
@@ -78,6 +79,12 @@ void SMTP::handleError(QAbstractSocket::SocketError err, const QString &msg)
emit error(msg);
}
+void SMTP::handleSslErrors(const QList<QSslError>& errors)
+{
+ auto msg = UiUtils::Formatting::sslErrorsToHtml(errors);
+ emit error(tr("<p>Cannot send message due to an SSL/TLS error</p>\n%1").arg(msg));
+}
+
void SMTP::setPassword(const QString &password)
{
pass = password;
diff --git a/src/MSA/SMTP.h b/src/MSA/SMTP.h
index 453407d3..913bb873 100644
--- a/src/MSA/SMTP.h
+++ b/src/MSA/SMTP.h
@@ -43,6 +43,7 @@ public slots:
virtual void setPassword(const QString &password);
void handleDone(bool ok);
void handleError(QAbstractSocket::SocketError err, const QString &msg);
+ void handleSslErrors(const QList<QSslError>& errors);
private:
QwwSmtpClient *qwwSmtp;
QString host;
--
GitLab
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment