Skip to content

Commit 16d59c1

Browse files
bfarias-godaddyMylesBorins
authored andcommitted
policy: add startup benchmark and make SRI lazier
PR-URL: #29527 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent ba878ee commit 16d59c1

File tree

4 files changed

+119
-33
lines changed

4 files changed

+119
-33
lines changed

benchmark/policy/policy-startup.js

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Tests the impact on eager operations required for policies affecting
2+
// general startup, does not test lazy operations
3+
'use strict';
4+
const common = require('../common.js');
5+
6+
const configs = {
7+
n: [1024]
8+
};
9+
10+
const options = {
11+
flags: ['--expose-internals']
12+
};
13+
14+
const bench = common.createBenchmark(main, configs, options);
15+
16+
function main(conf) {
17+
const hash = (str, algo) => {
18+
const hash = require('crypto').createHash(algo);
19+
return hash.update(str).digest('base64');
20+
};
21+
const resources = Object.fromEntries(
22+
// Simulate graph of 1k modules
23+
Array.from({ length: 1024 }, (_, i) => {
24+
return [`./_${i}`, {
25+
integrity: `sha256-${hash(`// ./_${i}`, 'sha256')}`,
26+
dependencies: Object.fromEntries(Array.from({
27+
// Average 3 deps per 4 modules
28+
length: Math.floor((i % 4) / 2)
29+
}, (_, ii) => {
30+
return [`_${ii}`, `./_${i - ii}`];
31+
})),
32+
}];
33+
})
34+
);
35+
const json = JSON.parse(JSON.stringify({ resources }), (_, o) => {
36+
if (o && typeof o === 'object') {
37+
Reflect.setPrototypeOf(o, null);
38+
Object.freeze(o);
39+
}
40+
return o;
41+
});
42+
const { Manifest } = require('internal/policy/manifest');
43+
44+
bench.start();
45+
46+
for (let i = 0; i < conf.n; i++) {
47+
new Manifest(json, 'file://benchmark/policy-relative');
48+
}
49+
50+
bench.end(conf.n);
51+
}

lib/internal/policy/manifest.js

+50-28
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const {
44
ArrayIsArray,
55
Map,
66
MapPrototypeSet,
7+
ObjectCreate,
78
ObjectEntries,
89
ObjectFreeze,
910
ObjectSetPrototypeOf,
@@ -31,7 +32,6 @@ const { URL } = require('internal/url');
3132
const { createHash, timingSafeEqual } = crypto;
3233
const HashUpdate = uncurryThis(crypto.Hash.prototype.update);
3334
const HashDigest = uncurryThis(crypto.Hash.prototype.digest);
34-
const BufferEquals = uncurryThis(Buffer.prototype.equals);
3535
const BufferToString = uncurryThis(Buffer.prototype.toString);
3636
const kRelativeURLStringPattern = /^\.{0,2}\//;
3737
const { getOptionValue } = require('internal/options');
@@ -56,9 +56,47 @@ function REACTION_LOG(error) {
5656
}
5757

5858
class Manifest {
59+
/**
60+
* @type {Map<string, true | string | SRI[]>}
61+
*
62+
* Used to compare a resource to the content body at the resource.
63+
* `true` is used to signify that all integrities are allowed, otherwise,
64+
* SRI strings are parsed to compare with the body.
65+
*
66+
* This stores strings instead of eagerly parsing SRI strings
67+
* and only converts them to SRI data structures when needed.
68+
* This avoids needing to parse all SRI strings at startup even
69+
* if some never end up being used.
70+
*/
5971
#integrities = new SafeMap();
72+
/**
73+
* @type {Map<string, (specifier: string) => true | URL>}
74+
*
75+
* Used to find where a dependency is located.
76+
*
77+
* This stores functions to lazily calculate locations as needed.
78+
* `true` is used to signify that the location is not specified
79+
* by the manifest and default resolution should be allowed.
80+
*/
6081
#dependencies = new SafeMap();
82+
/**
83+
* @type {(err: Error) => void}
84+
*
85+
* Performs default action for what happens when a manifest encounters
86+
* a violation such as abort()ing or exiting the process, throwing the error,
87+
* or logging the error.
88+
*/
6189
#reaction = null;
90+
/**
91+
* `obj` should match the policy file format described in the docs
92+
* it is expected to not have prototype pollution issues either by reassigning
93+
* the prototype to `null` for values or by running prior to any user code.
94+
*
95+
* `manifestURL` is a URL to resolve relative locations against.
96+
*
97+
* @param {object} obj
98+
* @param {string} manifestURL
99+
*/
62100
constructor(obj, manifestURL) {
63101
const integrities = this.#integrities;
64102
const dependencies = this.#dependencies;
@@ -98,35 +136,14 @@ class Manifest {
98136
let integrity = manifestEntries[i][1].integrity;
99137
if (!integrity) integrity = null;
100138
if (integrity != null) {
101-
debug(`Manifest contains integrity for url ${originalHREF}`);
139+
debug('Manifest contains integrity for url %s', originalHREF);
102140
if (typeof integrity === 'string') {
103-
const sri = ObjectFreeze(SRI.parse(integrity));
104141
if (integrities.has(resourceHREF)) {
105-
const old = integrities.get(resourceHREF);
106-
let mismatch = false;
107-
108-
if (old.length !== sri.length) {
109-
mismatch = true;
110-
} else {
111-
compare:
112-
for (let sriI = 0; sriI < sri.length; sriI++) {
113-
for (let oldI = 0; oldI < old.length; oldI++) {
114-
if (sri[sriI].algorithm === old[oldI].algorithm &&
115-
BufferEquals(sri[sriI].value, old[oldI].value) &&
116-
sri[sriI].options === old[oldI].options) {
117-
continue compare;
118-
}
119-
}
120-
mismatch = true;
121-
break compare;
122-
}
123-
}
124-
125-
if (mismatch) {
142+
if (integrities.get(resourceHREF) !== integrity) {
126143
throw new ERR_MANIFEST_INTEGRITY_MISMATCH(resourceURL);
127144
}
128145
}
129-
integrities.set(resourceHREF, sri);
146+
integrities.set(resourceHREF, integrity);
130147
} else if (integrity === true) {
131148
integrities.set(resourceHREF, true);
132149
} else {
@@ -138,7 +155,7 @@ class Manifest {
138155

139156
let dependencyMap = manifestEntries[i][1].dependencies;
140157
if (dependencyMap === null || dependencyMap === undefined) {
141-
dependencyMap = {};
158+
dependencyMap = ObjectCreate(null);
142159
}
143160
if (typeof dependencyMap === 'object' && !ArrayIsArray(dependencyMap)) {
144161
/**
@@ -200,13 +217,18 @@ class Manifest {
200217

201218
assertIntegrity(url, content) {
202219
const href = `${url}`;
203-
debug(`Checking integrity of ${href}`);
220+
debug('Checking integrity of %s', href);
204221
const integrities = this.#integrities;
205222
const realIntegrities = new Map();
206223

207224
if (integrities.has(href)) {
208-
const integrityEntries = integrities.get(href);
225+
let integrityEntries = integrities.get(href);
209226
if (integrityEntries === true) return true;
227+
if (typeof integrityEntries === 'string') {
228+
const sri = ObjectFreeze(SRI.parse(integrityEntries));
229+
integrities.set(href, sri);
230+
integrityEntries = sri;
231+
}
210232
// Avoid clobbered Symbol.iterator
211233
for (let i = 0; i < integrityEntries.length; i++) {
212234
const {

lib/internal/policy/sri.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
'use strict';
2-
// Value of https://w3c.github.io/webappsec-subresource-integrity/#the-integrity-attribute
2+
// Utility to parse the value of
3+
// https://w3c.github.io/webappsec-subresource-integrity/#the-integrity-attribute
34

45
const {
56
ObjectDefineProperty,
67
ObjectFreeze,
8+
ObjectGetPrototypeOf,
79
ObjectSeal,
10+
ObjectSetPrototypeOf,
811
RegExp,
912
RegExpPrototypeExec,
1013
RegExpPrototypeTest,
1114
StringPrototypeSlice,
1215
} = primordials;
1316

14-
// Returns [{algorithm, value (in base64 string), options,}]
1517
const {
1618
ERR_SRI_PARSE
1719
} = require('internal/errors').codes;
@@ -21,17 +23,19 @@ const kHASH_ALGO = 'sha(?:256|384|512)';
2123
// Base64
2224
const kHASH_VALUE = '[A-Za-z0-9+/]+[=]{0,2}';
2325
const kHASH_EXPRESSION = `(${kHASH_ALGO})-(${kHASH_VALUE})`;
24-
const kOPTION_EXPRESSION = `(${kVCHAR}*)`;
26+
// Ungrouped since unused
27+
const kOPTION_EXPRESSION = `(?:${kVCHAR}*)`;
2528
const kHASH_WITH_OPTIONS = `${kHASH_EXPRESSION}(?:[?](${kOPTION_EXPRESSION}))?`;
2629
const kSRIPattern = RegExp(`(${kWSP}*)(?:${kHASH_WITH_OPTIONS})`, 'g');
2730
ObjectSeal(kSRIPattern);
2831
const kAllWSP = RegExp(`^${kWSP}*$`);
2932
ObjectSeal(kAllWSP);
3033

3134
const BufferFrom = require('buffer').Buffer.from;
35+
const RealArrayPrototype = ObjectGetPrototypeOf([]);
3236

37+
// Returns {algorithm, value (in base64 string), options,}[]
3338
const parse = (str) => {
34-
kSRIPattern.lastIndex = 0;
3539
let prevIndex = 0;
3640
let match;
3741
const entries = [];
@@ -62,7 +66,7 @@ const parse = (str) => {
6266
throw new ERR_SRI_PARSE(str, str.charAt(prevIndex), prevIndex);
6367
}
6468
}
65-
return entries;
69+
return ObjectSetPrototypeOf(entries, RealArrayPrototype);
6670
};
6771

6872
module.exports = {
+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
3+
require('../common');
4+
5+
const runBenchmark = require('../common/benchmark');
6+
7+
runBenchmark('policy', [
8+
'n=1',
9+
]);

0 commit comments

Comments
 (0)