Skip to content

Commit

Permalink
Unify CSP: Detriplicate ContentSecurityPolicyDisposition
Browse files Browse the repository at this point in the history
Merge:
 - content::CSPDisposition
 - blink::ContentSecurityPolicyDisposition
 - blink::WebContentSecurityPolicyDisposition

Along the way, mojoifying it into network::mojom::CSPDisposition.

deleted files:
 - content/common/content_security_policy/csp_disposition_enum.h
 - third_party/blink/public/platform/web_content_security_policy.h

See UnifyCSP:
https://docs.google.com/document/d/1v5mJnXJ5dSVXE_rgvJnNM9bzH0ni0YzdhPQ7GLqyhao

Bug: 1021462
Change-Id: Ic7a018978ae525d66172e70aa88accb878621b46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2002570
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732831}
  • Loading branch information
ArthurSonzogni authored and Commit Bot committed Jan 17, 2020
1 parent a19369a commit 128a74d
Show file tree
Hide file tree
Showing 49 changed files with 82 additions and 162 deletions.
2 changes: 1 addition & 1 deletion content/browser/frame_host/form_submission_throttle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ FormSubmissionThrottle::CheckContentSecurityPolicyFormAction(
NavigationRequest* request = NavigationRequest::From(navigation_handle());

if (request->common_params().initiator_csp_info.should_check_main_world_csp ==
CSPDisposition::DO_NOT_CHECK) {
network::mojom::CSPDisposition::DO_NOT_CHECK) {
return NavigationThrottle::PROCEED;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ TEST_F(FormSubmissionTest, ContentSecurityPolicyFormActionNoCSP) {
auto form_submission =
NavigationSimulatorImpl::CreateRendererInitiated(kFormUrl, main_rfh());
form_submission->SetIsFormSubmission(true);
form_submission->set_should_check_main_world_csp(CSPDisposition::CHECK);
form_submission->set_should_check_main_world_csp(
network::mojom::CSPDisposition::CHECK);
form_submission->Start();
EXPECT_EQ(NavigationThrottle::PROCEED,
form_submission->GetLastThrottleCheckResult());
Expand All @@ -58,7 +59,8 @@ TEST_F(FormSubmissionTest, ContentSecurityPolicyFormActionNone) {
auto form_submission =
NavigationSimulatorImpl::CreateRendererInitiated(kFormUrl, main_rfh());
form_submission->SetIsFormSubmission(true);
form_submission->set_should_check_main_world_csp(CSPDisposition::CHECK);
form_submission->set_should_check_main_world_csp(
network::mojom::CSPDisposition::CHECK);

// Browser side checks have been disabled on the initial load. Only the
// renderer side checks occurs. Related issue: https://crbug.com/798698.
Expand Down Expand Up @@ -88,7 +90,7 @@ TEST_F(FormSubmissionTest, ContentSecurityPolicyFormActionBypassCSP) {
NavigationSimulatorImpl::CreateRendererInitiated(kFormUrl, main_rfh());
form_submission->SetIsFormSubmission(true);
form_submission->set_should_check_main_world_csp(
CSPDisposition::DO_NOT_CHECK);
network::mojom::CSPDisposition::DO_NOT_CHECK);
form_submission->Start();
EXPECT_EQ(NavigationThrottle::PROCEED,
form_submission->GetLastThrottleCheckResult());
Expand Down
2 changes: 1 addition & 1 deletion content/browser/frame_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2628,7 +2628,7 @@ net::Error NavigationRequest::CheckContentSecurityPolicy(
return net::OK;

if (common_params_->initiator_csp_info.should_check_main_world_csp ==
CSPDisposition::DO_NOT_CHECK) {
network::mojom::CSPDisposition::DO_NOT_CHECK) {
return net::OK;
}

Expand Down
9 changes: 2 additions & 7 deletions content/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ source_set("common") {
"content_security_policy/csp_context.h",
"content_security_policy/csp_directive.cc",
"content_security_policy/csp_directive.h",
"content_security_policy/csp_disposition_enum.h",
"content_security_policy/csp_source.cc",
"content_security_policy/csp_source.h",
"content_security_policy/csp_source_list.cc",
Expand Down Expand Up @@ -436,9 +435,7 @@ source_set("common") {
group("for_content_tests") {
visibility = [ "//content/test/*" ]
if (!is_component_build) {
public_deps = [
":common",
]
public_deps = [ ":common" ]
}
}

Expand Down Expand Up @@ -568,8 +565,6 @@ if (is_mac) {

write_file(_file, _lines)

sources = [
_file,
]
sources = [ _file ]
}
}
1 change: 0 additions & 1 deletion content/common/content_param_traits_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "services/network/public/mojom/content_security_policy.mojom.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_cursor_info.h"
#include "third_party/blink/public/platform/web_text_autosizer_page_info.h"
#include "third_party/blink/public/web/web_ime_text_span.h"
Expand Down
19 changes: 0 additions & 19 deletions content/common/content_security_policy/csp_disposition_enum.h

This file was deleted.

5 changes: 3 additions & 2 deletions content/common/frame_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ IPC_ENUM_TRAITS_MAX_VALUE(network::mojom::CSPDirectiveName,
network::mojom::CSPDirectiveName::kMaxValue)
IPC_ENUM_TRAITS_MAX_VALUE(blink::mojom::FeaturePolicyFeature,
blink::mojom::FeaturePolicyFeature::kMaxValue)
IPC_ENUM_TRAITS_MAX_VALUE(content::CSPDisposition,
content::CSPDisposition::LAST)
IPC_ENUM_TRAITS_MAX_VALUE(blink::TriggeringEventInfo,
blink::TriggeringEventInfo::kMaxValue)
IPC_ENUM_TRAITS_MAX_VALUE(blink::mojom::UserActivationUpdateType,
Expand Down Expand Up @@ -610,6 +608,9 @@ IPC_STRUCT_TRAITS_BEGIN(content::CSPViolationParams)
IPC_STRUCT_TRAITS_MEMBER(source_location)
IPC_STRUCT_TRAITS_END()

IPC_ENUM_TRAITS_MAX_VALUE(network::mojom::CSPDisposition,
network::mojom::CSPDisposition::kMaxValue)

IPC_STRUCT_BEGIN(FrameMsg_MixedContentFound_Params)
IPC_STRUCT_MEMBER(GURL, main_resource_url)
IPC_STRUCT_MEMBER(GURL, mixed_content_url)
Expand Down
2 changes: 1 addition & 1 deletion content/common/navigation_params.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ SourceLocation::~SourceLocation() = default;

InitiatorCSPInfo::InitiatorCSPInfo() = default;
InitiatorCSPInfo::InitiatorCSPInfo(
CSPDisposition should_check_main_world_csp,
network::mojom::CSPDisposition should_check_main_world_csp,
const std::vector<ContentSecurityPolicy>& initiator_csp,
const base::Optional<CSPSource>& initiator_self_source)
: should_check_main_world_csp(should_check_main_world_csp),
Expand Down
6 changes: 3 additions & 3 deletions content/common/navigation_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "build/build_config.h"
#include "content/common/content_export.h"
#include "content/common/content_security_policy/content_security_policy.h"
#include "content/common/content_security_policy/csp_disposition_enum.h"
#include "content/common/navigation_params.mojom-forward.h"
#include "content/common/prefetched_signed_exchange_info.mojom.h"
#include "content/public/common/navigation_policy.h"
Expand Down Expand Up @@ -61,7 +60,7 @@ struct CONTENT_EXPORT SourceLocation {
// Represents the Content Security Policy of the initator of the navigation.
struct CONTENT_EXPORT InitiatorCSPInfo {
InitiatorCSPInfo();
InitiatorCSPInfo(CSPDisposition should_check_main_world_csp,
InitiatorCSPInfo(network::mojom::CSPDisposition should_check_main_world_csp,
const std::vector<ContentSecurityPolicy>& initiator_csp,
const base::Optional<CSPSource>& initiator_self_source);
InitiatorCSPInfo(const InitiatorCSPInfo& other);
Expand All @@ -73,7 +72,8 @@ struct CONTENT_EXPORT InitiatorCSPInfo {
// TODO(arthursonzogni): Instead of this boolean, the origin of the isolated
// world which has initiated the navigation should be passed.
// See https://crbug.com/702540
CSPDisposition should_check_main_world_csp = CSPDisposition::CHECK;
network::mojom::CSPDisposition should_check_main_world_csp =
network::mojom::CSPDisposition::CHECK;

// The relevant CSP policies and the initiator 'self' source to be used.
std::vector<ContentSecurityPolicy> initiator_csp;
Expand Down
8 changes: 1 addition & 7 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,6 @@ mojom::CommonNavigationParamsPtr MakeCommonNavigationParams(
info->source_location.column_number);
}

CSPDisposition should_check_main_world_csp =
info->should_check_main_world_content_security_policy ==
blink::kWebContentSecurityPolicyDispositionCheck
? CSPDisposition::CHECK
: CSPDisposition::DO_NOT_CHECK;

const RequestExtraData* extra_data =
static_cast<RequestExtraData*>(info->url_request.GetExtraData());
DCHECK(extra_data);
Expand All @@ -577,7 +571,7 @@ mojom::CommonNavigationParamsPtr MakeCommonNavigationParams(
base::TimeTicks::Now(), info->url_request.HttpMethod().Latin1(),
GetRequestBodyForWebURLRequest(info->url_request), source_location,
false /* started_from_context_menu */, info->url_request.HasUserGesture(),
InitiatorCSPInfo(should_check_main_world_csp,
InitiatorCSPInfo(info->should_check_main_world_content_security_policy,
BuildContentSecurityPolicyList(info->initiator_csp),
info->initiator_csp.self_source.has_value()
? base::Optional<CSPSource>(BuildCSPSource(
Expand Down
1 change: 0 additions & 1 deletion content/renderer/worker/embedded_shared_worker_stub.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "third_party/blink/public/mojom/worker/shared_worker_info.mojom.h"
#include "third_party/blink/public/mojom/worker/worker_content_settings_proxy.mojom-forward.h"
#include "third_party/blink/public/mojom/worker/worker_main_script_load_params.mojom.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_content_settings_client.h"
#include "third_party/blink/public/web/web_shared_worker_client.h"
#include "url/gurl.h"
Expand Down
7 changes: 4 additions & 3 deletions content/test/navigation_simulator_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "base/callback.h"
#include "base/optional.h"
#include "content/browser/frame_host/navigation_request.h"
#include "content/common/content_security_policy/csp_disposition_enum.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_throttle.h"
#include "content/public/browser/web_contents_observer.h"
Expand Down Expand Up @@ -119,7 +118,8 @@ class NavigationSimulatorImpl : public NavigationSimulator,
// Set LoadURLParams and make browser initiated navigations use
// LoadURLWithParams instead of LoadURL.
void SetLoadURLParams(NavigationController::LoadURLParams* load_url_params);
void set_should_check_main_world_csp(CSPDisposition disposition) {
void set_should_check_main_world_csp(
network::mojom::CSPDisposition disposition) {
should_check_main_world_csp_ = disposition;
}

Expand Down Expand Up @@ -293,7 +293,8 @@ class NavigationSimulatorImpl : public NavigationSimulator,
mojo::PendingReceiver<blink::mojom::BrowserInterfaceBroker>
browser_interface_broker_receiver_;
std::string contents_mime_type_;
CSPDisposition should_check_main_world_csp_ = CSPDisposition::CHECK;
network::mojom::CSPDisposition should_check_main_world_csp_ =
network::mojom::CSPDisposition::CHECK;
net::HttpResponseInfo::ConnectionInfo http_connection_info_ =
net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN;
base::Optional<net::SSLInfo> ssl_info_;
Expand Down
4 changes: 1 addition & 3 deletions content/test/test_render_frame_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,7 @@ void TestRenderFrameHost::SendRendererInitiatedNavigationRequest(
GURL() /* client_side_redirect_url */,
base::nullopt /* devtools_initiator_info */,
false /* attach_same_site_cookies */);
mojom::CommonNavigationParamsPtr common_params =
mojom::CommonNavigationParams::New();
common_params->navigation_start = base::TimeTicks::Now();
auto common_params = CreateCommonNavigationParams();
common_params->url = url;
common_params->initiator_origin = GetLastCommittedOrigin();
common_params->referrer = blink::mojom::Referrer::New(
Expand Down
5 changes: 5 additions & 0 deletions services/network/public/mojom/content_security_policy.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ struct ContentSecurityPolicyHeader {
ContentSecurityPolicySource source = kHTTP;
};

enum CSPDisposition {
CHECK,
DO_NOT_CHECK,
};

// A CSPSource represents an expression that matches a set of urls.
// Examples of CSPSource:
// - domain.example.com
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/public/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ source_set("blink_headers") {
"platform/web_content_decryption_module_exception.h",
"platform/web_content_decryption_module_result.h",
"platform/web_content_decryption_module_session.h",
"platform/web_content_security_policy.h",
"platform/web_content_security_policy_struct.h",
"platform/web_content_settings_client.h",
"platform/web_crypto.h",
Expand Down
45 changes: 0 additions & 45 deletions third_party/blink/public/platform/web_content_security_policy.h

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#define THIRD_PARTY_BLINK_PUBLIC_PLATFORM_WEB_CONTENT_SECURITY_POLICY_STRUCT_H_

#include "services/network/public/mojom/content_security_policy.mojom-shared.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_source_location.h"
#include "third_party/blink/public/platform/web_string.h"
#include "third_party/blink/public/platform/web_url.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "base/unguessable_token.h"
#include "services/network/public/mojom/ip_address_space.mojom-shared.h"
#include "third_party/blink/public/mojom/service_worker/service_worker_registration.mojom-shared.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_fetch_client_settings_object.h"
#include "third_party/blink/public/platform/web_string.h"
#include "third_party/blink/public/platform/web_url.h"
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/public/web/web_local_frame_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
#include "third_party/blink/public/platform/blame_context.h"
#include "third_party/blink/public/platform/modules/service_worker/web_service_worker_provider.h"
#include "third_party/blink/public/platform/web_common.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_content_security_policy_struct.h"
#include "third_party/blink/public/platform/web_content_settings_client.h"
#include "third_party/blink/public/platform/web_effective_connection_type.h"
Expand Down
8 changes: 3 additions & 5 deletions third_party/blink/public/web/web_navigation_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-shared.h"
#include "third_party/blink/public/platform/modules/service_worker/web_service_worker_network_provider.h"
#include "third_party/blink/public/platform/web_common.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_content_security_policy_struct.h"
#include "third_party/blink/public/platform/web_data.h"
#include "third_party/blink/public/platform/web_http_body.h"
Expand Down Expand Up @@ -112,11 +111,10 @@ struct BLINK_EXPORT WebNavigationInfo {
// The initiator of this navigation used by DevTools.
WebString devtools_initiator_info;

// Whether this navigation should check CSP. See
// WebContentSecurityPolicyDisposition.
WebContentSecurityPolicyDisposition
// Whether this navigation should check CSP.
network::mojom::CSPDisposition
should_check_main_world_content_security_policy =
kWebContentSecurityPolicyDispositionCheck;
network::mojom::CSPDisposition::CHECK;

// When navigating to a blob url, this token specifies the blob.
mojo::ScopedMessagePipeHandle blob_url_token;
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/public/web/web_remote_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "third_party/blink/public/common/frame/frame_owner_element_type.h"
#include "third_party/blink/public/common/frame/sandbox_flags.h"
#include "third_party/blink/public/mojom/frame/user_activation_update_types.mojom-shared.h"
#include "third_party/blink/public/platform/web_content_security_policy.h"
#include "third_party/blink/public/platform/web_insecure_request_policy.h"
#include "third_party/blink/public/web/web_frame.h"
#include "ui/events/types/scroll_types.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,15 @@ void ScriptController::UpdateDocument() {

void ScriptController::ExecuteJavaScriptURL(
const KURL& url,
ContentSecurityPolicyDisposition check_main_world_csp) {
network::mojom::CSPDisposition check_main_world_csp) {
DCHECK(url.ProtocolIsJavaScript());

const int kJavascriptSchemeLength = sizeof("javascript:") - 1;
String script_source = DecodeURLEscapeSequences(
url.GetString(), DecodeURLMode::kUTF8OrIsomorphic);

bool should_bypass_main_world_content_security_policy =
check_main_world_csp == kDoNotCheckContentSecurityPolicy ||
check_main_world_csp == network::mojom::CSPDisposition::DO_NOT_CHECK ||
ContentSecurityPolicy::ShouldBypassMainWorld(GetFrame()->GetDocument());
if (!GetFrame()->GetPage())
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class CORE_EXPORT ScriptController final
const KURL& base_url,
SanitizeScriptErrors sanitize_script_errors);

void ExecuteJavaScriptURL(const KURL&, ContentSecurityPolicyDisposition);
void ExecuteJavaScriptURL(const KURL&, network::mojom::CSPDisposition);

// Creates a new isolated world for DevTools with the given human readable
// |world_name| and returns it id or nullptr on failure.
Expand Down
Loading

0 comments on commit 128a74d

Please sign in to comment.