Closed Bug 1878716 Opened 5 months ago Closed 2 months ago

Something screwy with window.open and canvas randomization

Categories

(Core :: Privacy: Anti-Tracking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
127 Branch
Tracking Status
firefox127 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Regressed 1 open bug)

Details

(Whiteboard: [fpp:m?])

Attachments

(18 files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

If I open https://canvasblocker.kkapsner.de/test/test.html in two browsers, with FPP enabled in both - the window.open values will be the same between browsers. Which shouldn't be happening because randomization.

A different issue is reported in https://github.com/arkenfox/user.js/issues/1716#issuecomment-1925180783 which shows that window.open is getting the same hash as a window without FPP enabled - however I could not reproduce that.

Maybe (hopefully?) these are symptoms of the same issue and fixing one will fix both...

So far I've traced this to the fact that GenerateCanvasKeyFromImageData fails because aCookieJarSettings->GetFingerprintingRandomizationKey fails. There's aCookieJarSettings (it's not null), but we never called SetFingerprintingRandomizationKey on it, so it has no key.

We are taking this branch because we don't have an inprocessparent for this popup it seems.

I confirmed that the attached patch fixes it, but I'll need feedback from Tim next week. This is basically the same problem we had with null-principal documents, opened windows, the RFP overrides.... just now applied to the randomization key.

It seems like our two options are to either copy from the opener window into the opened window (but this would probably be inconsistent with a bunch of other stuff not-RFP related that we don't copy, like the rest of CJS) or we generate a new randomization key in the else block, so the opened window has a different randomization key than the opener.

Flags: needinfo?(tihuang)

For the record, the opened window is in the same process.

It makes more sense if we copy the random key from the opener window. The random key is decided when we open the channel for the document in the parent process. However, a null principal document could have never gone through the parent process. So, we should copy the key from the opener window.

Flags: needinfo?(tihuang)
Severity: -- → S3
Priority: -- → P2
See Also: → 1885471
Assignee: nobody → tom
Attachment #9378989 - Attachment description: WIP: Bug 1878716: Copy the randomization key from the opener → Bug 1878716: Copy the randomization key from the opener r?timhuang
Status: NEW → ASSIGNED

Coming back to this, my patch seems to been screwed up - I can't remember or tell if my new block was inside if (inProcessParent) or after it. But it appears that it only works if it was after it, so I guess that's where it must have been.

It seems like we could copy a lot of the same properties we copy from inProcessParent from the opener. This patch only copies the CanvasRandomizationKey. But there's also CookieBehavior, PartitionKey, IsFirstPartyIsolated, IsOnContentBlockingAllowList, and ShouldResistFingerprinting. I think I'm going to need to look at this more because I have a feeling we're going to want to copy more of these...

It turns out that we had the override string wrong, but we
didn't notice because we happened to be only testing things
where it was supposed to be disabled anyway.

There was a test where this would have not been the case
(simplePBMFPPTest) - but this was only be tested when we were
exempting it with ETP.

I added this scenario to a place where it would be tested
positively, and confirmed that it failed, then when I fixed
the override, started working again.

This test ensures the expected number of pixels are randomized
in different scenarios of preferences

For the first test, the parent is a page and the child is an iframe, but
eventually we will have tests for lots of different children types.

We test that the number of pixels we expect to be randomized are, and
we also compare the differences between the parent and child.

This allows us to ensure that the FPP key is being propagated correctly.

This test is a little odd because (a) its name is super long and unhelpful
(b) it's testing a very important sitation we've regressed on before
(c) it disables the protections, so you actually don't expect anything
to be enabled.

We're going to need a very similar version of this test where the
protections are enabled, so let's pre-emptively rename this one to
prepare for that.

Attachment #9393619 - Attachment description: Bug 1878716: Add a test that parents the canvases netween a parent and child r?timhuang → Bug 1878716: Add a test that parents the canvases between a parent and child r?timhuang
Duplicate of this bug: 1885471
See Also: 1885471
Attachment #9393619 - Attachment description: Bug 1878716: Add a test that parents the canvases between a parent and child r?timhuang → Bug 1878716: Add a test that compares the canvases between a parent and child r?timhuang

Uhg, got several test failures on try from the last test run i will need to dig into

Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ed5d79d4629f
Copy the randomization key from the opener r=timhuang
https://hg.mozilla.org/integration/autoland/rev/8c994e63a5df
De-duplicate canvas comparison code in tests r=timhuang
https://hg.mozilla.org/integration/autoland/rev/734befa016cd
Fix some Hardware Concurrency Tests r=timhuang
https://hg.mozilla.org/integration/autoland/rev/a236ed107799
Add those tests to all the other files r=timhuang
https://hg.mozilla.org/integration/autoland/rev/7f4801b8afbd
Rename a shared test to hopefully be less confusing r=timhuang
https://hg.mozilla.org/integration/autoland/rev/f628d8a1eb58
Add a version of the test with protections enabled r=timhuang
https://hg.mozilla.org/integration/autoland/rev/5f0a42f88d73
Add a test for Canvas Randomization r=timhuang
https://hg.mozilla.org/integration/autoland/rev/c503aad0ca16
Add a test for popups r=timhuang
https://hg.mozilla.org/integration/autoland/rev/f87e9a61ca51
Add a test that compares the canvases between a parent and child r=timhuang
https://hg.mozilla.org/integration/autoland/rev/3978af2aa2fd
Add a test that compares the canvases in a popup that is same or cross domain r=timhuang
https://hg.mozilla.org/integration/autoland/rev/9b0ee8d0d4f6
Populate the key in another scenario that occurs for about:blank popups under certain conditions r=timhuang,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/6f3394ea8888
Add a test that compares the canvases in an about:blank popup r=timhuang
https://hg.mozilla.org/integration/autoland/rev/ac1a6456b77e
Add a test that compares the canvases in a blob popup r=timhuang
https://hg.mozilla.org/integration/autoland/rev/fceb109159c2
Add a test that compares the canvases in a data popup r=timhuang
https://hg.mozilla.org/integration/autoland/rev/99bdce781621
Add a test that compares the canvases in an about:blank iframe r=timhuang
https://hg.mozilla.org/integration/autoland/rev/cba20f6a8f60
Add a test that compares the canvases in a blob iframe r=timhuang
https://hg.mozilla.org/integration/autoland/rev/cef71eec96a0
Add a test that confirms the expected default behavior in PBM r=timhuang
https://hg.mozilla.org/integration/autoland/rev/76724efd86c9
Add a test that compares the canvases in a data iframe r=timhuang
Regressions: 1894730
No longer duplicate of this bug: 1885471
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: