Skip to content

Commit

Permalink
policy: add startup benchmark and make SRI lazier
Browse files Browse the repository at this point in the history
PR-URL: #29527
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bfarias-godaddy authored and bmeck committed Jul 14, 2020
1 parent e212955 commit 66810a0
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 33 deletions.
51 changes: 51 additions & 0 deletions benchmark/policy/policy-startup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Tests the impact on eager operations required for policies affecting
// general startup, does not test lazy operations
'use strict';
const common = require('../common.js');

const configs = {
n: [1024]
};

const options = {
flags: ['--expose-internals']
};

const bench = common.createBenchmark(main, configs, options);

function main(conf) {
const hash = (str, algo) => {
const hash = require('crypto').createHash(algo);
return hash.update(str).digest('base64');
};
const resources = Object.fromEntries(
// Simulate graph of 1k modules
Array.from({ length: 1024 }, (_, i) => {
return [`./_${i}`, {
integrity: `sha256-${hash(`// ./_${i}`, 'sha256')}`,
dependencies: Object.fromEntries(Array.from({
// Average 3 deps per 4 modules
length: Math.floor((i % 4) / 2)
}, (_, ii) => {
return [`_${ii}`, `./_${i - ii}`];
})),
}];
})
);
const json = JSON.parse(JSON.stringify({ resources }), (_, o) => {
if (o && typeof o === 'object') {
Reflect.setPrototypeOf(o, null);
Object.freeze(o);
}
return o;
});
const { Manifest } = require('internal/policy/manifest');

bench.start();

for (let i = 0; i < conf.n; i++) {
new Manifest(json, 'file://benchmark/policy-relative');
}

bench.end(conf.n);
}
78 changes: 50 additions & 28 deletions lib/internal/policy/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const {
ArrayIsArray,
Map,
MapPrototypeSet,
ObjectCreate,
ObjectEntries,
ObjectFreeze,
ObjectSetPrototypeOf,
Expand Down Expand Up @@ -31,7 +32,6 @@ const { URL } = require('internal/url');
const { createHash, timingSafeEqual } = crypto;
const HashUpdate = uncurryThis(crypto.Hash.prototype.update);
const HashDigest = uncurryThis(crypto.Hash.prototype.digest);
const BufferEquals = uncurryThis(Buffer.prototype.equals);
const BufferToString = uncurryThis(Buffer.prototype.toString);
const kRelativeURLStringPattern = /^\.{0,2}\//;
const { getOptionValue } = require('internal/options');
Expand All @@ -56,9 +56,47 @@ function REACTION_LOG(error) {
}

class Manifest {
/**
* @type {Map<string, true | string | SRI[]>}
*
* Used to compare a resource to the content body at the resource.
* `true` is used to signify that all integrities are allowed, otherwise,
* SRI strings are parsed to compare with the body.
*
* This stores strings instead of eagerly parsing SRI strings
* and only converts them to SRI data structures when needed.
* This avoids needing to parse all SRI strings at startup even
* if some never end up being used.
*/
#integrities = new SafeMap();
/**
* @type {Map<string, (specifier: string) => true | URL>}
*
* Used to find where a dependency is located.
*
* This stores functions to lazily calculate locations as needed.
* `true` is used to signify that the location is not specified
* by the manifest and default resolution should be allowed.
*/
#dependencies = new SafeMap();
/**
* @type {(err: Error) => void}
*
* Performs default action for what happens when a manifest encounters
* a violation such as abort()ing or exiting the process, throwing the error,
* or logging the error.
*/
#reaction = null;
/**
* `obj` should match the policy file format described in the docs
* it is expected to not have prototype pollution issues either by reassigning
* the prototype to `null` for values or by running prior to any user code.
*
* `manifestURL` is a URL to resolve relative locations against.
*
* @param {object} obj
* @param {string} manifestURL
*/
constructor(obj, manifestURL) {
const integrities = this.#integrities;
const dependencies = this.#dependencies;
Expand Down Expand Up @@ -98,35 +136,14 @@ class Manifest {
let integrity = manifestEntries[i][1].integrity;
if (!integrity) integrity = null;
if (integrity != null) {
debug(`Manifest contains integrity for url ${originalHREF}`);
debug('Manifest contains integrity for url %s', originalHREF);
if (typeof integrity === 'string') {
const sri = ObjectFreeze(SRI.parse(integrity));
if (integrities.has(resourceHREF)) {
const old = integrities.get(resourceHREF);
let mismatch = false;

if (old.length !== sri.length) {
mismatch = true;
} else {
compare:
for (let sriI = 0; sriI < sri.length; sriI++) {
for (let oldI = 0; oldI < old.length; oldI++) {
if (sri[sriI].algorithm === old[oldI].algorithm &&
BufferEquals(sri[sriI].value, old[oldI].value) &&
sri[sriI].options === old[oldI].options) {
continue compare;
}
}
mismatch = true;
break compare;
}
}

if (mismatch) {
if (integrities.get(resourceHREF) !== integrity) {
throw new ERR_MANIFEST_INTEGRITY_MISMATCH(resourceURL);
}
}
integrities.set(resourceHREF, sri);
integrities.set(resourceHREF, integrity);
} else if (integrity === true) {
integrities.set(resourceHREF, true);
} else {
Expand All @@ -138,7 +155,7 @@ class Manifest {

let dependencyMap = manifestEntries[i][1].dependencies;
if (dependencyMap === null || dependencyMap === undefined) {
dependencyMap = {};
dependencyMap = ObjectCreate(null);
}
if (typeof dependencyMap === 'object' && !ArrayIsArray(dependencyMap)) {
/**
Expand Down Expand Up @@ -200,13 +217,18 @@ class Manifest {

assertIntegrity(url, content) {
const href = `${url}`;
debug(`Checking integrity of ${href}`);
debug('Checking integrity of %s', href);
const integrities = this.#integrities;
const realIntegrities = new Map();

if (integrities.has(href)) {
const integrityEntries = integrities.get(href);
let integrityEntries = integrities.get(href);
if (integrityEntries === true) return true;
if (typeof integrityEntries === 'string') {
const sri = ObjectFreeze(SRI.parse(integrityEntries));
integrities.set(href, sri);
integrityEntries = sri;
}
// Avoid clobbered Symbol.iterator
for (let i = 0; i < integrityEntries.length; i++) {
const {
Expand Down
14 changes: 9 additions & 5 deletions lib/internal/policy/sri.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
'use strict';
// Value of https://w3c.github.io/webappsec-subresource-integrity/#the-integrity-attribute
// Utility to parse the value of
// https://w3c.github.io/webappsec-subresource-integrity/#the-integrity-attribute

const {
ObjectDefineProperty,
ObjectFreeze,
ObjectGetPrototypeOf,
ObjectSeal,
ObjectSetPrototypeOf,
RegExp,
RegExpPrototypeExec,
RegExpPrototypeTest,
StringPrototypeSlice,
} = primordials;

// Returns [{algorithm, value (in base64 string), options,}]
const {
ERR_SRI_PARSE
} = require('internal/errors').codes;
Expand All @@ -21,17 +23,19 @@ const kHASH_ALGO = 'sha(?:256|384|512)';
// Base64
const kHASH_VALUE = '[A-Za-z0-9+/]+[=]{0,2}';
const kHASH_EXPRESSION = `(${kHASH_ALGO})-(${kHASH_VALUE})`;
const kOPTION_EXPRESSION = `(${kVCHAR}*)`;
// Ungrouped since unused
const kOPTION_EXPRESSION = `(?:${kVCHAR}*)`;
const kHASH_WITH_OPTIONS = `${kHASH_EXPRESSION}(?:[?](${kOPTION_EXPRESSION}))?`;
const kSRIPattern = RegExp(`(${kWSP}*)(?:${kHASH_WITH_OPTIONS})`, 'g');
ObjectSeal(kSRIPattern);
const kAllWSP = RegExp(`^${kWSP}*$`);
ObjectSeal(kAllWSP);

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

// Returns {algorithm, value (in base64 string), options,}[]
const parse = (str) => {
kSRIPattern.lastIndex = 0;
let prevIndex = 0;
let match;
const entries = [];
Expand Down Expand Up @@ -62,7 +66,7 @@ const parse = (str) => {
throw new ERR_SRI_PARSE(str, str.charAt(prevIndex), prevIndex);
}
}
return entries;
return ObjectSetPrototypeOf(entries, RealArrayPrototype);
};

module.exports = {
Expand Down
9 changes: 9 additions & 0 deletions test/benchmark/test-benchmark-policy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict';

require('../common');

const runBenchmark = require('../common/benchmark');

runBenchmark('policy', [
'n=1',
]);

0 comments on commit 66810a0

Please sign in to comment.