Closed Bug 1638711 Opened 3 months ago Closed 2 months ago

Run Document content security checks in the parent process

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla79
Fission Milestone M6a
Tracking Status
firefox79 --- fixed

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(9 files, 2 obsolete files)

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

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.

Assignee: nobody → matt.woodrow
Status: NEW → ASSIGNED

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

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

Depends on D75725

Priority: -- → P2
Whiteboard: [domsecurity-active]
Severity: -- → N/A
Fission Milestone: --- → M6a
Attachment #9149734 - Attachment is obsolete: true
Attachment #9149735 - Attachment is obsolete: true
Pushed by mwoodrow@mozilla.com:
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
Pushed by mwoodrow@mozilla.com:
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
Attachment #9152238 - Attachment description: Bug 1638711 - Call DisplayLoadError for NS_ERROR_CONTENT_BLOCKED during EndPageLoad, since we would also have done this for the same error during AsyncOpen. r?nika → 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!
Flags: needinfo?(matt.woodrow)
Pushed by mwoodrow@mozilla.com:
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

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
Flags: needinfo?(matt.woodrow)
Pushed by mwoodrow@mozilla.com:
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
Flags: needinfo?(matt.woodrow)
You need to log in before you can comment on or make changes to this bug.