Bug 924708 - Fix regression of report-only CSP's that use policy-uri. r=sstamm, a=lsblakk
authorGarrett Robinson <grobinson@mozilla.com>
Mon, 28 Oct 2013 11:25:16 -0700
changeset 166414 3c9f15727c3161b416c6254f287c5ea574185b7c
parent 166413 52b2f1c08ae2b4e003a00c4563b75f2afa1831b6
child 166415 6047fcf502a785b5380330acd4da4bf53b6c6c1d
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstamm, lsblakk
bugs924708
milestone27.0a2
Bug 924708 - Fix regression of report-only CSP's that use policy-uri. r=sstamm, a=lsblakk
content/base/src/CSPUtils.jsm
content/base/src/contentSecurityPolicy.js
content/base/test/csp/file_policyuri_regression_from_multipolicy.html
content/base/test/csp/file_policyuri_regression_from_multipolicy.html^headers^
content/base/test/csp/file_policyuri_regression_from_multipolicy_policy
content/base/test/csp/mochitest.ini
content/base/test/csp/test_policyuri_regression_from_multipolicy.html
content/base/test/unit/test_bug558431.js
testing/mochitest/b2g.json
--- a/content/base/src/CSPUtils.jsm
+++ b/content/base/src/CSPUtils.jsm
@@ -237,33 +237,33 @@ CSPRep.ALLOW_DIRECTIVE   = "allow";
 
 /**
   * Factory to create a new CSPRep, parsed from a string.
   *
   * @param aStr
   *        string rep of a CSP
   * @param self (optional)
   *        URI representing the "self" source
+  * @param reportOnly (optional)
+  *        whether or not this CSP is report-only (defaults to false)
   * @param docRequest (optional)
   *        request for the parent document which may need to be suspended
   *        while the policy-uri is asynchronously fetched
   * @param csp (optional)
   *        the CSP object to update once the policy has been fetched
-  * @param reportOnly (optional)
-  *        whether or not this CSP is report-only (defaults to false)
   * @returns
   *        an instance of CSPRep
   */
-CSPRep.fromString = function(aStr, self, docRequest, csp, reportOnly) {
-  if (typeof reportOnly === 'undefined') reportOnly = false;
+CSPRep.fromString = function(aStr, self, reportOnly, docRequest, csp) {
   var SD = CSPRep.SRC_DIRECTIVES_OLD;
   var UD = CSPRep.URI_DIRECTIVES;
   var aCSPR = new CSPRep();
   aCSPR._originalText = aStr;
   aCSPR._innerWindowID = innerWindowFromRequest(docRequest);
+  if (typeof reportOnly === 'undefined') reportOnly = false;
   aCSPR._reportOnlyMode = reportOnly;
 
   var selfUri = null;
   if (self instanceof Ci.nsIURI) {
     selfUri = self.cloneIgnoringRef();
     // clean userpass out of the URI (not used for CSP origin checking, but
     // shows up in prePath).
     try {
@@ -411,50 +411,50 @@ CSPRep.fromString = function(aStr, self,
       continue directive;
     }
 
     // POLICY URI //////////////////////////////////////////////////////////
     if (dirname === UD.POLICY_URI) {
       // POLICY_URI can only be alone
       if (aCSPR._directives.length > 0 || dirs.length > 1) {
         cspError(aCSPR, CSPLocalizer.getStr("policyURINotAlone"));
-        return CSPRep.fromString("default-src 'none'");
+        return CSPRep.fromString("default-src 'none'", null, reportOnly);
       }
       // if we were called without a reference to the parent document request
       // we won't be able to suspend it while we fetch the policy -> fail closed
       if (!docRequest || !csp) {
         cspError(aCSPR, CSPLocalizer.getStr("noParentRequest"));
-        return CSPRep.fromString("default-src 'none'");
+        return CSPRep.fromString("default-src 'none'", null, reportOnly);
       }
 
       var uri = '';
       try {
         uri = gIoService.newURI(dirvalue, null, selfUri);
       } catch(e) {
         cspError(aCSPR, CSPLocalizer.getFormatStr("policyURIParseError",
                                                   [dirvalue]));
-        return CSPRep.fromString("default-src 'none'");
+        return CSPRep.fromString("default-src 'none'", null, reportOnly);
       }
 
       // Verify that policy URI comes from the same origin
       if (selfUri) {
         if (selfUri.host !== uri.host) {
           cspError(aCSPR, CSPLocalizer.getFormatStr("nonMatchingHost",
                                                     [uri.host]));
-          return CSPRep.fromString("default-src 'none'");
+          return CSPRep.fromString("default-src 'none'", null, reportOnly);
         }
         if (selfUri.port !== uri.port) {
           cspError(aCSPR, CSPLocalizer.getFormatStr("nonMatchingPort",
                                                     [uri.port.toString()]));
-          return CSPRep.fromString("default-src 'none'");
+          return CSPRep.fromString("default-src 'none'", null, reportOnly);
         }
         if (selfUri.scheme !== uri.scheme) {
           cspError(aCSPR, CSPLocalizer.getFormatStr("nonMatchingScheme",
                                                     [uri.scheme]));
-          return CSPRep.fromString("default-src 'none'");
+          return CSPRep.fromString("default-src 'none'", null, reportOnly);
         }
       }
 
       // suspend the parent document request while we fetch the policy-uri
       try {
         docRequest.suspend();
         var chan = gIoService.newChannel(uri.asciiSpec, null, null);
         // make request anonymous (no cookies, etc.) so the request for the
@@ -463,68 +463,66 @@ CSPRep.fromString = function(aStr, self,
         chan.loadGroup = docRequest.loadGroup;
         chan.asyncOpen(new CSPPolicyURIListener(uri, docRequest, csp, reportOnly), null);
       }
       catch (e) {
         // resume the document request and apply most restrictive policy
         docRequest.resume();
         cspError(aCSPR, CSPLocalizer.getFormatStr("errorFetchingPolicy",
                                                   [e.toString()]));
-        return CSPRep.fromString("default-src 'none'");
+        return CSPRep.fromString("default-src 'none'", null, reportOnly);
       }
 
       // return a fully-open policy to be used until the contents of the
       // policy-uri come back.
-      return CSPRep.fromString("default-src *");
+      return CSPRep.fromString("default-src *", null, reportOnly);
     }
 
     // UNIDENTIFIED DIRECTIVE /////////////////////////////////////////////
     cspWarn(aCSPR, CSPLocalizer.getFormatStr("couldNotProcessUnknownDirective",
                                              [dirname]));
 
   } // end directive: loop
 
   // the X-Content-Security-Policy syntax requires an allow or default-src
   // directive to be present.
   if (!aCSPR._directives[SD.DEFAULT_SRC]) {
     cspWarn(aCSPR, CSPLocalizer.getStr("allowOrDefaultSrcRequired"));
-    return CSPRep.fromString("default-src 'none'", selfUri, docRequest, csp,
-                             reportOnly);
+    return CSPRep.fromString("default-src 'none'", null, reportOnly);
   }
   return aCSPR;
 };
 
 /**
   * Factory to create a new CSPRep, parsed from a string, compliant
   * with the CSP 1.0 spec.
   *
   * @param aStr
   *        string rep of a CSP
   * @param self (optional)
   *        URI representing the "self" source
+  * @param reportOnly (optional)
+  *        whether or not this CSP is report-only (defaults to false)
   * @param docRequest (optional)
   *        request for the parent document which may need to be suspended
   *        while the policy-uri is asynchronously fetched
   * @param csp (optional)
   *        the CSP object to update once the policy has been fetched
-  * @param reportOnly (optional)
-  *        whether or not this CSP is report-only (defaults to false)
   * @returns
   *        an instance of CSPRep
   */
 // When we deprecate our original CSP implementation, we rename this to
 // CSPRep.fromString and remove the existing CSPRep.fromString above.
-CSPRep.fromStringSpecCompliant = function(aStr, self, docRequest, csp, reportOnly) {
-  if (typeof reportOnly === 'undefined') reportOnly = false;
-
+CSPRep.fromStringSpecCompliant = function(aStr, self, reportOnly, docRequest, csp) {
   var SD = CSPRep.SRC_DIRECTIVES_NEW;
   var UD = CSPRep.URI_DIRECTIVES;
   var aCSPR = new CSPRep(true);
   aCSPR._originalText = aStr;
   aCSPR._innerWindowID = innerWindowFromRequest(docRequest);
+  if (typeof reportOnly === 'undefined') reportOnly = false;
   aCSPR._reportOnlyMode = reportOnly;
 
   var selfUri = null;
   if (self instanceof Ci.nsIURI) {
     selfUri = self.cloneIgnoringRef();
     // clean userpass out of the URI (not used for CSP origin checking, but
     // shows up in prePath).
     try {
@@ -677,69 +675,69 @@ CSPRep.fromStringSpecCompliant = functio
       continue directive;
     }
 
     // POLICY URI //////////////////////////////////////////////////////////
     if (dirname === UD.POLICY_URI) {
       // POLICY_URI can only be alone
       if (aCSPR._directives.length > 0 || dirs.length > 1) {
         cspError(aCSPR, CSPLocalizer.getStr("policyURINotAlone"));
-        return CSPRep.fromStringSpecCompliant("default-src 'none'");
+        return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
       }
       // if we were called without a reference to the parent document request
       // we won't be able to suspend it while we fetch the policy -> fail closed
       if (!docRequest || !csp) {
         cspError(aCSPR, CSPLocalizer.getStr("noParentRequest"));
-        return CSPRep.fromStringSpecCompliant("default-src 'none'");
+        return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
       }
 
       var uri = '';
       try {
         uri = gIoService.newURI(dirvalue, null, selfUri);
       } catch(e) {
         cspError(aCSPR, CSPLocalizer.getFormatStr("policyURIParseError", [dirvalue]));
-        return CSPRep.fromStringSpecCompliant("default-src 'none'");
+        return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
       }
 
       // Verify that policy URI comes from the same origin
       if (selfUri) {
         if (selfUri.host !== uri.host){
           cspError(aCSPR, CSPLocalizer.getFormatStr("nonMatchingHost", [uri.host]));
-          return CSPRep.fromStringSpecCompliant("default-src 'none'");
+          return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
         }
         if (selfUri.port !== uri.port){
           cspError(aCSPR, CSPLocalizer.getFormatStr("nonMatchingPort", [uri.port.toString()]));
-          return CSPRep.fromStringSpecCompliant("default-src 'none'");
+          return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
         }
         if (selfUri.scheme !== uri.scheme){
           cspError(aCSPR, CSPLocalizer.getFormatStr("nonMatchingScheme", [uri.scheme]));
-          return CSPRep.fromStringSpecCompliant("default-src 'none'");
+          return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
         }
       }
 
       // suspend the parent document request while we fetch the policy-uri
       try {
         docRequest.suspend();
         var chan = gIoService.newChannel(uri.asciiSpec, null, null);
         // make request anonymous (no cookies, etc.) so the request for the
         // policy-uri can't be abused for CSRF
         chan.loadFlags |= Components.interfaces.nsIChannel.LOAD_ANONYMOUS;
         chan.loadGroup = docRequest.loadGroup;
-        chan.asyncOpen(new CSPPolicyURIListener(uri, docRequest, csp), null);
+        chan.asyncOpen(new CSPPolicyURIListener(uri, docRequest, csp, reportOnly), null);
       }
       catch (e) {
         // resume the document request and apply most restrictive policy
         docRequest.resume();
         cspError(aCSPR, CSPLocalizer.getFormatStr("errorFetchingPolicy", [e.toString()]));
-        return CSPRep.fromStringSpecCompliant("default-src 'none'");
+        return CSPRep.fromStringSpecCompliant("default-src 'none'", null, reportOnly);
       }
 
       // return a fully-open policy to be used until the contents of the
       // policy-uri come back
-      return CSPRep.fromStringSpecCompliant("default-src *");
+      return CSPRep.fromStringSpecCompliant("default-src *", null, reportOnly);
     }
 
     // UNIDENTIFIED DIRECTIVE /////////////////////////////////////////////
     cspWarn(aCSPR, CSPLocalizer.getFormatStr("couldNotProcessUnknownDirective", [dirname]));
 
   } // end directive: loop
 
   return aCSPR;
--- a/content/base/src/contentSecurityPolicy.js
+++ b/content/base/src/contentSecurityPolicy.js
@@ -306,25 +306,25 @@ ContentSecurityPolicy.prototype = {
     // CSPSource only the scheme, host, and port are kept.
 
     // If we want to be CSP 1.0 spec compliant, use the new parser.
     // The old one will be deprecated in the future and will be
     // removed at that time.
     if (aSpecCompliant) {
       newpolicy = CSPRep.fromStringSpecCompliant(aPolicy,
                                                  selfURI,
+                                                 aReportOnly,
                                                  this._docRequest,
-                                                 this,
-                                                 aReportOnly);
+                                                 this);
     } else {
       newpolicy = CSPRep.fromString(aPolicy,
                                     selfURI,
+                                    aReportOnly,
                                     this._docRequest,
-                                    this,
-                                    aReportOnly);
+                                    this);
     }
 
     newpolicy._specCompliant = !!aSpecCompliant;
     newpolicy._isInitialized = true;
     this._policies.push(newpolicy);
     this._cache = {}; // reset cache since effective policy changes
   },
 
new file mode 100644
--- /dev/null
+++ b/content/base/test/csp/file_policyuri_regression_from_multipolicy.html
@@ -0,0 +1,9 @@
+<!doctype html>
+<html>
+  <body>
+    <div id=testdiv>Inline script didn't run</div>
+    <script>
+      document.getElementById('testdiv').innerHTML = "Inline Script Executed";
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/content/base/test/csp/file_policyuri_regression_from_multipolicy.html^headers^
@@ -0,0 +1,1 @@
+content-security-policy-report-only: policy-uri /tests/content/base/test/csp/file_CSP_policyuri_regression_from_multipolicy_policy
new file mode 100644
--- /dev/null
+++ b/content/base/test/csp/file_policyuri_regression_from_multipolicy_policy
@@ -0,0 +1,1 @@
+default-src 'self';
--- a/content/base/test/csp/mochitest.ini
+++ b/content/base/test/csp/mochitest.ini
@@ -77,16 +77,19 @@ support-files =
   file_csp_redirects_resource.sjs
   file_CSP_bug910139.sjs
   file_CSP_bug910139.xml
   file_CSP_bug910139.xsl
   file_CSP_bug909029_star.html
   file_CSP_bug909029_star.html^headers^
   file_CSP_bug909029_none.html
   file_CSP_bug909029_none.html^headers^
+  file_policyuri_regression_from_multipolicy.html
+  file_policyuri_regression_from_multipolicy.html^headers^
+  file_policyuri_regression_from_multipolicy_policy
 
 [test_CSP.html]
 [test_CSP_bug663567.html]
 [test_CSP_bug802872.html]
 [test_CSP_bug885433.html]
 [test_CSP_bug888172.html]
 [test_CSP_bug916446.html]
 [test_CSP_evalscript.html]
@@ -94,8 +97,9 @@ support-files =
 [test_CSP_frameancestors.html]
 [test_CSP_inlinescript.html]
 [test_CSP_inlinestyle.html]
 [test_bothCSPheaders.html]
 [test_bug836922_npolicies.html]
 [test_csp_redirects.html]
 [test_CSP_bug910139.html]
 [test_CSP_bug909029.html]
+[test_policyuri_regression_from_multipolicy.html]
new file mode 100644
--- /dev/null
+++ b/content/base/test/csp/test_policyuri_regression_from_multipolicy.html
@@ -0,0 +1,29 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for Bug 924708</title>
+   <!--
+   test that a report-only policy that uses policy-uri is not incorrectly
+   enforced due to regressions introduced by Bug 836922.
+   -->
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<iframe style="width:200px;height:200px;" id='testframe'></iframe>
+<script class="testbody" type="text/javascript">
+SpecialPowers.pushPrefEnv(
+  {'set':[["security.csp.speccompliant", true]]},
+  function() {
+    var testframe = document.getElementById('testframe');
+    testframe.src = 'file_policyuri_regression_from_multipolicy.html';
+    testframe.addEventListener('load', function checkInlineScriptExecuted () {
+      is(this.contentDocument.getElementById('testdiv').innerHTML,
+         'Inline Script Executed',
+         'Inline script should execute (it would be blocked by the policy, but the policy is report-only)');
+    });
+  }
+);
+</script>
+</body>
+</html>
--- a/content/base/test/unit/test_bug558431.js
+++ b/content/base/test/unit/test_bug558431.js
@@ -134,17 +134,17 @@ function test_CSPRep_fromPolicyURI() {
   // simulates the request for the parent document
   var docChan = makeChan(DOCUMENT_URI);
   docChan.asyncOpen(new listener(csp, cspr_static), null);
 
   // the resulting policy here can be discarded, since it's going to be
   // "allow *"; when the policy-uri fetching call-back happens, the *real*
   // policy will be in csp.policy
   CSPRep.fromString("policy-uri " + POLICY_URI,
-                    mkuri(DOCUMENT_URI), docChan, csp);
+                    mkuri(DOCUMENT_URI), false, docChan, csp);
 }
 
 function test_CSPRep_fromRelativePolicyURI() {
   do_test_pending();
   let csp = Cc["@mozilla.org/contentsecuritypolicy;1"]
               .createInstance(Ci.nsIContentSecurityPolicy);
   // once the policy-uri is returned we will compare our static CSPRep with one
   // we generated from the content we got back from the network to make sure
@@ -154,10 +154,10 @@ function test_CSPRep_fromRelativePolicyU
   // simulates the request for the parent document
   var docChan = makeChan(DOCUMENT_URI);
   docChan.asyncOpen(new listener(csp, cspr_static), null);
 
   // the resulting policy here can be discarded, since it's going to be
   // "allow *"; when the policy-uri fetching call-back happens, the *real*
   // policy will be in csp.policy
   CSPRep.fromString("policy-uri " + POLICY_URI_RELATIVE,
-                    mkuri(DOCUMENT_URI), docChan, csp);
+                    mkuri(DOCUMENT_URI), false, docChan, csp);
 }
--- a/testing/mochitest/b2g.json
+++ b/testing/mochitest/b2g.json
@@ -205,16 +205,17 @@
 
     "content/base/test/csp/test_CSP_evalscript.html":"observer not working",
     "content/base/test/csp/test_CSP_evalscript_getCRMFRequest.html":"observer not working",
     "content/base/test/csp/test_CSP_frameancestors.html":"observer not working",
     "content/base/test/csp/test_CSP.html":"observer not working",
     "content/base/test/csp/test_bug836922_npolicies.html":"observer not working",
     "content/base/test/csp/test_CSP_bug916446.html":"observer not working",
     "content/base/test/csp/test_CSP_bug909029.html":"observer not working",
+    "content/base/test/csp/test_policyuri_regression_from_multipolicy.html":"observer not working",
 
     "content/base/test/test_CrossSiteXHR_origin.html":"https not working, bug 907770",
     "content/base/test/test_plugin_freezing.html":"",
     "content/base/test/test_bug466409.html":"",
     "content/base/test/test_bug482935.html":"",
     "content/base/test/test_bug498433.html":"",
     "content/base/test/test_bug650386_redirect_301.html":"",
     "content/base/test/test_bug650386_redirect_302.html":"",