From 317621b4a12562eb75055a67bb2c5556f53fe017 Mon Sep 17 00:00:00 2001 From: patr0nus Date: Thu, 18 Jun 2020 22:41:56 +0800 Subject: [PATCH 1/5] src: tolerate EPERM returned from tcsetattr macOS app sandbox makes tcsetattr return EPERM. The CHECK_EQ(0, err) here would fail when a sandboxed Node.js process is exiting. This commit fixes this issue. --- src/node.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/node.cc b/src/node.cc index b7435d67f839ea..438b08391dc49c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -737,7 +737,10 @@ void ResetStdio() { err = tcsetattr(fd, TCSANOW, &s.termios); while (err == -1 && errno == EINTR); // NOLINT CHECK_EQ(0, pthread_sigmask(SIG_UNBLOCK, &sa, nullptr)); - CHECK_EQ(0, err); + + // Normally we expect err == 0. But if macOS App Sandbox is enabled, + // tcsetattr will fail with err == -1 and errno == EPERM. + CHECK_IMPLIES(err != 0, err == -1 && errno == EPERM); } } #endif // __POSIX__ From 5f4f969813980b25a3cb75bd79c02c68268a6930 Mon Sep 17 00:00:00 2001 From: patr0nus Date: Thu, 18 Jun 2020 22:57:22 +0800 Subject: [PATCH 2/5] test: add test for running in macOS app sandbox Bare-bone command-line executables cannot run directly in the app sandbox. To test that Node.js is able to run in the sandbox (and to test the fix in 317621b4a12562eb75055a67bb2c5556f53fe017), this commit creates a typical Cocoa app bundle, puts the node executable in it and calles Apple's codesign command to enable sandbox. --- test/fixtures/macos-app-sandbox/Info.plist | 24 +++++++ .../node_sandboxed.entitlements | 8 +++ test/parallel/test-macos-app-sandbox.js | 67 +++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 test/fixtures/macos-app-sandbox/Info.plist create mode 100644 test/fixtures/macos-app-sandbox/node_sandboxed.entitlements create mode 100644 test/parallel/test-macos-app-sandbox.js diff --git a/test/fixtures/macos-app-sandbox/Info.plist b/test/fixtures/macos-app-sandbox/Info.plist new file mode 100644 index 00000000000000..38362085af4bf8 --- /dev/null +++ b/test/fixtures/macos-app-sandbox/Info.plist @@ -0,0 +1,24 @@ + + + + + CFBundleExecutable + node + CFBundleIdentifier + org.nodejs.test.node_sandboxed + CFBundleInfoDictionaryVersion + 6.0 + CFBundleName + node_sandboxed + CFBundlePackageType + APPL + CFBundleShortVersionString + 1.0 + CFBundleSupportedPlatforms + + MacOSX + + CFBundleVersion + 1 + + \ No newline at end of file diff --git a/test/fixtures/macos-app-sandbox/node_sandboxed.entitlements b/test/fixtures/macos-app-sandbox/node_sandboxed.entitlements new file mode 100644 index 00000000000000..852fa1a4728ae4 --- /dev/null +++ b/test/fixtures/macos-app-sandbox/node_sandboxed.entitlements @@ -0,0 +1,8 @@ + + + + + com.apple.security.app-sandbox + + + diff --git a/test/parallel/test-macos-app-sandbox.js b/test/parallel/test-macos-app-sandbox.js new file mode 100644 index 00000000000000..a14e1c91722fd8 --- /dev/null +++ b/test/parallel/test-macos-app-sandbox.js @@ -0,0 +1,67 @@ +'use strict'; +const common = require('../common'); +if (process.platform !== 'darwin') + common.skip('App Sandbox is only avaliable on Darwin'); + +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const child_process = require('child_process'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +const nodeBinary = path.resolve( + __dirname, '..', '..', + `out/${common.buildType}/node`); + +tmpdir.refresh(); + +const appBundlePath = path.join(tmpdir.path, 'node_sandboxed.app'); +const appBundleContentPath = path.join(appBundlePath, 'Contents'); +const appExecutablePath = path.join( + appBundleContentPath, 'MacOS', 'node'); + +// Construct the app bundle and put the node executable in it: +// node_sandboxed.app/ +// └── Contents +// ├── Info.plist +// ├── MacOS +// │ └── node +fs.mkdirSync(appBundlePath); +fs.mkdirSync(appBundleContentPath); +fs.mkdirSync(path.join(appBundleContentPath, 'MacOS')); +fs.copyFileSync( + fixtures.path('macos-app-sandbox', 'Info.plist'), + path.join(appBundleContentPath, 'Info.plist')); +fs.copyFileSync( + nodeBinary, + appExecutablePath); + + +// Sign the app bundle with sandbox entitlements: +assert.strictEqual( + child_process.spawnSync('/usr/bin/codesign', [ + '--entitlements', fixtures.path( + 'macos-app-sandbox', 'node_sandboxed.entitlements'), + '-s', '-', + appBundlePath + ]).status, + 0); + +// Sandboxed app shouldn't be able to read the home dir +assert.notStrictEqual( + child_process.spawnSync(appExecutablePath, [ + '-e', 'fs.readdirSync(process.argv[1])', os.homedir() + ]).status, + 0); + +if (process.stdin.isTTY) { + // Run the sandboxed node instance with inherited tty stdin + const spawnResult = child_process.spawnSync( + appExecutablePath, ['-e', ''], + { stdio: 'inherit' } + ); + + assert.strictEqual(spawnResult.signal, null); +} From 3cb770ede7fc522f7c6a43cc6795f8bd6bc6ef75 Mon Sep 17 00:00:00 2001 From: patr0nus Date: Thu, 18 Jun 2020 23:32:08 +0800 Subject: [PATCH 3/5] test: use process.execPath to get path of testing node --- test/parallel/test-macos-app-sandbox.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-macos-app-sandbox.js b/test/parallel/test-macos-app-sandbox.js index a14e1c91722fd8..f7fcf5e5728815 100644 --- a/test/parallel/test-macos-app-sandbox.js +++ b/test/parallel/test-macos-app-sandbox.js @@ -11,9 +11,7 @@ const path = require('path'); const fs = require('fs'); const os = require('os'); -const nodeBinary = path.resolve( - __dirname, '..', '..', - `out/${common.buildType}/node`); +const nodeBinary = process.execPath; tmpdir.refresh(); From d939f7f344e638d8390cdb767038338dde9c670f Mon Sep 17 00:00:00 2001 From: patr0nus Date: Fri, 19 Jun 2020 10:52:23 +0800 Subject: [PATCH 4/5] test: log codesign result (ci debug) --- test/parallel/test-macos-app-sandbox.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-macos-app-sandbox.js b/test/parallel/test-macos-app-sandbox.js index f7fcf5e5728815..77b84b17db6db5 100644 --- a/test/parallel/test-macos-app-sandbox.js +++ b/test/parallel/test-macos-app-sandbox.js @@ -38,14 +38,19 @@ fs.copyFileSync( // Sign the app bundle with sandbox entitlements: -assert.strictEqual( - child_process.spawnSync('/usr/bin/codesign', [ +const codesignResult = child_process.spawnSync( + '/usr/bin/codesign', + [ '--entitlements', fixtures.path( 'macos-app-sandbox', 'node_sandboxed.entitlements'), '-s', '-', appBundlePath - ]).status, - 0); + ]) + +console.error(codesignResult) +console.error(codesignResult.stdout?.toString()) +console.error(codesignResult.stderr?.toString()) +assert.strictEqual(codesignResult.status, 0); // Sandboxed app shouldn't be able to read the home dir assert.notStrictEqual( From 59e434d20b13d4294c71304c8a07d8704543bd46 Mon Sep 17 00:00:00 2001 From: patr0nus Date: Fri, 19 Jun 2020 14:36:38 +0800 Subject: [PATCH 5/5] Revert "test: log codesign result (ci debug)" This reverts commit d939f7f344e638d8390cdb767038338dde9c670f. --- test/parallel/test-macos-app-sandbox.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-macos-app-sandbox.js b/test/parallel/test-macos-app-sandbox.js index 77b84b17db6db5..f7fcf5e5728815 100644 --- a/test/parallel/test-macos-app-sandbox.js +++ b/test/parallel/test-macos-app-sandbox.js @@ -38,19 +38,14 @@ fs.copyFileSync( // Sign the app bundle with sandbox entitlements: -const codesignResult = child_process.spawnSync( - '/usr/bin/codesign', - [ +assert.strictEqual( + child_process.spawnSync('/usr/bin/codesign', [ '--entitlements', fixtures.path( 'macos-app-sandbox', 'node_sandboxed.entitlements'), '-s', '-', appBundlePath - ]) - -console.error(codesignResult) -console.error(codesignResult.stdout?.toString()) -console.error(codesignResult.stderr?.toString()) -assert.strictEqual(codesignResult.status, 0); + ]).status, + 0); // Sandboxed app shouldn't be able to read the home dir assert.notStrictEqual(