Index: content/browser/frame_host/navigation_request.cc |
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc |
index c74c6031fab6ffaaf33cb9384de74038b571f597..1cb627fad9e296500f99e2eac7d91a9659052940 100644 |
--- a/content/browser/frame_host/navigation_request.cc |
+++ b/content/browser/frame_host/navigation_request.cc |
@@ -945,8 +945,40 @@ NavigationRequest::CheckContentSecurityPolicyFrameSrc(bool is_redirect) { |
SourceLocation source_location; |
if (common_params_.source_location) |
source_location = common_params_.source_location.value(); |
+ |
+ // CSP checking happens in three phases, per steps 3-5 of |
+ // https://fetch.spec.whatwg.org/#main-fetch: |
+ // |
+ // (1) Check report-only policies and trigger reports for any violations. |
+ // (2) Upgrade the request to HTTPS if necessary. |
+ // (3) Check enforced policies (triggering reports for any violations of those |
+ // policies) and block the request if necessary. |
+ // |
+ // This sequence of events allows site owners to learn about (via step 1) any |
+ // requests that are upgraded in step 2. |
+ |
+ bool allowed = parent->IsAllowedByCsp( |
+ CSPDirective::FrameSrc, common_params_.url, is_redirect, source_location, |
+ CSPContext::CHECK_REPORT_ONLY_CSP); |
+ // Checking report-only CSP should never return false because no requests are |
nasko
2017/06/05 21:31:49
nit: empty line before the comment.
estark
2017/06/06 19:16:14
Done.
|
+ // blocked by report-only policies. |
+ DCHECK(allowed); |
+ |
+ // TODO(mkwst,estark): upgrade-insecure-requests does not work when following |
+ // redirects. Trying to uprade the new URL on redirect here is fruitless: the |
+ // redirect URL cannot be changed at this point. upgrade-insecure-requests |
+ // needs to move to the net stack to resolve this. https://crbug.com/615885 |
+ if (!is_redirect) { |
+ GURL new_url; |
+ if (parent->ShouldModifyRequestUrlForCsp( |
+ common_params_.url, true /* is subresource */, &new_url)) { |
nasko
2017/06/05 21:31:49
We will have to be careful here with the changes c
estark
2017/06/06 19:16:14
Yes. As discussed in person, there are some open q
|
+ common_params_.url = new_url; |
+ } |
+ } |
+ |
if (parent->IsAllowedByCsp(CSPDirective::FrameSrc, common_params_.url, |
- is_redirect, source_location)) { |
+ is_redirect, source_location, |
+ CSPContext::CHECK_ENFORCED_CSP)) { |
return CONTENT_SECURITY_POLICY_CHECK_PASSED; |
} |