diff --git a/filenames.gni b/filenames.gni index 0146238c36cfb..1a8fc5fe52023 100644 --- a/filenames.gni +++ b/filenames.gni @@ -199,6 +199,8 @@ filenames = { "shell/common/language_util_mac.mm", "shell/common/mac/main_application_bundle.h", "shell/common/mac/main_application_bundle.mm", + "shell/common/mac/codesign_util.cc", + "shell/common/mac/codesign_util.h", "shell/common/node_bindings_mac.cc", "shell/common/node_bindings_mac.h", "shell/common/platform_util_mac.mm", diff --git a/shell/app/node_main.cc b/shell/app/node_main.cc index c905ece0e760d..15c37a0ba13a3 100644 --- a/shell/app/node_main.cc +++ b/shell/app/node_main.cc @@ -13,6 +13,7 @@ #include "base/base_switches.h" #include "base/command_line.h" #include "base/containers/fixed_flat_set.h" +#include "base/environment.h" #include "base/feature_list.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -20,6 +21,7 @@ #include "base/task/thread_pool/thread_pool_instance.h" #include "content/public/common/content_switches.h" #include "electron/electron_version.h" +#include "electron/fuses.h" #include "gin/array_buffer.h" #include "gin/public/isolate_holder.h" #include "gin/v8_initializer.h" @@ -35,13 +37,16 @@ #endif #if BUILDFLAG(IS_LINUX) -#include "base/environment.h" #include "base/posix/global_descriptors.h" #include "base/strings/string_number_conversions.h" #include "components/crash/core/app/crash_switches.h" // nogncheck #include "content/public/common/content_descriptors.h" #endif +#if BUILDFLAG(IS_MAC) +#include "shell/common/mac/codesign_util.h" +#endif + #if !IS_MAS_BUILD() #include "components/crash/core/app/crashpad.h" // nogncheck #include "shell/app/electron_crash_reporter_client.h" @@ -100,12 +105,36 @@ int NodeMain(int argc, char* argv[]) { exit(1); } + auto os_env = base::Environment::Create(); + bool node_options_enabled = electron::fuses::IsNodeOptionsEnabled(); +#if BUILDFLAG(IS_MAC) + if (node_options_enabled && os_env->HasVar("NODE_OPTIONS")) { + // On macOS, it is forbidden to run sandboxed app with custom arguments + // from another app, i.e. args are discarded in following call: + // exec("Sandboxed.app", ["--custom-args-will-be-discarded"]) + // However it is possible to bypass the restriction by abusing the node mode + // of Electron apps: + // exec("Electron.app", {env: {ELECTRON_RUN_AS_NODE: "1", + // NODE_OPTIONS: "--require 'bad.js'"}}) + // To prevent Electron apps from being used to work around macOS security + // restrictions, when NODE_OPTIONS is passed it will be checked whether + // this process is invoked by its own app. + if (!ProcessBelongToCurrentApp(getppid())) { + LOG(ERROR) << "NODE_OPTIONS is disabled because this process is invoked " + "by other apps."; + node_options_enabled = false; + } + } +#endif // BUILDFLAG(IS_MAC) + if (!node_options_enabled) { + os_env->UnSetVar("NODE_OPTIONS"); + } + #if BUILDFLAG(IS_WIN) v8_crashpad_support::SetUp(); #endif #if BUILDFLAG(IS_LINUX) - auto os_env = base::Environment::Create(); std::string fd_string, pid_string; if (os_env->GetVar("CRASHDUMP_SIGNAL_FD", &fd_string) && os_env->GetVar("CRASHPAD_HANDLER_PID", &pid_string)) { diff --git a/shell/common/mac/codesign_util.cc b/shell/common/mac/codesign_util.cc new file mode 100644 index 0000000000000..5171f90aac117 --- /dev/null +++ b/shell/common/mac/codesign_util.cc @@ -0,0 +1,55 @@ +// Copyright 2023 Microsoft, Inc. +// Copyright 2013 The Chromium Authors +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "shell/common/mac/codesign_util.h" + +#include "base/apple/osstatus_logging.h" +#include "base/apple/scoped_cftyperef.h" + +#include +#include + +namespace electron { + +bool ProcessBelongToCurrentApp(pid_t pid) { + // Get and check the code signature of current app. + base::apple::ScopedCFTypeRef self_code; + OSStatus status = + SecCodeCopySelf(kSecCSDefaultFlags, self_code.InitializeInto()); + if (status != errSecSuccess) { + OSSTATUS_LOG(ERROR, status) << "SecCodeCopyGuestWithAttributes"; + return false; + } + // Get the code signature of process. + base::apple::ScopedCFTypeRef process_cf( + CFNumberCreate(nullptr, kCFNumberIntType, &pid)); + const void* attribute_keys[] = {kSecGuestAttributePid}; + const void* attribute_values[] = {process_cf.get()}; + base::apple::ScopedCFTypeRef attributes(CFDictionaryCreate( + nullptr, attribute_keys, attribute_values, std::size(attribute_keys), + &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks)); + base::apple::ScopedCFTypeRef process_code; + status = SecCodeCopyGuestWithAttributes(nullptr, attributes.get(), + kSecCSDefaultFlags, + process_code.InitializeInto()); + if (status != errSecSuccess) { + OSSTATUS_LOG(ERROR, status) << "SecCodeCopyGuestWithAttributes"; + return false; + } + // Get the requirement of current app's code signature. + base::apple::ScopedCFTypeRef self_requirement; + status = SecCodeCopyDesignatedRequirement(self_code.get(), kSecCSDefaultFlags, + self_requirement.InitializeInto()); + if (status != errSecSuccess) { + OSSTATUS_LOG(ERROR, status) << "SecCodeCopyDesignatedRequirement"; + return false; + } + // Check whether the process meets the signature requirement of current app. + status = SecCodeCheckValidity(process_code.get(), kSecCSDefaultFlags, + self_requirement.get()); + return status == errSecSuccess; +} + +} // namespace electron diff --git a/shell/common/mac/codesign_util.h b/shell/common/mac/codesign_util.h new file mode 100644 index 0000000000000..8a2fb94ee8e6f --- /dev/null +++ b/shell/common/mac/codesign_util.h @@ -0,0 +1,19 @@ +// Copyright 2023 Microsoft, Inc. +// Copyright 2013 The Chromium Authors +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_MAC_CODESIGN_UTIL_H_ +#define SHELL_COMMON_MAC_CODESIGN_UTIL_H_ + +#include + +namespace electron { + +// Given a pid, check if the process belongs to current app by comparing its +// code signature with current app. +bool ProcessBelongToCurrentApp(pid_t pid); + +} // namespace electron + +#endif // SHELL_COMMON_MAC_CODESIGN_UTIL_H_ diff --git a/spec/api-autoupdater-darwin-spec.ts b/spec/api-autoupdater-darwin-spec.ts index 8ceebc04bd034..373ee884b7a61 100644 --- a/spec/api-autoupdater-darwin-spec.ts +++ b/spec/api-autoupdater-darwin-spec.ts @@ -3,36 +3,26 @@ import * as cp from 'node:child_process'; import * as http from 'node:http'; import * as express from 'express'; import * as fs from 'fs-extra'; -import * as os from 'node:os'; import * as path from 'node:path'; import * as psList from 'ps-list'; import { AddressInfo } from 'node:net'; import { ifdescribe, ifit } from './lib/spec-helpers'; +import { copyApp, getCodesignIdentity, shouldRunCodesignTests, signApp, spawn, withTempDirectory } from './lib/codesign-helpers'; import * as uuid from 'uuid'; import { autoUpdater, systemPreferences } from 'electron'; -const features = process._linkedBinding('electron_common_features'); - -const fixturesPath = path.resolve(__dirname, 'fixtures'); - // We can only test the auto updater on darwin non-component builds -ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === 'arm64') && !process.mas && !features.isComponentBuild())('autoUpdater behavior', function () { +ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () { this.timeout(120000); let identity = ''; beforeEach(function () { - const result = cp.spawnSync(path.resolve(__dirname, '../script/codesign/get-trusted-identity.sh')); - if (result.status !== 0 || result.stdout.toString().trim().length === 0) { - // Per https://circleci.com/docs/2.0/env-vars: - // CIRCLE_PR_NUMBER is only present on forked PRs - if (process.env.CI && !process.env.CIRCLE_PR_NUMBER) { - throw new Error('No valid signing identity available to run autoUpdater specs'); - } - + const result = getCodesignIdentity(); + if (result === null) { this.skip(); } else { - identity = result.stdout.toString().trim(); + identity = result; } }); @@ -40,59 +30,6 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === expect(identity).to.be.a('string').with.lengthOf.at.least(1); }); - const copyApp = async (newDir: string, fixture = 'initial') => { - const appBundlePath = path.resolve(process.execPath, '../../..'); - const newPath = path.resolve(newDir, 'Electron.app'); - cp.spawnSync('cp', ['-R', appBundlePath, path.dirname(newPath)]); - const appDir = path.resolve(newPath, 'Contents/Resources/app'); - await fs.mkdirp(appDir); - await fs.copy(path.resolve(fixturesPath, 'auto-update', fixture), appDir); - const plistPath = path.resolve(newPath, 'Contents', 'Info.plist'); - await fs.writeFile( - plistPath, - (await fs.readFile(plistPath, 'utf8')).replace('BuildMachineOSBuild', `NSAppTransportSecurity - - NSAllowsArbitraryLoads - - NSExceptionDomains - - localhost - - NSExceptionAllowsInsecureHTTPLoads - - NSIncludesSubdomains - - - - BuildMachineOSBuild`) - ); - return newPath; - }; - - const spawn = (cmd: string, args: string[], opts: any = {}) => { - let out = ''; - const child = cp.spawn(cmd, args, opts); - child.stdout.on('data', (chunk: Buffer) => { - out += chunk.toString(); - }); - child.stderr.on('data', (chunk: Buffer) => { - out += chunk.toString(); - }); - return new Promise<{ code: number, out: string }>((resolve) => { - child.on('exit', (code, signal) => { - expect(signal).to.equal(null); - resolve({ - code: code!, - out - }); - }); - }); - }; - - const signApp = (appPath: string) => { - return spawn('codesign', ['-s', identity, '--deep', '--force', appPath]); - }; - const launchApp = (appPath: string, args: string[] = []) => { return spawn(path.resolve(appPath, 'Contents/MacOS/Electron'), args); }; @@ -107,17 +44,6 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === return activeShipIts; }; - const withTempDirectory = async (fn: (dir: string) => Promise, autoCleanUp = true) => { - const dir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-update-spec-')); - try { - await fn(dir); - } finally { - if (autoCleanUp) { - cp.spawnSync('rm', ['-r', dir]); - } - } - }; - const logOnError = (what: any, fn: () => void) => { try { fn(); @@ -151,7 +77,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === (await fs.readFile(infoPath, 'utf8')).replace(/(CFBundleShortVersionString<\/key>\s+)[^<]+/g, `$1${version}`) ); await mutateAppPreSign?.mutate(secondAppPath); - await signApp(secondAppPath); + await signApp(secondAppPath, identity); await mutateAppPostSign?.mutate(secondAppPath); updateZipPath = path.resolve(dir, 'update.zip'); await spawn('zip', ['-0', '-r', '--symlinks', updateZipPath, './'], { @@ -183,7 +109,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === it('should cleanly set the feed URL when the app is signed', async () => { await withTempDirectory(async (dir) => { const appPath = await copyApp(dir); - await signApp(appPath); + await signApp(appPath, identity); const launchResult = await launchApp(appPath, ['http://myupdate']); expect(launchResult.code).to.equal(0); expect(launchResult.out).to.include('Feed URL Set: http://myupdate'); @@ -224,7 +150,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === it('should hit the update endpoint when checkForUpdates is called', async () => { await withTempDirectory(async (dir) => { const appPath = await copyApp(dir, 'check'); - await signApp(appPath); + await signApp(appPath, identity); server.get('/update-check', (req, res) => { res.status(204).send(); }); @@ -241,7 +167,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === it('should hit the update endpoint with customer headers when checkForUpdates is called', async () => { await withTempDirectory(async (dir) => { const appPath = await copyApp(dir, 'check-with-headers'); - await signApp(appPath); + await signApp(appPath, identity); server.get('/update-check', (req, res) => { res.status(204).send(); }); @@ -258,7 +184,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === it('should hit the download endpoint when an update is available and error if the file is bad', async () => { await withTempDirectory(async (dir) => { const appPath = await copyApp(dir, 'update'); - await signApp(appPath); + await signApp(appPath, identity); server.get('/update-file', (req, res) => { res.status(500).send('This is not a file'); }); @@ -298,7 +224,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === infoPath, (await fs.readFile(infoPath, 'utf8')).replace(/(CFBundleShortVersionString<\/key>\s+)[^<]+/g, '$11.0.0') ); - await signApp(appPath); + await signApp(appPath, identity); const updateZipPath = await getOrCreateUpdateZipPath(opts.nextVersion, opts.endFixture, opts.mutateAppPreSign, opts.mutateAppPostSign); diff --git a/spec/fixtures/api/fork-with-node-options.js b/spec/fixtures/api/fork-with-node-options.js new file mode 100644 index 0000000000000..49dad89944b69 --- /dev/null +++ b/spec/fixtures/api/fork-with-node-options.js @@ -0,0 +1,22 @@ +const { execFileSync } = require('node:child_process'); +const path = require('node:path'); + +const fixtures = path.resolve(__dirname, '..'); + +const env = { + ELECTRON_RUN_AS_NODE: 'true', + // Process will exit with 1 if NODE_OPTIONS is accepted. + NODE_OPTIONS: `--require "${path.join(fixtures, 'module', 'fail.js')}"` +}; +// Provide a lower cased NODE_OPTIONS in case some code ignores case sensitivity +// when reading NODE_OPTIONS. +env.node_options = env.NODE_OPTIONS; +try { + execFileSync(process.argv[2], + ['--require', path.join(fixtures, 'module', 'noop.js')], + { env, stdio: 'inherit' }); + process.exit(0); +} catch (error) { + console.log('NODE_OPTIONS passed to child'); + process.exit(1); +} diff --git a/spec/lib/codesign-helpers.ts b/spec/lib/codesign-helpers.ts new file mode 100644 index 0000000000000..37f129c9119c5 --- /dev/null +++ b/spec/lib/codesign-helpers.ts @@ -0,0 +1,99 @@ +import * as cp from 'node:child_process'; +import * as fs from 'fs-extra'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { expect } from 'chai'; + +const features = process._linkedBinding('electron_common_features'); +const fixturesPath = path.resolve(__dirname, '..', 'fixtures'); + +export const shouldRunCodesignTests = + process.platform === 'darwin' && + !(process.env.CI && process.arch === 'arm64') && + !process.mas && + !features.isComponentBuild(); + +let identity: string | null; + +export function getCodesignIdentity () { + if (identity === undefined) { + const result = cp.spawnSync(path.resolve(__dirname, '../../script/codesign/get-trusted-identity.sh')); + if (result.status !== 0 || result.stdout.toString().trim().length === 0) { + // Per https://circleci.com/docs/2.0/env-vars: + // CIRCLE_PR_NUMBER is only present on forked PRs + if (process.env.CI && !process.env.CIRCLE_PR_NUMBER) { + throw new Error('No valid signing identity available to run autoUpdater specs'); + } + identity = null; + } else { + identity = result.stdout.toString().trim(); + } + } + return identity; +} + +export async function copyApp (newDir: string, fixture: string | null = 'initial') { + const appBundlePath = path.resolve(process.execPath, '../../..'); + const newPath = path.resolve(newDir, 'Electron.app'); + cp.spawnSync('cp', ['-R', appBundlePath, path.dirname(newPath)]); + if (fixture) { + const appDir = path.resolve(newPath, 'Contents/Resources/app'); + await fs.mkdirp(appDir); + await fs.copy(path.resolve(fixturesPath, 'auto-update', fixture), appDir); + } + const plistPath = path.resolve(newPath, 'Contents', 'Info.plist'); + await fs.writeFile( + plistPath, + (await fs.readFile(plistPath, 'utf8')).replace('BuildMachineOSBuild', `NSAppTransportSecurity + + NSAllowsArbitraryLoads + + NSExceptionDomains + + localhost + + NSExceptionAllowsInsecureHTTPLoads + + NSIncludesSubdomains + + + + BuildMachineOSBuild`) + ); + return newPath; +}; + +export function spawn (cmd: string, args: string[], opts: any = {}) { + let out = ''; + const child = cp.spawn(cmd, args, opts); + child.stdout.on('data', (chunk: Buffer) => { + out += chunk.toString(); + }); + child.stderr.on('data', (chunk: Buffer) => { + out += chunk.toString(); + }); + return new Promise<{ code: number, out: string }>((resolve) => { + child.on('exit', (code, signal) => { + expect(signal).to.equal(null); + resolve({ + code: code!, + out + }); + }); + }); +}; + +export function signApp (appPath: string, identity: string) { + return spawn('codesign', ['-s', identity, '--deep', '--force', appPath]); +}; + +export async function withTempDirectory (fn: (dir: string) => Promise, autoCleanUp = true) { + const dir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-update-spec-')); + try { + await fn(dir); + } finally { + if (autoCleanUp) { + cp.spawnSync('rm', ['-r', dir]); + } + } +}; diff --git a/spec/node-spec.ts b/spec/node-spec.ts index 2f8dd5218d567..1c7f7097d585f 100644 --- a/spec/node-spec.ts +++ b/spec/node-spec.ts @@ -1,9 +1,10 @@ import { expect } from 'chai'; import * as childProcess from 'node:child_process'; -import * as fs from 'node:fs'; +import * as fs from 'fs-extra'; import * as path from 'node:path'; import * as util from 'node:util'; import { getRemoteContext, ifdescribe, ifit, itremote, useRemoteContext } from './lib/spec-helpers'; +import { copyApp, getCodesignIdentity, shouldRunCodesignTests, signApp, spawn, withTempDirectory } from './lib/codesign-helpers'; import { webContents } from 'electron/main'; import { EventEmitter } from 'node:stream'; import { once } from 'node:events'; @@ -659,6 +660,66 @@ describe('node feature', () => { }); }); + ifdescribe(shouldRunCodesignTests)('NODE_OPTIONS in signed app', function () { + let identity = ''; + + beforeEach(function () { + const result = getCodesignIdentity(); + if (result === null) { + this.skip(); + } else { + identity = result; + } + }); + + const script = path.join(fixtures, 'api', 'fork-with-node-options.js'); + const nodeOptionsWarning = 'NODE_OPTIONS is disabled because this process is invoked by other apps'; + + it('is disabled when invoked by other apps in ELECTRON_RUN_AS_NODE mode', async () => { + await withTempDirectory(async (dir) => { + const appPath = await copyApp(dir); + await signApp(appPath, identity); + // Invoke Electron by using the system node binary as middle layer, so + // the check of NODE_OPTIONS will think the process is started by other + // apps. + const { code, out } = await spawn('node', [script, path.join(appPath, 'Contents/MacOS/Electron')]); + expect(code).to.equal(0); + expect(out).to.include(nodeOptionsWarning); + }); + }); + + it('is disabled when invoked by alien binary in app bundle in ELECTRON_RUN_AS_NODE mode', async function () { + await withTempDirectory(async (dir) => { + const appPath = await copyApp(dir); + await signApp(appPath, identity); + // Find system node and copy it to app bundle. + const nodePath = process.env.PATH?.split(path.delimiter).find(dir => fs.existsSync(path.join(dir, 'node'))); + if (!nodePath) { + this.skip(); + return; + } + const alienBinary = path.join(appPath, 'Contents/MacOS/node'); + await fs.copy(path.join(nodePath, 'node'), alienBinary); + // Try to execute electron app from the alien node in app bundle. + const { code, out } = await spawn(alienBinary, [script, path.join(appPath, 'Contents/MacOS/Electron')]); + expect(code).to.equal(0); + expect(out).to.include(nodeOptionsWarning); + }); + }); + + it('is respected when invoked from self', async () => { + await withTempDirectory(async (dir) => { + const appPath = await copyApp(dir, null); + await signApp(appPath, identity); + const appExePath = path.join(appPath, 'Contents/MacOS/Electron'); + const { code, out } = await spawn(appExePath, [script, appExePath]); + expect(code).to.equal(1); + expect(out).to.not.include(nodeOptionsWarning); + expect(out).to.include('NODE_OPTIONS passed to child'); + }); + }); + }); + describe('Node.js cli flags', () => { let child: childProcess.ChildProcessWithoutNullStreams; let exitPromise: Promise;