Bug 1519302 - Add pref to restrict BinAST feature to specific hosts. r=baku
☠☠ backed out by 7c002ce6b6cc ☠ ☠
authorTooru Fujisawa <arai_a@mac.com>
Wed, 16 Jan 2019 13:12:00 +0000
changeset 514085 8c88a33dc39f0b2089e4def7ca9feaf5f78a1505
parent 514084 c6ec6b0911d20e4672b6409771a1a6e1495fff9b
child 514086 d50d152c74c71a2581ecfb373f9efb8255aef4a2
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1519302
milestone66.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 1519302 - Add pref to restrict BinAST feature to specific hosts. r=baku To reduce the attack surface in early test for BinAST, add a preference to restrict the hosts that Firefox accepts BinAST file from. The preference is turned on by default (BinAST itself is turned off by default for now), and the list contains hosts which is going to be used in early test. For hosts not listed in the list, Firefox doesn't send BinAST MIME-Type in Accept field, and doesn't handle BinAST file in case the server returns BinAST file. Differential Revision: https://phabricator.services.mozilla.com/D16517
dom/script/ScriptLoadHandler.cpp
dom/script/ScriptLoadRequest.cpp
modules/libpref/init/StaticPrefList.h
modules/libpref/init/all.js
testing/web-platform/mozilla/meta/binast/domain-restrict-excluded.https.html.ini
testing/web-platform/mozilla/meta/binast/domain-restrict-included.https.html.ini
testing/web-platform/mozilla/meta/binast/large.https.html.ini
testing/web-platform/mozilla/meta/binast/not-secure.html.ini
testing/web-platform/mozilla/meta/binast/small.https.html.ini
testing/web-platform/mozilla/tests/binast/domain-restrict-excluded.https.html
testing/web-platform/mozilla/tests/binast/domain-restrict-included.https.html
testing/web-platform/mozilla/tests/binast/not-secure.html
testing/web-platform/mozilla/tests/binast/serve.py
--- a/dom/script/ScriptLoadHandler.cpp
+++ b/dom/script/ScriptLoadHandler.cpp
@@ -307,17 +307,22 @@ nsresult ScriptLoadHandler::EnsureKnownD
       nsAutoCString mimeType;
       httpChannel->GetContentType(mimeType);
       if (mimeType.LowerCaseEqualsASCII(APPLICATION_JAVASCRIPT_BINAST)) {
         if (mRequest->ShouldAcceptBinASTEncoding()) {
           mRequest->SetBinASTSource();
           TRACE_FOR_TEST(mRequest->Element(), "scriptloader_load_source");
           return NS_OK;
         }
-        return NS_ERROR_FAILURE;
+
+        // If the request isn't allowed to accept BinAST, fallback to text
+        // source.  The possibly binary source will be passed to normal
+        // JS parser and will throw error there.
+        mRequest->SetTextSource();
+        return NS_OK;
       }
     }
   }
 
   mRequest->SetTextSource();
   TRACE_FOR_TEST(mRequest->Element(), "scriptloader_load_source");
 
   MOZ_ASSERT(!mRequest->IsUnknownDataType());
--- a/dom/script/ScriptLoadRequest.cpp
+++ b/dom/script/ScriptLoadRequest.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ScriptLoadRequest.h"
 
 #include "mozilla/HoldDropJSObjects.h"
 #include "mozilla/Unused.h"
 
+#include "nsContentUtils.h"
 #include "nsICacheInfoChannel.h"
 #include "ScriptLoadRequest.h"
 #include "ScriptSettings.h"
 
 namespace mozilla {
 namespace dom {
 
 //////////////////////////////////////////////////////////////
@@ -190,17 +191,28 @@ bool ScriptLoadRequest::ShouldAcceptBinA
 #ifdef JS_BUILD_BINAST
   // We accept the BinAST encoding if we're using a secure connection.
 
   bool isHTTPS = false;
   nsresult rv = mURI->SchemeIs("https", &isHTTPS);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
   Unused << rv;
 
-  return isHTTPS;
+  if (!isHTTPS) {
+    return false;
+  }
+
+  if (StaticPrefs::dom_script_loader_binast_encoding_domain_restrict()) {
+    if (!nsContentUtils::IsURIInPrefList(
+            mURI, "dom.script_loader.binast_encoding.domain.restrict.list")) {
+      return false;
+    }
+  }
+
+  return true;
 #else
   MOZ_CRASH("BinAST not supported");
 #endif
 }
 
 void ScriptLoadRequest::ClearScriptSource() {
   if (IsTextSource()) {
     ScriptText().clearAndFree();
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -414,16 +414,22 @@ VARCACHE_PREF(
 )
 
 #ifdef JS_BUILD_BINAST
 VARCACHE_PREF(
   "dom.script_loader.binast_encoding.enabled",
    dom_script_loader_binast_encoding_enabled,
   RelaxedAtomicBool, false
 )
+
+VARCACHE_PREF(
+  "dom.script_loader.binast_encoding.domain.restrict",
+   dom_script_loader_binast_encoding_domain_restrict,
+  bool, true
+)
 #endif
 
 // IMPORTANT: Keep this in condition in sync with all.js. The value
 // of MOZILLA_OFFICIAL is different between full and artifact builds, so without
 // it being specified there, dump is disabled in artifact builds (see Bug 1490412).
 #ifdef MOZILLA_OFFICIAL
 # define PREF_VALUE false
 #else
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -259,16 +259,17 @@ pref("dom.script_loader.bytecode_cache.e
 //          page-load time.
 //
 // Other values might lead to experimental strategies. For more details, have a
 // look at: ScriptLoader::ShouldCacheBytecode function.
 pref("dom.script_loader.bytecode_cache.strategy", 0);
 
 #ifdef JS_BUILD_BINAST
 pref("dom.script_loader.binast_encoding.enabled", false);
+pref("dom.script_loader.binast_encoding.domain.restrict.list", "*.facebook.com,static.xx.fbcdn.net");
 #endif
 
 // Whether window.event is enabled
 pref("dom.window.event.enabled", true);
 
 // Fastback caching - if this pref is negative, then we calculate the number
 // of content viewers to cache based on the amount of available memory.
 pref("browser.sessionhistory.max_total_viewers", -1);
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/meta/binast/domain-restrict-excluded.https.html.ini
@@ -0,0 +1,7 @@
+[domain-restrict-excluded.https.html]
+  prefs: [dom.script_loader.binast_encoding.domain.restrict:true,
+          dom.script_loader.binast_encoding.domain.restrict.list:]
+  [Check we can't load BinAST if the host is excluded in the list]
+    expected:
+      if release_or_beta or (os == "android") or ((os == "win") and (processor == "x86")): FAIL
+
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/meta/binast/domain-restrict-included.https.html.ini
@@ -0,0 +1,7 @@
+[domain-restrict-included.https.html]
+  prefs: [dom.script_loader.binast_encoding.domain.restrict:true,
+          dom.script_loader.binast_encoding.domain.restrict.list:web-platform.test]
+  [Check we can load BinAST if the host is included in the list]
+    expected:
+      if release_or_beta or (os == "android") or ((os == "win") and (processor == "x86")): FAIL
+
--- a/testing/web-platform/mozilla/meta/binast/large.https.html.ini
+++ b/testing/web-platform/mozilla/meta/binast/large.https.html.ini
@@ -1,5 +1,6 @@
 [large.https.html]
+  prefs: [dom.script_loader.binast_encoding.domain.restrict:false]
   [Check we can load BinAST over HTTPS]
     expected:
       if release_or_beta or (os == "android") or ((os == "win") and (processor == "x86")): FAIL
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/meta/binast/not-secure.html.ini
@@ -0,0 +1,5 @@
+[not-secure.html]
+  prefs: [dom.script_loader.binast_encoding.domain.restrict:false]
+  [Check we can't load BinAST over HTTP]
+    expected:
+      if release_or_beta or (os == "android") or ((os == "win") and (processor == "x86")): FAIL
--- a/testing/web-platform/mozilla/meta/binast/small.https.html.ini
+++ b/testing/web-platform/mozilla/meta/binast/small.https.html.ini
@@ -1,5 +1,6 @@
 [small.https.html]
+  prefs: [dom.script_loader.binast_encoding.domain.restrict:false]
   [Check we can load BinAST over HTTPS]
     expected:
       if release_or_beta or (os == "android") or ((os == "win") and (processor == "x86")): FAIL
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/tests/binast/domain-restrict-excluded.https.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<title>Check we can't load BinAST if the host is excluded in the list</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    var hadScriptLoadError = false;
+    function setLoadError() {
+      // Load error happens if the server side throws an exception,
+      // for 'expect_accept' check on server side.
+      hadScriptLoadError = true;
+    }
+
+    var hadSyntaxError = false;
+    var hadOtherError = false;
+    function checkSyntaxError(event) {
+      // Server returns BinAST and the browser parses it as plain JS.
+      if (event.message.startsWith("SyntaxError:")) {
+        hadSyntaxError = true;
+      } else {
+        hadOtherError = true;
+      }
+    }
+
+    window.addEventListener("error", checkSyntaxError);
+
+    const test_load = async_test("Check we can't load BinAST if the host is excluded in the list");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(hadScriptLoadError, false, "Didn't expect a load error event");
+      assert_equals(hadSyntaxError, true, "Expect a syntax error event for receiving binast");
+      assert_equals(hadOtherError, false, "Didn't expect other error event");
+      assert_equals(typeof binASTLoaded, "undefined", "Expected not to load either version");
+    }));
+
+</script>
+<script src="./serve.py?name=small&amp;expect_accept=false&amp;force_binast=true" onerror="setLoadError()"></script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/tests/binast/domain-restrict-included.https.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<title>Check we can load BinAST if the host is included in the list</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    var hadScriptLoadError = false;
+    function setLoadError() {
+      // Load error happens if the server side throws an exception,
+      // for 'expect_accept' check on server side.
+      hadScriptLoadError = true;
+    }
+
+    var hadOtherError = false;
+    function setOtherError() {
+      hadOtherError = true;
+    }
+
+    window.addEventListener("error", setOtherError);
+
+    const test_load = async_test("Check we can load BinAST if the host is included in the list");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(hadScriptLoadError, false, "Didn't expect a load error event");
+      assert_equals(hadOtherError, false, "Didn't expect other error event");
+      assert_equals(binASTLoaded, true, "Expected to load BinAST version");
+    }));
+
+</script>
+<script src="./serve.py?name=small&amp;expect_accept=true" onerror="setLoadError()"></script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/tests/binast/not-secure.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<title>Check we can't load BinAST over HTTP</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+    setup({allow_uncaught_exception: true});
+
+    var hadScriptLoadError = false;
+    function setLoadError() {
+      // Load error happens if the server side throws an exception,
+      // for 'expect_accept' check on server side.
+      hadScriptLoadError = true;
+    }
+
+    var hadSyntaxError = false;
+    var hadOtherError = false;
+    function checkSyntaxError(event) {
+      // Server returns BinAST and the browser parses it as plain JS.
+      if (event.message.startsWith("SyntaxError:")) {
+        hadSyntaxError = true;
+      } else {
+        hadOtherError = true;
+      }
+    }
+
+    window.addEventListener("error", checkSyntaxError);
+
+    const test_load = async_test("Check we can't load BinAST over HTTP");
+    window.addEventListener("load", test_load.step_func_done(ev => {
+      assert_equals(hadScriptLoadError, false, "Didn't expect a load error event");
+      assert_equals(hadSyntaxError, true, "Expect a syntax error event for receiving binast");
+      assert_equals(hadOtherError, false, "Didn't expect other error event");
+      assert_equals(typeof binASTLoaded, "undefined", "Expected not to load either version");
+    }));
+
+</script>
+<script src="./serve.py?name=small&amp;expect_accept=false&amp;force_binast=true" onerror="setLoadError()"></script>
--- a/testing/web-platform/mozilla/tests/binast/serve.py
+++ b/testing/web-platform/mozilla/tests/binast/serve.py
@@ -12,24 +12,42 @@ BinASTExtension = ".binjs"
 
 def clientAcceptsBinAST(request):
     if "accept" not in request.headers:
         return False
 
     encodings = [s.strip().lower() for s in request.headers["accept"].split(",")]
     return BinASTMimeType in encodings
 
+def get_mine_type_and_extension(request, params):
+    accept_binast = clientAcceptsBinAST(request)
+
+    if 'expect_accept' in params:
+        expect_accept = params['expect_accept'][0]
+        if expect_accept == 'false':
+            if accept_binast:
+                raise Exception("Expect not accept binast")
+        elif expect_accept == 'true':
+            if not accept_binast:
+                raise Exception("Expect accept binast")
+        else:
+            raise Exception("Bad check_header parameter: " + check_header)
+
+    if 'force_binast' in params and params['force_binast'][0] == 'true':
+        return BinASTMimeType, BinASTExtension
+
+    if accept_binast:
+        return BinASTMimeType, BinASTExtension
+
+    return JavaScriptMimeType, SourceTextExtension
+
+
 def main(request, response):
     params = urlparse.parse_qs(request.url_parts[3])
     name = params['name'][0]
     if not re.match(r'\w+$', name):
         raise Exception("Bad name parameter: " + name)
 
-    if clientAcceptsBinAST(request):
-        mimeType = BinASTMimeType
-        extension = BinASTExtension
-    else:
-        mimeType = JavaScriptMimeType
-        extension = SourceTextExtension
+    mimeType, extension = get_mine_type_and_extension(request, params)
 
     scriptDir = os.path.dirname(__file__)
     contentPath = os.path.join(scriptDir, name + extension)
     return [("Content-Type", mimeType)], open(contentPath, "rb")