Skip to content

Commit

Permalink
Bug 1519302 - Add pref to restrict BinAST feature to specific hosts. …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
arai-a committed Jan 17, 2019
1 parent 045872b commit f78d669
Show file tree
Hide file tree
Showing 13 changed files with 171 additions and 8 deletions.
7 changes: 6 additions & 1 deletion dom/script/ScriptLoadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,12 @@ nsresult ScriptLoadHandler::EnsureKnownDataType(
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;
}
}
}
Expand Down
14 changes: 13 additions & 1 deletion dom/script/ScriptLoadRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "mozilla/HoldDropJSObjects.h"
#include "mozilla/Unused.h"

#include "nsContentUtils.h"
#include "nsICacheInfoChannel.h"
#include "ScriptLoadRequest.h"
#include "ScriptSettings.h"
Expand Down Expand Up @@ -195,7 +196,18 @@ bool ScriptLoadRequest::ShouldAcceptBinASTEncoding() const {
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
Expand Down
6 changes: 6 additions & 0 deletions modules/libpref/init/StaticPrefList.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,12 @@ VARCACHE_PREF(
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
Expand Down
1 change: 1 addition & 0 deletions modules/libpref/init/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ 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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[domain-restrict-excluded.https.html]
prefs: [dom.script_loader.binast_encoding.domain.restrict:true,
dom.script_loader.binast_encoding.domain.restrict.list:]
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[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
Expand Down
2 changes: 2 additions & 0 deletions testing/web-platform/mozilla/meta/binast/not-secure.html.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[not-secure.html]
prefs: [dom.script_loader.binast_encoding.domain.restrict:false]
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[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
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
38 changes: 38 additions & 0 deletions testing/web-platform/mozilla/tests/binast/not-secure.html
Original file line number Diff line number Diff line change
@@ -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>
30 changes: 24 additions & 6 deletions testing/web-platform/mozilla/tests/binast/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,36 @@ def clientAcceptsBinAST(request):
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)
Expand Down

0 comments on commit f78d669

Please sign in to comment.