Closed Bug 1173371 Opened 9 years ago Closed 8 years ago

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

Categories

(Core :: Security: Process Sandboxing, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
e10s m9+ ---
firefox41 --- affected
firefox46 --- wontfix
firefox47 --- verified

People

(Reporter: alice0775, Assigned: bobowen)

References

Details

Attachments

(3 files)

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
Alice, can you reproduce with the sandbox disabled?
Flags: needinfo?(alice0775)
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #1)
> Alice, can you reproduce with the sandbox disabled?

How ?
Flags: needinfo?(alice0775) → needinfo?(blassey.bugs)
(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.
Flags: needinfo?(blassey.bugs)
*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)
Thanks Alice, this is a sandboxing problem then
Assignee: nobody → bobowen.code
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.
Component: General → Security: Process Sandboxing
Product: Firefox → Core
Blocks: 1151767
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.
Blocks: 1105816
Assignee: bobowen.code → blassey.bugs
Assignee: blassey.bugs → bobowen.code
Status: NEW → ASSIGNED
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.
Attachment #8711662 - Flags: review?(aklotz)
Hopefully I will be able to get these changes upstreamed.
I'll need to write some tests for the chromium sandbox first though.
Attachment #8711669 - Flags: review?(aklotz)
Comment on attachment 8711662 [details] [diff] [review]
Part 1: Take commit 0e49d029d5a1a25d971880b9e44d67ac70b31a80 for sandbox code

rs=me
Attachment #8711662 - Flags: review?(aklotz) → review+
Attachment #8711669 - Flags: review?(aklotz) → review+
Attachment #8711670 - Flags: review?(aklotz) → review+
Thanks Aaron, I just want to test this on a samba share before I land.
That'll have to be tomorrow.
(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.
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.
https://hg.mozilla.org/mozilla-central/rev/7aea6010b596
https://hg.mozilla.org/mozilla-central/rev/afa4f68de47c
https://hg.mozilla.org/mozilla-central/rev/673e2072dfae
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
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.
Depends on: 1244774
(In reply to Toshihiro Yamada from comment #19)
> This causes build failure with VS2015.

Filed as bug 1244774.
Depends on: 1244952
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
Attachment #8711662 - Flags: approval-mozilla-aurora?
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.
Attachment #8711669 - Flags: approval-mozilla-aurora?
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.
Attachment #8711670 - Flags: approval-mozilla-aurora?
Can we get more testing or verification for this? 
Bob, can you describe what testing you did for network drives?
Flags: needinfo?(bobowen.code)
(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. :-)
Flags: needinfo?(bobowen.code)
Blocks: 1246505
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.
Attachment #8711662 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8711669 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8711670 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Andrei, here is a good area for exploratory testing for your team.
Flags: needinfo?(andrei.vaida)
(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.
Flags: needinfo?(lhenry)
Thanks Bob. Yes, considering that, I'd like to keep this in 47.
Flags: needinfo?(lhenry)
Attachment #8711662 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
Attachment #8711669 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
Attachment #8711670 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
I reproduced the problem on nightly 2016-01-05.
Verified fixed Fx 47.0a1 (2016-02-09) Win 7
Status: RESOLVED → VERIFIED
Flags: needinfo?(andrei.vaida)
marking status-firefox46=wontfix as per comment 29
Depends on: 1321256
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: