Make OffscreenCanvas WebGL(2) context consider taintedness of image source

Make OffscreenCanvas WebGL(2) context consider taintedness of image source

BUG=696222
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2806803003
Cr-Commit-Position: refs/heads/master@{#464467}
Committed: https://chromium.googlesource.com/chromium/src/+/a618ae85f99de9dfcd3596d40e247b92b52eb745

[xlai (Olivia), 2017-04-07 21:46:11]

Description was changed from

==========
Make OffscreenCanvas WebGL(2) context consider taintedness of image source

BUG=696222
==========

to

==========
Make OffscreenCanvas WebGL(2) context consider taintedness of image source

BUG=696222
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[xlai (Olivia), 2017-04-10 19:03:14]

xlai@chromium.org changed reviewers:
+ junov@chromium.org, kbr@chromium.org

[xlai (Olivia), 2017-04-10 19:03:14]

junov@: pls take a look at the security layout test and some general change in
CanvasRenderingContext.

kbr@: pls take a look at the webgl change.

Thanks!

[Justin Novosad, 2017-04-12 14:53:41]

lgtm with comments.

Should we upstream the test to webgl conformance tests?

https://codereview.chromium.org/2806803003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp
(right):

https://codereview.chromium.org/2806803003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp:235:
image_source->WouldTaintOrigin(destination_security_origin);
Just checking: I assume OCRC2D was already calling this correctly and with a
non-null destination_security_origin, and we already have tests for that?

[xlai (Olivia), 2017-04-12 19:37:29]

On 2017/04/12 14:53:41, Justin Novosad wrote:
> Should we upstream the test to webgl conformance tests?
> 

I'm not quite sure whether it's fine to upload a cross-origin security test
instead of a functionality test to webgl conformance test suite. kbr@: WDYT?

https://codereview.chromium.org/2806803003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp
(right):

https://codereview.chromium.org/2806803003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp:235:
image_source->WouldTaintOrigin(destination_security_origin);
On 2017/04/12 14:53:40, Justin Novosad wrote:
> Just checking: I assume OCRC2D was already calling this correctly and with a
> non-null destination_security_origin, and we already have tests for that?

Yes, OCRC2D is already doing this correct. It calls WouldTaintOrigin in two
functions and each has its own security test:
drawImage:
LayoutTests/http/tests/security/offscreencanvas-read-blocked-no-crossorigin.html
createPattern:
LayoutTests/http/tests/security/cross-origin-OffscreenCanvas2D-createPattern.html

[Ken Russell (switch to Gerrit), 2017-04-12 23:31:14]

On 2017/04/12 19:37:29, xlai (Olivia) wrote:
> On 2017/04/12 14:53:41, Justin Novosad wrote:
> > Should we upstream the test to webgl conformance tests?
> > 
> 
> I'm not quite sure whether it's fine to upload a cross-origin security test
> instead of a functionality test to webgl conformance test suite. kbr@: WDYT?

Yes, please do add tests to the WebGL conformance suite. There are already some
tests for properly rejecting cross-origin HTMLImageElements, and tainted
HTMLCanvasElements. Please see these tests:

sdk/tests/conformance/textures/misc/origin-clean-conformance.html
sdk/tests/conformance/more/functions/readPixelsBadArgs.html
sdk/tests/conformance/more/functions/texImage2DHTML.html
sdk/tests/conformance/more/functions/texSubImage2DHTML.html

If you have any questions about the structure of the tests please tell me. One
possibly confusing note is that "SOP" in some of these tests stands for
"Standard Operating Procedure".

LGTM

[xlai (Olivia), 2017-04-13 14:17:04]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 14:17:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xlai (Olivia), 2017-04-13 16:00:48]

The CQ bit was unchecked by xlai@chromium.org

[xlai (Olivia), 2017-04-13 16:00:52]

The CQ bit was checked by xlai@chromium.org

[commit-bot: I haz the power, 2017-04-13 16:01:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 18:05:00]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492099252772270,
"parent_rev": "311458dc726c28f8d091aa496ca6ce70778269ef", "commit_rev":
"a618ae85f99de9dfcd3596d40e247b92b52eb745"}

[commit-bot: I haz the power, 2017-04-13 18:05:51]

Description was changed from

==========
Make OffscreenCanvas WebGL(2) context consider taintedness of image source

BUG=696222
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Make OffscreenCanvas WebGL(2) context consider taintedness of image source

BUG=696222
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2806803003
Cr-Commit-Position: refs/heads/master@{#464467}
Committed:
https://chromium.googlesource.com/chromium/src/+/a618ae85f99de9dfcd3596d40e24...
==========

[commit-bot: I haz the power, 2017-04-13 18:05:53]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/a618ae85f99de9dfcd3596d40e24...

[xlai (Olivia), 2017-04-19 20:35:30]

On 2017/04/12 23:31:14, Ken Russell wrote:
> On 2017/04/12 19:37:29, xlai (Olivia) wrote:
> > On 2017/04/12 14:53:41, Justin Novosad wrote:
> > > Should we upstream the test to webgl conformance tests?
> > > 
> > 
> > I'm not quite sure whether it's fine to upload a cross-origin security test
> > instead of a functionality test to webgl conformance test suite. kbr@: WDYT?
> 
> Yes, please do add tests to the WebGL conformance suite. There are already
some
> tests for properly rejecting cross-origin HTMLImageElements, and tainted
> HTMLCanvasElements. Please see these tests:
> 
> sdk/tests/conformance/textures/misc/origin-clean-conformance.html
> sdk/tests/conformance/more/functions/readPixelsBadArgs.html
> sdk/tests/conformance/more/functions/texImage2DHTML.html
> sdk/tests/conformance/more/functions/texSubImage2DHTML.html
> 
> If you have any questions about the structure of the tests please tell me. One
> possibly confusing note is that "SOP" in some of these tests stands for
> "Standard Operating Procedure".
> 
> LGTM

As a follow-up of this CL, I've created a GitHub pull request 
https://github.com/KhronosGroup/WebGL/pull/2376
to add security tests.