Bug 56274 – Delegate some read-only DOM properties to FrameLoaderClient and ChromeClient

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Robert Hogan]

Delegate some read-only DOM properties to FrameLoaderClient and ChromeClient

[Robert Hogan]

Created attachment 85625 [details]
Patch

[Early Warning System Bot]

Attachment 85625 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8176088

[WebKit Review Bot]

Attachment 85625 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8181018

[Build Bot]

Attachment 85625 [details] did not build on win:
Build output: http://queues.webkit.org/results/8176093

[WebKit Review Bot]

Attachment 85625 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8176104

[Collabora GTK+ EWS bot]

Attachment 85625 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8176106

[WebKit Review Bot]

Attachment 85625 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8175109

[Adam Barth]

Comment on attachment 85625 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=85625&action=review

> Source/WebCore/loader/FrameLoaderClient.h:235
> +        virtual String referrer(const String& referrer) {return referrer;};
> +        virtual String windowName(const String& name) {return name;};
> +        virtual unsigned historyLength(unsigned length) {return length;};

No need for ; after these lines.

[Robert Hogan]

Created attachment 85705 [details]
Patch

[Hajime Morrita]

> Created an attachment (id=85705) [details]
> Patch

Just for curious, is it possible to have "drop in" support for fingerprint prevention?
It would be great if we can share the mechanism between ports.

[Robert Hogan]

(In reply to comment #10)
> > Created an attachment (id=85705) [details] [details]
> > Patch
> 
> Just for curious, is it possible to have "drop in" support for fingerprint prevention?
> It would be great if we can share the mechanism between ports.

Hi Morita,

If there are items for which the 'right thing to do' can be agreed on then there's no reason why WebKit couldn't implement them.

I'm trying to build up a picture of what those might be at:

https://trac.webkit.org/wiki/Fingerprinting

[Nate Chapin]

Comment on attachment 85705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=85705&action=review

Drive-by comment:

> Source/WebCore/loader/FrameLoader.cpp:3365
>  String FrameLoader::referrer() const
>  {
> -    return m_documentLoader ? m_documentLoader->request().httpReferrer() : "";
> +    return m_documentLoader ? m_client->referrer(m_documentLoader->request().httpReferrer()) : "";
> +}

I'm pretty sure that this will only change the referrer as displayed to javascript, not what is sent over the network.

Is that intentional?  I feel like if we're going to hide referrers, we should hide it in both HTTP and JS, not just one or the other.

[Robert Hogan]

(In reply to comment #12)
> (From update of attachment 85705 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=85705&action=review
> 
> Drive-by comment:
> 
> > Source/WebCore/loader/FrameLoader.cpp:3365
> >  String FrameLoader::referrer() const
> >  {
> > -    return m_documentLoader ? m_documentLoader->request().httpReferrer() : "";
> > +    return m_documentLoader ? m_client->referrer(m_documentLoader->request().httpReferrer()) : "";
> > +}
> 
> I'm pretty sure that this will only change the referrer as displayed to javascript, not what is sent over the network.

That's correct, yes.

> 
> Is that intentional?  I feel like if we're going to hide referrers, we should hide it in both HTTP and JS, not just one or the other.

It's intentional insofar as there is already a mechanism, in some ports at least, for the client to manipulate HTTP headers. In Qt, this is QNetworkAccessManager::createRequest() - which admittedly sits outside WebKit in Qt's case.

The other factor is that there may be a better 'generic' rule for making the referrer header more privacy-sensitive in all cases. This is tracked at https://bugs.webkit.org/show_bug.cgi?id=51638

[Alexey Proskuryakov]

It would probably be quite confusing that this client call only affects DOM APIs.

[Eric Seidel (no email)]

Comment on attachment 85705 [details]
Patch

I really don't like this design.

I think we should have a new AntiFingerprinting class or something that we can optionally pass these values to and have it reply with lies.

I don't want to plumb all of these up through the client in every different callsite.  Instead, it would be better to plumb through this lier class, which can plumb through the client on ports which care.  Then all the anti-fingerprinting logic can be implemented inside WebCore too.

[Eric Seidel (no email)]

What I dislike so much about this design is that previously all these simple data values are self-contained inside WebCore.  Now they're needlessly piping up through WebKit, which is bad.  Only in the anti-fingerprinting case do we need to ask WebKIt.  And even then we only need to ask it if we've turne don the anti-fingerprinting stuff, and then the anti-fingerprinting code (which should be inside WebCore) can do the lying.