policy: add startup benchmark and make SRI lazier

PR-URL: #29527
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: bfarias-godaddy

benchmark/policy/policy-startup.js

@@ -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);
+}

lib/internal/policy/manifest.js

@@ -4,6 +4,7 @@ const {
   ArrayIsArray,
   Map,
   MapPrototypeSet,
+  ObjectCreate,
   ObjectEntries,
   ObjectFreeze,
   ObjectSetPrototypeOf,
@@ -29,7 +30,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');
@@ -54,9 +54,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;
@@ -96,35 +134,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 {
@@ -136,7 +153,7 @@ class Manifest {
 
       let dependencyMap = manifestEntries[i][1].dependencies;
       if (dependencyMap === null || dependencyMap === undefined) {
-        dependencyMap = {};
+        dependencyMap = ObjectCreate(null);
       }
       if (typeof dependencyMap === 'object' && !ArrayIsArray(dependencyMap)) {
         /**
@@ -198,13 +215,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 {

lib/internal/policy/sri.js

@@ -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;
@@ -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 = [];
@@ -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 = {

test/benchmark/test-benchmark-policy.js

@@ -0,0 +1,9 @@
+'use strict';
+
+require('../common');
+
+const runBenchmark = require('../common/benchmark');
+
+runBenchmark('policy', [
+  'n=1',
+]);