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: add startup benchmark and make SRI lazier #29527

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
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',
]);