Bug 41802 - Give WebKit clients a way to replace window.screen to foil attempts to track users with it
Summary: Give WebKit clients a way to replace window.screen to foil attempts to track ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 41801
  Show dependency treegraph
 
Reported: 2010-07-07 14:34 PDT by Robert Hogan
Modified: 2010-09-09 11:46 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.54 KB, patch)
2010-09-02 12:37 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (5.06 KB, patch)
2010-09-02 13:58 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (3.73 KB, patch)
2010-09-04 03:07 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (6.02 KB, patch)
2010-09-07 12:43 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (6.93 KB, patch)
2010-09-08 13:25 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (8.40 KB, patch)
2010-09-09 04:07 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Hogan 2010-07-07 14:34:30 PDT
From https://bugzilla.mozilla.org/show_bug.cgi?id=418986
"The window.screen object can be used to build an identifier that can be used to
track users independent of IP. Based on my rough calculations, there should be
at least 29 bits of extractable state from desktop geometry, toolbar geometry,
window size, etc.

Currently, there is no way to obscure this information without injecting
javascript into the contentWindow to hook the screen object. It would be nice
if the browser provided a pref to say "only report my content window dimensions
as inner and outer height." This pref would cause the screen object to behave
as if the contentWindow was the size of the entire desktop, with no chrome or
desktop toolbar size information."

What he says!
Comment 1 Robert Hogan 2010-09-02 12:37:23 PDT
Created attachment 66397 [details]
Patch
Comment 2 Adam Barth 2010-09-02 12:44:16 PDT
Comment on attachment 66397 [details]
Patch

This changes is great, but we need a test.
Comment 3 Robert Hogan 2010-09-02 12:46:42 PDT
(In reply to comment #2)
> (From update of attachment 66397 [details])
> This changes is great, but we need a test.

Cool, I'll come up with one and resubmit. Thanks for the quick feedback!
Comment 4 Robert Hogan 2010-09-02 13:58:08 PDT
Created attachment 66408 [details]
Patch
Comment 5 Adam Barth 2010-09-02 16:06:05 PDT
Comment on attachment 66408 [details]
Patch

Can't we test this in a LayoutTest?  I thought the Replacable IDL attribue had some web-platform facing behavior change.
Comment 6 Robert Hogan 2010-09-04 03:07:01 PDT
Created attachment 66582 [details]
Patch
Comment 7 Robert Hogan 2010-09-04 03:08:36 PDT
Chromium-Win and Chromium-Mac have platform-specific test results for this test - not sure how to generate png/checksum for those two so have not touched either.
Comment 8 Adam Barth 2010-09-05 23:22:06 PDT
Comment on attachment 66582 [details]
Patch

Thanks for the patch.  It would be polite to give the Chromium WebKit gardener a heads up about the expected test behavior change when you land this patch.  If you ask in #webkit, someone can lookup who's on duty in the schedule.
Comment 9 WebKit Commit Bot 2010-09-07 12:06:41 PDT
Comment on attachment 66582 [details]
Patch

Rejecting patch 66582 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 20932 test cases.
fast/js/var-declarations-shadowing.html -> failed

Exiting early after 1 failures. 8583 tests run.
193.02s total testing time

8582 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
1 test case (<1%) had stderr output

Full output: http://queues.webkit.org/results/3905279
Comment 10 Robert Hogan 2010-09-07 12:43:46 PDT
Created attachment 66752 [details]
Patch
Comment 11 Darin Adler 2010-09-07 14:06:38 PDT
Is there a way we can give this power to WebKit clients without giving it to webpages too? This seems an unwanted behavior change for webpages, no?
Comment 12 Adam Barth 2010-09-07 14:33:07 PDT
(In reply to comment #11)
> Is there a way we can give this power to WebKit clients without giving it to webpages too? This seems an unwanted behavior change for webpages, no?

Why is it unwanted for webpages?
Comment 13 Darin Adler 2010-09-07 14:39:39 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Is there a way we can give this power to WebKit clients without giving it to webpages too? This seems an unwanted behavior change for webpages, no?
> 
> Why is it unwanted for webpages?

I had presumed that the previous lack of the [Replaceable] flag was based on website compatibility and browser consistency considerations, and I see no argument here in this bug saying those have changed.
Comment 14 Adam Barth 2010-09-07 14:50:36 PDT
> I had presumed that the previous lack of the [Replaceable] flag was based on website compatibility and browser consistency considerations, and I see no argument here in this bug saying those have changed.

I don't know the history here.  We had a meeting some folks from the Tor project recently and they expressed interest in being able to override the screen property also, which tells me there's desire to make it Replaceable.  I'd be inclined to try making it Replaceable and see if there are compat problems.
Comment 15 WebKit Commit Bot 2010-09-07 14:52:14 PDT
Comment on attachment 66752 [details]
Patch

Rejecting patch 66752 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 20933 test cases.
fast/js/sputnik/Conformance/08_Types/8.6_The_Object_Type/8.6.2_Internal_Properties_and_Methods/S8.6.2_A5_T1.html -> failed

Exiting early after 1 failures. 9223 tests run.
216.25s total testing time

9222 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
1 test case (<1%) had stderr output

Full output: http://queues.webkit.org/results/3946289
Comment 16 Robert Hogan 2010-09-08 13:25:15 PDT
Created attachment 66932 [details]
Patch
Comment 17 Robert Hogan 2010-09-08 13:27:52 PDT
(In reply to comment #16)
> Created an attachment (id=66932) [details]
> Patch

Apologies for the churn. There are definitely no other tests that can fail!
Comment 18 WebKit Commit Bot 2010-09-08 23:27:15 PDT
Comment on attachment 66932 [details]
Patch

Rejecting patch 66932 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 20944 test cases.
http/tests/security/cross-frame-access-put.html -> failed

Exiting early after 1 failures. 20358 tests run.
641.36s total testing time

20357 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
27 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/3974338
Comment 19 Robert Hogan 2010-09-09 03:47:05 PDT
(In reply to comment #18)
> http/tests/security/cross-frame-access-put.html -> failed

This test is skipped in Qt but doesn't need to be - I can recreate the failure and get it to pass otherwise.

The difference in results is an extra console message:

CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-put-test.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-put.html. Domains, protocols and ports must match.

for the attempt to shadow the screen object cross-origin. The 'replaceable' attribute generates this new piece of code that the test invokes and which generates the console message:

void setJSDOMWindowScreen(ExecState* exec, JSObject* thisObject, JSValue value)
{
    if (!static_cast<JSDOMWindow*>(thisObject)->allowsAccessFrom(exec))
        return;
    // Shadowing a built-in object
    static_cast<JSDOMWindow*>(thisObject)->putDirect(Identifier(exec, "screen"), value);
}

So the test hasn't identified a regression. Updating results and resubmitting.
Comment 20 Robert Hogan 2010-09-09 04:07:33 PDT
Created attachment 67019 [details]
Patch
Comment 21 Adam Barth 2010-09-09 06:51:48 PDT
Comment on attachment 67019 [details]
Patch

Ok.  Please run all the tests next time before uploading patches.  It's a bit bit of a time sink to keep spinning on these patches.
Comment 22 Antonio Gomes 2010-09-09 06:57:34 PDT
(In reply to comment #20)
> Created an attachment (id=67019) [details]
> Patch

tip: Robert, it has been already reviewed. In cases when you are just updating a non-harmful test expectation, you likely do not need to re-requestion review.

Pre-full the "Reviewed by" with the reviewer name, and upload it setting cq+ to the patch. It's been working fine for me.
Comment 23 WebKit Commit Bot 2010-09-09 11:46:52 PDT
Comment on attachment 67019 [details]
Patch

Clearing flags on attachment: 67019

Committed r67100: <http://trac.webkit.org/changeset/67100>
Comment 24 WebKit Commit Bot 2010-09-09 11:46:58 PDT
All reviewed patches have been landed.  Closing bug.