Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable mixed-sandbox mode by default #15894

Merged
merged 6 commits into from Jan 22, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -204,12 +204,6 @@ void AtomMainDelegate::PreSandboxStartup() {
// linux (namespace sandbox is available on most distros).
command_line->AppendSwitch(service_manager::switches::kDisableSetuidSandbox);

if (!command_line->HasSwitch(switches::kEnableMixedSandbox) &&
!command_line->HasSwitch(switches::kEnableSandbox)) {
// Disable renderer sandbox for most of node's functions.
command_line->AppendSwitch(service_manager::switches::kNoSandbox);
}

// Allow file:// URIs to read other file:// URIs by default.
command_line->AppendSwitch(::switches::kAllowFileAccessFromFiles);

@@ -1250,19 +1250,6 @@ void App::EnableSandbox(mate::Arguments* args) {
command_line->AppendSwitch(switches::kEnableSandbox);
}

void App::EnableMixedSandbox(mate::Arguments* args) {
if (Browser::Get()->is_ready()) {
args->ThrowError(
"app.enableMixedSandbox() can only be called "
"before app is ready");
return;
}

auto* command_line = base::CommandLine::ForCurrentProcess();
RemoveNoSandboxSwitch(command_line);
command_line->AppendSwitch(switches::kEnableMixedSandbox);
}

#if defined(OS_MACOSX)
bool App::MoveToApplicationsFolder(mate::Arguments* args) {
return ui::cocoa::AtomBundleMover::Move(args);
@@ -1370,8 +1357,7 @@ void App::BuildPrototype(v8::Isolate* isolate,
.SetMethod("startAccessingSecurityScopedResource",
&App::StartAccessingSecurityScopedResource)
#endif
.SetMethod("enableSandbox", &App::EnableSandbox)
.SetMethod("enableMixedSandbox", &App::EnableMixedSandbox);
.SetMethod("enableSandbox", &App::EnableSandbox);
}

} // namespace api
@@ -206,7 +206,6 @@ class App : public AtomBrowserClient::Delegate,
v8::Local<v8::Promise> GetGPUInfo(v8::Isolate* isolate,
const std::string& info_type);
void EnableSandbox(mate::Arguments* args);
void EnableMixedSandbox(mate::Arguments* args);

#if defined(OS_MACOSX)
bool MoveToApplicationsFolder(mate::Arguments* args);
@@ -71,6 +71,7 @@
#include "services/device/public/cpp/geolocation/location_provider.h"
#include "services/network/public/cpp/resource_request_body.h"
#include "services/proxy_resolver/public/mojom/proxy_resolver.mojom.h"
#include "services/service_manager/sandbox/switches.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "v8/include/v8.h"
@@ -497,6 +498,16 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(

content::WebContents* web_contents = GetWebContentsFromProcessID(process_id);
if (web_contents) {
// devtools processes must be launched unsandboxed in order for the remote
// API to work in devtools extensions. This is due to the fact that the
// remote API assumes that it will only be used from the main frame, but
// devtools extensions are loaded from an iframe.
// It would be possible to sandbox devtools extensions processes by default
// if we made the remote API work with multiple frames.
if (web_contents->GetVisibleURL().SchemeIs("chrome-devtools")) {
command_line->AppendSwitch(service_manager::switches::kNoSandbox);
command_line->AppendSwitch(::switches::kNoZygote);
}
auto* web_preferences = WebContentsPreferences::From(web_contents);
if (web_preferences)
web_preferences->AppendCommandLineSwitches(command_line);
@@ -161,9 +161,6 @@ namespace switches {
// Enable chromium sandbox.
const char kEnableSandbox[] = "enable-sandbox";

// Enable sandbox in only remote content windows.
const char kEnableMixedSandbox[] = "enable-mixed-sandbox";

// Enable plugins.
const char kEnablePlugins[] = "enable-plugins";

@@ -83,7 +83,6 @@ extern const char kOffscreen[];
namespace switches {

extern const char kEnableSandbox[];
extern const char kEnableMixedSandbox[];
extern const char kEnablePlugins[];
extern const char kPpapiFlashPath[];
extern const char kPpapiFlashVersion[];
@@ -1225,12 +1225,6 @@ Enables full sandbox mode on the app.

This method can only be called before app is ready.

### `app.enableMixedSandbox()` _Experimental_ _macOS_ _Windows_

Enables mixed sandbox mode on the app.

This method can only be called before app is ready.

### `app.isInApplicationsFolder()` _macOS_

Returns `Boolean` - Whether the application is currently running from the
@@ -60,10 +60,6 @@ It is important to note that this option alone won't enable the OS-enforced sand
`--enable-sandbox` command-line argument must be passed to electron, which will
force `sandbox: true` for all `BrowserWindow` instances.

To enable OS-enforced sandbox on `BrowserWindow` or `webview` process with `sandbox:true` without causing
entire app to be in sandbox, `--enable-mixed-sandbox` command-line argument must be passed to electron.
This option is currently only supported on macOS and Windows.

```js
let win
app.on('ready', () => {
@@ -30,6 +30,9 @@ Object.assign(app, {
getSwitchValue: (...args) => commandLine.getSwitchValue(...args.map(String)),
appendSwitch: (...args) => commandLine.appendSwitch(...args.map(String)),
appendArgument: (...args) => commandLine.appendArgument(...args.map(String))
},
enableMixedSandbox () {
deprecate.log(`'enableMixedSandbox' is deprecated. Mixed-sandbox mode is now enabled by default. You can safely remove the call to enableMixedSandbox().`)
}
})

@@ -1111,56 +1111,6 @@ describe('app module', () => {
})
})
})

describe('when app.enableMixedSandbox() is called', () => {
it('adds --enable-sandbox to renderer processes created with sandbox: true', done => {
const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--app-enable-mixed-sandbox'])

server.once('error', error => { done(error) })

server.on('connection', client => {
client.once('data', data => {
const argv = JSON.parse(data)
expect(argv.sandbox).to.include('--enable-sandbox')
expect(argv.sandbox).to.not.include('--no-sandbox')

expect(argv.noSandbox).to.not.include('--enable-sandbox')
expect(argv.noSandbox).to.include('--no-sandbox')

expect(argv.noSandboxDevtools).to.be.true()
expect(argv.sandboxDevtools).to.be.true()

done()
})
})
})
})

describe('when the app is launched with --enable-mixed-sandbox', () => {
it('adds --enable-sandbox to renderer processes created with sandbox: true', done => {
const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'])

server.once('error', error => { done(error) })

server.on('connection', client => {
client.once('data', data => {
const argv = JSON.parse(data)
expect(argv.sandbox).to.include('--enable-sandbox')
expect(argv.sandbox).to.not.include('--no-sandbox')

expect(argv.noSandbox).to.not.include('--enable-sandbox')
expect(argv.noSandbox).to.include('--no-sandbox')

expect(argv.noSandboxDevtools).to.be.true()
expect(argv.sandboxDevtools).to.be.true()

done()
})
})
})
})
})

describe('disableDomainBlockingFor3DAPIs() API', () => {
@@ -1873,7 +1873,6 @@ describe('BrowserWindow module', () => {

it('validates process APIs access in sandboxed renderer', (done) => {
ipcMain.once('answer', function (event, test) {
assert.strictEqual(test.pid, w.webContents.getOSProcessId())
assert.strictEqual(test.arch, remote.process.arch)
assert.strictEqual(test.platform, remote.process.platform)
assert.deepStrictEqual(...resolveGetters(test.env, remote.process.env))
@@ -10,10 +10,6 @@ if (process.argv.includes('--app-enable-sandbox')) {
app.enableSandbox()
}

if (process.argv.includes('--app-enable-mixed-sandbox')) {
app.enableMixedSandbox()
}

let currentWindowSandboxed = false

app.once('ready', () => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.