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

VERIFIED FIXED in Firefox 47

Status

()

Core
Security: Process Sandboxing
VERIFIED FIXED
3 years ago
a year ago

People

(Reporter: Alice0775 White, Assigned: bobowen)

Tracking

Trunk
mozilla47
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm9+, firefox41 affected, firefox46 wontfix, firefox47 verified)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
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)
(Reporter)

Comment 2

3 years ago
(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)
(Assignee)

Comment 3

3 years ago
(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)
(Reporter)

Comment 4

3 years ago
*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
tracking-e10s: ? → -
Comment hidden (spam)
(Assignee)

Comment 7

3 years ago
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
(Assignee)

Updated

2 years ago
tracking-e10s: - → ?
(Assignee)

Updated

2 years ago
Blocks: 1151767
tracking-e10s: ? → m8+
(Assignee)

Comment 8

2 years ago
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
tracking-e10s: m8+ → m9+
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 9

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a94d1619add
(Assignee)

Comment 10

2 years ago
Created attachment 8711662 [details] [diff] [review]
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.
Attachment #8711662 - Flags: review?(aklotz)
(Assignee)

Comment 11

2 years ago
Created attachment 8711669 [details] [diff] [review]
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.
Attachment #8711669 - Flags: review?(aklotz)
(Assignee)

Comment 12

2 years ago
Created 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
Attachment #8711670 - 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+

Updated

2 years ago
Attachment #8711669 - Flags: review?(aklotz) → review+

Updated

2 years ago
Attachment #8711670 - Flags: review?(aklotz) → review+
(Assignee)

Comment 14

2 years ago
Thanks Aaron, I just want to test this on a samba share before I land.
That'll have to be tomorrow.
(Assignee)

Comment 15

2 years ago
(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.
(Assignee)

Comment 16

2 years ago
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

2 years ago
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

2 years ago
bugherder
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
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Comment 19

2 years ago
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
(Assignee)

Comment 21

2 years ago
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?
(Assignee)

Comment 22

2 years ago
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?
(Assignee)

Comment 23

2 years ago
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)
(Assignee)

Comment 25

2 years ago
(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)
(Assignee)

Updated

2 years ago
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)
(Assignee)

Comment 28

2 years ago
(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
status-firefox47: fixed → verified
Flags: needinfo?(andrei.vaida)
marking status-firefox46=wontfix as per comment 29
status-firefox46: --- → wontfix
(Assignee)

Updated

a year ago
Depends on: 1321256
You need to log in before you can comment on or make changes to this bug.