Skip to content

Commit e476fc8

Browse files
bmeckgntem
authored andcommitted
policy: add policy-integrity to mitigate policy tampering
PR-URL: nodejs#28734 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 791a022 commit e476fc8

File tree

9 files changed

+148
-0
lines changed

9 files changed

+148
-0
lines changed

doc/api/cli.md

+13
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,17 @@ unless either the `--pending-deprecation` command line flag, or the
446446
are used to provide a kind of selective "early warning" mechanism that
447447
developers may leverage to detect deprecated API usage.
448448

449+
### `--policy-integrity=sri`
450+
<!-- YAML
451+
added: REPLACEME
452+
-->
453+
454+
> Stability: 1 - Experimental
455+
456+
Instructs Node.js to error prior to running any code if the policy does not have
457+
the specified integrity. It expects a [Subresource Integrity][] string as a
458+
parameter.
459+
449460
### `--preserve-symlinks`
450461
<!-- YAML
451462
added: v6.3.0
@@ -980,6 +991,7 @@ Node.js options that are allowed are:
980991
- `--no-warnings`
981992
- `--openssl-config`
982993
- `--pending-deprecation`
994+
- `--policy-integrity`
983995
- `--preserve-symlinks-main`
984996
- `--preserve-symlinks`
985997
- `--prof-process`
@@ -1184,6 +1196,7 @@ greater than `4` (its current default value). For more information, see the
11841196
[Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
11851197
[REPL]: repl.html
11861198
[ScriptCoverage]: https://chromedevtools.github.io/devtools-protocol/tot/Profiler#type-ScriptCoverage
1199+
[Subresource Integrity]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity
11871200
[V8 JavaScript code coverage]: https://v8project.blogspot.com/2017/12/javascript-code-coverage.html
11881201
[customizing esm specifier resolution]: esm.html#esm_customizing_esm_specifier_resolution_algorithm
11891202
[debugger]: debugger.html

doc/api/policy.md

+9
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ node --experimental-policy=policy.json app.js
3838
The policy manifest will be used to enforce constraints on code loaded by
3939
Node.js.
4040

41+
In order to mitigate tampering with policy files on disk, an integrity for
42+
the policy file itself may be provided via `--policy-integrity`.
43+
This allows running `node` and asserting the policy file contents
44+
even if the file is changed on disk.
45+
46+
```sh
47+
node --experimental-policy=policy.json --policy-integrity="sha384-SggXRQHwCG8g+DktYYzxkXRIkTiEYWBHqev0xnpCxYlqMBufKZHAHQM3/boDaI/0" app.js
48+
```
49+
4150
## Features
4251

4352
### Error Behavior

doc/node.1

+3
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,9 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
228228
.It Fl -pending-deprecation
229229
Emit pending deprecation warnings.
230230
.
231+
.It Fl -policy-integrity Ns = Ns Ar sri
232+
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.
233+
.
231234
.It Fl -preserve-symlinks
232235
Instructs the module loader to preserve symbolic links when resolving and caching modules other than the main module.
233236
.

lib/internal/bootstrap/pre_execution.js

+27
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const { Object, SafeWeakMap } = primordials;
44

55
const { getOptionValue } = require('internal/options');
66
const { Buffer } = require('buffer');
7+
const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
78

89
function prepareMainThreadExecution(expandArgv1 = false) {
910
// Patch the process object with legacy properties and normalizations
@@ -332,6 +333,32 @@ function initializePolicy() {
332333
}
333334
const fs = require('fs');
334335
const src = fs.readFileSync(manifestURL, 'utf8');
336+
const experimentalPolicyIntegrity = getOptionValue('--policy-integrity');
337+
if (experimentalPolicyIntegrity) {
338+
const SRI = require('internal/policy/sri');
339+
const { createHash, timingSafeEqual } = require('crypto');
340+
const realIntegrities = new Map();
341+
const integrityEntries = SRI.parse(experimentalPolicyIntegrity);
342+
let foundMatch = false;
343+
for (var i = 0; i < integrityEntries.length; i++) {
344+
const {
345+
algorithm,
346+
value: expected
347+
} = integrityEntries[i];
348+
const hash = createHash(algorithm);
349+
hash.update(src);
350+
const digest = hash.digest();
351+
if (digest.length === expected.length &&
352+
timingSafeEqual(digest, expected)) {
353+
foundMatch = true;
354+
break;
355+
}
356+
realIntegrities.set(algorithm, digest.toString('base64'));
357+
}
358+
if (!foundMatch) {
359+
throw new ERR_MANIFEST_ASSERT_INTEGRITY(manifestURL, realIntegrities);
360+
}
361+
}
335362
require('internal/process/policy')
336363
.setup(src, manifestURL.href);
337364
}

src/node_options.cc

+16
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,13 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
116116
if (!userland_loader.empty() && !experimental_modules) {
117117
errors->push_back("--loader requires --experimental-modules be enabled");
118118
}
119+
if (has_policy_integrity_string && experimental_policy.empty()) {
120+
errors->push_back("--policy-integrity requires "
121+
"--experimental-policy be enabled");
122+
}
123+
if (has_policy_integrity_string && experimental_policy_integrity.empty()) {
124+
errors->push_back("--policy-integrity cannot be empty");
125+
}
119126

120127
if (!module_type.empty()) {
121128
if (!experimental_modules) {
@@ -321,6 +328,15 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
321328
"security policy",
322329
&EnvironmentOptions::experimental_policy,
323330
kAllowedInEnvironment);
331+
AddOption("[has_policy_integrity_string]",
332+
"",
333+
&EnvironmentOptions::has_policy_integrity_string);
334+
AddOption("--policy-integrity",
335+
"ensure the security policy contents match "
336+
"the specified integrity",
337+
&EnvironmentOptions::experimental_policy_integrity,
338+
kAllowedInEnvironment);
339+
Implies("--policy-integrity", "[has_policy_integrity_string]");
324340
AddOption("--experimental-repl-await",
325341
"experimental await keyword support in REPL",
326342
&EnvironmentOptions::experimental_repl_await,

src/node_options.h

+2
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ class EnvironmentOptions : public Options {
106106
bool experimental_wasm_modules = false;
107107
std::string module_type;
108108
std::string experimental_policy;
109+
std::string experimental_policy_integrity;
110+
bool has_policy_integrity_string;
109111
bool experimental_repl_await = false;
110112
bool experimental_vm_modules = false;
111113
bool expose_internals = false;

test/fixtures/policy/dep-policy.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"resources": {
3+
"./dep.js": {
4+
"integrity": "sha512-7CMcc2oytFfMnGQaXbJk84gYWF2J7p/fmWPW7dsnJyniD+vgxtK9VAZ/22UxFOA4q5d27RoGLxSqNZ/nGCJkMw== sha512-scgN9Td0bGMlGH2lUHvEeHtz92Hx6AO+sYhU3WRI6bn3jEUCXbXJs68nOOsGzRWR7a2tbqGoETnOCpHHf1Njhw=="
5+
}
6+
}
7+
}

test/fixtures/policy/dep.js

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
'use strict';
2+
module.exports = 'The Secret Ingredient';
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const fixtures = require('../common/fixtures');
8+
9+
const assert = require('assert');
10+
const { spawnSync } = require('child_process');
11+
const fs = require('fs');
12+
const crypto = require('crypto');
13+
14+
const depPolicy = fixtures.path('policy', 'dep-policy.json');
15+
const dep = fixtures.path('policy', 'dep.js');
16+
17+
const emptyHash = crypto.createHash('sha512');
18+
emptyHash.update('');
19+
const emptySRI = `sha512-${emptyHash.digest('base64')}`;
20+
const policyHash = crypto.createHash('sha512');
21+
policyHash.update(fs.readFileSync(depPolicy));
22+
23+
/* eslint-disable max-len */
24+
// When using \n only
25+
const nixPolicySRI = 'sha512-u/nXI6UacK5fKDC2bopcgnuQY4JXJKlK3dESO3GIKKxwogVHjJqpF9rgk7Zw+TJXIc96xBUWKHuUgOzic8/4tQ==';
26+
// When \n is turned into \r\n
27+
const windowsPolicySRI = 'sha512-OeyCPRo4OZMosHyquZXDHpuU1F4KzG9UHFnn12FMaHsvqFUt3TFZ+7wmZE7ThZ5rsQWkUjc9ZH0knGZ2e8BYPQ==';
28+
/* eslint-enable max-len */
29+
30+
const depPolicySRI = `${nixPolicySRI} ${windowsPolicySRI}`;
31+
console.dir({
32+
depPolicySRI,
33+
body: JSON.stringify(fs.readFileSync(depPolicy).toString('utf8'))
34+
});
35+
{
36+
const { status, stderr } = spawnSync(
37+
process.execPath,
38+
[
39+
'--policy-integrity', emptySRI,
40+
'--experimental-policy', depPolicy, dep,
41+
]
42+
);
43+
44+
assert.ok(stderr.includes('ERR_MANIFEST_ASSERT_INTEGRITY'));
45+
assert.strictEqual(status, 1);
46+
}
47+
{
48+
const { status, stderr } = spawnSync(
49+
process.execPath,
50+
[
51+
'--policy-integrity', '',
52+
'--experimental-policy', depPolicy, dep,
53+
]
54+
);
55+
56+
assert.ok(stderr.includes('--policy-integrity'));
57+
assert.strictEqual(status, 9);
58+
}
59+
{
60+
const { status, stderr } = spawnSync(
61+
process.execPath,
62+
[
63+
'--policy-integrity', depPolicySRI,
64+
'--experimental-policy', depPolicy, dep,
65+
]
66+
);
67+
68+
assert.strictEqual(status, 0, `status: ${status}\nstderr: ${stderr}`);
69+
}

0 commit comments

Comments
 (0)