1386832 - On Linux, System extensions fail to load in local builds run outside of ./mach

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P1)

Description

[Haik Aftandilian [:haik]]

When starting a developer unpackaged local build on Linux, (e.g., running ./firefox) system extensions will encounter errors when loading resources. For example, the screenshot extension fails when 

As a workaround, set MOZ_DEVELOPER_REPO_DIR to be the path to the repo. As in

  $ MOZ_DEVELOPER_REPO_DIR=/path/to/my/mozilla-central/ ./obj/dist/bin/firefox

This problem occurs because ExtensionProtocolHandler only allows unpacked extensions to load resources outside of their extension directory if the resource resides within the repo dir. The repo dir is obtained from MOZ_DEVELOPER_REPO_DIR, but that isn't always set when Firefox is run. I'm working on a fix for this that doesn't depend on MOZ_DEVELOPER_REPO_DIR.

Comment 1

[Haik Aftandilian [:haik]]

Attachment: Bug 1386832 - Part 1 - Move non-sandbox-specific routines out of SandboxSettings.

Moves IsDevelopmentBuild(), GetRepoDir(), and GetObjectDir() out of
SandboxSettings because they also need to be used by ExtensionProtocolHandler
to do security checks on developer builds as a result of how developer builds
rely on symlinks to the repo dir from system extension directories.

Remove the Linux-implementation of GetRepoDir() and GetObjectDir()
because the Linux content sandbox implementation and the
ExtensionProtocolHandler checks don't need them.

Review commit: https://reviewboard.mozilla.org/r/164994/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164994/

Comment 2

[Haik Aftandilian [:haik]]

Attachment: Bug 1386832 - Part 2 - Make Linux dev build security check exception not depend on MOZ_DEVELOPER_REPO.

For Linux dev builds, change the developer build unpacked security check
exception to not depend on knowing the repo dir because MOZ_DEVELOPER_REPO
isn't reliably set whenever the firefox binary is run. Instead, make sure the
extension root directory is within NS_GRE_DIR. Use both checks on Mac.

Review commit: https://reviewboard.mozilla.org/r/166078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166078/

Comment 3

[Haik Aftandilian [:haik]]

The posted patches 1) fix the problem when firefox is run outside of ./mach without the MOZ_DEVELOPER_REPO_DIR environment variable being set and 2) make ExtensionProtocolHandler not depend on MOZ_DEVELOPER_REPO_DIR ever being set.

When the dev build security check exception was added, I expected we would always require MOZ_DEVELOPER_REPO_DIR to be set on Linux. In the end, our Linux sandboxing implementation didn't require MOZ_DEVELOPER_REPO_DIR due to how it resolves symlinks. On the Mac, the sandboxing implementation does require knowing the full path to the repo, but it is obtained from the Info.plist file, not an environment variable.

Comment 4

[Haik Aftandilian [:haik]]

Comment on attachment 8894968 [details]
Bug 1386832 - Part 1 - Move non-sandbox-specific routines out of SandboxSettings.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164994/diff/1-2/

Comment 5

[Haik Aftandilian [:haik]]

Comment on attachment 8894969 [details]
Bug 1386832 - Part 2 - Make Linux dev build security check exception not depend on MOZ_DEVELOPER_REPO.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166078/diff/1-2/

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8894968 [details]
Bug 1386832 - Part 1 - Move non-sandbox-specific routines out of SandboxSettings.

https://reviewboard.mozilla.org/r/164994/#review171846

Comment 7

[Jim Mathies [:jimm]]

Comment on attachment 8894969 [details]
Bug 1386832 - Part 2 - Make Linux dev build security check exception not depend on MOZ_DEVELOPER_REPO.

https://reviewboard.mozilla.org/r/166078/#review171852

Comment 8

[Haik Aftandilian [:haik]]

Comment on attachment 8894968 [details]
Bug 1386832 - Part 1 - Move non-sandbox-specific routines out of SandboxSettings.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164994/diff/2-3/

Comment 9

[Haik Aftandilian [:haik]]

Comment on attachment 8894969 [details]
Bug 1386832 - Part 2 - Make Linux dev build security check exception not depend on MOZ_DEVELOPER_REPO.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166078/diff/2-3/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2779bfca2a01
Part 1 - Move non-sandbox-specific routines out of SandboxSettings. r=jimm
https://hg.mozilla.org/integration/autoland/rev/86b17cf120ca
Part 2 - Make Linux dev build security check exception not depend on MOZ_DEVELOPER_REPO. r=jimm

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/2779bfca2a01
https://hg.mozilla.org/mozilla-central/rev/86b17cf120ca