1422862 - Make OffscreenCanvas respect Canvas Permission Prompt so you don't always get a placeholder

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, enhancement, P3)

Description

[Tom Ritter [:tjr]]

We have a permission prompt for Canvas we added in Bug 967895.  Some light testing indicates that OffscreenCanvas does not trigger the prompt.

Comment 1

[Yuan Lyu]

Here is a page to reproduce this issue, https://lyuyuan92.github.io/test_canvas_on_and_off_screen.html
With gfx.offscreencanvas.enabled and privacy.resistFingerprinting set to True, Firefox allows OffscreenCanvas API call without prompting the user.

Comment 2

[Thorin [:thorin]]

• link is: https://lyuyuan.github.io/lyuyuan92.github.io/test_canvas_on_and_off_screen.html
• github source: https://github.com/lyuyuan/lyuyuan92.github.io/blob/master/test_canvas_on_and_off_screen.html

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

This test case works with my latest patches (in flight on autoland still) but there is no permissions prompt still.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

If I understand this correctly, should the prompt only show when the raw pixels are exposed to JavaScript? I presume that is why copyOne triggers the prompt, because this line pulls out the data:

https://github.com/0x00A5/lyuyuan92.github.io/blob/master/test_canvas_on_and_off_screen.html#L30

but createTwo moves it around via an ImageBitmap, it doesn't:

https://github.com/0x00A5/lyuyuan92.github.io/blob/master/test_canvas_on_and_off_screen.html#L40-L41

I would only expect the prompt to be generated for Canvas2D.getImageData and HTMLCanvasElement.toDataURL (maybe HTMLCanvasElement.toBlob?). So it doesn't seem like there is actually a problem here....

Comment 5

[Tom Ritter [:tjr]]

Yes, this sounds like we misunderstood the feature when we initially filed this bug (so as not to forget about it.) If there is no way for an OffscreenCanvas to actually give its rendered content via an API (like there is for canvas through e.g. toDataURL), and all it can do it stick it into a canvas (where permission prompts work) or another object that can't be read via Javascript (like an <img> tag) - then we are fine here.

Comment 6

[Tom Schuster (MoCo)]

If there is no way for an OffscreenCanvas to actually give its rendered content via an API (like there is for canvas through e.g. toDataURL),

There are ways to get the rendered content out of the OffscreenCanvas, e.g. convertToBlob() or transferToImageBitmap().

It seems like we currently check ShouldResistFingerprinting(RFPTarget::CanvasImageExtractionPrompt) in OffscreenCanvas::ConvertToBlob, but there doesn't seem to be any way to actually show a prompt. (Which might be tricky to do, I guess, because OffscreenCanvas can run on a Worker thread) So to me it seems like we will always get a placeholder image when using this API.

Comment 7

[Tom Ritter [:tjr]]

So transferToImageBitmap gives you an ImageBitmap, and I get the impression from MDN that the only useful thing you can do with that is put it into a canvas. (Which will lead to the normal canvas extraction paths.) I guess you can measure the width and height, which could leak data - except maybe these values are fixed to the initial size you specify when you create the OffscreenCanvas...?

convertToBlog gives you a Blob, which I see does have e.g. arrayBuffer() which will let you see the actual contents. That's definitely a problem. You're right - we don't do any permission checking here so you're always getting a placeholder.

OffscreenCanvas has a GlobalObject associated with it when its created, so it seems like we ought to be able to call IsImageExtractionAllowed?

Comment 8

[Tom Schuster (MoCo)]

OffscreenCanvas has a GlobalObject associated with it when its created, so it seems like we ought to be able to call IsImageExtractionAllowed?

The current code in IsImageExtractionAllowed is unlikely to work without changes in a Worker context. It seems to require a Document and I think not all Workers are associated with a document (but I keep forgetting why, maybe SharedWorker or ServiceWorkers?). We would probably also need to dispatch the call to the main thread.

Comment 9

[Tim Huang[:timhuang]]

We might be able to trigger the canvas prompt from the worker context. If the canvas prompt is requested from a content process, it sends an IPC message to the parent process via the browserChild. So, if we can access the browserChild in the worker context, we can use it to show the prompt.

The workerLoadInfo maintains a browserChild list, so we probably can use an active browserChild to request canvas prompt. However, this might not work for service workers because it could run without any opened tab for its origin. In this case, we can only check the canvas permission.

Comment 10

[László Károlyi]

Hey,

my investigation in Element (the 'official' Matrix client) yielded finding this bug, so I'd add its issue link here, in hopes of this getting a bit more priority:

https://github.com/element-hq/element-web/issues/23936

Comment 11

[Tom Ritter [:tjr]]

Tom, you were just in this code, it seems like it might be an easy fix?

Comment 12

[Tom Schuster (MoCo)]

I don't think this is easy per se. While can use the BrowserChild to prompt for canvas extraction permission, we actually need to use nsIPermissionManager to query for it. That interface does not look thread-safe.

Comment 13

[Fatih Kilic [:fkilic]]

Attachment: Bug 1422862: Check canvas permission in offscreen canvas. r?tjr

Comment 14

[Fatih Kilic [:fkilic]]

Attachment: Bug 1422862: Show canvas permission prompt for offscreen canvases. r?tjr

Comment 15

[Fatih Kilic [:fkilic]]

Attachment: Bug 1422862: Implement IsImageExtractionAllowed_impl. r?tjr

Comment 16

[Kagami Rosylight [:saschanaz] (they/them)]

(Given there are known real world breakages)

Comment 17

[Kagami Rosylight [:saschanaz] (they/them)]

Actually never mind, this is disabled by default.

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The following patches are waiting for review from an inactive reviewer:

ID	Title	Author	Reviewer Status
D222686	Bug 1422862: Check canvas permission in offscreen canvas. r?tjr	fkilic	jgilbert: Disabled
D222687	Bug 1422862: Show canvas permission prompt for offscreen canvases. r?tjr	fkilic	jgilbert: Disabled
D227385	Bug 1422862: Implement IsImageExtractionAllowed_impl. r?tjr	fkilic	jgilbert: Disabled

:fkilic, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit BugBot documentation.

Comment 19

[Fatih Kilic [:fkilic]]

Changed it to gfx-reviewers

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ad6b0a25036d
https://hg.mozilla.org/integration/autoland/rev/4f53b7be32db
Check canvas permission in offscreen canvas. r=tjr,gfx-reviewers,nical
https://github.com/mozilla-firefox/firefox/commit/b87087b00fb4
https://hg.mozilla.org/integration/autoland/rev/01896e14f710
Show canvas permission prompt for offscreen canvases. r=tjr,gfx-reviewers,nical
https://github.com/mozilla-firefox/firefox/commit/26aa4db4c0e1
https://hg.mozilla.org/integration/autoland/rev/62c818afc63a
Implement IsImageExtractionAllowed_impl. r=tjr,gfx-reviewers,nical

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/414cd1ce2e11
https://hg.mozilla.org/integration/autoland/rev/cd5ee27a8527
Revert "Bug 1422862: Implement IsImageExtractionAllowed_impl. r=tjr,gfx-reviewers,nical" for causing bc failures @ browser_etp_permission.js

Comment 22

[amarc]

Backed out for causing bc failures @ browser_etp_permission.js

• Backout link
• Push with failures
• Failure Log

Comment 23

[Fatih Kilic [:fkilic]]

Thank you I'll look into it

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2178fbf85f22
https://hg.mozilla.org/integration/autoland/rev/386dab1ef198
Check canvas permission in offscreen canvas. r=tjr,gfx-reviewers,nical
https://github.com/mozilla-firefox/firefox/commit/f37c6503fc52
https://hg.mozilla.org/integration/autoland/rev/492226fffccc
Show canvas permission prompt for offscreen canvases. r=tjr,gfx-reviewers,nical
https://github.com/mozilla-firefox/firefox/commit/ae12ef53771f
https://hg.mozilla.org/integration/autoland/rev/c4efb37a6427
Implement IsImageExtractionAllowed_impl. r=tjr,gfx-reviewers,nical

Comment 25

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/386dab1ef198
https://hg.mozilla.org/mozilla-central/rev/492226fffccc
https://hg.mozilla.org/mozilla-central/rev/c4efb37a6427

Comment 26

[Fatih Kilic [:fkilic]]

Attachment: Bug 1422862: Check canvas permission in offscreen canvas. r?tjr

This patch uses a Runnable to get "canvas" permission for the given principal in the main thread.
The runnable is only dispatched if we are not in main thread.

Original Revision: https://phabricator.services.mozilla.com/D222686

Comment 27

[Fatih Kilic [:fkilic]]

Attachment: Bug 1422862: Show canvas permission prompt for offscreen canvases. r?tjr

This patch implements IsImageExtractionAllowed for Offscreen Canvas. It gets the windowId from offscreen canvas, queries it, attempts to get window's document, shows canvas permission prompt if successful.

Original Revision: https://phabricator.services.mozilla.com/D222687

Comment 28

[Fatih Kilic [:fkilic]]

Attachment: Bug 1422862: Implement IsImageExtractionAllowed_impl. r?tjr

Original Revision: https://phabricator.services.mozilla.com/D227385

Comment 29

[Tom Ritter [:tjr]]

To be clear, we are requesting uplift to put this into ESR for Tor. It affects he behavior of Resist Fingerprinting, but not for supported FF users. It doesn't need to be uplifted, but given that it's been stable for us and doesn't change the functionality of ESR we thought we'd ask.

Comment 30

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/ef0d5b28123e
https://hg.mozilla.org/releases/mozilla-esr140/rev/f2463405c3f7
https://hg.mozilla.org/releases/mozilla-esr140/rev/4c1517a386c0