Place Canvas MediaStream behind site permission

The Canvas is specified as a potential MediaStream source, to allow live video capture of Canvas data by Javascript. This is a fingerprinting risk, and should be placed behind our Canvas site permission.

https://dvcs.w3.org/hg/audio/raw-file/tip/streams/StreamProcessing.html#canvas-recording

Apparently support for this has not yet been added to Firefox, but they are currently working on landing it. Adding the ff45-esr tag accordingly.

added TorBrowserTeam201604R component::applications/tor browser ff45-esr owner::mcs priority::medium resolution::fixed severity::normal sponsor:: status::closed tbb-6.0a5 tbb-fingerprinting type::defect labels

Trac:
Cc: N/A to gk

There is also a preference to disable this feature: canvas.capturestream.enabled That said, unless we run out of time during our ESR45 work, we should protect access to this feature using the existing canvas prompt that is in Tor Browser.

Trac:
Cc: gk to gk, brade, mcs
Reviewer: N/A to N/A
Severity: N/A to Normal
Sponsor: N/A to None

Trac:
Keywords: N/A deleted, tbb-6.0a5 added

Taking ownership of some ff45-esr tickets.

Trac:
Status: new to accepted
Owner: tbb-team to mcs

Trac:
0001-fixup-Bug-6253-Add-canvas-image-extraction-prompt.patch

proposed fix

Please review the attached patch.

Trac:
Status: accepted to needs_review
Keywords: N/A deleted, TorBrowserTeam201603R added

No easy way to do reviews in March anymore.

Trac:
Keywords: TorBrowserTeam201603R deleted, TorBrowserTeam201604R added

How can one test this?

Trac:
Status: needs_review to needs_information

We have a somewhat ugly test page that we have been using. A copy is available here: https://people.torproject.org/~brade/tests/canvasTest.html

Clicking the "captureStream" button will cause the contents of the "Source canvas" to be mirrored to the "Destination video."

Thanks, looks good to me. I was a bit confused by the pointInPath() and pointIStroke() buttons as the tests behind them always pass even if I don't have Data Extraction Allowed checked. Is that supposed to be this way?

Ah, it tests only the case where data extraction is falsely allowed, nevermind then. Adding Arthur into Cc for a second review.

Trac:
Cc: gk, brade, mcs to gk, brade, mcs, arthuredelstein
Status: needs_information to needs_review

Replying to gk:

Ah, it tests only the case where data extraction is falsely allowed, nevermind then. Adding Arthur into Cc for a second review.

Yes. That test page is a little quirky and not very well documented. Sorry.

Arthur: could you have a look at this one as well?

I am taking that one as well as we want to have it in the alpha (commit 916c78a0a56a38763077634cd492cfdcb2e37b81 in tor-browser-45.0.2esr-6.x-1). I leave it open, so that we get reminded that we want to have a second review before putting that patch into the stable version.

Sorry I didn't manage to review this earlier. I looked over the code carefully and I ran the tests and it all looks good to me. The tests are very nice and we might consider porting them into an automated regression test. I opened a ticket in case one us feels like working on that: #18903 (moved)

Okay, thanks.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

mentioned in issue #18545 (moved)

mentioned in issue #20680 (moved)

mentioned in issue #20680 (moved)

moved to tpo/applications/tor-browser#15640 (closed)