Skip to content

Commit

Permalink
policy: add policy-integrity to mitigate policy tampering
Browse files Browse the repository at this point in the history
PR-URL: #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>
  • Loading branch information
bmeck authored and targos committed Jul 22, 2019
1 parent cf811ec commit 2eeb44f
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 0 deletions.
13 changes: 13 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions doc/api/policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
.
Expand Down
27 changes: 27 additions & 0 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
16 changes: 16 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/policy/dep-policy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"resources": {
"./dep.js": {
"integrity": "sha512-7CMcc2oytFfMnGQaXbJk84gYWF2J7p/fmWPW7dsnJyniD+vgxtK9VAZ/22UxFOA4q5d27RoGLxSqNZ/nGCJkMw== sha512-scgN9Td0bGMlGH2lUHvEeHtz92Hx6AO+sYhU3WRI6bn3jEUCXbXJs68nOOsGzRWR7a2tbqGoETnOCpHHf1Njhw=="
}
}
}
2 changes: 2 additions & 0 deletions test/fixtures/policy/dep.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use strict';
module.exports = 'The Secret Ingredient';
69 changes: 69 additions & 0 deletions test/parallel/test-policy-integrity-flag.js
Original file line number Diff line number Diff line change
@@ -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}`);
}

0 comments on commit 2eeb44f

Please sign in to comment.