Detect static securecookies to trivialize more rules (#16029)

utils/trivialize-rules/package.json

@@ -4,6 +4,7 @@
   "main": "trivialize-rules.js",
   "dependencies": {
     "chalk": "^2.1.0",
+    "escape-string-regexp": "^1.0.5",
     "highland": "^2.7.1",
     "regulex": "0.0.2",
     "xml2js": "^0.4.16"

utils/trivialize-rules/trivialize-rules.js

@@ -9,6 +9,7 @@ const readFile = _.wrapCallback(fs.readFile);
 const writeFile = _.wrapCallback(fs.writeFile);
 const parseXML = _.wrapCallback(require('xml2js').parseString);
 const { explodeRegExp, UnsupportedRegExp } = require('./explode-regexp');
+const escapeStringRegexp = require('escape-string-regexp');
 const chalk = require('chalk');
 
 const rulesDir = `${__dirname}/../../src/chrome/content/rules`;
@@ -99,6 +100,7 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
   const fail = createTag('FAIL', chalk.red, console.error);
 
   let targets = ruleset.target.map(target => target.$.host);
+  let securecookies = ruleset.securecookie ? ruleset.securecookie.map(sc => sc.$) : new Array();
   let rules = ruleset.rule.map(rule => rule.$);
 
   if (rules.length === 1 && isTrivial(rules[0])) {
@@ -181,9 +183,97 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
 
   domains = Array.from(domains);
 
+  // It is assumed that if all securecookies are static,
+  // they can be safely ignored.
+  //
+  // A securecookie is called to be static either it is a trivial securecookie
+  // or ALL of the following conditions are satisfied:
+  //
+  // 1. securecookie.host match cookie.host from the beginning ^ to the end $.
+  // Otherwise, it might match subdomains/ partial patterns, thus a non-trivial
+  // securecookie.
+  //
+  // 2. securecookie.host will not throw an error when passed to explodeRegExp().
+  // Otherwise, it might match patterns too complicated for our interests.
+  //
+  // 3. Each exploded securecookie.host should be included in ruleset.target/
+  // exploded target. Otherwise, this ruleset is likely problematic itself. It
+  // is dangerous for a rewrite.
+  function isStaticCookie(securecookie) {
+    if (securecookie.host === '.+' && securecookie.name === '.+') {
+      return [true, false];
+    }
+
+    if (!securecookie.host.startsWith('^') || !securecookie.host.endsWith('$')) {
+      return [false, false];
+    }
+
+    let localDomains = new Set();
+    let unsupportedDomains = new Set();
+
+    try {
+      explodeRegExp(securecookie.host, domain => {
+        if (domain.startsWith('.')) {
+          domain = domain.slice(1);
+        }
+        localDomains.add(domain);
+      });
+    } catch (e) {
+      if (!(e instanceof UnsupportedRegExp)) {
+        throw e;
+      }
+      warn`Unsupported regexp part ${e.message} while traversing securecookie : ${JSON.stringify(securecookie)}`;
+      return [false, false];
+    }
+
+    for (const domain of localDomains) {
+      if (domains.indexOf(domain) === -1) {
+        warn`Ruleset does not cover target ${domain} for securecookie : ${JSON.stringify(securecookie)}`;
+        unsupportedDomains.add(domain);
+      }
+    }
+
+    // For cookies to be covered, there must be at least one rule covering the
+    // same domain. This is guaranteed by safeToSecureCookie(cookie) in rules.js
+    //
+    // Since securecookie.host will only match cookie.domain if there there is
+    // a rule covers cookie.domain. Given the target are trivial, cookie.domain
+    // cannot be anything other than domain.example.com and .domain.example.com
+    // (possibly with more leading dots) for a securecookie rule ever to take
+    // place.
+    //
+    // With condition (1) effective, securecookie.host should explode to either
+    // one of the aforementioned patterns. Otherwise, the securecookie rules
+    // will never be applied. Such dangling securecookie rules can be removed
+    // safely.
+    if (unsupportedDomains.size > 0) {
+      if (unsupportedDomains.size === localDomains.size) {
+        return [true, true];
+      }
+      fail`Tag securecookie ${JSON.stringify(securecookie)} matches ${unsupportedDomains} which are not in targets ${targets}`;
+    }
+    return [true, false];
+  }
+
   if (domains.slice().sort().join('\n') !== targets.sort().join('\n')) {
-    if (ruleset.securecookie) {
-      return;
+    // For each securecookie rule, check if it is a static securecookie.
+    // If it is non-static, we do not trivialize the ruleset; Otherwise,
+    // we remove the securecookie if it contain only unsupported hosts.
+    // This removal is better done one by one to avoid side effects.
+    // Else if ALL securecookie rules are static, trivialize the targets.
+    for (const securecookie of securecookies) {
+      let [isStatic, shouldRemove] = isStaticCookie(securecookie);
+
+      if (isStatic) {
+        if (shouldRemove) {
+          let scReSrc = `\n([\t ]*)<securecookie\\s*host=\\s*"${escapeStringRegexp(securecookie.host)}"(\\s*)name=\\s*"${escapeStringRegexp(securecookie.name)}"\\s*?/>[\t ]*\n`;
+          let scRe = new RegExp(scReSrc);
+          source = source.replace(scRe, '');
+        }
+      } else {
+        // Skip this ruleset as it contain non-static securecookies
+        return;
+      }
     }
 
     source = replaceXML(source, 'target', domains.map(domain => `<target host="${domain}" />`));
@@ -194,7 +284,6 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
   info`trivialized`;
 
   return writeFile(`${rulesDir}/${name}`, source);
-
 })
   .filter(Boolean)
   .parallel(10)