diff options
author | Bradley Farias <bradley.meck@gmail.com> | 2019-06-05 13:33:07 -0500 |
---|---|---|
committer | Michaƫl Zasso <targos@protonmail.com> | 2019-07-22 21:20:42 +0200 |
commit | 2eeb44f3facb58dacbcb2f270d4f169a2c81ee08 (patch) | |
tree | cb3ecdb07852362d181312eb6ffd204d86199b09 | |
parent | cf811ecd47cf2c4f5bec2b27577c6d414842b703 (diff) | |
download | android-node-v8-2eeb44f3facb58dacbcb2f270d4f169a2c81ee08.tar.gz android-node-v8-2eeb44f3facb58dacbcb2f270d4f169a2c81ee08.tar.bz2 android-node-v8-2eeb44f3facb58dacbcb2f270d4f169a2c81ee08.zip |
policy: add policy-integrity to mitigate policy tampering
PR-URL: https://github.com/nodejs/node/pull/28734
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
-rw-r--r-- | doc/api/cli.md | 13 | ||||
-rw-r--r-- | doc/api/policy.md | 9 | ||||
-rw-r--r-- | doc/node.1 | 3 | ||||
-rw-r--r-- | lib/internal/bootstrap/pre_execution.js | 27 | ||||
-rw-r--r-- | src/node_options.cc | 16 | ||||
-rw-r--r-- | src/node_options.h | 2 | ||||
-rw-r--r-- | test/fixtures/policy/dep-policy.json | 7 | ||||
-rw-r--r-- | test/fixtures/policy/dep.js | 2 | ||||
-rw-r--r-- | test/parallel/test-policy-integrity-flag.js | 69 |
9 files changed, 148 insertions, 0 deletions
diff --git a/doc/api/cli.md b/doc/api/cli.md index d63749819e..5e60b509e3 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -457,6 +457,17 @@ unless either the `--pending-deprecation` command line flag, or the are used to provide a kind of selective "early warning" mechanism that developers may leverage to detect deprecated API usage. +### `--policy-integrity=sri` +<!-- YAML +added: REPLACEME +--> + +> Stability: 1 - Experimental + +Instructs Node.js to error prior to running any code if the policy does not have +the specified integrity. It expects a [Subresource Integrity][] string as a +parameter. + ### `--preserve-symlinks` <!-- YAML added: v6.3.0 @@ -992,6 +1003,7 @@ Node.js options that are allowed are: - `--no-warnings` - `--openssl-config` - `--pending-deprecation` +- `--policy-integrity` - `--preserve-symlinks-main` - `--preserve-symlinks` - `--prof-process` @@ -1196,6 +1208,7 @@ greater than `4` (its current default value). For more information, see the [Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/ [REPL]: repl.html [ScriptCoverage]: https://chromedevtools.github.io/devtools-protocol/tot/Profiler#type-ScriptCoverage +[Subresource Integrity]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity [V8 JavaScript code coverage]: https://v8project.blogspot.com/2017/12/javascript-code-coverage.html [customizing esm specifier resolution]: esm.html#esm_customizing_esm_specifier_resolution_algorithm [debugger]: debugger.html diff --git a/doc/api/policy.md b/doc/api/policy.md index be7ea3480b..a1955f2b3e 100644 --- a/doc/api/policy.md +++ b/doc/api/policy.md @@ -38,6 +38,15 @@ node --experimental-policy=policy.json app.js The policy manifest will be used to enforce constraints on code loaded by Node.js. +In order to mitigate tampering with policy files on disk, an integrity for +the policy file itself may be provided via `--policy-integrity`. +This allows running `node` and asserting the policy file contents +even if the file is changed on disk. + +```sh +node --experimental-policy=policy.json --policy-integrity="sha384-SggXRQHwCG8g+DktYYzxkXRIkTiEYWBHqev0xnpCxYlqMBufKZHAHQM3/boDaI/0" app.js +``` + ## Features ### Error Behavior diff --git a/doc/node.1 b/doc/node.1 index 14266a3f57..0c8de4b75f 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -231,6 +231,9 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is .It Fl -pending-deprecation Emit pending deprecation warnings. . +.It Fl -policy-integrity Ns = Ns Ar sri +Instructs Node.js to error prior to running any code if the policy does not have the specified integrity. It expects a Subresource Integrity string as a parameter. +. .It Fl -preserve-symlinks Instructs the module loader to preserve symbolic links when resolving and caching modules other than the main module. . diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index bbb0786dcd..104ebaff32 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -4,6 +4,7 @@ const { Object, SafeWeakMap } = primordials; const { getOptionValue } = require('internal/options'); const { Buffer } = require('buffer'); +const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes; function prepareMainThreadExecution(expandArgv1 = false) { // Patch the process object with legacy properties and normalizations @@ -332,6 +333,32 @@ function initializePolicy() { } const fs = require('fs'); const src = fs.readFileSync(manifestURL, 'utf8'); + const experimentalPolicyIntegrity = getOptionValue('--policy-integrity'); + if (experimentalPolicyIntegrity) { + const SRI = require('internal/policy/sri'); + const { createHash, timingSafeEqual } = require('crypto'); + const realIntegrities = new Map(); + const integrityEntries = SRI.parse(experimentalPolicyIntegrity); + let foundMatch = false; + for (var i = 0; i < integrityEntries.length; i++) { + const { + algorithm, + value: expected + } = integrityEntries[i]; + const hash = createHash(algorithm); + hash.update(src); + const digest = hash.digest(); + if (digest.length === expected.length && + timingSafeEqual(digest, expected)) { + foundMatch = true; + break; + } + realIntegrities.set(algorithm, digest.toString('base64')); + } + if (!foundMatch) { + throw new ERR_MANIFEST_ASSERT_INTEGRITY(manifestURL, realIntegrities); + } + } require('internal/process/policy') .setup(src, manifestURL.href); } diff --git a/src/node_options.cc b/src/node_options.cc index 829154c3bf..2eed3f8222 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -116,6 +116,13 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) { if (!userland_loader.empty() && !experimental_modules) { errors->push_back("--loader requires --experimental-modules be enabled"); } + if (has_policy_integrity_string && experimental_policy.empty()) { + errors->push_back("--policy-integrity requires " + "--experimental-policy be enabled"); + } + if (has_policy_integrity_string && experimental_policy_integrity.empty()) { + errors->push_back("--policy-integrity cannot be empty"); + } if (!module_type.empty()) { if (!experimental_modules) { @@ -321,6 +328,15 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "security policy", &EnvironmentOptions::experimental_policy, kAllowedInEnvironment); + AddOption("[has_policy_integrity_string]", + "", + &EnvironmentOptions::has_policy_integrity_string); + AddOption("--policy-integrity", + "ensure the security policy contents match " + "the specified integrity", + &EnvironmentOptions::experimental_policy_integrity, + kAllowedInEnvironment); + Implies("--policy-integrity", "[has_policy_integrity_string]"); AddOption("--experimental-repl-await", "experimental await keyword support in REPL", &EnvironmentOptions::experimental_repl_await, diff --git a/src/node_options.h b/src/node_options.h index c55aaf17a0..dbd85b2d58 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -106,6 +106,8 @@ class EnvironmentOptions : public Options { bool experimental_wasm_modules = false; std::string module_type; std::string experimental_policy; + std::string experimental_policy_integrity; + bool has_policy_integrity_string; bool experimental_repl_await = false; bool experimental_vm_modules = false; bool expose_internals = false; diff --git a/test/fixtures/policy/dep-policy.json b/test/fixtures/policy/dep-policy.json new file mode 100644 index 0000000000..6cc483a578 --- /dev/null +++ b/test/fixtures/policy/dep-policy.json @@ -0,0 +1,7 @@ +{ + "resources": { + "./dep.js": { + "integrity": "sha512-7CMcc2oytFfMnGQaXbJk84gYWF2J7p/fmWPW7dsnJyniD+vgxtK9VAZ/22UxFOA4q5d27RoGLxSqNZ/nGCJkMw== sha512-scgN9Td0bGMlGH2lUHvEeHtz92Hx6AO+sYhU3WRI6bn3jEUCXbXJs68nOOsGzRWR7a2tbqGoETnOCpHHf1Njhw==" + } + } +} diff --git a/test/fixtures/policy/dep.js b/test/fixtures/policy/dep.js new file mode 100644 index 0000000000..1c61a090d2 --- /dev/null +++ b/test/fixtures/policy/dep.js @@ -0,0 +1,2 @@ +'use strict'; +module.exports = 'The Secret Ingredient'; diff --git a/test/parallel/test-policy-integrity-flag.js b/test/parallel/test-policy-integrity-flag.js new file mode 100644 index 0000000000..3b332758d1 --- /dev/null +++ b/test/parallel/test-policy-integrity-flag.js @@ -0,0 +1,69 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const fixtures = require('../common/fixtures'); + +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const fs = require('fs'); +const crypto = require('crypto'); + +const depPolicy = fixtures.path('policy', 'dep-policy.json'); +const dep = fixtures.path('policy', 'dep.js'); + +const emptyHash = crypto.createHash('sha512'); +emptyHash.update(''); +const emptySRI = `sha512-${emptyHash.digest('base64')}`; +const policyHash = crypto.createHash('sha512'); +policyHash.update(fs.readFileSync(depPolicy)); + +/* eslint-disable max-len */ +// When using \n only +const nixPolicySRI = 'sha512-u/nXI6UacK5fKDC2bopcgnuQY4JXJKlK3dESO3GIKKxwogVHjJqpF9rgk7Zw+TJXIc96xBUWKHuUgOzic8/4tQ=='; +// When \n is turned into \r\n +const windowsPolicySRI = 'sha512-OeyCPRo4OZMosHyquZXDHpuU1F4KzG9UHFnn12FMaHsvqFUt3TFZ+7wmZE7ThZ5rsQWkUjc9ZH0knGZ2e8BYPQ=='; +/* eslint-enable max-len */ + +const depPolicySRI = `${nixPolicySRI} ${windowsPolicySRI}`; +console.dir({ + depPolicySRI, + body: JSON.stringify(fs.readFileSync(depPolicy).toString('utf8')) +}); +{ + const { status, stderr } = spawnSync( + process.execPath, + [ + '--policy-integrity', emptySRI, + '--experimental-policy', depPolicy, dep, + ] + ); + + assert.ok(stderr.includes('ERR_MANIFEST_ASSERT_INTEGRITY')); + assert.strictEqual(status, 1); +} +{ + const { status, stderr } = spawnSync( + process.execPath, + [ + '--policy-integrity', '', + '--experimental-policy', depPolicy, dep, + ] + ); + + assert.ok(stderr.includes('--policy-integrity')); + assert.strictEqual(status, 9); +} +{ + const { status, stderr } = spawnSync( + process.execPath, + [ + '--policy-integrity', depPolicySRI, + '--experimental-policy', depPolicy, dep, + ] + ); + + assert.strictEqual(status, 0, `status: ${status}\nstderr: ${stderr}`); +} |