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 8 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
10 changes: 10 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,14 @@ 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
-->

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.
bmeck marked this conversation as resolved.
Show resolved Hide resolved

### `--preserve-symlinks`
<!-- YAML
added: v6.3.0
Expand Down Expand Up @@ -959,6 +967,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 @@ -1171,3 +1180,4 @@ greater than `4` (its current default value). For more information, see the
[experimental ECMAScript Module]: esm.html#esm_resolve_hook
[libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html
[remote code execution]: https://www.owasp.org/index.php/Code_Injection
[Subresource Integrity]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity

This comment was marked as resolved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these don't seem to be in alphabetical order though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bottom references are usually sorted in ASCII order, case and punctuation are taken into account.

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=sri
bmeck marked this conversation as resolved.
Show resolved Hide resolved
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.

This comment was marked as resolved.

.
.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=="
}
}
}
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';
57 changes: 57 additions & 0 deletions test/parallel/test-policy-integrity-flag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'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));
const depPolicySRI = `sha512-${policyHash.digest('base64')}`;
{
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 } = spawnSync(
process.execPath,
[
'--policy-integrity', depPolicySRI,
'--experimental-policy', depPolicy, dep,
]
);

assert.strictEqual(status, 0);
bmeck marked this conversation as resolved.
Show resolved Hide resolved
}