1345573 - Improve granularity of content process permission manager initialization

Status: RESOLVED FIXED

(Core :: Permission Manager, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Only send permissions for file URIs to the file process when avaliable

Not asking for review yet because I'm doing a try run first: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bb23c128749b46e3c74d4bf520d6ef87dc62472

MozReview-Commit-ID: 9jZMw9ir0mr

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Key http, https, and ftp URIs on origin instead of eTLD+1

MozReview-Commit-ID: Gihc4QFf11R

Comment 3

[Nika Layzell [:nika] (ni? for response)]

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?

Comment 4

[Benjamin Smedberg]

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?

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(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 ^.^

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Remove old tests for permissions syncing, and add test for new logic

MozReview-Commit-ID: A2vYqdCpeu6

Comment 7

[Andrea Marchesini [:baku]]

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.

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Only send permissions for file URIs to the file process when avaliable

MozReview-Commit-ID: 9jZMw9ir0mr

Comment 9

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 10

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Key http, https, and ftp URIs on origin instead of eTLD+1

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

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Remove old tests for permissions syncing, and add test for new logic

MozReview-Commit-ID: A2vYqdCpeu6

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/035042b8b4aa
https://hg.mozilla.org/mozilla-central/rev/63e7bf43da65