Commit b0fb3c83 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

More diagnostics for tracking opaque request_initiator_site_lock.

Most DwoC reports for https://crbug.com/1056949 seem to indicate that
the request_initiator_site_lock has been set to an opaque, unique
origin.  This CL tweaks two code paths that can lead to such an opaque
origin:
1. mhtml subframes
2. committing a browser-initiated navigation to an about:blank URL
After this CL, the origins above will be derived from an artificial
origin that should show up in crash keys of DwoC reports.

This CL supplements r779447 which so far has come back empty handed.

Bug: 1056949
Change-Id: I0b6eeb266ebe4d625b2a9a949f6fec0710872c92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2273486
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783795}
parent 45e7a303
...@@ -551,8 +551,16 @@ url::Origin GetOriginForURLLoaderFactoryUnchecked( ...@@ -551,8 +551,16 @@ url::Origin GetOriginForURLLoaderFactoryUnchecked(
// make network requests on behalf of the real origin). // make network requests on behalf of the real origin).
// //
// TODO(lukasza): Cover MHTML main frames here. // TODO(lukasza): Cover MHTML main frames here.
if (navigation_request->IsForMhtmlSubframe()) if (navigation_request->IsForMhtmlSubframe()) {
return url::Origin(); // TODO(lukasza): https://crbug.com/1056949: Stop using
// mhtml.subframe.invalid as the precursor (once https://crbug.com/1056949
// is debugged OR once network::VerifyRequestInitiatorLock starts to compare
// precursors).
url::Origin mhtml_subframe_origin =
url::Origin::Create(GURL("https://mhtml.subframe.invalid"))
.DeriveNewOpaqueOrigin();
return mhtml_subframe_origin;
}
// Srcdoc subframes need to inherit their origin from their parent frame. // Srcdoc subframes need to inherit their origin from their parent frame.
if (navigation_request->GetURL().IsAboutSrcdoc()) { if (navigation_request->GetURL().IsAboutSrcdoc()) {
...@@ -564,11 +572,19 @@ url::Origin GetOriginForURLLoaderFactoryUnchecked( ...@@ -564,11 +572,19 @@ url::Origin GetOriginForURLLoaderFactoryUnchecked(
return parent->GetLastCommittedOrigin(); return parent->GetLastCommittedOrigin();
} }
// TODO(lukasza): https://crbug.com/1056949: Stop using
// browser.initiated.invalid as the precursor (once https://crbug.com/1056949
// is debugged OR once network::VerifyRequestInitiatorLock starts to compare
// precursors).
url::Origin browser_initiated_fallback_origin =
url::Origin::Create(GURL("https://browser.initiated.invalid"))
.DeriveNewOpaqueOrigin();
// In cases not covered above, URLLoaderFactory should be associated with the // In cases not covered above, URLLoaderFactory should be associated with the
// origin of |common_params.url| and/or |common_params.initiator_origin|. // origin of |common_params.url| and/or |common_params.initiator_origin|.
return url::Origin::Resolve( return url::Origin::Resolve(common_params.url,
common_params.url, common_params.initiator_origin.value_or(
common_params.initiator_origin.value_or(url::Origin())); browser_initiated_fallback_origin));
} }
url::Origin GetOriginForURLLoaderFactory( url::Origin GetOriginForURLLoaderFactory(
...@@ -588,7 +604,12 @@ url::Origin GetOriginForURLLoaderFactory( ...@@ -588,7 +604,12 @@ url::Origin GetOriginForURLLoaderFactory(
// Check that |result| origin is allowed to be accessed from the process that // Check that |result| origin is allowed to be accessed from the process that
// is the target of this navigation. // is the target of this navigation.
if (ShouldBypassChecksForErrorPage(target_frame, navigation_request)) //
// TODO(lukasza): https://crbug.com/1056949: Stop excluding opaque origins
// from the security checks once the *.invalid diagnostics are no longer
// needed.
if (ShouldBypassChecksForErrorPage(target_frame, navigation_request) ||
result.opaque())
return result; return result;
int process_id = target_frame->GetProcess()->GetID(); int process_id = target_frame->GetProcess()->GetID();
auto* policy = ChildProcessSecurityPolicyImpl::GetInstance(); auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
......
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