Bug 1196740 - Consider redirects when looking for SRI-eligibility. r=ckerschb
authorFrancois Marier <francois@mozilla.com>
Tue, 25 Aug 2015 13:38:39 -0700
changeset 259326 793bc02002203267dfa825221fe0b6a18faf54ad
parent 259325 42a4c4b8d8534268ba1ef26ac0ec89df0ce4b6dc
child 259327 3a5257ac933dce09eb38c8f3615aa14ce996f1ee
push id29277
push userryanvm@gmail.com
push dateWed, 26 Aug 2015 18:32:23 +0000
treeherdermozilla-central@fea87cbeaa6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb
bugs1196740
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1196740 - Consider redirects when looking for SRI-eligibility. r=ckerschb
dom/base/nsScriptLoader.cpp
dom/security/SRICheck.cpp
dom/security/SRICheck.h
dom/security/test/sri/iframe_script_crossdomain.html
dom/security/test/sri/iframe_style_crossdomain.html
dom/security/test/sri/mochitest.ini
dom/security/test/sri/script_301.js
dom/security/test/sri/script_301.js^headers^
dom/security/test/sri/script_crossdomain5.js
dom/security/test/sri/script_crossdomain5.js^headers^
dom/security/test/sri/style_301.css
dom/security/test/sri/style_301.css^headers^
dom/security/test/sri/test_style_crossdomain.html
layout/style/Loader.cpp
--- a/dom/base/nsScriptLoader.cpp
+++ b/dom/base/nsScriptLoader.cpp
@@ -1431,20 +1431,26 @@ nsScriptLoader::OnStreamComplete(nsIStre
                                  nsresult aStatus,
                                  uint32_t aStringLen,
                                  const uint8_t* aString)
 {
   nsScriptLoadRequest* request = static_cast<nsScriptLoadRequest*>(aContext);
   NS_ASSERTION(request, "null request in stream complete handler");
   NS_ENSURE_TRUE(request, NS_ERROR_FAILURE);
 
+  nsCOMPtr<nsIHttpChannel> httpChannel;
+  {
+    nsCOMPtr<nsIRequest> req;
+    aLoader->GetRequest(getter_AddRefs(req));
+    httpChannel = do_QueryInterface(req);
+  } // throw away req, we only need the channel
+
   nsresult rv = NS_ERROR_SRI_CORRUPT;
   if (request->mIntegrity.IsEmpty() ||
-      NS_SUCCEEDED(SRICheck::VerifyIntegrity(request->mIntegrity,
-                                             request->mURI,
+      NS_SUCCEEDED(SRICheck::VerifyIntegrity(request->mIntegrity, httpChannel,
                                              request->mCORSMode, aStringLen,
                                              aString, mDocument))) {
     rv = PrepareLoadedRequest(request, aLoader, aStatus, aStringLen, aString);
   }
 
   if (NS_FAILED(rv)) {
     /*
      * Handle script not loading error because source was a tracking URL.
--- a/dom/security/SRICheck.cpp
+++ b/dom/security/SRICheck.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SRICheck.h"
 
 #include "mozilla/Base64.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Preferences.h"
 #include "nsContentUtils.h"
+#include "nsIChannel.h"
 #include "nsICryptoHash.h"
 #include "nsIDocument.h"
 #include "nsIProtocolHandler.h"
 #include "nsIScriptError.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsIURI.h"
 #include "nsNetUtil.h"
 #include "nsWhitespaceTokenizer.h"
@@ -36,48 +37,58 @@ namespace mozilla {
 namespace dom {
 
 /**
  * Returns whether or not the sub-resource about to be loaded is eligible
  * for integrity checks. If it's not, the checks will be skipped and the
  * sub-resource will be loaded.
  */
 static nsresult
-IsEligible(nsIURI* aRequestURI, const CORSMode aCORSMode,
+IsEligible(nsIChannel* aChannel, const CORSMode aCORSMode,
            const nsIDocument* aDocument)
 {
-  NS_ENSURE_ARG_POINTER(aRequestURI);
+  NS_ENSURE_ARG_POINTER(aChannel);
   NS_ENSURE_ARG_POINTER(aDocument);
 
-  nsAutoCString requestSpec;
-  nsresult rv = aRequestURI->GetSpec(requestSpec);
-  NS_ENSURE_SUCCESS(rv, rv);
-  NS_ConvertUTF8toUTF16 requestSpecUTF16(requestSpec);
-
   // Was the sub-resource loaded via CORS?
   if (aCORSMode != CORS_NONE) {
     SRILOG(("SRICheck::IsEligible, CORS mode"));
     return NS_OK;
   }
 
+  nsCOMPtr<nsIURI> finalURI;
+  nsresult rv = NS_GetFinalChannelURI(aChannel, getter_AddRefs(finalURI));
+  NS_ENSURE_SUCCESS(rv, rv);
+  nsCOMPtr<nsIURI> originalURI;
+  rv = aChannel->GetOriginalURI(getter_AddRefs(originalURI));
+  NS_ENSURE_SUCCESS(rv, rv);
+  nsAutoCString requestSpec;
+  rv = originalURI->GetSpec(requestSpec);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (MOZ_LOG_TEST(GetSriLog(), mozilla::LogLevel::Debug)) {
+    nsAutoCString documentSpec, finalSpec;
+    aDocument->GetDocumentURI()->GetAsciiSpec(documentSpec);
+    if (finalURI) {
+      finalURI->GetSpec(finalSpec);
+    }
+    SRILOG(("SRICheck::IsEligible, documentURI=%s; requestURI=%s; finalURI=%s",
+            documentSpec.get(), requestSpec.get(), finalSpec.get()));
+  }
+
   // Is the sub-resource same-origin?
   nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
   if (NS_SUCCEEDED(ssm->CheckSameOriginURI(aDocument->GetDocumentURI(),
-                                           aRequestURI, false))) {
+                                           finalURI, false))) {
     SRILOG(("SRICheck::IsEligible, same-origin"));
     return NS_OK;
   }
-  if (MOZ_LOG_TEST(GetSriLog(), mozilla::LogLevel::Debug)) {
-    nsAutoCString documentURI;
-    aDocument->GetDocumentURI()->GetAsciiSpec(documentURI);
-    // documentURI will be empty if GetAsciiSpec failed
-    SRILOG(("SRICheck::IsEligible, NOT same origin: documentURI=%s; requestURI=%s",
-            documentURI.get(), requestSpec.get()));
-  }
+  SRILOG(("SRICheck::IsEligible, NOT same origin"));
 
+  NS_ConvertUTF8toUTF16 requestSpecUTF16(requestSpec);
   const char16_t* params[] = { requestSpecUTF16.get() };
   nsContentUtils::ReportToConsole(nsIScriptError::errorFlag,
                                   NS_LITERAL_CSTRING("Sub-resource Integrity"),
                                   aDocument,
                                   nsContentUtils::eSECURITY_PROPERTIES,
                                   "IneligibleResource",
                                   params, ArrayLength(params));
   return NS_ERROR_SRI_NOT_ELIGIBLE;
@@ -222,49 +233,53 @@ SRICheck::IntegrityMetadata(const nsAStr
       SRILOG(("SRICheck::IntegrityMetadata, no valid metadata found"));
     }
   }
   return NS_OK;
 }
 
 /* static */ nsresult
 SRICheck::VerifyIntegrity(const SRIMetadata& aMetadata,
-                          nsIURI* aRequestURI,
+                          nsIChannel* aChannel,
                           const CORSMode aCORSMode,
                           const nsAString& aString,
                           const nsIDocument* aDocument)
 {
   NS_ConvertUTF16toUTF8 utf8Hash(aString);
-  return VerifyIntegrity(aMetadata, aRequestURI, aCORSMode, utf8Hash.Length(),
+  return VerifyIntegrity(aMetadata, aChannel, aCORSMode, utf8Hash.Length(),
                          (uint8_t*)utf8Hash.get(), aDocument);
 }
 
 /* static */ nsresult
 SRICheck::VerifyIntegrity(const SRIMetadata& aMetadata,
-                          nsIURI* aRequestURI,
+                          nsIChannel* aChannel,
                           const CORSMode aCORSMode,
                           uint32_t aStringLen,
                           const uint8_t* aString,
                           const nsIDocument* aDocument)
 {
   if (MOZ_LOG_TEST(GetSriLog(), mozilla::LogLevel::Debug)) {
     nsAutoCString requestURL;
-    aRequestURI->GetAsciiSpec(requestURL);
-    // requestURL will be empty if GetAsciiSpec fails
+    nsCOMPtr<nsIURI> originalURI;
+    if (NS_SUCCEEDED(aChannel->GetOriginalURI(getter_AddRefs(originalURI))) &&
+        originalURI) {
+      originalURI->GetAsciiSpec(requestURL);
+      // requestURL will be empty if GetAsciiSpec fails
+    }
     SRILOG(("SRICheck::VerifyIntegrity, url=%s (length=%u)",
             requestURL.get(), aStringLen));
   }
 
   MOZ_ASSERT(!aMetadata.IsEmpty()); // should be checked by caller
 
   // IntegrityMetadata() checks this and returns "no metadata" if
   // it's disabled so we should never make it this far
   MOZ_ASSERT(Preferences::GetBool("security.sri.enable", false));
 
-  if (NS_FAILED(IsEligible(aRequestURI, aCORSMode, aDocument))) {
+  if (NS_FAILED(IsEligible(aChannel, aCORSMode, aDocument))) {
     return NS_OK; // ignore non-CORS resources for forward-compatibility
   }
   if (!aMetadata.IsValid()) {
     nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
                                     NS_LITERAL_CSTRING("Sub-resource Integrity"),
                                     aDocument,
                                     nsContentUtils::eSECURITY_PROPERTIES,
                                     "NoValidMetadata");
--- a/dom/security/SRICheck.h
+++ b/dom/security/SRICheck.h
@@ -6,18 +6,18 @@
 
 #ifndef mozilla_dom_SRICheck_h
 #define mozilla_dom_SRICheck_h
 
 #include "mozilla/CORSMode.h"
 #include "nsCOMPtr.h"
 #include "SRIMetadata.h"
 
+class nsIChannel;
 class nsIDocument;
-class nsIHttpChannel;
 class nsIScriptSecurityManager;
 class nsIStreamLoader;
 class nsIURI;
 
 namespace mozilla {
 namespace dom {
 
 class SRICheck final
@@ -34,27 +34,27 @@ public:
                                     const nsIDocument* aDocument,
                                     SRIMetadata* outMetadata);
 
   /**
    * Process the integrity attribute of the element.  A result of false
    * must prevent the resource from loading.
    */
   static nsresult VerifyIntegrity(const SRIMetadata& aMetadata,
-                                  nsIURI* aRequestURI,
+                                  nsIChannel* aChannel,
                                   const CORSMode aCORSMode,
                                   const nsAString& aString,
                                   const nsIDocument* aDocument);
 
   /**
    * Process the integrity attribute of the element.  A result of false
    * must prevent the resource from loading.
    */
   static nsresult VerifyIntegrity(const SRIMetadata& aMetadata,
-                                  nsIURI* aRequestURI,
+                                  nsIChannel* aChannel,
                                   const CORSMode aCORSMode,
                                   uint32_t aStringLen,
                                   const uint8_t* aString,
                                   const nsIDocument* aDocument);
 };
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/security/test/sri/iframe_script_crossdomain.html
+++ b/dom/security/test/sri/iframe_script_crossdomain.html
@@ -39,16 +39,30 @@
   function bad_onloadCalled() {
     window.onloadCalled = true;
   }
 
   function good_onerrorCalled() {
     window.onerrorCalled = true;
   }
 
+  function good_incorrect301Loaded() {
+    ok(true, "A non-CORS load redirected to a different origin was loaded correctly despite an incorrect hash value.");
+  }
+  function bad_incorrect301Blocked() {
+    ok(false, "Non-CORS loads redirecting to a different origin should be loaded despite an incorrect hash value!");
+  }
+
+  function good_correct301Loaded() {
+    ok(true, "A non-CORS load redirected to a different origin was loaded correctly.");
+  }
+  function bad_correct301Blocked() {
+    ok(false, "Non-CORS loads redirecting to a different origin should be loaded!");
+  }
+
   window.onload = function() {
     SimpleTest.finish()
   }
 </script>
 
 <!-- cors-enabled. should be loaded -->
 <script src="http://example.com/tests/dom/security/test/sri/script_crossdomain1.js"
         crossorigin=""
@@ -68,16 +82,28 @@
         onerror="bad_nonsriBlocked()"></script>
 
 <!-- non-cors with invalid metadata. should trigger onload -->
 <script src="http://example.com/tests/dom/security/test/sri/script_crossdomain4.js"
         integrity="sha256-bogus"
         onload="good_nonCORSInvalidLoaded()"
         onerror="bad_nonCORSInvalidBlocked()"></script>
 
+<!-- non-cors that's same-origin initially but redirected to another origin -->
+<script src="script_301.js"
+        integrity="sha384-invalid"
+        onerror="bad_incorrect301Blocked()"
+        onload="good_incorrect301Loaded()"></script>
+
+<!-- non-cors that's same-origin initially but redirected to another origin -->
+<script src="script_301.js"
+        integrity="sha384-1NpiDI6decClMaTWSCAfUjTdx1BiOffsCPgH4lW5hCLwmHk0VyV/g6B9Sw2kD2K3"
+        onerror="bad_correct301Blocked()"
+        onload="good_correct301Loaded()"></script>
+
 <script>
   ok(window.hasCORSLoaded, "CORS-enabled resource with a correct hash");
   ok(!window.hasNonCORSLoaded, "Correct hash, but non-CORS, should be blocked");
   ok(!window.onloadCalled, "Failed loads should not call onload when they're cross-domain");
   ok(window.onerrorCalled, "Failed loads should call onerror when they're cross-domain");
 </script>
 </body>
 </html>
copy from dom/security/test/sri/iframe_style_sameorigin.html
copy to dom/security/test/sri/iframe_style_crossdomain.html
--- a/dom/security/test/sri/iframe_style_sameorigin.html
+++ b/dom/security/test/sri/iframe_style_crossdomain.html
@@ -1,74 +1,51 @@
 <!DOCTYPE HTML>
 <!-- Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/ -->
 <html>
 <head>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <script type="application/javascript">
-    function check_styles() {
-      var redText = document.getElementById('red-text');
-      var blackText = document.getElementById('black-text');
-      var redTextColor = window.getComputedStyle(redText, null).getPropertyValue('color');
-      var blackTextColor = window.getComputedStyle(blackText, null).getPropertyValue('color');
-      ok(redTextColor == 'rgb(255, 0, 0)', "The first part should be red.");
-      ok(blackTextColor == 'rgb(0, 0, 0)', "The second part should still be black.");
-    }
-
     SimpleTest.waitForExplicitFinish();
     window.onload = function() {
-      check_styles();
       SimpleTest.finish();
     }
   </script>
   <script>
     function good_correctHashLoaded() {
-      ok(true, "A stylesheet was correctly loaded when integrity matched");
+      ok(true, "A non-CORS cross-domain stylesheet was correctly loaded when integrity matched.");
     }
     function bad_correctHashBlocked() {
-      ok(false, "We should load stylesheets with hashes that match!");
+      ok(false, "We should load non-CORS cross-domain stylesheets with hashes that match!");
     }
 
-    function good_emptyIntegrityLoaded() {
-      ok(true, "A stylesheet was correctly loaded when the integrity attribute was empty");
+    function good_incorrectHashLoaded() {
+      ok(true, "A non-CORS cross-domain stylesheet was correctly loaded even when integrity didn't match.");
     }
-    function bad_emptyIntegrityBlocked() {
-      ok(false, "We should load stylesheets with empty integrity attributes!");
+    function bad_incorrectHashBlocked() {
+      ok(false, "We should load non-CORS cross-domain stylesheets even when hashes don't match!");
     }
 
-    function good_incorrectHashBlocked() {
-      ok(true, "A stylesheet was correctly blocked, because the hash digest was wrong");
-    }
-    function bad_incorrectHashLoaded() {
-      ok(false, "We should not load stylesheets with hashes that do not match the content!");
-    }
   </script>
 
-  <!-- valid sha256 hash. should trigger onload -->
-  <link rel="stylesheet" href="style1.css"
+  <!-- valid non-CORS sha256 hash. should trigger onload -->
+  <link rel="stylesheet" href="style_301.css"
         integrity="sha256-qs8lnkunWoVldk5d5E+652yth4VTSHohlBKQvvgGwa8="
         onerror="bad_correctHashBlocked()"
         onload="good_correctHashLoaded()">
 
-  <!-- empty metadata. should trigger onload -->
-  <link rel="stylesheet" href="style2.css"
-        integrity=""
-        onerror="bad_emptyIntegrityBlocked()"
-        onload="good_emptyIntegrityLoaded()">
-
-  <!-- invalid sha256 hash. should trigger onerror -->
-  <link rel="stylesheet" href="style3.css"
+  <!-- invalid non-CORS sha256 hash. should trigger onload -->
+  <link rel="stylesheet" href="style_301.css?again"
         integrity="sha256-bogus"
-        onerror="good_incorrectHashBlocked()"
-        onload="bad_incorrectHashLoaded()">
+        onerror="bad_incorrectHashBlocked()"
+        onload="good_incorrectHashLoaded()">
 </head>
 <body>
-<p><span id="red-text">This should be red </span> and
-  <span id="black-text">this should stay black.</p>
+<p><span id="red-text">This should be red.</span></p>
 <p id="display"></p>
 <div id="content" style="display: none">
 </div>
 <pre id="test">
 </pre>
 </body>
 </html>
--- a/dom/security/test/sri/mochitest.ini
+++ b/dom/security/test/sri/mochitest.ini
@@ -1,30 +1,35 @@
 [DEFAULT]
 support-files =
   iframe_script_crossdomain.html
   iframe_script_sameorigin.html
   iframe_sri_disabled.html
+  iframe_style_crossdomain.html
   iframe_style_sameorigin.html
   script_crossdomain1.js
   script_crossdomain1.js^headers^
   script_crossdomain2.js
   script_crossdomain3.js
   script_crossdomain3.js^headers^
   script_crossdomain4.js
   script_crossdomain4.js^headers^
+  script_crossdomain5.js
+  script_crossdomain5.js^headers^
   script.js
   script.js^headers^
+  script_301.js
+  script_301.js^headers^
   script_302.js
   script_302.js^headers^
   script_401.js
   script_401.js^headers^
   style1.css
   style2.css
   style3.css
+  style_301.css
+  style_301.css^headers^
 
 [test_script_sameorigin.html]
-
 [test_script_crossdomain.html]
-
 [test_sri_disabled.html]
-
+[test_style_crossdomain.html]
 [test_style_sameorigin.html]
new file mode 100644
--- /dev/null
+++ b/dom/security/test/sri/script_301.js
@@ -0,0 +1,1 @@
+var load=false;
new file mode 100644
--- /dev/null
+++ b/dom/security/test/sri/script_301.js^headers^
@@ -0,0 +1,2 @@
+HTTP 301 Moved Permanently
+Location: http://example.com/tests/dom/security/test/sri/script_crossdomain5.js
copy from dom/security/test/sri/script_crossdomain4.js
copy to dom/security/test/sri/script_crossdomain5.js
new file mode 100644
--- /dev/null
+++ b/dom/security/test/sri/script_crossdomain5.js^headers^
@@ -0,0 +1,1 @@
+Access-Control-Allow-Origin: *
copy from dom/security/test/sri/style1.css
copy to dom/security/test/sri/style_301.css
new file mode 100644
--- /dev/null
+++ b/dom/security/test/sri/style_301.css^headers^
@@ -0,0 +1,2 @@
+HTTP 301 Moved Permanently
+Location: http://example.com/tests/dom/security/test/sri/style1.css
copy from dom/security/test/sri/test_style_sameorigin.html
copy to dom/security/test/sri/test_style_crossdomain.html
--- a/dom/security/test/sri/test_style_sameorigin.html
+++ b/dom/security/test/sri/test_style_crossdomain.html
@@ -1,18 +1,18 @@
 <!DOCTYPE HTML>
 <!-- Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/ -->
 <html>
 <head>
   <meta charset="utf-8">
-  <title>Same-origin stylesheet tests for Bug 992096</title>
+  <title>Cross-domain stylesheet tests for Bug 1196740</title>
   <script>
     SpecialPowers.setBoolPref("security.sri.enable", true);
   </script>
 </head>
 <body>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=992096">Mozilla Bug 992096</a>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1196740">Mozilla Bug 1196740</a>
 <div>
-  <iframe src="iframe_style_sameorigin.html" height="100%" width="90%" frameborder="0"></iframe>
+  <iframe src="iframe_style_crossdomain.html" height="100%" width="90%" frameborder="0"></iframe>
 </div>
 </body>
 </html>
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -936,17 +936,17 @@ SheetLoadData::OnStreamComplete(nsIUnich
                 contentType.get()));
       mLoader->SheetComplete(this, NS_ERROR_NOT_AVAILABLE);
       return NS_OK;
     }
   }
 
   SRIMetadata sriMetadata = mSheet->GetIntegrity();
   if (!sriMetadata.IsEmpty() &&
-      NS_FAILED(SRICheck::VerifyIntegrity(sriMetadata, channelURI,
+      NS_FAILED(SRICheck::VerifyIntegrity(sriMetadata, httpChannel,
                                           mSheet->GetCORSMode(), aBuffer,
                                           mLoader->mDocument))) {
     LOG(("  Load was blocked by SRI"));
     MOZ_LOG(GetSriLog(), mozilla::LogLevel::Debug,
             ("css::Loader::OnStreamComplete, bad metadata"));
     mLoader->SheetComplete(this, NS_ERROR_SRI_CORRUPT);
     return NS_OK;
   }