Commit 500a74f5 authored by Mike West's avatar Mike West Committed by Commit Bot

Add a runtime flag to enforce strict MIME type checks for workers.

We're currently enforcing strict MIME type checks for `importScripts()`
and nested workers. Firefox would like to enforce them for the top-level
worker script as well. We should follow suit.

This patch also excludes `blob:`, `filesystem:`, and `data:` from
scripty MIME type checks. This is the behavior Firefox is shipping,
and seems reasonable, given that the risk of sniffing scripts is
highest when dealing with resources delivered from a server, rather
than resources which are already accessible to the page causing
their execution.

https://github.com/whatwg/html/issues/3255
https://bugzilla.mozilla.org/show_bug.cgi?id=1583657

Bug: 794548
Change-Id: Ia55621243b8e279a30f457c6a3a20b9fa13cd3d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1917528
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: default avatarDaniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716121}
parent 36d524c2
......@@ -2471,6 +2471,7 @@ enum WebFeature {
kFetchBodyStreamOutsideServiceWorker = 3087,
kGetComputedStyleOutsideFlatTree = 3088,
kARIADescriptionAttribute = 3089,
kStrictMimeTypeChecksWouldBlockWorker = 3090,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
......
......@@ -180,7 +180,8 @@ void WebSharedWorkerImpl::StartWorkerContext(
network::mojom::ReferrerPolicy::kDefault,
/*outgoing_referrer=*/String(),
CalculateHttpsState(starter_origin.get()),
AllowedByNosniff::MimeTypeCheck::kLax, creation_address_space,
AllowedByNosniff::MimeTypeCheck::kLaxForWorker,
creation_address_space,
/*insecure_request_policy=*/kBlockAllMixedContent,
FetchClientSettingsObject::InsecureNavigationsSet(),
/*mixed_autoupgrade_opt_out=*/false);
......
......@@ -347,7 +347,8 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url) const {
// If the MIME check fails, which is considered as load failure.
if (!AllowedByNosniff::MimeTypeAsScript(
fetcher->GetUseCounter(), &fetcher->GetConsoleLogger(),
resource->GetResponse(), AllowedByNosniff::MimeTypeCheck::kLax)) {
resource->GetResponse(),
AllowedByNosniff::MimeTypeCheck::kLaxForElement)) {
return nullptr;
}
......
......@@ -6,6 +6,7 @@
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/execution_context/security_context.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
namespace blink {
......@@ -48,6 +49,9 @@ HttpsState FetchClientSettingsObjectImpl::GetHttpsState() const {
AllowedByNosniff::MimeTypeCheck
FetchClientSettingsObjectImpl::MimeTypeCheckForClassicWorkerScript() const {
if (RuntimeEnabledFeatures::StrictMimeTypesForWorkersEnabled())
return AllowedByNosniff::MimeTypeCheck::kStrict;
if (execution_context_->IsDocument()) {
// For worker creation on a document, don't impose strict MIME-type checks
// on the top-level worker script for backward compatibility. Note that
......@@ -57,7 +61,7 @@ FetchClientSettingsObjectImpl::MimeTypeCheckForClassicWorkerScript() const {
// For worker creation on a document with off-the-main-thread top-level
// worker classic script loading, this value is propagated to
// outsideSettings FCSO.
return AllowedByNosniff::MimeTypeCheck::kLax;
return AllowedByNosniff::MimeTypeCheck::kLaxForWorker;
}
// For importScripts() and nested worker top-level scripts impose the strict
......
......@@ -327,8 +327,8 @@ WebEmbeddedWorkerImpl::CreateFetchClientSettingsObjectData(
passed_settings_object.referrer_policy,
KURL::CreateIsolated(
passed_settings_object.outgoing_referrer.GetString()),
https_state, AllowedByNosniff::MimeTypeCheck::kLax, address_space,
insecure_requests_policy,
https_state, AllowedByNosniff::MimeTypeCheck::kLaxForWorker,
address_space, insecure_requests_policy,
FetchClientSettingsObject::InsecureNavigationsSet(),
false /* mixed_autoupgrade_opt_out */);
}
......
......@@ -98,7 +98,8 @@ bool AllowMimeTypeAsScript(const String& mime_type,
if (mime_type_check_mode == MimeTypeCheck::kStrict) {
return false;
}
DCHECK_EQ(mime_type_check_mode, MimeTypeCheck::kLax);
DCHECK(mime_type_check_mode == MimeTypeCheck::kLaxForWorker ||
mime_type_check_mode == MimeTypeCheck::kLaxForElement);
// Beyond this point we handle legacy MIME types, where it depends whether
// we still wish to accept them (or log them using UseCounter, or add a
......@@ -131,17 +132,20 @@ bool AllowedByNosniff::MimeTypeAsScript(UseCounter& use_counter,
ConsoleLogger* console_logger,
const ResourceResponse& response,
MimeTypeCheck mime_type_check_mode) {
// The content type is really only meaningful for the http:-family & data
// schemes.
bool is_http_family_or_data =
response.CurrentRequestUrl().ProtocolIsInHTTPFamily() ||
response.CurrentRequestUrl().ProtocolIsData();
if (!is_http_family_or_data &&
// The content type is really only meaningful for `http:`-family schemes.
if (!response.CurrentRequestUrl().ProtocolIsInHTTPFamily() &&
(response.CurrentRequestUrl().LastPathComponent().EndsWith(".js") ||
response.CurrentRequestUrl().LastPathComponent().EndsWith(".mjs"))) {
return true;
}
// Exclude `data:`, `blob:` and `filesystem:` URLs from MIME checks.
if (response.CurrentRequestUrl().ProtocolIsData() ||
response.CurrentRequestUrl().ProtocolIs(url::kBlobScheme) ||
response.CurrentRequestUrl().ProtocolIs(url::kFileSystemScheme)) {
return true;
}
String mime_type = response.HttpContentType();
// Allowed by nosniff?
......@@ -191,6 +195,11 @@ bool AllowedByNosniff::MimeTypeAsScript(UseCounter& use_counter,
"Refused to execute script from '" +
response.CurrentRequestUrl().ElidedString() +
"' because its MIME type ('" + mime_type + "') is not executable.");
} else if (mime_type_check_mode == MimeTypeCheck::kLaxForWorker) {
bool strict_allow = AllowMimeTypeAsScript(mime_type, same_origin,
MimeTypeCheck::kStrict, counter);
if (!strict_allow)
use_counter.CountUse(WebFeature::kStrictMimeTypeChecksWouldBlockWorker);
}
return allow;
}
......
......@@ -15,7 +15,7 @@ class ResourceResponse;
class PLATFORM_EXPORT AllowedByNosniff final {
public:
enum class MimeTypeCheck { kStrict, kLax };
enum class MimeTypeCheck { kStrict, kLaxForElement, kLaxForWorker };
static bool MimeTypeAsScript(UseCounter&,
ConsoleLogger*,
......
......@@ -111,12 +111,20 @@ TEST_F(AllowedByNosniffTest, AllowedOrNot) {
ResourceResponse response(url);
response.SetHttpHeaderField("Content-Type", testcase.mimetype);
EXPECT_CALL(*use_counter, CountUse(_)).Times(::testing::AnyNumber());
if (!testcase.allowed)
EXPECT_CALL(*logger, AddConsoleMessageImpl(_, _, _, _));
EXPECT_EQ(testcase.allowed, AllowedByNosniff::MimeTypeAsScript(
*use_counter, logger, response,
MimeTypeCheck::kLaxForElement));
::testing::Mock::VerifyAndClear(use_counter);
EXPECT_CALL(*use_counter, CountUse(_)).Times(::testing::AnyNumber());
if (!testcase.allowed)
EXPECT_CALL(*logger, AddConsoleMessageImpl(_, _, _, _));
EXPECT_EQ(testcase.allowed,
AllowedByNosniff::MimeTypeAsScript(*use_counter, logger, response,
MimeTypeCheck::kLax));
MimeTypeCheck::kLaxForWorker));
::testing::Mock::VerifyAndClear(use_counter);
EXPECT_CALL(*use_counter, CountUse(_)).Times(::testing::AnyNumber());
......@@ -182,7 +190,24 @@ TEST_F(AllowedByNosniffTest, Counters) {
EXPECT_CALL(*use_counter, CountUse(::testing::Ne(testcase.expected)))
.Times(::testing::AnyNumber());
AllowedByNosniff::MimeTypeAsScript(*use_counter, logger, response,
MimeTypeCheck::kLax);
MimeTypeCheck::kLaxForElement);
::testing::Mock::VerifyAndClear(use_counter);
EXPECT_CALL(*use_counter, CountUse(testcase.expected));
EXPECT_CALL(*use_counter, CountUse(::testing::Ne(testcase.expected)))
.Times(::testing::AnyNumber());
AllowedByNosniff::MimeTypeAsScript(*use_counter, logger, response,
MimeTypeCheck::kLaxForWorker);
::testing::Mock::VerifyAndClear(use_counter);
EXPECT_CALL(*use_counter,
CountUse(WebFeature::kStrictMimeTypeChecksWouldBlockWorker));
EXPECT_CALL(*use_counter,
CountUse(::testing::Ne(
WebFeature::kStrictMimeTypeChecksWouldBlockWorker)))
.Times(::testing::AnyNumber());
AllowedByNosniff::MimeTypeAsScript(*use_counter, logger, response,
MimeTypeCheck::kLaxForWorker);
::testing::Mock::VerifyAndClear(use_counter);
}
}
......@@ -210,6 +235,12 @@ TEST_F(AllowedByNosniffTest, AllTheSchemes) {
{"file://home/potato.js", true},
{"file://home/potato.mjs", true},
{"chrome://dino/dino.mjs", true},
// `blob:` and `filesystem:` are excluded:
{"blob:https://example.com/bla.js", true},
{"blob:https://example.com/bla.txt", true},
{"filesystem:https://example.com/temporary/bla.js", true},
{"filesystem:https://example.com/temporary/bla.txt", true},
};
for (auto& testcase : data) {
......@@ -225,7 +256,13 @@ TEST_F(AllowedByNosniffTest, AllTheSchemes) {
response.SetHttpHeaderField("X-Content-Type-Options", "nosniff");
EXPECT_EQ(testcase.allowed,
AllowedByNosniff::MimeTypeAsScript(*use_counter, logger, response,
MimeTypeCheck::kLax));
MimeTypeCheck::kStrict));
EXPECT_EQ(testcase.allowed, AllowedByNosniff::MimeTypeAsScript(
*use_counter, logger, response,
MimeTypeCheck::kLaxForElement));
EXPECT_EQ(testcase.allowed,
AllowedByNosniff::MimeTypeAsScript(*use_counter, logger, response,
MimeTypeCheck::kLaxForWorker));
}
}
......
......@@ -1581,6 +1581,10 @@
name: "StorageQuotaDetails",
status: "stable"
},
{
name: "StrictMimeTypesForWorkers",
status: "experimental"
},
{
name: "SurfaceEmbeddingFeatures",
status: "stable",
......
......@@ -11,10 +11,11 @@ self.onfetch = event => {
// Request handler for a synthesized response.
if (url.indexOf('synthesized') != -1) {
let script_headers = new Headers({ "Content-Type": "text/javascript" });
if (destination === 'worker')
event.respondWith(new Response(dedicated_worker_script));
event.respondWith(new Response(dedicated_worker_script, { 'headers': script_headers }));
else if (destination === 'sharedworker')
event.respondWith(new Response(shared_worker_script));
event.respondWith(new Response(shared_worker_script, { 'headers': script_headers }));
else
event.respondWith(new Response('Unexpected request! ' + destination));
return;
......
This is a testharness.js-based test.
FAIL Worker constructor with script inside text file assert_unreached: Worker should load. Reached unreachable code
Harness: the test ran to completion.
......@@ -9,6 +9,8 @@ async_test(function(t) {
worker.onmessage = t.step_func_done(function(e) {
assert_equals(e.data, "Pass");
});
// TODO(mkwst): Revisit this after https://github.com/whatwg/html/issues/3255.
worker.onerror = t.unreached_func("Worker should load.");
worker.postMessage("start");
});
</script>
<!DOCTYPE html>
<html>
<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<meta name="referrer" content="no-referrer">
</head>
<body>
<script>
async_test(function () {
var workerUrl = 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/worker.php?type=report-referrer';
var worker = new Worker(workerUrl);
worker.onmessage = this.step_func(function (event) {
assert_equals(workerUrl, event.data);
this.done();
});
}, "Worker with no referrer policy does not inherit document's policy");
async_test(function () {
var workerUrl = 'http://127.0.0.1:8000/security/contentSecurityPolicy/resources/worker.php?type=shared-report-referrer';
var worker = new SharedWorker(workerUrl);
worker.port.onmessage = this.step_func(function (event) {
assert_equals(workerUrl, event.data);
this.done();
});
worker.port.start();
}, "Shared worker with no referrer policy does not inherit document's policy");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
<script>
async_test(function () {
var worker = new Worker('http://127.0.0.1:8000/security/contentSecurityPolicy/resources/worker.php?type=report-referrer&referrerpolicy=' +
encodeURIComponent('no-referrer'));
worker.onmessage = this.step_func(function (event) {
assert_equals("HTTP Referer header is empty", event.data);
this.done();
});
}, "Worker with no-referrer CSP");
</script>
</body>
</html>
......@@ -25483,6 +25483,7 @@ Called by update_net_error_codes.py.-->
<int value="3087" label="FetchBodyStreamOutsideServiceWorker"/>
<int value="3088" label="GetComputedStyleOutsideFlatTree"/>
<int value="3089" label="ARIADescriptionAttribute"/>
<int value="3090" label="StrictMimeTypeChecksWouldBlockWorker"/>
</enum>
<enum name="FeaturePolicyAllowlistType">
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment