Bug 1828375 - Do gradual ORB transition. r=sefeng,necko-reviewers

Add a separate check for spec breaking allows of certain MIME
types. Having this separated out means that we can make the rest of
the implementation behave exactly like spec.

Some tradeoffs that we need in the current state are:

* Allowing "application/dash+xml"
* Allowing "application/vnd.apple.mpegurl"
* Allowing "text/vtt"
* Allow all MIME types beginning with "audio/mpeg"
* Allow "text/plain" when there is a no-sniff header.

Differential Revision: https://phabricator.services.mozilla.com/D176821
This commit is contained in:
Andreas Farre 2023-05-10 14:35:52 +00:00
parent 99240722e4
commit 1125476080
8 changed files with 107 additions and 5 deletions

View File

@ -214,6 +214,7 @@
#define VIDEO_MATROSKA "video/x-matroska"
#define APPLICATION_OGG "application/ogg"
#define APPLICATION_MPEGURL "application/vnd.apple.mpegurl"
#define APPLICATION_DASH_XML "application/dash+xml"
/* x-uuencode-apple-single. QuickMail made me do this. */
#define UUENCODE_APPLE_SINGLE "x-uuencode-apple-single"

View File

@ -3234,6 +3234,9 @@ HttpBaseChannel::PerformOpaqueResponseSafelistCheckBeforeSniff() {
case OpaqueResponseBlockedReason::ALLOWED_SAFE_LISTED:
// Step 3.1
return OpaqueResponse::Allow;
case OpaqueResponseBlockedReason::ALLOWED_SAFE_LISTED_SPEC_BREAKING:
LOGORB("Allowed %s in a spec breaking way", contentType.get());
return OpaqueResponse::Allow;
case OpaqueResponseBlockedReason::BLOCKED_BLOCKLISTED_NEVER_SNIFFED:
return BlockOrFilterOpaqueResponse(
mORB, u"mimeType is an opaque-blocklisted-never-sniffed MIME type"_ns,

View File

@ -37,6 +37,31 @@ static bool IsOpaqueSafeListedMIMEType(const nsACString& aContentType) {
return nsContentUtils::IsJavascriptMIMEType(typeString);
}
static bool IsOpaqueSafeListedSpecBreakingMIMEType(
const nsACString& aContentType, bool aNoSniff) {
// Avoid trouble with DASH/HLS. See bug 1698040.
if (aContentType.EqualsLiteral(APPLICATION_DASH_XML) ||
aContentType.EqualsLiteral(APPLICATION_MPEGURL) ||
aContentType.EqualsLiteral(TEXT_VTT)) {
return true;
}
// Do what Chromium does. This is from bug 1828375, and we should ideally
// revert this.
if (aContentType.EqualsLiteral(TEXT_PLAIN) && aNoSniff) {
return true;
}
// This is not a good solution, but we need this until we solve Bug 1827684 in
// a better way. Chromium currently allows all "audio/*" and "video/*", but
// from discussion in bug, we want to try only "audio/mpeg".
if (StringBeginsWith(aContentType, "audio/mpeg"_ns)) {
return true;
}
return false;
}
static bool IsOpaqueBlockListedMIMEType(const nsACString& aContentType) {
return aContentType.EqualsLiteral(TEXT_HTML) ||
StringEndsWith(aContentType, "+json"_ns) ||
@ -89,7 +114,8 @@ static bool IsOpaqueBlockListedNeverSniffedMIMEType(
aContentType.EqualsLiteral(MULTIPART_SIGNED) ||
aContentType.EqualsLiteral(TEXT_EVENT_STREAM) ||
aContentType.EqualsLiteral(TEXT_CSV) ||
aContentType.EqualsLiteral(TEXT_VTT);
aContentType.EqualsLiteral(TEXT_VTT) ||
aContentType.EqualsLiteral(APPLICATION_DASH_XML);
}
OpaqueResponseBlockedReason GetOpaqueResponseBlockedReason(
@ -102,6 +128,12 @@ OpaqueResponseBlockedReason GetOpaqueResponseBlockedReason(
return OpaqueResponseBlockedReason::ALLOWED_SAFE_LISTED;
}
// For some MIME types we deviate from spec and allow when we ideally
// shouldn't. These are returnened before any blocking takes place.
if (IsOpaqueSafeListedSpecBreakingMIMEType(aContentType, aNoSniff)) {
return OpaqueResponseBlockedReason::ALLOWED_SAFE_LISTED_SPEC_BREAKING;
}
if (IsOpaqueBlockListedNeverSniffedMIMEType(aContentType)) {
return OpaqueResponseBlockedReason::BLOCKED_BLOCKLISTED_NEVER_SNIFFED;
}

View File

@ -40,6 +40,7 @@ class nsHttpResponseHead;
enum class OpaqueResponseBlockedReason : uint32_t {
ALLOWED_SAFE_LISTED,
ALLOWED_SAFE_LISTED_SPEC_BREAKING,
BLOCKED_BLOCKLISTED_NEVER_SNIFFED,
BLOCKED_206_AND_BLOCKLISTED,
BLOCKED_NOSNIFF_AND_EITHER_BLOCKLISTED_OR_TEXTPLAIN,

View File

@ -1,4 +0,0 @@
[img-mime-types-coverage.tentative.sub.html]
prefs: [browser.opaqueResponseBlocking:true]
[CORB should allow the response if Content-Type is: 'application/dash+xml'. ]
expected: FAIL

View File

@ -0,0 +1,12 @@
[img-mime-types-coverage.tentative.sub.html]
[ORB should block the response if Content-Type is: 'application/dash+xml'. ]
expected: FAIL
[ORB should block the response if Content-Type is: 'application/vnd.apple.mpegurl'. ]
expected: FAIL
[ORB should block the response if Content-Type is: 'audio/mpegurl'. ]
expected: FAIL
[ORB should block the response if Content-Type is: 'text/vtt'. ]
expected: FAIL

View File

@ -0,0 +1,43 @@
<!-- Test verifies that cross-origin, nosniff images are 1) blocked when their
MIME type is covered by ORB and 2) allowed otherwise.
This test is very similar to fetch/orb/img-mime-types-coverage.tentative.sub.html,
except that it focuses on MIME types relevant to ORB.
-->
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<div id=log></div>
<script>
var passes = [
// These are exceptions that allow more MIME types than the ORB spec does.
// This is due to web compat, but might be removed in the future.
// See Bug 1828375
"application/dash+xml",
"application/vnd.apple.mpegurl",
"audio/mpegurl",
"audio/mpeg",
"text/vtt",
]
const get_url = (mime) => {
// www1 is cross-origin, so the HTTP response is ORB-eligible -->
url = "http://{{domains[www1]}}:{{ports[http][0]}}"
url = url + "/fetch/nosniff/resources/image.py"
if (mime != null) {
url += "?type=" + encodeURIComponent(mime)
}
return url
}
passes.forEach(function (mime) {
async_test(function (t) {
var img = document.createElement("img")
img.onerror = t.unreached_func("Unexpected error event")
img.onload = t.step_func_done(function () {
assert_equals(img.width, 96)
})
img.src = get_url(mime)
document.body.appendChild(img)
}, "ORB should allow the response if Content-Type is: '" + mime + "'. ")
})
</script>

View File

@ -3,6 +3,20 @@
const path = "http://{{domains[www1]}}:{{ports[http][0]}}/fetch/orb/resources";
// This is an exception that allow more MIME types than the ORB spec does.
// This is due to web compatibility, but might be removed in the future.
// See Bug 1828375
promise_test(
async () =>
await fetchORB(
`${path}/text.txt`,
null,
contentType("text/plain"),
contentTypeOptions("nosniff")
),
"ORB shouldn't block opaque text/plain with nosniff"
);
// Due to web compatibility we filter opaque Response object from the
// fetch() function in the Fetch specification. See Bug 1823877. This
// might be removed in the future.