Skip to content

Commit

Permalink
[M91][ResourceTiming] Fix Timing Allow Origin checks
Browse files Browse the repository at this point in the history 
(cherry picked from commit 61d41d9)

Bug: 1197675
Change-Id: I70b2774c827e8b2190b5e16b6285c05874e900c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2819318
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Tom McKee <tommckee@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#871953}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2826652
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4472@{#69}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}
  • Loading branch information
@npm1
npm1 authored and Chromium LUCI CQ committed Apr 14, 2021
1 parent 75b42f5 commit 619f57c
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 133 deletions.
14 changes: 4 additions & 10 deletions third_party/blink/renderer/core/timing/performance.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,6 @@ bool Performance::PassesTimingAllowCheck(
if (header == security_origin)
contains_security_origin = true;
}

// If the tainted origin flag is set and the header contains the origin, this
// means that this method currently passes the check but once we implement the
// tainted origin flag properly then it will fail the check. Record this in a
Expand Down Expand Up @@ -511,18 +510,13 @@ mojom::blink::ResourceTimingInfoPtr Performance::GenerateResourceTiming(
result->context_type = info.ContextType();
result->request_destination = info.RequestDestination();

bool response_tainting_not_basic = false;
bool tainted_origin_flag = false;
result->allow_timing_details = PassesTimingAllowCheck(
final_response, final_response, destination_origin,
&context_for_use_counter, &response_tainting_not_basic,
&tainted_origin_flag);
result->allow_timing_details =
AllowsTimingRedirect(info.RedirectChain(), final_response,
destination_origin, &context_for_use_counter);

const Vector<ResourceResponse>& redirect_chain = info.RedirectChain();
if (!redirect_chain.IsEmpty()) {
result->allow_redirect_details =
AllowsTimingRedirect(redirect_chain, final_response, destination_origin,
&context_for_use_counter);
result->allow_redirect_details = result->allow_timing_details;

// TODO(https://crbug.com/817691): is |last_chained_timing| being null a bug
// or is this if statement reasonable?
Expand Down
10 changes: 7 additions & 3 deletions third_party/blink/renderer/core/timing/performance_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ TEST_F(PerformanceTest, AllowsTimingRedirect) {
EXPECT_FALSE(AllowsTimingRedirect(redirect_chain, empty_final_response,
*security_origin.get(),
GetExecutionContext()));
// Final response is same origin as requestor.
ResourceResponse final_response(url);
EXPECT_TRUE(AllowsTimingRedirect(redirect_chain, final_response,
*security_origin.get(),
Expand All @@ -184,9 +185,12 @@ TEST_F(PerformanceTest, AllowsTimingRedirect) {
EXPECT_FALSE(AllowsTimingRedirect(redirect_chain, final_response,
*security_origin.get(),
GetExecutionContext()));
// When cross-origin redirect opts in, and the final response has as well.
final_response.SetHttpHeaderField(http_names::kTimingAllowOrigin,
origin_domain);
// TODO(npm): when tainted origin flag is set and header is origin,
// we should fail.

// Change the opt ins to be '*' and then the check should pass.
redirect_chain.back().SetHttpHeaderField(http_names::kTimingAllowOrigin, "*");
final_response.SetHttpHeaderField(http_names::kTimingAllowOrigin, "*");
EXPECT_TRUE(AllowsTimingRedirect(redirect_chain, final_response,
*security_origin.get(),
GetExecutionContext()));
Expand Down
2 changes: 1 addition & 1 deletion ...link/web_tests/external/wpt/resource-timing/tao-origin-SO-XO-SO-redirect-chain.https.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<body>
<script>
const {HTTPS_REMOTE_ORIGIN} = get_host_info();
const SAME_ORIGIN = document.location.host;
const SAME_ORIGIN = location.origin;
let destUrl = `${HTTPS_REMOTE_ORIGIN}/resource-timing/resources/multi_redirect.py?`;
destUrl += `page_origin=http://${SAME_ORIGIN}`;
destUrl += `&cross_origin=${HTTPS_REMOTE_ORIGIN}`;
Expand Down
106 changes: 53 additions & 53 deletions third_party/blink/web_tests/http/tests/misc/resource-timing-sizes-redirect-img.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,102 +3,102 @@
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/get-host-info.js?pipe=sub"></script>
<script>
// Redirects for fetch() always apply CORS rules, whereas normal resources
// don't, so this test covers extra code paths beyond those covered by
// resource-timing-sizes-redirect.html.
// Redirects for fetch() always apply CORS rules, whereas normal resources
// don't, so this test covers extra code paths beyond those covered by
// resource-timing-sizes-redirect.html.

const baseUrl = new URL('/resources/square20.png', location.href).href;
const baseUrl = new URL('/resources/square20-with-tao.php', location.href).href;

// Because apache decrements the Keep-Alive max value on each request, the
// transferSize will vary slightly between requests for the same resource.
const fuzzFactor = 3; // bytes
// Because apache decrements the Keep-Alive max value on each request, the
// transferSize will vary slightly between requests for the same resource.
const fuzzFactor = 3; // bytes

const minHeaderSize = 100;
const minHeaderSize = 100;

const hostInfo = get_host_info();
const hostInfo = get_host_info();

var directUrl, sameOriginRedirect, crossOriginRedirect, mixedRedirect;
var complexRedirect;
var t = async_test('PerformanceResourceTiming sizes redirects img');
var directUrl, sameOriginRedirect, crossOriginRedirect, mixedRedirect;
var complexRedirect;
var t = async_test('PerformanceResourceTiming sizes redirects img');

function checkResourceSizes() {
function checkResourceSizes() {
var entries = performance.getEntriesByType('resource');
var lowerBound, upperBound, withRedirectLowerBound;
var seenCount = 0;
for (var entry of entries) {
switch (entry.name) {
switch (entry.name) {
case directUrl:
assert_greater_than(entry.transferSize, minHeaderSize,
'direct transferSize');
lowerBound = entry.transferSize - fuzzFactor;
upperBound = entry.transferSize + fuzzFactor;
withRedirectLowerBound = entry.transferSize + minHeaderSize;
++seenCount;
break;
assert_greater_than(entry.transferSize, minHeaderSize,
'direct transferSize');
lowerBound = entry.transferSize - fuzzFactor;
upperBound = entry.transferSize + fuzzFactor;
withRedirectLowerBound = entry.transferSize + minHeaderSize;
++seenCount;
break;

case sameOriginRedirect:
assert_greater_than(entry.transferSize, withRedirectLowerBound,
'same origin transferSize');
++seenCount;
break;
assert_greater_than(entry.transferSize, withRedirectLowerBound,
'same origin transferSize');
++seenCount;
break;

case crossOriginRedirect:
case mixedRedirect:
case complexRedirect:
assert_between_exclusive(entry.transferSize, lowerBound, upperBound,
'cross origin transferSize');
++seenCount;
break;
assert_between_exclusive(entry.transferSize, lowerBound, upperBound,
'cross origin transferSize');
++seenCount;
break;

default:
break;
}
break;
}
}
assert_equals(seenCount, 5, 'seenCount');
t.done();
}
}

function redirectUrl(redirectSourceOrigin, targetUrl) {
function redirectUrl(redirectSourceOrigin, targetUrl) {
return redirectSourceOrigin +
'/resources/redirect.php?url=' + encodeURIComponent(targetUrl);
}
'/resources/redirect.php?url=' + encodeURIComponent(targetUrl) + "&timing_allow_origin=*";
}

// Loads the images in |urlArray| in sequence, finally calling |callback|.
// |callback| will be wrapped in t.step_func() when called.
function loadImages(urlArray, callback) {
// Loads the images in |urlArray| in sequence, finally calling |callback|.
// |callback| will be wrapped in t.step_func() when called.
function loadImages(urlArray, callback) {
var url = urlArray.shift();
var onload;
if (urlArray.length === 0) {
onload = t.step_func(callback);
onload = t.step_func(callback);
} else {
onload = () => { loadImages(urlArray, callback); };
onload = () => { loadImages(urlArray, callback); };
}
var img = document.createElement('img');
img.src = url;
img.onload = onload;
img.onerror = t.step_func(() => assert_unreached('Failed to load ' + url));
img.style = 'display: none;';
}
}

function cacheBustedUrl() {
function cacheBustedUrl() {
return baseUrl + '?unique=' + Math.random().toString().substring(2);
}
}

function runTest() {
function runTest() {
directUrl = cacheBustedUrl();
sameOriginRedirect = redirectUrl(hostInfo.HTTP_ORIGIN, cacheBustedUrl());
crossOriginRedirect = redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
cacheBustedUrl());
cacheBustedUrl());
mixedRedirect = redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
redirectUrl(
hostInfo.HTTP_ORIGIN, cacheBustedUrl()));
redirectUrl(
hostInfo.HTTP_ORIGIN, cacheBustedUrl()));
complexRedirect = redirectUrl(hostInfo.HTTP_ORIGIN,
redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
redirectUrl(hostInfo.HTTP_ORIGIN,
cacheBustedUrl())));
redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
redirectUrl(hostInfo.HTTP_ORIGIN,
cacheBustedUrl())));
loadImages([directUrl, sameOriginRedirect, crossOriginRedirect,
mixedRedirect, complexRedirect], checkResourceSizes);
}
mixedRedirect, complexRedirect], checkResourceSizes);
}

t.step(runTest);
t.step(runTest);
</script>
133 changes: 67 additions & 66 deletions third_party/blink/web_tests/http/tests/misc/resources/resource-timing-sizes-redirect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// resource-timing-sizes-redirect-worker.html

if (typeof document === 'undefined') {
importScripts('/resources/testharness.js',
'/resources/get-host-info.js?pipe=sub',
'/misc/resources/run-async-tasks-promise.js');
importScripts('/resources/testharness.js',
'/resources/get-host-info.js?pipe=sub',
'/misc/resources/run-async-tasks-promise.js');
}

const baseUrl =
new URL('/security/resources/cors-hello.php', location.href).href;
new URL('/security/resources/cors-hello.php', location.href).href;
const expectedSize = 73;

// Because apache decrements the Keep-Alive max value on each request, the
Expand All @@ -23,81 +23,82 @@ var directUrl, sameOriginRedirect, crossOriginRedirect, mixedRedirect;
var complexRedirect;

function checkBodySizeFields(entry) {
assert_equals(entry.decodedBodySize, expectedSize, 'decodedBodySize');
assert_equals(entry.encodedBodySize, expectedSize, 'encodedBodySize');
assert_equals(entry.decodedBodySize, expectedSize, 'decodedBodySize');
assert_equals(entry.encodedBodySize, expectedSize, 'encodedBodySize');
}

function checkResourceSizes() {
var entries = performance.getEntriesByType('resource');
var lowerBound, upperBound, withRedirectLowerBound;
var seenCount = 0;
for (var entry of entries) {
switch (entry.name) {
case directUrl:
checkBodySizeFields(entry);
assert_greater_than(entry.transferSize, expectedSize,
'transferSize');
lowerBound = entry.transferSize - fuzzFactor;
upperBound = entry.transferSize + fuzzFactor;
withRedirectLowerBound = entry.transferSize + minHeaderSize;
++seenCount;
break;
var entries = performance.getEntriesByType('resource');
var lowerBound, upperBound, withRedirectLowerBound;
var seenCount = 0;
for (var entry of entries) {
switch (entry.name) {
case directUrl:
checkBodySizeFields(entry);
assert_greater_than(entry.transferSize, expectedSize,
'transferSize');
lowerBound = entry.transferSize - fuzzFactor;
upperBound = entry.transferSize + fuzzFactor;
withRedirectLowerBound = entry.transferSize + minHeaderSize;
++seenCount;
break;

case sameOriginRedirect:
checkBodySizeFields(entry);
assert_greater_than(entry.transferSize, withRedirectLowerBound,
'transferSize');
++seenCount;
break;
case sameOriginRedirect:
checkBodySizeFields(entry);
assert_greater_than(entry.transferSize, withRedirectLowerBound,
'transferSize');
++seenCount;
break;

case crossOriginRedirect:
case mixedRedirect:
case complexRedirect:
checkBodySizeFields(entry);
assert_between_exclusive(entry.transferSize, lowerBound, upperBound,
'transferSize');
++seenCount;
break;
case crossOriginRedirect:
case mixedRedirect:
case complexRedirect:
checkBodySizeFields(entry);
assert_between_exclusive(entry.transferSize, lowerBound, upperBound,
'transferSize');
++seenCount;
break;

default:
break;
}
default:
break;
}
assert_equals(seenCount, 5, 'seenCount');
}
assert_equals(seenCount, 5, 'seenCount');
}

function redirectUrl(redirectSourceOrigin, allowOrigin, targetUrl) {
return redirectSourceOrigin +
'/resources/redirect.php?cors_allow_origin=' +
encodeURIComponent(allowOrigin) +
'&url=' + encodeURIComponent(targetUrl);
return redirectSourceOrigin +
'/resources/redirect.php?cors_allow_origin=' +
encodeURIComponent(allowOrigin) +
'&url=' + encodeURIComponent(targetUrl) +
'&timing_allow_origin=*';
}

promise_test(() => {
// Use a different URL every time so that the cache behaviour does not
// depend on execution order.
directUrl = baseUrl + '?unique=' + Math.random().toString().substring(2) +
'&cors=*';
sameOriginRedirect = redirectUrl(hostInfo.HTTP_ORIGIN, '*', directUrl);
crossOriginRedirect = redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
hostInfo.HTTP_ORIGIN, directUrl);
mixedRedirect = redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
hostInfo.HTTP_ORIGIN, sameOriginRedirect);
complexRedirect = redirectUrl(hostInfo.HTTP_ORIGIN,
hostInfo.HTTP_REMOTE_ORIGIN, mixedRedirect);
var eatBody = response => response.arrayBuffer();
return fetch(directUrl)
.then(eatBody)
.then(() => fetch(sameOriginRedirect))
.then(eatBody)
.then(() => fetch(crossOriginRedirect))
.then(eatBody)
.then(() => fetch(mixedRedirect))
.then(eatBody)
.then(() => fetch(complexRedirect))
.then(eatBody)
.then(runAsyncTasks)
.then(checkResourceSizes);
// Use a different URL every time so that the cache behaviour does not
// depend on execution order.
directUrl = baseUrl + '?unique=' + Math.random().toString().substring(2) +
'&cors=*';
sameOriginRedirect = redirectUrl(hostInfo.HTTP_ORIGIN, '*', directUrl);
crossOriginRedirect = redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
hostInfo.HTTP_ORIGIN, directUrl);
mixedRedirect = redirectUrl(hostInfo.HTTP_REMOTE_ORIGIN,
hostInfo.HTTP_ORIGIN, sameOriginRedirect);
complexRedirect = redirectUrl(hostInfo.HTTP_ORIGIN,
hostInfo.HTTP_REMOTE_ORIGIN, mixedRedirect);
var eatBody = response => response.arrayBuffer();
return fetch(directUrl)
.then(eatBody)
.then(() => fetch(sameOriginRedirect))
.then(eatBody)
.then(() => fetch(crossOriginRedirect))
.then(eatBody)
.then(() => fetch(mixedRedirect))
.then(eatBody)
.then(() => fetch(complexRedirect))
.then(eatBody)
.then(runAsyncTasks)
.then(checkResourceSizes);
}, 'PerformanceResourceTiming sizes Fetch with redirect test');

done();
2 changes: 2 additions & 0 deletions third_party/blink/web_tests/http/tests/resources/redirect.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
header("Location: $url");
if (isset($_GET['cors_allow_origin']))
header("Access-Control-Allow-Origin: " . $_GET['cors_allow_origin']);
if (isset($_GET['timing_allow_origin']))
header("Timing-Allow-Origin: " . $_GET['timing_allow_origin']);

$code = $_GET['code'];
if (!isset($code))
Expand Down

0 comments on commit 619f57c

Please sign in to comment.