1173371 - [e10s] Web page is not shown when launch Firefox from network drive on Windows

Status: VERIFIED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Alice0775 White]

This problem is only reproduced with e10s.

Steps To Reproduce:
1. launch Firefox from sheard network folder
   (\\host\folder\firefox.exe)

Actual Results:
Web page and about:home is not shown (blank page).
And process remains forever after exit browser.

It works as expected if Disabled e10s

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Alice, can you reproduce with the sandbox disabled?

Comment 2

[Alice0775 White]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #1)
> Alice, can you reproduce with the sandbox disabled?

How ?

Comment 3

[Bob Owen (:bobowen)]

(In reply to Alice0775 White from comment #2)
> (In reply to Brad Lassey [:blassey] (use needinfo?) from comment #1)
> > Alice, can you reproduce with the sandbox disabled?
> 
> How ?

Hi Alice, there are two options.
The first is to make the content sandbox weaker by setting the following pref:
security.sandbox.content.level=0

(This requires a restart, but that's inevitable for this test :-) )

The second is to turn off the sandbox completely by setting an environment variable:
MOZ_DISABLE_CONTENT_SANDBOX=1

For tracking down the specific problem, it's normally useful if you try both of these.

Comment 4

[Alice0775 White]

*Set security.sandbox.content.level=0 in about:config and restart
 I can still reproduce this problem.

*Set MOZ_DISABLE_CONTENT_SANDBOX=1 in system env
 It works as expected. Web page is shown properly.

Tested with Nightly 41.0a1(2015-06-10)

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Thanks Alice, this is a sandboxing problem then

Comment 7

[Bob Owen (:bobowen)]

Had a first quick look at this and even, if I turn of all of the sandbox settings, this still fails.

So, it appears to be something to do with the way the sandbox process launch differs from the old e10s one.

Comment 8

[Bob Owen (:bobowen)]

Seems this is a problem for Chrome as well:
https://code.google.com/p/chromium/issues/detail?id=103902

Looks like it fails when it tries to load a DLL (xul.dll is first) from the network drive.

Comment 9

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a94d1619add

Comment 10

[Bob Owen (:bobowen)]

Attachment: Part 1: Take commit 0e49d029d5a1a25d971880b9e44d67ac70b31a80 for sandbox code

From Chromium commit comment:
Sandbox: Add support for file system policies that use implied device paths.

A policy rule of the form \HarddiskVolume0\Foo\bar allows sandboxed code
to use \\.\HarddiskVolume0\Foo\bar directly.

Comment 11

[Bob Owen (:bobowen)]

Attachment: Part 2: Change chromium sandbox to allow rules for files on network drives to be added

Hopefully I will be able to get these changes upstreamed.
I'll need to write some tests for the chromium sandbox first though.

Comment 12

[Bob Owen (:bobowen)]

Attachment: Part 3: Add sandbox policy rule to allow read access to the Firefox program directory when it is on a network drive

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8711662 [details] [diff] [review]
Part 1: Take commit 0e49d029d5a1a25d971880b9e44d67ac70b31a80 for sandbox code

rs=me

Comment 14

[Bob Owen (:bobowen)]

Thanks Aaron, I just want to test this on a samba share before I land.
That'll have to be tomorrow.

Comment 15

[Bob Owen (:bobowen)]

(In reply to Bob Owen (:bobowen) from comment #14)
> Thanks Aaron, I just want to test this on a samba share before I land.
> That'll have to be tomorrow.

Running from a samba share also works with these fixes.
I'll not land this late (for me) on a Friday though.

Comment 16

[Bob Owen (:bobowen)]

Just a quick note for the record before landing ...
In Part 3, I noticed that I was only using the directory service once in AddContentSandboxAllowedFiles, so I changed it to use NS_GetSpecialDirectory to make it a bit tidier.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7aea6010b596
https://hg.mozilla.org/integration/mozilla-inbound/rev/afa4f68de47c
https://hg.mozilla.org/integration/mozilla-inbound/rev/673e2072dfae

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7aea6010b596
https://hg.mozilla.org/mozilla-central/rev/afa4f68de47c
https://hg.mozilla.org/mozilla-central/rev/673e2072dfae

Comment 19

[Toshihiro Yamada]

This causes build failure with VS2015.
main problem is

ipc/glue/GeckoChildProcessHost.cpp(640): error C2784: 'void nsAString_internal::InsertLiteral(const nsAString_internal::char_type (&)[N],nsAString_internal::index_type)': could not deduce template argument for 'const nsAString_internal::char_type (&)[N]' from 'const wchar_t [7]'
ipc/glue/GeckoChildProcessHost.cpp(643): error C2784: 'void nsAString_internal::AppendLiteral(const char (&)[N])': could not deduce template argument for 'const char (&)[N]' from 'const wchar_t [3]'
ipc/glue/GeckoChildProcessHost.cpp(643): error C2784: 'void nsAString_internal::AppendLiteral(const nsAString_internal::char_type (&)[N])': could not deduce template argument for 'const nsAString_internal::char_type (&)[N]' from 'const wchar_t [3]'

Looks like similar problem as bug 1242295.

Comment 20

[Jonathan Kew [:jfkthame]]

(In reply to Toshihiro Yamada from comment #19)
> This causes build failure with VS2015.

Filed as bug 1244774.

Comment 21

[Bob Owen (:bobowen)]

Comment on attachment 8711662 [details] [diff] [review]
Part 1: Take commit 0e49d029d5a1a25d971880b9e44d67ac70b31a80 for sandbox code

Note that if these patches are uplifted, we will need to take the VS2015 compilation fix from bug 1244774 as well.

Approval Request Comment
[Feature/regressing bug #]:
This is needed to be able to enable a low integrity sandbox on Aurora.

[User impact if declined]:
We won't be able to get a low integrity content sandbox turned on for Windows on Fx46.

[Describe test coverage new/current, TreeHerder]:
The specific case of running from a network drive has been manually tested, this would be difficult to automate.
Any tests using a sandboxed child process (e10s, GMP or NPAPI 64-bit), will be testing the existing use of the policy setting code.

[Risks and why]:
Medium - this adds new functionality to the sandboxing code, but it is only used if you are launching Firefox from a network drive.
However, it does involve changing the existing file system sandbox policy setting code.

[String/UUID change made/needed]:
None

Comment 22

[Bob Owen (:bobowen)]

Comment on attachment 8711669 [details] [diff] [review]
Part 2: Change chromium sandbox to allow rules for files on network drives to be added

See comment 21.

Comment 23

[Bob Owen (:bobowen)]

Comment on attachment 8711670 [details] [diff] [review]
Part 3: Add sandbox policy rule to allow read access to the Firefox program directory when it is on a network drive

See comment 21.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Can we get more testing or verification for this? 
Bob, can you describe what testing you did for network drives?

Comment 25

[Bob Owen (:bobowen)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #24)
> Can we get more testing or verification for this? 
> Bob, can you describe what testing you did for network drives?

I've tested with Firefox installed to a shared folder on another Windows machine.
Running with \\host\sharedfolder\firefox.exe and \\host\sharedfolder\subfolder\firefox.exe

Again with \\host\sharedfolder mapped to a drive letter.

Same again with the files copied onto a Linux machine and using a samba share.

Any extra testing would be great of course. :-)

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8711662 [details] [diff] [review]
Part 1: Take commit 0e49d029d5a1a25d971880b9e44d67ac70b31a80 for sandbox code

Let's uplift and test this, necessary for e10s to ship. 
It seems likely to uncover some new issues.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei, here is a good area for exploratory testing for your team.

Comment 28

[Bob Owen (:bobowen)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #26)
> Comment on attachment 8711662 [details] [diff] [review]
> Part 1: Take commit 0e49d029d5a1a25d971880b9e44d67ac70b31a80 for sandbox code
> 
> Let's uplift and test this, necessary for e10s to ship. 
> It seems likely to uncover some new issues.

Thanks Liz.
Just to be clear, this is necessary for the content sandbox to ship, not e10s.

The issue with running from a network drive, that this fixes, doesn't appear until we turn the sandbox on.

Some of the sandboxing code changes are in code used by both the GMP and NPAPI (64-bit) process sandbox.
So, the only places we ought to see any issues are there.

It looks like we won't uplift the sandbox for 46 now (see bug 1246505 comment 6).
So, I'd understand if you wanted to change your decision on this and sorry for the churn.

Comment 29

[Liz Henry (:lizzard) (relman/hg->git project)]

Thanks Bob. Yes, considering that, I'd like to keep this in 47.

Comment 30

[Paul Silaghi, QA [:pauly]]

I reproduced the problem on nightly 2016-01-05.
Verified fixed Fx 47.0a1 (2016-02-09) Win 7

Comment 31

[Chris Peterson [:cpeterson]]

marking status-firefox46=wontfix as per comment 29