Early Hint crashes when preloading stylesheets
Categories
(Core :: Networking: HTTP, defect, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox105 | --- | fixed |
People
(Reporter: mleclair, Assigned: manuel)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-triaged])
Attachments
(2 files)
On windows 11, when connecting to shopify.com with early hints enable, I get the following crash when launching firefox with shopify in a tab which opens right away:
[Parent 20688, Main Thread] WARNING: NS_ENSURE_SUCCESS(status, status) failed with result 0x804B0002 (NS_BINDING_ABORTED): file C:/mozcentral/mozcentral/netwerk/protocol/http/nsCORSListenerProxy.cpp:1343
Assertion failure: false (Disallowing SystemPrincipal load of stylesheets on HTTP(S).), at C:/mozcentral/mozcentral/dom/security/nsContentSecurityManager.cpp:1184
#01: nsContentSecurityManager::CheckAllowLoadInSystemPrivilegedContext (C:\mozcentral\mozcentral\dom\security\nsContentSecurityManager.cpp:1183)
#02: nsContentSecurityManager::doContentSecurityCheck (C:\mozcentral\mozcentral\dom\security\nsContentSecurityManager.cpp:1384)
#03: mozilla::net::nsHttpChannel::AsyncOpen (C:\mozcentral\mozcentral\netwerk\protocol\http\nsHttpChannel.cpp:5752)
#04: mozilla::net::EarlyHintPreloader::OpenChannel (C:\mozcentral\mozcentral\netwerk\protocol\http\EarlyHintPreloader.cpp:253)
#05: mozilla::net::EarlyHintPreloader::MaybeCreateAndInsertPreload (C:\mozcentral\mozcentral\netwerk\protocol\http\EarlyHintPreloader.cpp:214)
#06: mozilla::net::EarlyHintsService::EarlyHint (C:\mozcentral\mozcentral\netwerk\protocol\http\EarlyHintsService.cpp:47)
#07: mozilla::net::DocumentLoadListener::EarlyHint (C:\mozcentral\mozcentral\netwerk\ipc\DocumentLoadListener.cpp:2849)
#08: mozilla::net::nsHttpChannel::EarlyHint (C:\mozcentral\mozcentral\netwerk\protocol\http\nsHttpChannel.cpp:9734)
#09: mozilla::detail::RunnableFunction<`lambda at C:/mozcentral/mozcentral/netwerk/protocol/http/nsHttpTransaction.cpp:1973:19'>::Run (C:\mozcentral\mozcentral\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h:532)
#10: mozilla::RunnableTask::Run (C:\mozcentral\mozcentral\xpcom\threads\TaskController.cpp:539)
#11: mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal (C:\mozcentral\mozcentral\xpcom\threads\TaskController.cpp:851)
#12: mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal (C:\mozcentral\mozcentral\xpcom\threads\TaskController.cpp:683)
#13: mozilla::TaskController::ProcessPendingMTTask (C:\mozcentral\mozcentral\xpcom\threads\TaskController.cpp:461)
#14: mozilla::detail::RunnableFunction<`lambda at C:/mozcentral/mozcentral/xpcom/threads/TaskController.cpp:190:7'>::Run (C:\mozcentral\mozcentral\xpcom\threads\nsThreadUtils.h:532)
#15: nsThread::ProcessNextEvent (C:\mozcentral\mozcentral\xpcom\threads\nsThread.cpp:1209)
#16: NS_ProcessNextEvent (C:\mozcentral\mozcentral\xpcom\threads\nsThreadUtils.cpp:465)
#17: mozilla::ipc::MessagePump::Run (C:\mozcentral\mozcentral\ipc\glue\MessagePump.cpp:107)
#18: MessageLoop::RunHandler (C:\mozcentral\mozcentral\ipc\chromium\src\base\message_loop.cc:374)
#19: MessageLoop::Run (C:\mozcentral\mozcentral\ipc\chromium\src\base\message_loop.cc:356)
#20: nsBaseAppShell::Run (C:\mozcentral\mozcentral\widget\nsBaseAppShell.cpp:152)
#21: nsAppShell::Run (C:\mozcentral\mozcentral\widget\windows\nsAppShell.cpp:613)
#22: nsAppStartup::Run (C:\mozcentral\mozcentral\toolkit\components\startup\nsAppStartup.cpp:296)
#23: XREMain::XRE_mainRun (C:\mozcentral\mozcentral\toolkit\xre\nsAppRunner.cpp:5719)
#24: XREMain::XRE_main (C:\mozcentral\mozcentral\toolkit\xre\nsAppRunner.cpp:5913)
#25: XRE_main (C:\mozcentral\mozcentral\toolkit\xre\nsAppRunner.cpp:5981)
#26: NS_internal_main (C:\mozcentral\mozcentral\browser\app\nsBrowserApp.cpp:406)
#27: wmain (C:\mozcentral\mozcentral\toolkit\xre\nsWindowsWMain.cpp:167)
#28: __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
#29: BaseThreadInitThunk[C:\Windows\System32\KERNEL32.DLL +0x154e0]
This only happens on my local build and I couldn't reproduce on Mac or linux. This is happening on this commit : 696551:04b02c492dab
| Assignee | ||
Comment 1•4 years ago
|
||
I could reproduce on Linux. Steps to reproduce:
- enable early hints in
about:config:network.early-hints.enabled true - start firefox with the shopify url:
./mach run -- https://www.shopify.com/
| Assignee | ||
Comment 2•4 years ago
|
||
We are currently using the triggeringPrincipal. It seems to be the systemPrincipal when the website loads on startup, which is weird. Is there a principal, that is not the systemPrincipal when loading a website with ./mach run -- https://www.shopify.com/?
Maybe we need to switch to the loadingPrincipal, principalToInherit or some other principal (topLevelPrincipal doesn't seem like the one we are looking for). The principal is needed to call NS_NewChannel. I initially tried to pass nsILoadNode to NS_NewChannel, but I think it isn't available during this stage of the code.
| Assignee | ||
Comment 3•4 years ago
|
||
both loadingPrincipal and principalToInherit are nullptr on startup when opening the web page with ./mach run -- https://www.shopify.com/.
| Assignee | ||
Updated•4 years ago
|
| Assignee | ||
Comment 4•4 years ago
|
||
| Assignee | ||
Comment 5•4 years ago
|
||
Even using FindPrincipalToInherit leads to the systemPrincipal (I've tried that in the attached patch)
The triggeringPrincipal is the systemPrincipal for user initiated loads (entering the URL into the URL-bar or starting firefox with that specific url).
Tom, do you know how to refactor the code, that we get the correct (non-system) principal to start the early hint loads?
Comment 6•4 years ago
|
||
Short answer: no. But I might be able to help you fumble around...
I don't know this as a fact, but only as an observation that I've gotten r+ on in the past - document types (which this would be AIUI) don't have a loading principal in the LoadInfo. Instead you get get the channelURI which is a building block of the principals created for the channel (which I think might be done in these functions but I'm really not sure about that.)
The CheckAllowLoadInSystemPrivilegedContext check is to ensure we don't load resources with privileges we shouldn't - it doesn't care about things like OriginAttributes - but a solution that actively ignores OA and just creates a principal using the URI is playing with fire, because if that principal was used for anything else (like constructing a hashkey or whatnot) - it would actively leak content across OA boundaries (e.g. PBM, Containers, ETP, FPI, etc).
I think what you want to do is use nsContentUtils::GetSecurityManager()->GetChannelResultPrincipal() like this to construct a principal from the channel. But I'm really not certain about that - it might seem to work, but be the wrong approach. Someone will need to review it; but I can't do it :)
As an aside, I noticed in the spec that Early Hints can come with CSP headers - do we process/honor those?
| Assignee | ||
Comment 7•4 years ago
|
||
No, we don't look at the CSP header yet. We are tracking that in Bug 1765289.
Updated•4 years ago
|
| Assignee | ||
Comment 8•4 years ago
|
||
Depends on D152861
Updated•4 years ago
|
Updated•4 years ago
|
| Assignee | ||
Comment 9•4 years ago
|
||
I think testing this is hard, because the systemprincipal checks seem to be disabled during browser-tests: https://searchfox.org/mozilla-central/rev/5644fae86d5122519a0e34ee03117c88c6ed9b47/dom/security/nsContentSecurityManager.cpp#1134-1151.
I've work around that by enabling security.disallow_non_local_systemprincipal_in_tests during the page load. The test fails before the commit and passes after the commit, so seems to work (aside from a probably unrelated test failure):
netwerk/test/browser/browser_103_user_load.js
FAIL waiting for vsync to be disabled - timed out after 50 tries. - false == true - JS frame :: chrome://mochikit/content/browser-test.js :: ensureVsyncDisabled :: line 575
Stack trace:
chrome://mochikit/content/browser-test.js:ensureVsyncDisabled:575
FAIL vsync remained enabled at the end of the test. Is there an animation still running? Consider talking to the performance team for tips to solve this. - false == true - JS frame :: chrome://mochikit/content/browser-test.js :: ensureVsyncDisabled :: line 576
Stack trace:
chrome://mochikit/content/browser-test.js:ensureVsyncDisabled:576
| Assignee | ||
Comment 10•4 years ago
|
||
It turns out that all browser tests are currently failing on wayland with this error message. So the test failure is not related to my test case and can be ignored.
| Assignee | ||
Comment 11•4 years ago
•
|
||
That (unrelated) test failure is tracked in Bug 1783818
Comment 12•4 years ago
|
||
Comment 13•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/1085415eea61
https://hg.mozilla.org/mozilla-central/rev/7e3f9c4adac2
Updated•4 years ago
|
I was unable to reproduce this issue on Windows 11 nor on Ubuntu 22.04 on a 2022-07-22 Nightly build. Would you be so kind as to verify the fix on the latest beta/nightly builds?
Thank you.
| Assignee | ||
Comment 15•3 years ago
|
||
Yeah, this only crashed in debug builds (MOZ_CRASH), so neither in Nightly nor Beta.
Download debug builds from Treeherder:
- downloaded current version from treeherder: doesn't crash
- downloaded parent commit of my patch from treeherder: crashes
| Assignee | ||
Updated•3 years ago
|
Description
•