Closed Bug 1345573 Opened 8 years ago Closed 8 years ago

Improve granularity of content process permission manager initialization

Categories

(Core :: Permission Manager, enhancement)

Product:
Core
Component:
Permission Manager
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: nika, Assigned: nika)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

Part 1: Key http, https, and ftp URIs on origin instead of eTLD+1
9.23 KB, patch
Details | Diff | Splinter Review
Part 2: Remove old tests for permissions syncing, and add test for new logic
14.21 KB, patch
Details | Diff | Splinter Review
This is a follow up to bug 1337056. It expands on the original bug by making 2 changes to increase the granularity at which the permission manager is initialized: a) Permission Keys are now keyed on Origin for http:// ftp:// and https:// URIs, meaning that a single load may send down multiple permission keys, (for example, loading http://a.b.example.com:80 would send down http://a.b.example.com:80, http://b.example.com:80 and http://example.com:80). b) file:// URIs now have a separate permission key ("file") which, when the dedicated file URI process is enabled, is only sent down to the file URI process. This shouldn't change the amount of data being sent too much, but does improve security somewhat, by sending down less unnecessary info to the content process.
MozReview-Commit-ID: Gihc4QFf11R
I'm getting this failure with the file:// changes: https://treeherder.mozilla.org/logviewer.html#?job_id=82544344&repo=try&lineNumber=13105 It seems to be being caused by the use of an NPAPI plugin to feed content to an iframe through NPN_NewStream. For some reason that iframe is being loaded with the uri `file:///tmp/testframe-N`. This is a problem with these changes because it means that in this case a file:// URI is having questions about it's permissions asked in a non-file:// content process. bsmedberg, bz suggested that you'd know things about NPAPI plugins. Perhaps you could help me figure out what's going on here better?
Flags: needinfo?(benjamin)
Hey immaculate timing. In fixing bug 1335475 I caused a similar test failure. I'm going to remove that test entirely because we no longer support Java nor are we going to allow plugin loads from file: URIs. The basic problem appears to be that this line still loads a file: URI: https://dxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/test_bug406541.html?q=file%3A406541&redirect_type=single#65 SpecialPowers.wrap(i.contentWindow).location.href = "file://" + file.path; Perhaps it would be worthwhile to more aggressively fail if a file: URI ever attempts to load as a subframe of a non-file page?
Flags: needinfo?(benjamin)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #4) > Hey immaculate timing. In fixing bug 1335475 I caused a similar test > failure. I'm going to remove that test entirely because we no longer support > Java nor are we going to allow plugin loads from file: URIs. > > The basic problem appears to be that this line still loads a file: URI: > https://dxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/ > test_bug406541.html?q=file%3A406541&redirect_type=single#65 > > SpecialPowers.wrap(i.contentWindow).location.href = "file://" + file.path; > > Perhaps it would be worthwhile to more aggressively fail if a file: URI ever > attempts to load as a subframe of a non-file page? Perhaps. Once we get sandboxing up it should fail pretty aggressively (as the file won't be accessible) but it might be nice to get something in tree before then. Thanks for killing the test - makes my life easier ^.^
MozReview-Commit-ID: A2vYqdCpeu6
Attachment #8845625 - Flags: review?(amarchesini)
Attachment #8845066 - Flags: review?(amarchesini)
Attachment #8845067 - Flags: review?(amarchesini)
Comment on attachment 8845066 [details] [diff] [review] Part 1: Only send permissions for file URIs to the file process when avaliable Review of attachment 8845066 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/ContentParent.cpp @@ +1338,5 @@ > EnsurePermissionsByKey(EmptyCString()); > > + // We want to send down the File URI permissions only if either we don't have > + // the seperate file URI process enabled, or we are the file URI process. > + if (!Preferences::GetBool("browser.tabs.remote.separateFileUriProcess") || use a cache var for this pref.
Attachment #8845066 - Flags: review?(amarchesini) → review+
Attachment #8845067 - Flags: review?(amarchesini) → review+
Attachment #8845625 - Flags: review?(amarchesini) → review+
Attachment #8845066 - Attachment is obsolete: true
Part 1 of this is currently blocked on us making sure that flash doesn't use the NewStream NPAPI function to determine if we can remove the second test which is falling down in the failure in comment 3. If we need to support this API I will pull part 1 into a separate patch and land everything except that part.
I moved part 1, as we can't land it until we can remove some NPAPI commands, into another bug, so we can land this right now. MozReview-Commit-ID: Gihc4QFf11R
Attachment #8845067 - Attachment is obsolete: true
Attachment #8845625 - Attachment is obsolete: true
Attachment #8845940 - Attachment is obsolete: true
Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1101f931d2d7 Part 1: Key http, https, and ftp URIs on origin instead of eTLD+1, r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/f5f05a774023 Part 2: Remove old tests for permissions syncing, and add test for new logic, r=baku
Backed out bug 1337056 and bug 1345573 for crashing xpcshell test test_bug930456_child.js on debug: bug 1337056: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba0c83461506c421e5c7473cce9a53d1bfee06d8 https://hg.mozilla.org/integration/mozilla-inbound/rev/ddcfb1e70d7230c491dc0a2cb756320de28eea17 https://hg.mozilla.org/integration/mozilla-inbound/rev/dd3ecc8e83179b40e50e39645b9efa7e53fda7d5 https://hg.mozilla.org/integration/mozilla-inbound/rev/36c22158aa94be8bbe82e970462a36b157f775e1 https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b5387adabe9cabf299ca25ea22a42df00511ca https://hg.mozilla.org/integration/mozilla-inbound/rev/95a8674a048d7ce09de02b9340e075b321bcb68a https://hg.mozilla.org/integration/mozilla-inbound/rev/dc53ad27cd41785974bc83cf2d015a7cf1bee33a https://hg.mozilla.org/integration/mozilla-inbound/rev/a7b2c6dacc63afc23dcaffe377a9e4b1df2d1b30 https://hg.mozilla.org/integration/mozilla-inbound/rev/1b93025f476945108ac6988ad00a61f3c54f331b https://hg.mozilla.org/integration/mozilla-inbound/rev/a5fb7e5eb46b2083a6c424307c961bd255458f83 bug 1345573: https://hg.mozilla.org/integration/mozilla-inbound/rev/8767bffeadca0b8d907eb251149965745ecd8b85 https://hg.mozilla.org/integration/mozilla-inbound/rev/30bb3bc160d3e6022892c717c00d114695546a07 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f5f05a774023c7e9cc8eedbfbc91ba117f49274a&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=83785901&repo=mozilla-inbound 12:53:31 WARNING - PROCESS-CRASH | toolkit/components/search/tests/xpcshell/test_bug930456_child.js | application crashed [@ mozalloc_abort(char const*)] 12:53:31 INFO - Crash dump filename: /var/folders/64/m7jhlc3x3b32lz348yvd80pc00000w/T/xpc-other-OWG_6b/0B714EBA-12B1-4D08-B04E-DE790AF0F45F.dmp 12:53:31 INFO - Operating system: Mac OS X 12:53:31 INFO - 10.10.5 14F27 12:53:31 INFO - CPU: amd64 12:53:31 INFO - family 6 model 69 stepping 1 12:53:31 INFO - 4 CPUs 12:53:31 INFO - GPU: UNKNOWN 12:53:31 INFO - Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS 12:53:31 INFO - Crash address: 0x0 12:53:31 INFO - Process uptime: 6 seconds 12:53:31 INFO - Thread 0 (crashed) 12:53:31 INFO - 0 libmozglue.dylib!mozalloc_abort(char const*) [mozalloc_abort.cpp:f5f05a774023 : 33 + 0x0] 12:53:31 INFO - rax = 0x0000000000000000 rdx = 0x00007fff7c5691f8 12:53:31 INFO - rcx = 0x0000000000000000 rbx = 0x00007fff7c569c50 12:53:31 INFO - rsi = 0x00002b0000002b00 rdi = 0x00002a0000002b03 12:53:31 INFO - rbp = 0x00007fff534e4c20 rsp = 0x00007fff534e4c10 12:53:31 INFO - r8 = 0x00007fff534e4bc0 r9 = 0x00007fff7c4f4300 12:53:31 INFO - r10 = 0x0000000000000000 r11 = 0x0000000000000246 12:53:31 INFO - r12 = 0x0000000000000001 r13 = 0x0000000111872568 12:53:31 INFO - r14 = 0x00007fff534e4c90 r15 = 0x00007fff534e6e8b 12:53:31 INFO - rip = 0x0000000113032439 12:53:31 INFO - Found by: given as instruction pointer in context 12:53:31 INFO - 1 XUL!Abort(char const*) [nsDebugImpl.cpp:f5f05a774023 : 441 + 0x5] 12:53:31 INFO - rbx = 0x00007fff7c569c50 rbp = 0x00007fff534e4c30 12:53:31 INFO - rsp = 0x00007fff534e4c30 r12 = 0x0000000000000001 12:53:31 INFO - r13 = 0x0000000111872568 r14 = 0x00007fff534e4c90 12:53:31 INFO - r15 = 0x00007fff534e6e8b rip = 0x000000010ca3d7a9 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 2 XUL!NS_DebugBreak [nsDebugImpl.cpp:f5f05a774023 : 428 + 0x8] 12:53:31 INFO - rbx = 0x00007fff7c569c50 rbp = 0x00007fff534e50c0 12:53:31 INFO - rsp = 0x00007fff534e4c40 r12 = 0x0000000000000001 12:53:31 INFO - r13 = 0x0000000111872568 r14 = 0x00007fff534e4c90 12:53:31 INFO - r15 = 0x00007fff534e6e8b rip = 0x000000010ca3d590 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 3 XUL!nsPermissionManager::PermissionKey::CreateFromPrincipal(nsIPrincipal*, nsresult&) [nsPermissionManager.cpp:f5f05a774023 : 661 + 0x1e] 12:53:31 INFO - rbx = 0x00007fff534e50d8 rbp = 0x00007fff534e51f0 12:53:31 INFO - rsp = 0x00007fff534e50d0 r12 = 0x00007fff534e5168 12:53:31 INFO - r13 = 0x0000000000000000 r14 = 0x0000000119482920 12:53:31 INFO - r15 = 0x00007fff534e5128 rip = 0x000000010d5ecc76 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 4 XUL!nsPermissionManager::AddInternal(nsIPrincipal*, nsCString const&, unsigned int, long long, unsigned int, long long, long long, nsPermissionManager::NotifyOperationType, nsPermissionManager::DBOperationType, bool) [nsPermissionManager.cpp:f5f05a774023 : 1638 + 0xc] 12:53:31 INFO - rbx = 0x0000000119482920 rbp = 0x00007fff534e5390 12:53:31 INFO - rsp = 0x00007fff534e5200 r12 = 0x0000000113362980 12:53:31 INFO - r13 = 0x0000000000000000 r14 = 0x00007fff534e5300 12:53:31 INFO - r15 = 0x0000000000000000 rip = 0x000000010d5f4aff 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 5 XUL!mozilla::dom::ContentChild::RecvAddPermission(IPC::Permission const&) [ContentChild.cpp:f5f05a774023 : 2234 + 0x25] 12:53:31 INFO - rbx = 0x0000000119482920 rbp = 0x00007fff534e54b0 12:53:31 INFO - rsp = 0x00007fff534e53a0 r12 = 0x00007fff534e55a0 12:53:31 INFO - r13 = 0x00007fff534e5420 r14 = 0x0000000113362980 12:53:31 INFO - r15 = 0x00007fff534e53f8 rip = 0x000000010f1d396c 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 6 XUL!mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) [PContentChild.cpp:f5f05a774023 : 6832 + 0x13] 12:53:31 INFO - rbx = 0x00007fff534e56b0 rbp = 0x00007fff534e5700 12:53:31 INFO - rsp = 0x00007fff534e54c0 r12 = 0x0000000113393bc8 12:53:31 INFO - r13 = 0x00007fff534e55b0 r14 = 0x00000001133ac020 12:53:31 INFO - r15 = 0x0000000113393bd0 rip = 0x000000010d40eba9 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 7 XUL!mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) [MessageChannel.cpp:f5f05a774023 : 1872 + 0x9] 12:53:31 INFO - rbx = 0x0000000000000001 rbp = 0x00007fff534e5760 12:53:31 INFO - rsp = 0x00007fff534e5710 r12 = 0x0000000000000000 12:53:31 INFO - r13 = 0x00000001133ac118 r14 = 0x0000000113393bc8 12:53:31 INFO - r15 = 0x00000000ffffff00 rip = 0x000000010d0893cb 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 8 XUL!mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) [MessageChannel.cpp:f5f05a774023 : 1807 + 0xb] 12:53:31 INFO - rbx = 0x0000000113389330 rbp = 0x00007fff534e5820 12:53:31 INFO - rsp = 0x00007fff534e5770 r12 = 0x0000000113393bc8 12:53:31 INFO - r13 = 0x0000000113306e70 r14 = 0x00000001133ac118 12:53:31 INFO - r15 = 0x00000000ffffffff rip = 0x000000010d087197 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 9 XUL!mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) [MessageChannel.cpp:f5f05a774023 : 1680 + 0xb] 12:53:31 INFO - rbx = 0x0000000113393b00 rbp = 0x00007fff534e5880 12:53:31 INFO - rsp = 0x00007fff534e5830 r12 = 0x0000000113393b70 12:53:31 INFO - r13 = 0x0000000113306e70 r14 = 0x00000001133ac118 12:53:31 INFO - r15 = 0x0000000113393bc8 rip = 0x000000010d0883a2 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 10 XUL!mozilla::ipc::MessageChannel::MessageTask::Run() [MessageChannel.cpp:f5f05a774023 : 1713 + 0x8] 12:53:31 INFO - rbx = 0x0000000113393b70 rbp = 0x00007fff534e58a0 12:53:31 INFO - rsp = 0x00007fff534e5890 r12 = 0x00007fff534e5918 12:53:31 INFO - r13 = 0x0000000113306e70 r14 = 0x0000000113389330 12:53:31 INFO - r15 = 0x00000001133c8040 rip = 0x000000010d088b48 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 11 XUL!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:f5f05a774023 : 1269 + 0x6] 12:53:31 INFO - rbx = 0x00007fff534e5900 rbp = 0x00007fff534e59a0 12:53:31 INFO - rsp = 0x00007fff534e58b0 r12 = 0x00007fff534e5918 12:53:31 INFO - r13 = 0x0000000113306e70 r14 = 0x00000001133c8090 12:53:31 INFO - r15 = 0x00000001133c8040 rip = 0x000000010caf939c 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 12 XUL!NS_ProcessNextEvent(nsIThread*, bool) [nsThreadUtils.cpp:f5f05a774023 : 389 + 0xd] 12:53:31 INFO - rbx = 0x0000000000000000 rbp = 0x00007fff534e59d0 12:53:31 INFO - rsp = 0x00007fff534e59b0 r12 = 0x0000000000000001 12:53:31 INFO - r13 = 0x0000000113306e70 r14 = 0x00007fff534e5bd8 12:53:31 INFO - r15 = 0x0000000000000001 rip = 0x000000010caf798e 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 13 XUL!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [MessagePump.cpp:f5f05a774023 : 96 + 0xb] 12:53:31 INFO - rbx = 0x0000000113306e90 rbp = 0x00007fff534e5a40 12:53:31 INFO - rsp = 0x00007fff534e59e0 r12 = 0x0000000000000001 12:53:31 INFO - r13 = 0x0000000113306e70 r14 = 0x00007fff534e5bd8 12:53:31 INFO - r15 = 0x0000000000000001 rip = 0x000000010d08c833 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 14 XUL!MessageLoop::Run() [message_loop.cc:f5f05a774023 : 231 + 0x5] 12:53:31 INFO - rbx = 0x000000011961a640 rbp = 0x00007fff534e5a70 12:53:31 INFO - rsp = 0x00007fff534e5a50 r12 = 0x0000000000000003 12:53:31 INFO - r13 = 0x00007fff534e5e40 r14 = 0x00000000534e5b01 12:53:31 INFO - r15 = 0x00007fff534e5bd8 rip = 0x000000010d040efc 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 15 XUL!XRE_RunAppShell() [nsEmbedFunctions.cpp:f5f05a774023 : 866 + 0x8] 12:53:31 INFO - rbx = 0x000000011961a640 rbp = 0x00007fff534e5ac0 12:53:31 INFO - rsp = 0x00007fff534e5a80 r12 = 0x0000000000000003 12:53:31 INFO - r13 = 0x00007fff534e5e40 r14 = 0x00000000534e5b01 12:53:31 INFO - r15 = 0x00007fff534e5bd8 rip = 0x0000000110a63683 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 16 XUL!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [MessagePump.cpp:f5f05a774023 : 269 + 0x5] 12:53:31 INFO - rbx = 0x00007fff534e5bd8 rbp = 0x00007fff534e5af0 12:53:31 INFO - rsp = 0x00007fff534e5ad0 r12 = 0x0000000000000003 12:53:31 INFO - r13 = 0x00007fff534e5e40 r14 = 0x0000000113306e70 12:53:31 INFO - r15 = 0x0000000080004005 rip = 0x000000010d08d282 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 17 XUL!MessageLoop::Run() [message_loop.cc:f5f05a774023 : 231 + 0x5] 12:53:31 INFO - rbx = 0x0000000000000001 rbp = 0x00007fff534e5b20 12:53:31 INFO - rsp = 0x00007fff534e5b00 r12 = 0x0000000000000003 12:53:31 INFO - r13 = 0x00007fff534e5e40 r14 = 0x0000000000000012 12:53:31 INFO - r15 = 0x0000000080004005 rip = 0x000000010d040efc 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 18 XUL!XRE_InitChildProcess(int, char**, XREChildData const*) [nsEmbedFunctions.cpp:f5f05a774023 : 695 + 0x5] 12:53:31 INFO - rbx = 0x0000000000000001 rbp = 0x00007fff534e5de0 12:53:31 INFO - rsp = 0x00007fff534e5b30 r12 = 0x0000000000000003 12:53:31 INFO - r13 = 0x00007fff534e5e40 r14 = 0x0000000000000012 12:53:31 INFO - r15 = 0x0000000080004005 rip = 0x0000000110a63297 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 19 plugin-container!main [plugin-container.cpp:f5f05a774023 : 64 + 0x13] 12:53:31 INFO - rbx = 0x0000000113334188 rbp = 0x00007fff534e5e20 12:53:31 INFO - rsp = 0x00007fff534e5df0 r12 = 0x0000000000000000 12:53:31 INFO - r13 = 0x0000000000000000 r14 = 0x00007fff534e5e40 12:53:31 INFO - r15 = 0x0000000000000015 rip = 0x000000010c719ee9 12:53:31 INFO - Found by: call frame info 12:53:31 INFO - 20 plugin-container!start + 0x34 12:53:31 INFO - rbx = 0x0000000000000000 rbp = 0x00007fff534e5e30 12:53:31 INFO - rsp = 0x00007fff534e5e30 r12 = 0x0000000000000000 12:53:31 INFO - r13 = 0x0000000000000000 r14 = 0x0000000000000000 12:53:31 INFO - r15 = 0x0000000000000000 rip = 0x000000010c719e84 12:53:31 INFO - Found by: call frame info
Flags: needinfo?(michael)
Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/035042b8b4aa Part 1: Key http, https, and ftp URIs on origin instead of eTLD+1, r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/63e7bf43da65 Part 2: Remove old tests for permissions syncing, and add test for new logic, r=baku
Flags: needinfo?(michael)
https://hg.mozilla.org/mozilla-central/rev/035042b8b4aa https://hg.mozilla.org/mozilla-central/rev/63e7bf43da65
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: