First attempt to not secure cookies on HTTP origins

pde · pde · commit 4fb5cfbf7383 · 2012-12-11T18:26:27.000Z
https://trac.torproject.org/projects/tor/ticket/7491
diff --git a/src/chrome/content/code/HTTPS.js b/src/chrome/content/code/HTTPS.js
@@ -50,6 +50,9 @@ const HTTPS = {
         https_everywhere_blacklist[channel.URI.spec] = true;
       }
       https_everywhere_blacklist[channel.URI.spec] = blob.applied_ruleset;
+      var domain = null;
+      try { domain = channel.URI.host; } catch (e) {}
+      if (domain) https_blacklist_domains[domain] = true;
       return false;
     }
 
@@ -224,7 +227,7 @@ const HTTPS = {
       for each (var cs in cookies.split("\n")) {
         this.log(DBUG, "Examining cookie: ");
         c = new Cookie(cs, host);
-        if (!c.secure && HTTPSRules.shouldSecureCookie(alist, c)) {
+        if (!c.secure && HTTPSRules.shouldSecureCookie(alist, c, true)) {
           this.log(INFO, "Securing cookie: " + c.domain + " " + c.name);
           c.secure = true;
           req.setResponseHeader("Set-Cookie", c.source + ";Secure", true);
@@ -235,7 +238,7 @@ const HTTPS = {
   },
 
   handleInsecureCookie: function(c) {
-    if (HTTPSRules.shouldSecureCookie(null, c)) {
+    if (HTTPSRules.shouldSecureCookie(null, c, false)) {
       this.log(INFO, "Securing cookie from event: " + c.domain + " " + c.name);
       var cookieManager = Components.classes["@mozilla.org/cookiemanager;1"]
                             .getService(Components.interfaces.nsICookieManager2);
diff --git a/src/chrome/content/code/HTTPSRules.js b/src/chrome/content/code/HTTPSRules.js
@@ -92,13 +92,14 @@ RuleSet.prototype = {
  wouldMatch: function(hypothetical_uri, alist) {
    // return true if this ruleset would match the uri, assuming it were http
    // used for judging moot / inactive rulesets
+   // alist is optional
 
    // if the ruleset is already somewhere in this applicable list, we don't
-   // care about hypothetical wouldMatch questios
-   if (this.name in alist.all) return false;
+   // care about hypothetical wouldMatch questions
+   if (alist && (this.name in alist.all)) return false;
 
    this.log(DBUG,"Would " +this.name + " match " +hypothetical_uri.spec +
-            "?  serial " + alist.serial);
+            "?  serial " + (alist && alist.serial));
     
    var uri = hypothetical_uri.clone();
    if (uri.scheme == "https") uri.scheme = "http";
@@ -587,19 +588,25 @@ const HTTPSRules = {
     return results;
   },
 
-  shouldSecureCookie: function(applicable_list, c) {
+  shouldSecureCookie: function(applicable_list, c, known_https) {
     // Check to see if the Cookie object c meets any of our cookierule citeria
-    // for being marked as secure
+    // for being marked as secure.
+    // @applicable_list : an ApplicableList or record keeping
+    // @c : an nsICookie2
+    // @known_https : true if we know the page setting the cookie is https
     //this.log(DBUG, "Testing cookie:");
     //this.log(DBUG, "  name: " + c.name);
     //this.log(DBUG, "  host: " + c.host);
     //this.log(DBUG, "  domain: " + c.domain);
-    //this.log(DBUG, "  rawhost: " + c.rawHost);
+    this.log(DBUG, "  rawhost: " + c.rawHost);
     var i,j;
     var rs = this.potentiallyApplicableRulesets(c.host);
     for (i = 0; i < rs.length; ++i) {
       var ruleset = rs[i];
       if (ruleset.active) {
+        // Never secure a cookie if this page might be HTTP
+        if (!known_https && !this.safeToSecureCookie(c.rawhost)) 
+          continue;
         for (j = 0; j < ruleset.cookierules.length; j++) {
           var cr = ruleset.cookierules[j];
           if (cr.host_c.test(c.host) && cr.name_c.test(c.name)) {
@@ -616,6 +623,36 @@ const HTTPSRules = {
       }
     }
     return false;
+  },
+
+  safeToSecureCookie(domain) {
+    // Check if the domain might be being served over HTTP.  If so, it isn't
+    // safe to secure a cookie!  We can't always know this for sure because
+    // observing cookie-changed doesn't give us enough context to know the
+    // full origin URI.
+
+    // If there are any redirect loops on this domain, don't secure cookies.
+    // XXX This is not a very satisfactory heuristic.  Sometimes we would want
+    // to secure the cookie anyway, because the URLs that loop are not
+    // authenticated or not important.  Also by the time 
+
+    if (domain in https_blacklist_domains) return false;
+
+    // If we passed that test, make up a random URL on the domain, and see if
+    // we would HTTPSify that.
+
+    var ios = CC['@mozilla.org/network/io-service;1']
+              .getService(CI.nsIIOService);
+    try {
+      var nonce_path = Math.random().toString() + "/" + Math.random().toString();
+      var test_uri = ios.newURI("http://" + nonce_path, "UTF-8", null);
+    } catch (e) {
+      this.log(WARN, "explosion in safeToSecureCookie for " + domain + "\n" 
+                      + "(" + e + ")");
+      return false;
+    }
+    return wouldMatch(test_uri, null);
   }
 
+
 };
diff --git a/src/components/https-everywhere.js b/src/components/https-everywhere.js
@@ -14,6 +14,9 @@ https_domains = {};              // maps domain patterns (with at most one
 https_everywhere_blacklist = {}; // URLs we've given up on rewriting because
                                  // of redirection loops
 
+https_blacklist_domains = {};    // domains for which there is at least one
+                                 // blacklisted URL
+
 //
 const CI = Components.interfaces;
 const CC = Components.classes;
