Investigate why dom/security/test/csp/test_frame_src.html is not working if we do not enforce contentSecurity Checks in the DocumentChannelChild
Categories
(Core :: DOM: Security, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: ckerschb, Assigned: mattwoodrow)
References
Details
(Whiteboard: [domsecurity-active])
Attachments
(3 files)
Let's investigate why that test is not working when we remove the doContentSecurityCheck call from within https://searchfox.org/mozilla-central/rev/4d2a9d5dc8f0e65807ee66e2b04c64596c643b7a/netwerk/ipc/DocumentChannelChild.cpp#47
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 1•4 years ago
|
||
Hey Matt, I removed the nsContentSecurityManager::doContentSecurityCheck
within DocumentChannelChild::AsyncOpen
and tried to narrow down the problem why test_frame_src.html is failing. So it seems that CSP handling itself in the parent is working correctly and the load gets blocked within nsCSPContext::ShouldLoad which propagates the error back to nsContentSecurityManager.
The problem is that the onerror handler for the testframe does not fire anymore. I don't know why that is the case. I assume that problem causes more csp tests to fail in your TRY run.
If you have any suggestions or ideas where to start debugging and why onerror events on iframes don't fire anymore, please let me know.
Assignee | ||
Comment 2•4 years ago
|
||
So it appears nsFrameLoader::FireErrorEvent is responsible for firing the onerror event to iframe element, but it's only called if the frame loader has an in-process docshell, and the error is returned synchronously from nsDocShell::LoadURI.
Previously, this test was catching the error during AsyncOpen in the content process, so we successfully fired the error event.
Now that the error is detected asynchronously (in the parent), nothing lets the frame loader know :(
I'll look into this a bit, since it seems like it's already pretty broken with OOP iframes.
Assignee | ||
Comment 3•4 years ago
|
||
WIP patches for fixing this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f24bea1cdbca48dfa3ea2df2cce27ad48570b61
With those patches, and the content process security checks removed, this test passed. I haven't checked any others.
Try run doesn't have the security checks moved, just to see what potential fallout there is as a baseline.
Assignee | ||
Comment 4•4 years ago
|
||
Trying to reason about which failures need an error event is super hard with the current code, so I've filed bug 1629201 for trying to refactor the code into a state where that's possible.
That's fairly involved though, and this bug is blocking fission, so I'll upload my patches for making this specific case work in the mean time.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/98c748c67725 Move docshell embedder unblock load to a helper. r=nika https://hg.mozilla.org/integration/autoland/rev/732e95f852ef Add option to MaybeFireEmbedderLoadEvents to also fire the error event to the embedder element. r=nika https://hg.mozilla.org/integration/autoland/rev/c885d9bdcc53 Fire onerror event for CONTENT_BLOCKED in EndPageLoad. r=nika,ckerschb
Comment 9•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/98c748c67725
https://hg.mozilla.org/mozilla-central/rev/732e95f852ef
https://hg.mozilla.org/mozilla-central/rev/c885d9bdcc53
Description
•