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

--policy-integrity #28734

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,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 @@ -959,6 +970,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 @@ -1163,6 +1175,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 @@ -228,6 +228,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 @@ -121,6 +121,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 @@ -313,6 +320,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 @@ -105,6 +105,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';
65 changes: 65 additions & 0 deletions test/parallel/test-policy-integrity-flag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
'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));
// When using \n only
const nixPolicySRI = 'sha512-u/nXI6UacK5fKDC2bopcgnuQY4JXJKlK3dESO3GIKKxwogVHjJqpF9rgk7Zw+TJXIc96xBUWKHuUgOzic8/4tQ==';
// When \n is turned into \r\n
const windowsPolicySRI = 'sha512-OeyCPRo4OZMosHyquZXDHpuU1F4KzG9UHFnn12FMaHsvqFUt3TFZ+7wmZE7ThZ5rsQWkUjc9ZH0knGZ2e8BYPQ==';
const depPolicySRI = `${nixPolicySRI} ${windowsPolicySRI}`;
console.dir({
depPolicySRI,
body: JSON.stringify(fs.readFileSync(depPolicy).toString('utf8'))
})
Trott marked this conversation as resolved.
Show resolved Hide resolved
{
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}`);
}