1638711 - Run Document content security checks in the parent process

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P2)

Description

[Matt Woodrow (:mattwoodrow)]

With fission, the requesting Document for a subdocument load might be in a different process, so the LoadInfo we compute in the subdoc's content process may be incomplete.

Using this LoadInfo for security checks may result in incorrect checking being done.

We also have checks done on redirect (AsyncOnChannelRedirect) in the content process, which doesn't run if the load results in a process switch. Having these checks run may hide bugs where the parent process equivalent isn't successfully blocking, and we don't have sufficient tests for process switching cases.

I think we should move to only doing security checks for document loads in the parent process, so that we use the same code for all our tests, and can have more confidence in its correctness.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Do document security checks in parent process. r?ckerschb

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Update WindowGlobalParent's copy of ClientInfo when we mutate it on the window. r?nika

Depends on D75720

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Allow test_frameNavigation to use the error event to detect when a load was blocked. r?ckerschb

The current state is that we fire error events for content blocking if the error happens synchronously and src was set when the iframe was in-document, or if the error happens asynchronously (from the parent process).
This test is currently setting src before appending the iframe to the document, and thus was expecting no error event to be fired. We have other content security tests that do rely on the error event being fired.

Since we're doing security checks in the parent, the error event now fires, and this changes the test to report success in that case.

Depends on D75721

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Fix test_CSP to detect CSP events from the parent process. r?ckerschb

Depends on D75722

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Convert test_contentpolicytest to a browser test. r?ckerschb!,kmag!

Content security checks for Documents run in the parent now, so we need a browser test to add the new policy in the parent process too.

Depends on D75723

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Convert (sub)document pieces of test_bug375314 to a browser test. r?ckerschb!,kmag!

Depends on D75724

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Mark WPT as passing. r?ckerschb

Depends on D75725

Comment 8

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Convert tests that try to install a content policy for Document loads to use SpecialPowers.loadChromeScript to do so in the parent process. r?kmag!,ckerschb!

Depends on D75723

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/164b842bea99
Do document security checks in parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/bebe76fca022
Update WindowGlobalParent's copy of ClientInfo when we mutate it on the window. r=nika
https://hg.mozilla.org/integration/autoland/rev/97d1d2390586
Allow test_frameNavigation to use the error event to detect when a load was blocked. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/254871b49de9
Fix test_CSP to detect CSP events from the parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/75d97b8df5ba
Convert tests that try to install a content policy for Document loads to use SpecialPowers.loadChromeScript to do so in the parent process. r=kmag,ckerschb
https://hg.mozilla.org/integration/autoland/rev/22a7b8f16c44
Mark WPT as passing. r=ckerschb

Comment 10

[Dorel Luca [:dluca]]

Backed out 6 changesets (bug 1638711) for Geckoview failures in NavigationDelegateTest.loadUriReferrerSessionFileUrl.CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=303899462&repo=autoland&lineNumber=6999

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=22a7b8f16c4478819bbb0b6648058cbaac9b3498

Backout:
https://hg.mozilla.org/integration/autoland/rev/1caff03435d24141061d029f0b0acd8f93b5c759

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Call DisplayLoadError for NS_ERROR_DOM_BAD_URI during EndPageLoad, since we would also have done this for the same error during AsyncOpen. r?nika!,ckerschb!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47da03dd8365
Do document security checks in parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/b34546384903
Update WindowGlobalParent's copy of ClientInfo when we mutate it on the window. r=nika
https://hg.mozilla.org/integration/autoland/rev/f776c30279f3
Allow test_frameNavigation to use the error event to detect when a load was blocked. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/9eda9f6c5877
Fix test_CSP to detect CSP events from the parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/c2a1461890c3
Convert tests that try to install a content policy for Document loads to use SpecialPowers.loadChromeScript to do so in the parent process. r=kmag,ckerschb
https://hg.mozilla.org/integration/autoland/rev/3d06b48e2e3a
Mark WPT as passing. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/9643bf18ac31
Call DisplayLoadError for NS_ERROR_CONTENT_BLOCKED during EndPageLoad, since we would also have done this for the same error during AsyncOpen. r=nika

Comment 13

[Razvan Maries]

Backed out for perma failures on test_bug375314-2.html.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=CPZRk1ImSnK1xvfQpahXog-0&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=android%2C7.0%2Cx86-64%2Cwebrender%2Cdebug%2Cmochitests%2Ctest-android-em-7.0-x86_64-qr%2Fdebug-geckoview-mochitest-plain-e10s-3%2Cm%283%29&revision=9643bf18ac311a7e175f7ed1e9cb1717c3bac6d2

Failure logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304044116&repo=autoland&lineNumber=2100
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304042793&repo=autoland&lineNumber=4043
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304047680&repo=autoland&lineNumber=6912
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304050275&repo=autoland&lineNumber=3736

Backout: https://hg.mozilla.org/integration/autoland/rev/1eef124eff9bc4ead0c08a9ae7504da6b9f38558

Comment 14

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Report correct error code in DocumentLoadListener if AsyncOpen fails. r?jya

Depends on D77171

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1638711 - Mark WPTs that incorrectly expect synchronous security checks as failing. r?asuth

Depends on D77954

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e5a049b57ed
Do document security checks in parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/6e702b519ef0
Update WindowGlobalParent's copy of ClientInfo when we mutate it on the window. r=nika
https://hg.mozilla.org/integration/autoland/rev/c1d12e04bc27
Allow test_frameNavigation to use the error event to detect when a load was blocked. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/77f6f2396343
Fix test_CSP to detect CSP events from the parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/d6d396498f5e
Convert tests that try to install a content policy for Document loads to use SpecialPowers.loadChromeScript to do so in the parent process. r=kmag,ckerschb
https://hg.mozilla.org/integration/autoland/rev/5b312893e8e2
Mark WPT as passing. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/417df721766f
Call DisplayLoadError for NS_ERROR_DOM_BAD_URI during EndPageLoad, since we would also have done this for the same error during AsyncOpen. r=nika,ckerschb
https://hg.mozilla.org/integration/autoland/rev/3529cf4e69b6
Report correct error code in DocumentLoadListener if AsyncOpen fails. r=jya,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/b137e5dfcc57
Mark WPTs that incorrectly expect synchronous security checks as failing. r=asuth

Comment 17

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 9 changesets (bug 1638711) for test_block_toplevel_data_navigation.html failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=1bba10286a53216cda51188f1baba314085baea2&searchStr=mochitests&tochange=b8097850787bd00814b7a4858d8553979e67d3c4&selectedTaskRun=eBBALpWqTamJdb-tQP5T_g-1

Backout link: https://hg.mozilla.org/integration/autoland/rev/b8097850787bd00814b7a4858d8553979e67d3c4

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305433961&repo=autoland&lineNumber=4755

[task 2020-06-08T09:23:38.958Z] 09:23:38     INFO -  1525 INFO TEST-START | dom/security/test/general/test_block_toplevel_data_navigation.html
[task 2020-06-08T09:23:38.959Z] 09:23:38     INFO -  Buffered messages logged at 09:23:29
[task 2020-06-08T09:23:38.959Z] 09:23:38     INFO -  1526 INFO TEST-PASS | dom/security/test/general/test_block_toplevel_data_navigation.html | A valid string reason is expected
[task 2020-06-08T09:23:38.959Z] 09:23:38     INFO -  1527 INFO TEST-PASS | dom/security/test/general/test_block_toplevel_data_navigation.html | Reason cannot be empty
[task 2020-06-08T09:23:38.959Z] 09:23:38     INFO -  1528 INFO TEST-FAIL | dom/security/test/general/test_block_toplevel_data_navigation.html | The author of the test has indicated that flaky timeouts are expected.  Reason: have to test that top level data: URI navgiation is blocked
[task 2020-06-08T09:23:38.960Z] 09:23:38     INFO -  Buffered messages logged at 09:23:30
[task 2020-06-08T09:23:38.960Z] 09:23:38     INFO -  1529 INFO TEST-PASS | dom/security/test/general/test_block_toplevel_data_navigation.html | toplevel data: URI navigation through click() should be blocked
[task 2020-06-08T09:23:38.960Z] 09:23:38     INFO -  Buffered messages logged at 09:23:31
[task 2020-06-08T09:23:38.960Z] 09:23:38     INFO -  1530 INFO TEST-PASS | dom/security/test/general/test_block_toplevel_data_navigation.html | data: URI navigation using _blank from data: URI should be blocked
[task 2020-06-08T09:23:38.960Z] 09:23:38     INFO -  1531 INFO TEST-FAIL | dom/security/test/general/test_block_toplevel_data_navigation.html | The author of the test has indicated that flaky timeouts are expected.  Reason: have to test that top level data: URI navgiation is blocked
[task 2020-06-08T09:23:38.960Z] 09:23:38     INFO -  Buffered messages logged at 09:23:32
[task 2020-06-08T09:23:38.961Z] 09:23:38     INFO -  1532 INFO TEST-PASS | dom/security/test/general/test_block_toplevel_data_navigation.html | data: URI navigation through win.loc.href should be blocked
[task 2020-06-08T09:23:38.961Z] 09:23:38     INFO -  1533 INFO TEST-FAIL | dom/security/test/general/test_block_toplevel_data_navigation.html | The author of the test has indicated that flaky timeouts are expected.  Reason: have to test that top level data: URI navgiation is blocked
[task 2020-06-08T09:23:38.961Z] 09:23:38     INFO -  Buffered messages finished
[task 2020-06-08T09:23:38.961Z] 09:23:38  WARNING -  1534 INFO TEST-UNEXPECTED-FAIL | dom/security/test/general/test_block_toplevel_data_navigation.html | navigating to a data: URI using window.open() should be blocked - got "Error!", expected ""
[task 2020-06-08T09:23:38.961Z] 09:23:38     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:383:14
[task 2020-06-08T09:23:38.961Z] 09:23:38     INFO -      test4/<@dom/security/test/general/test_block_toplevel_data_navigation.html:87:7
[task 2020-06-08T09:23:38.962Z] 09:23:38     INFO -  1535 INFO TEST-FAIL | dom/security/test/general/test_block_toplevel_data_navigation.html | The author of the test has indicated that flaky timeouts are expected.  Reason: have to test that top level data: URI navgiation is blocked
[task 2020-06-08T09:23:38.962Z] 09:23:38  WARNING -  1536 INFO TEST-UNEXPECTED-FAIL | dom/security/test/general/test_block_toplevel_data_navigation.html | navigating to URI which redirects to a data: URI using window.open() should be blocked - got "Error!", expected ""
[task 2020-06-08T09:23:38.962Z] 09:23:38     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:383:14
[task 2020-06-08T09:23:38.962Z] 09:23:38     INFO -      test5/<@dom/security/test/general/test_block_toplevel_data_navigation.html:100:7
[task 2020-06-08T09:23:38.962Z] 09:23:38     INFO -  1537 INFO TEST-OK | dom/security/test/general/test_block_toplevel_data_navigation.html | took 5690ms

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1dcb2873841a
Do document security checks in parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/f1a23a62e826
Update WindowGlobalParent's copy of ClientInfo when we mutate it on the window. r=nika
https://hg.mozilla.org/integration/autoland/rev/ca1e2087c0af
Allow test_frameNavigation to use the error event to detect when a load was blocked. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/034f641d64bc
Fix test_CSP to detect CSP events from the parent process. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/903baf6ab111
Convert tests that try to install a content policy for Document loads to use SpecialPowers.loadChromeScript to do so in the parent process. r=kmag,ckerschb
https://hg.mozilla.org/integration/autoland/rev/624f2306ac72
Mark WPT as passing. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/b4cc6d55dc09
Call DisplayLoadError for NS_ERROR_DOM_BAD_URI during EndPageLoad, since we would also have done this for the same error during AsyncOpen. r=nika,ckerschb
https://hg.mozilla.org/integration/autoland/rev/c448f9dcdedc
Report correct error code in DocumentLoadListener if AsyncOpen fails. r=jya,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/cb212bb7b6de
Mark WPTs that incorrectly expect synchronous security checks as failing. r=asuth

Comment 19

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1dcb2873841a
https://hg.mozilla.org/mozilla-central/rev/f1a23a62e826
https://hg.mozilla.org/mozilla-central/rev/ca1e2087c0af
https://hg.mozilla.org/mozilla-central/rev/034f641d64bc
https://hg.mozilla.org/mozilla-central/rev/903baf6ab111
https://hg.mozilla.org/mozilla-central/rev/624f2306ac72
https://hg.mozilla.org/mozilla-central/rev/b4cc6d55dc09
https://hg.mozilla.org/mozilla-central/rev/c448f9dcdedc
https://hg.mozilla.org/mozilla-central/rev/cb212bb7b6de