Skip to content

Commit 02ea320

Browse files
bfarias-godaddyMylesBorins
authored andcommitted
policy: add startup benchmark and make SRI lazier
PR-URL: #29527 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 17174e6 commit 02ea320

4 files changed

Lines changed: 119 additions & 33 deletions

File tree

‎benchmark/policy/policy-startup.js‎

Lines changed: 51 additions & 0 deletions
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+
constcommon=require('../common.js');
5+
6+
constconfigs={
7+
n: [1024]
8+
};
9+
10+
constoptions={
11+
flags: ['--expose-internals']
12+
};
13+
14+
constbench=common.createBenchmark(main,configs,options);
15+
16+
functionmain(conf){
17+
consthash=(str,algo)=>{
18+
consthash=require('crypto').createHash(algo);
19+
returnhash.update(str).digest('base64');
20+
};
21+
constresources=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+
constjson=JSON.parse(JSON.stringify({ resources }),(_,o)=>{
36+
if(o&&typeofo==='object'){
37+
Reflect.setPrototypeOf(o,null);
38+
Object.freeze(o);
39+
}
40+
returno;
41+
});
42+
const{ Manifest }=require('internal/policy/manifest');
43+
44+
bench.start();
45+
46+
for(leti=0;i<conf.n;i++){
47+
newManifest(json,'file://benchmark/policy-relative');
48+
}
49+
50+
bench.end(conf.n);
51+
}

‎lib/internal/policy/manifest.js‎

Lines changed: 50 additions & 28 deletions
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
constHashUpdate=uncurryThis(crypto.Hash.prototype.update);
3334
constHashDigest=uncurryThis(crypto.Hash.prototype.digest);
34-
constBufferEquals=uncurryThis(Buffer.prototype.equals);
3535
constBufferToString=uncurryThis(Buffer.prototype.toString);
3636
constkRelativeURLStringPattern=/^\.{0,2}\//;
3737
const{ getOptionValue }=require('internal/options');
@@ -56,9 +56,47 @@ function REACTION_LOG(error) {
5656
}
5757

5858
classManifest{
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 =newSafeMap();
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 =newSafeMap();
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
constintegrities=this.#integrities;
64102
constdependencies=this.#dependencies;
@@ -98,35 +136,14 @@ class Manifest {
98136
letintegrity=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(typeofintegrity==='string'){
103-
constsri=ObjectFreeze(SRI.parse(integrity));
104141
if(integrities.has(resourceHREF)){
105-
constold=integrities.get(resourceHREF);
106-
letmismatch=false;
107-
108-
if(old.length!==sri.length){
109-
mismatch=true;
110-
}else{
111-
compare:
112-
for(letsriI=0;sriI<sri.length;sriI++){
113-
for(letoldI=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
thrownewERR_MANIFEST_INTEGRITY_MISMATCH(resourceURL);
127144
}
128145
}
129-
integrities.set(resourceHREF,sri);
146+
integrities.set(resourceHREF,integrity);
130147
}elseif(integrity===true){
131148
integrities.set(resourceHREF,true);
132149
}else{
@@ -138,7 +155,7 @@ class Manifest {
138155

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

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

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

‎lib/internal/policy/sri.js‎

Lines changed: 9 additions & 5 deletions
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
constkHASH_VALUE='[A-Za-z0-9+/]+[=]{0,2}';
2325
constkHASH_EXPRESSION=`(${kHASH_ALGO})-(${kHASH_VALUE})`;
24-
constkOPTION_EXPRESSION=`(${kVCHAR}*)`;
26+
// Ungrouped since unused
27+
constkOPTION_EXPRESSION=`(?:${kVCHAR}*)`;
2528
constkHASH_WITH_OPTIONS=`${kHASH_EXPRESSION}(?:[?](${kOPTION_EXPRESSION}))?`;
2629
constkSRIPattern=RegExp(`(${kWSP}*)(?:${kHASH_WITH_OPTIONS})`,'g');
2730
ObjectSeal(kSRIPattern);
2831
constkAllWSP=RegExp(`^${kWSP}*$`);
2932
ObjectSeal(kAllWSP);
3033

3134
constBufferFrom=require('buffer').Buffer.from;
35+
constRealArrayPrototype=ObjectGetPrototypeOf([]);
3236

37+
// Returns {algorithm, value (in base64 string), options,}[]
3338
constparse=(str)=>{
34-
kSRIPattern.lastIndex=0;
3539
letprevIndex=0;
3640
letmatch;
3741
constentries=[];
@@ -62,7 +66,7 @@ const parse = (str) => {
6266
thrownewERR_SRI_PARSE(str,str.charAt(prevIndex),prevIndex);
6367
}
6468
}
65-
returnentries;
69+
returnObjectSetPrototypeOf(entries,RealArrayPrototype);
6670
};
6771

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

0 commit comments

Comments
 (0)