[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 Jan Kundrát changed: What|Removed |Added Status|CONFIRMED |RESOLVED Resolution|--- |UPSTREAM -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #12 from Caspar Schutijser --- 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. -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #11 from Caspar Schutijser --- (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 -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #10 from Caspar Schutijser --- (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. -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #9 from Jan Kundrát --- Caspar, did you have luck reporting this bug to the Qt upstream? -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #8 from Jan Kundrát --- > 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. -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452
--- Comment #7 from Caspar Schutijser ---
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(""));
+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"));
+
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #6 from Caspar Schutijser --- (In reply to Thomas Lübking from comment #5) > The way the code looks, injecting > > > > 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. -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #5 from Thomas Lübking --- The way the code looks, injecting 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 -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452
--- Comment #4 from Caspar Schutijser ---
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
+
#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;
}
--
You are receiving this mail because:
You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 Caspar Schutijser changed: What|Removed |Added CC||cas...@schutijser.com --- Comment #3 from Caspar Schutijser --- 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. -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 --- Comment #2 from Jens Mueller --- 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 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: -- You are receiving this mail because: You are watching all bug changes.
[trojita] [Bug 390452] HTML Backchannel in Trojitá Mail Client: DNS Prefetching
https://bugs.kde.org/show_bug.cgi?id=390452 Jan Kundrát changed: What|Removed |Added Ever confirmed|0 |1 Status|UNCONFIRMED |CONFIRMED --- Comment #1 from Jan Kundrát --- 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 -- You are receiving this mail because: You are watching all bug changes.
