Commit e5f5c1f8 authored by Lucas Furukawa Gadani's avatar Lucas Furukawa Gadani Committed by Commit Bot

Report CSP frame-ancestors violations enforced in the browser.

The browser process will now notify the renderer to report a
content security policy violation.

Bug: 759184
Change-Id: I1a4f8fc82d522c67d7587c0dd9b65550c9354ac7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857202Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713494}
parent 69ee66f3
...@@ -114,8 +114,8 @@ AncestorThrottle::WillRedirectRequest() { ...@@ -114,8 +114,8 @@ AncestorThrottle::WillRedirectRequest() {
// so we can't log reliably to the console. We should be able to work around // so we can't log reliably to the console. We should be able to work around
// this iff we decide to ship the redirect-blocking behavior, but for now // this iff we decide to ship the redirect-blocking behavior, but for now
// we'll just skip the console-logging bits to collect metrics. // we'll just skip the console-logging bits to collect metrics.
NavigationThrottle::ThrottleCheckResult result = NavigationThrottle::ThrottleCheckResult result = ProcessResponseImpl(
ProcessResponseImpl(LoggingDisposition::DO_NOT_LOG_TO_CONSOLE); LoggingDisposition::DO_NOT_LOG_TO_CONSOLE, false /* is_response_check */);
if (result.action() == NavigationThrottle::BLOCK_RESPONSE) if (result.action() == NavigationThrottle::BLOCK_RESPONSE)
RecordXFrameOptionsUsage(XFrameOptionsHistogram::REDIRECT_WOULD_BE_BLOCKED); RecordXFrameOptionsUsage(XFrameOptionsHistogram::REDIRECT_WOULD_BE_BLOCKED);
...@@ -129,11 +129,13 @@ AncestorThrottle::WillRedirectRequest() { ...@@ -129,11 +129,13 @@ AncestorThrottle::WillRedirectRequest() {
NavigationThrottle::ThrottleCheckResult NavigationThrottle::ThrottleCheckResult
AncestorThrottle::WillProcessResponse() { AncestorThrottle::WillProcessResponse() {
return ProcessResponseImpl(LoggingDisposition::LOG_TO_CONSOLE); return ProcessResponseImpl(LoggingDisposition::LOG_TO_CONSOLE,
true /* is_response_check */);
} }
NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl( NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl(
LoggingDisposition logging) { LoggingDisposition logging,
bool is_response_check) {
DCHECK(!navigation_handle()->IsInMainFrame()); DCHECK(!navigation_handle()->IsInMainFrame());
NavigationRequest* request = NavigationRequest::From(navigation_handle()); NavigationRequest* request = NavigationRequest::From(navigation_handle());
...@@ -150,9 +152,13 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl( ...@@ -150,9 +152,13 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl(
if (network::mojom::ContentSecurityPolicyPtr policy = if (network::mojom::ContentSecurityPolicyPtr policy =
request->response()->head.content_security_policy) { request->response()->head.content_security_policy) {
if (auto& frame_ancestors = policy->frame_ancestors) { if (auto& frame_ancestors = policy->frame_ancestors) {
// TODO(lfg, arthursonzogni): Move the frame-ancestors check to a common
// ContentSecurityPolicy object instead of checking directly against the
// CSPSourceList.
CSPSourceList frame_ancestors_list(*frame_ancestors); CSPSourceList frame_ancestors_list(*frame_ancestors);
frame_ancestors_list.allow_response_redirects = true; frame_ancestors_list.allow_response_redirects = true;
FrameTreeNode* parent = request->frame_tree_node()->parent(); FrameTreeNode* parent = request->frame_tree_node()->parent();
bool has_followed_redirect = navigation_handle()->WasServerRedirect();
// Since the navigation hasn't committed yet, we need to create a // Since the navigation hasn't committed yet, we need to create a
// CSPContext for the navigation handle. // CSPContext for the navigation handle.
CSPContext csp_context; CSPContext csp_context;
...@@ -162,12 +168,36 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl( ...@@ -162,12 +168,36 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl(
parent->current_frame_host() parent->current_frame_host()
->GetLastCommittedOrigin() ->GetLastCommittedOrigin()
.GetURL(), .GetURL(),
&csp_context, &csp_context, has_followed_redirect,
false /* has_followed_redirect */, is_response_check)) {
true /* is_response_check) */)) {
parent = parent->parent(); parent = parent->parent();
continue; continue;
} }
auto* frame_to_commit = static_cast<RenderFrameHostImpl*>(
navigation_handle()->GetRenderFrameHost());
GURL blocked_url = navigation_handle()->GetURL();
SourceLocation source_location;
frame_to_commit->SanitizeDataForUseInCspViolation(
has_followed_redirect, CSPDirective::FrameAncestors, &blocked_url,
&source_location);
std::vector<std::string> report_endpoints;
for (auto& url : policy->report_endpoints)
report_endpoints.push_back(url.spec());
frame_to_commit->ReportContentSecurityPolicyViolation(
// The browser doesn't have the raw CSP text to report in the
// message.
CSPViolationParams(
"frame-ancestors", "frame-ancestors",
base::StringPrintf(
"Refused to display '%s' in a frame because an ancestor "
"violates the frame-ancestors Content Security Policy.",
blocked_url.spec().c_str()),
blocked_url, report_endpoints, policy->use_reporting_api,
"" /* header */,
blink::mojom::ContentSecurityPolicyType::kEnforce,
has_followed_redirect, source_location));
return NavigationThrottle::BLOCK_RESPONSE; return NavigationThrottle::BLOCK_RESPONSE;
} }
return NavigationThrottle::PROCEED; return NavigationThrottle::PROCEED;
......
...@@ -52,7 +52,8 @@ class CONTENT_EXPORT AncestorThrottle : public NavigationThrottle { ...@@ -52,7 +52,8 @@ class CONTENT_EXPORT AncestorThrottle : public NavigationThrottle {
explicit AncestorThrottle(NavigationHandle* handle); explicit AncestorThrottle(NavigationHandle* handle);
NavigationThrottle::ThrottleCheckResult ProcessResponseImpl( NavigationThrottle::ThrottleCheckResult ProcessResponseImpl(
LoggingDisposition); LoggingDisposition logging,
bool is_response_check);
void ParseError(const std::string& value, HeaderDisposition disposition); void ParseError(const std::string& value, HeaderDisposition disposition);
void ConsoleError(HeaderDisposition disposition); void ConsoleError(HeaderDisposition disposition);
......
...@@ -17,6 +17,7 @@ static CSPDirective::Name CSPFallback(CSPDirective::Name directive) { ...@@ -17,6 +17,7 @@ static CSPDirective::Name CSPFallback(CSPDirective::Name directive) {
case CSPDirective::FormAction: case CSPDirective::FormAction:
case CSPDirective::UpgradeInsecureRequests: case CSPDirective::UpgradeInsecureRequests:
case CSPDirective::NavigateTo: case CSPDirective::NavigateTo:
case CSPDirective::FrameAncestors:
return CSPDirective::Unknown; return CSPDirective::Unknown;
case CSPDirective::FrameSrc: case CSPDirective::FrameSrc:
......
...@@ -32,6 +32,8 @@ std::string CSPDirective::NameToString(CSPDirective::Name name) { ...@@ -32,6 +32,8 @@ std::string CSPDirective::NameToString(CSPDirective::Name name) {
return "upgrade-insecure-requests"; return "upgrade-insecure-requests";
case NavigateTo: case NavigateTo:
return "navigate-to"; return "navigate-to";
case FrameAncestors:
return "frame-ancestors";
case Unknown: case Unknown:
return ""; return "";
} }
...@@ -53,6 +55,8 @@ CSPDirective::Name CSPDirective::StringToName(const std::string& name) { ...@@ -53,6 +55,8 @@ CSPDirective::Name CSPDirective::StringToName(const std::string& name) {
return CSPDirective::UpgradeInsecureRequests; return CSPDirective::UpgradeInsecureRequests;
if (name == "navigate-to") if (name == "navigate-to")
return CSPDirective::NavigateTo; return CSPDirective::NavigateTo;
if (name == "frame-ancestors")
return CSPDirective::FrameAncestors;
return CSPDirective::Unknown; return CSPDirective::Unknown;
} }
......
...@@ -28,6 +28,7 @@ struct CONTENT_EXPORT CSPDirective { ...@@ -28,6 +28,7 @@ struct CONTENT_EXPORT CSPDirective {
FormAction, FormAction,
UpgradeInsecureRequests, UpgradeInsecureRequests,
NavigateTo, NavigateTo,
FrameAncestors,
Unknown, Unknown,
NameLast = Unknown, NameLast = Unknown,
......
...@@ -2107,14 +2107,19 @@ void WebLocalFrameImpl::ReportContentSecurityPolicyViolation( ...@@ -2107,14 +2107,19 @@ void WebLocalFrameImpl::ReportContentSecurityPolicyViolation(
Vector<String> report_endpoints; Vector<String> report_endpoints;
for (const WebString& end_point : violation.report_endpoints) for (const WebString& end_point : violation.report_endpoints)
report_endpoints.push_back(end_point); report_endpoints.push_back(end_point);
auto directive_type =
ContentSecurityPolicy::GetDirectiveType(violation.effective_directive);
LocalFrame* context_frame =
directive_type == ContentSecurityPolicy::DirectiveType::kFrameAncestors
? GetFrame()
: nullptr;
document->GetContentSecurityPolicy()->ReportViolation( document->GetContentSecurityPolicy()->ReportViolation(
violation.directive, violation.directive, directive_type, violation.console_message,
ContentSecurityPolicy::GetDirectiveType(violation.effective_directive), violation.blocked_url, report_endpoints, violation.use_reporting_api,
violation.console_message, violation.blocked_url, report_endpoints, violation.header,
violation.use_reporting_api, violation.header,
static_cast<ContentSecurityPolicyHeaderType>(violation.disposition), static_cast<ContentSecurityPolicyHeaderType>(violation.disposition),
ContentSecurityPolicy::ViolationType::kURLViolation, ContentSecurityPolicy::ViolationType::kURLViolation,
std::move(source_location), nullptr /* LocalFrame */, std::move(source_location), context_frame,
violation.after_redirect ? RedirectStatus::kFollowedRedirect violation.after_redirect ? RedirectStatus::kFollowedRedirect
: RedirectStatus::kNoRedirect, : RedirectStatus::kNoRedirect,
nullptr /* Element */); nullptr /* Element */);
......
This is a testharness.js-based test.
FAIL Violation report status OK. assert_equals: No such report. expected "" but got "false"
Harness: the test ran to completion.
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