1584839 - move OpenBSD pledge() promises to files

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P5)

Description

[jcs]

Attachment: ff-pledge-files1.diff

We are heading in a different direction for OpenBSD's sandbox support and want to move the pledge() promises to flat files rather than preference keys.

Landry suggested /usr/local/lib/firefox/browser/defaults/preferences/ for this directory, which I can't resolve via NS_GetSpecialDirectory(NS_GRE_DIR) because that is not available in a content/gpu process due to where the sandboxing is being done. I'd rather just keep this hard-coded at build-time anyway to keep it immutable, but I'm not sure how to add that.

Comment 1

[jcs]

In the long term, these promises will be moved to hard-coded strings inside OpenBSDPledgePromises and files won't be needed.

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 9097218 [details] [diff] [review]
ff-pledge-files1.diff

Review of attachment 9097218 [details] [diff] [review]:
-----------------------------------------------------------------

The prefs changes seem fine. I don't know much about the C++ side but it's all BSD-only code so the bar for r+ isn't very high.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 9097218 [details] [diff] [review]
ff-pledge-files1.diff

Review of attachment 9097218 [details] [diff] [review]:
-----------------------------------------------------------------

Hardcoding the path seems fine but if OpenBSD allows users to install Nightly/Release/ESR alongside each other you might want to have separate paths.

::: dom/ipc/ContentChild.cpp
@@ +4050,2 @@
>  
> +#define PLEDGE_FILE_MAIN     "pledge.main"

You're defining these but not using them.

@@ +4108,3 @@
>  
>  bool StartOpenBSDSandbox(GeckoProcessType type) {
> +  // XXX: make this a build-time hard-coded string, we can't use

What's the follow-up on this XXX?

If the location of the file is guaranteed, I see no issue with hard-coding the path.

Maybe there's a question what to do if the user also has a Nightly install (in as so far that's applicable to OpenBSD), though? Maybe the package name here should be parametrized?

Comment 4

[jcs]

Landry, should we pass a -D define for the path during the port/package build (to expand $LOCALBASE), and then this code can use that define and error out at build time if it's not set?

I think the prospect of anyone building and using Firefox on OpenBSD not using our packages or Landry's beta packages is pretty slim, especially when these pledge/unveil files will not be distributed with Firefox itself and will have to come from our package build.

Comment 5

[Landry Breuil (:gaston)]

I'd like to avoid breaking the 'running a build from mozilla-central' case (even if i'm probably the only user), so far trunk builds provide a .tar.bz2 which is self contained and can run without files outside of it.

hardcoding 'firefox' should be avoided, $MOZ_APP_NAME (or the corresponding #define) should be used, as it expands to firefox or firefox-esr, depending on --with-app-name - to make sure mainline and esr can coexist with different values/params.

So my proposal would be:

• check if $sysconfdir/$MOZ_APP_NAME/pledge.main exists then use it
• if not found, use $instdir/defaults/preferences/pledge.main - there really should be a way to get the "dynamic" value for instdir at runtime, i cant believe it's not available early at startup.. if not, hardcode usr/local for OpenBSD, that $LOCALBASE ship has sailed
• if not found either, abort. Shouldnt happen as i want to upstream default values, allowing for override via files in $sysconfdir (even if i still dont like $sysconfdir as it's a deviation from upstream, that's how we configure things in ports)

this way, params are upstreamed at some point, one can run a build from m-c, and package end users can override values in /etc:.

Comment 6

[jcs]

MOZ_APP_NAME doesn't seem to be available as a constant in any header file, and is only passed as a -D define in certain subdirectories:

toolkit/xre/backend.mk
3:DEFINES += -DNDEBUG=1 -DTRIMMED=1 -DTELEMETRY_PING_FORMAT_VERSION=4 -DPROXY_PRINTING=1 -DOS_POSIX=1 -DOS_BSD=1 -DOS_OPENBSD=1 -DUSE_GLX_TEST '-DMOZ_APP_NAME="firefox"' [...]

This is not defined when in dom/ipc:

/home/jcs/code/firefox/dom/ipc/ContentChild.cpp:4111:47: error: use of undeclared identifier 'MOZ_APP_NAME'
  result.Assign(nsPrintfCString("/etc/%s/%s", MOZ_APP_NAME, file));

Comment 7

[Gian-Carlo Pascutto [:gcp]]

MOZ_APP_NAME doesn't seem to be available as a constant in any header file, and is only passed as a -D define in certain subdirectories:

Change dom/ipc/moz.build to contain

DEFINES['MOZ_APP_NAME'] = CONFIG['MOZ_APP_NAME']

Comment 8

[jcs]

Attachment: ff-pledge-files2.diff

Thanks, this diff defines MOZ_APP_NAME.

Comment 9

[jcs]

Attachment: ff-pledge-files3.diff

New patch against Firefox 70

Comment 10

[Landry Breuil (:gaston)]

Attachment: Bug 1584839 - Move OpenBSD pledge promises to files r?gcp

this way, preferences cant be modified by an extension, and they're
locked down in root-owned files.

Pledge promises files consist of a promise by line, are read first from
/etc/MOZ_APP_NAME/pledge.${processtype} (allowing overriding by a local
root if needed), and if not found
/usr/local/lib/MOZ_APP_NAME/browser/defaults/preferences is used, which
is where the OpenBSD packaging system will install the defaults.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/396a73e240df
Move OpenBSD pledge promises to files r=gcp

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/396a73e240df