Skip to content

Commit

Permalink
CSP: Remove 'plugin-types' directive
Browse files Browse the repository at this point in the history 
The Content Security Policy directive 'plugin-types' is being removed
by the specification
(w3c/webappsec-csp#456). This CL removes the
code parsing and checking the 'plugin-types' directive from Blink and
from the services/network CSP parser. All WP tests for plugin-types
are removed, too.

When parsing a plugin-types directive, we display a console error
message informing that the directive has been removed and that
object-src can be used instead.

Bug: 1168001
Change-Id: I61420677a0f11f8daf46c473e578d66c932751d1
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643282
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/master@{#851760}
GitOrigin-RevId: 8f48b81f329ad794c71d05e85c7f7ca4772bdea4
  • Loading branch information
@antosart @copybara-github
antosart authored and copybara-github committed Feb 8, 2021
1 parent 8abadbe commit 5cb87e7
Show file tree
Hide file tree
Showing 38 changed files with 20 additions and 770 deletions.
1 change: 0 additions & 1 deletion blink/public/platform/web_content_security_policy_struct.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ struct WebContentSecurityPolicy {
WebContentSecurityPolicyHeader header;
bool use_reporting_api;
WebVector<WebString> report_endpoints;
base::Optional<WebVector<WebString>> plugin_types;
network::mojom::CSPRequireTrustedTypesFor require_trusted_types_for;
base::Optional<WebCSPTrustedTypes> trusted_types;
WebVector<WebString> parsing_errors;
Expand Down
2 changes: 0 additions & 2 deletions blink/renderer/core/frame/build.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ blink_core_sources_frame = [
"csp/csp_violation_report_body.h",
"csp/execution_context_csp_delegate.cc",
"csp/execution_context_csp_delegate.h",
"csp/media_list_directive.cc",
"csp/media_list_directive.h",
"csp/navigation_initiator_impl.cc",
"csp/navigation_initiator_impl.h",
"csp/require_trusted_types_for_directive.cc",
Expand Down
66 changes: 8 additions & 58 deletions blink/renderer/core/frame/csp/content_security_policy.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,23 +290,6 @@ void ContentSecurityPolicy::CopyStateFrom(const ContentSecurityPolicy* other) {
ReportAccumulatedHeaders();
}

void ContentSecurityPolicy::CopyPluginTypesFrom(
const ContentSecurityPolicy* other) {
for (const auto& policy : other->policies_) {
if (policy->plugin_types) {
KURL url;
url.SetProtocol(policy->self_origin->scheme);
url.SetHost(policy->self_origin->host);
url.SetPort(policy->self_origin->port == url::PORT_UNSPECIFIED
? 0
: policy->self_origin->port);
scoped_refptr<SecurityOrigin> self_origin = SecurityOrigin::Create(url);
DidReceiveHeader(CSPDirectiveListPluginTypesText(*policy), *self_origin,
policy->header->type, policy->header->source);
}
}
}

void ContentSecurityPolicy::DidReceiveHeaders(
const ContentSecurityPolicyResponseHeaders& headers) {
scoped_refptr<SecurityOrigin> self_origin =
Expand Down Expand Up @@ -620,19 +603,6 @@ String ContentSecurityPolicy::EvalDisabledErrorMessage() const {
return String();
}

bool ContentSecurityPolicy::AllowPluginType(
const String& type,
const String& type_attribute,
const KURL& url,
ReportingDisposition reporting_disposition) {
for (const auto& policy : policies_) {
if (!CSPDirectiveListAllowPluginType(*policy, this, type, type_attribute,
url, reporting_disposition))
return false;
}
return true;
}

static base::Optional<CSPDirectiveName> GetDirectiveTypeFromRequestContextType(
mojom::blink::RequestContextType context) {
switch (context) {
Expand Down Expand Up @@ -1106,7 +1076,6 @@ void ContentSecurityPolicy::ReportViolation(
if (!delegate_ && !context_frame) {
DCHECK(effective_type == CSPDirectiveName::ChildSrc ||
effective_type == CSPDirectiveName::FrameSrc ||
effective_type == CSPDirectiveName::PluginTypes ||
effective_type == CSPDirectiveName::TrustedTypes ||
effective_type == CSPDirectiveName::RequireTrustedTypesFor);
return;
Expand Down Expand Up @@ -1278,6 +1247,7 @@ void ContentSecurityPolicy::ReportUnsupportedDirective(const String& name) {
static const char kAllow[] = "allow";
static const char kOptions[] = "options";
static const char kPolicyURI[] = "policy-uri";
static const char kPluginTypes[] = "plugin-types";
static const char kAllowMessage[] =
"The 'allow' directive has been replaced with 'default-src'. Please use "
"that directive instead, as 'allow' has no effect.";
Expand All @@ -1290,6 +1260,11 @@ void ContentSecurityPolicy::ReportUnsupportedDirective(const String& name) {
"The 'policy-uri' directive has been removed from the "
"specification. Please specify a complete policy via "
"the Content-Security-Policy header.";
static const char kPluginTypesMessage[] =
"The Content-Security-Policy directive 'plugin-types' has been removed "
"from the specification. "
"If you want to block plugins, consider specifying \"object-src 'none'\" "
"instead.";

String message =
"Unrecognized Content-Security-Policy directive '" + name + "'.\n";
Expand All @@ -1300,6 +1275,8 @@ void ContentSecurityPolicy::ReportUnsupportedDirective(const String& name) {
message = kOptionsMessage;
} else if (EqualIgnoringASCIICase(name, kPolicyURI)) {
message = kPolicyURIMessage;
} else if (EqualIgnoringASCIICase(name, kPluginTypes)) {
message = kPluginTypesMessage;
} else if (GetDirectiveType(name) != CSPDirectiveName::Unknown) {
message = "The Content-Security-Policy directive '" + name +
"' is implemented behind a flag which is currently disabled.\n";
Expand All @@ -1325,29 +1302,6 @@ void ContentSecurityPolicy::ReportDuplicateDirective(const String& name) {
LogToConsole(message);
}

void ContentSecurityPolicy::ReportInvalidPluginTypes(
const String& plugin_type) {
String message;
if (plugin_type.IsNull()) {
message =
"'plugin-types' Content Security Policy directive is empty; all "
"plugins will be blocked. To disallow all plugins, the \"object-src "
"'none'\" directive should be used instead.\n";
} else if (plugin_type == "'none'") {
message =
"Invalid plugin type in 'plugin-types' Content Security Policy "
"directive: '" +
plugin_type +
"'. Did you mean to set the object-src directive to 'none'?\n";
} else {
message =
"Invalid plugin type in 'plugin-types' Content Security Policy "
"directive: '" +
plugin_type + "'.\n";
}
LogToConsole(message);
}

void ContentSecurityPolicy::ReportInvalidRequireTrustedTypesFor(
const String& require_trusted_types_for) {
String message;
Expand Down Expand Up @@ -1595,8 +1549,6 @@ const char* ContentSecurityPolicy::GetDirectiveName(CSPDirectiveName type) {
return "navigate-to";
case CSPDirectiveName::ObjectSrc:
return "object-src";
case CSPDirectiveName::PluginTypes:
return "plugin-types";
case CSPDirectiveName::PrefetchSrc:
return "prefetch-src";
case CSPDirectiveName::ReportTo:
Expand Down Expand Up @@ -1666,8 +1618,6 @@ CSPDirectiveName ContentSecurityPolicy::GetDirectiveType(const String& name) {
return CSPDirectiveName::NavigateTo;
if (name == "object-src")
return CSPDirectiveName::ObjectSrc;
if (name == "plugin-types")
return CSPDirectiveName::PluginTypes;
if (name == "prefetch-src")
return CSPDirectiveName::PrefetchSrc;
if (name == "report-to")
Expand Down
6 changes: 0 additions & 6 deletions blink/renderer/core/frame/csp/content_security_policy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ class CORE_EXPORT ContentSecurityPolicy final
bool IsBound();
void BindToDelegate(ContentSecurityPolicyDelegate&);
void CopyStateFrom(const ContentSecurityPolicy*);
void CopyPluginTypesFrom(const ContentSecurityPolicy*);

void DidReceiveHeaders(const ContentSecurityPolicyResponseHeaders&);
void DidReceiveHeader(const String&,
Expand Down Expand Up @@ -225,10 +224,6 @@ class CORE_EXPORT ContentSecurityPolicy final
bool AllowWasmEval(ReportingDisposition,
ExceptionStatus,
const String& script_content);
bool AllowPluginType(const String& type,
const String& type_attribute,
const KURL&,
ReportingDisposition = ReportingDisposition::kReport);

// AllowFromSource() wrappers.
bool AllowBaseURI(const KURL&);
Expand Down Expand Up @@ -336,7 +331,6 @@ class CORE_EXPORT ContentSecurityPolicy final
void ReportInvalidPathCharacter(const String& directive_name,
const String& value,
const char);
void ReportInvalidPluginTypes(const String&);
void ReportInvalidRequireTrustedTypesFor(const String&);
void ReportInvalidSandboxFlags(const String&);
void ReportInvalidSourceExpression(const String& directive_name,
Expand Down
48 changes: 2 additions & 46 deletions blink/renderer/core/frame/csp/content_security_policy_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ TEST_F(ContentSecurityPolicyTest, ParseInsecureRequestPolicy) {
}

TEST_F(ContentSecurityPolicyTest, CopyStateFrom) {
csp->DidReceiveHeader("script-src 'none'; plugin-types application/x-type-1",
*secure_origin, ContentSecurityPolicyType::kReport,
csp->DidReceiveHeader("script-src 'none'", *secure_origin,
ContentSecurityPolicyType::kReport,
ContentSecurityPolicySource::kHTTP);
csp->DidReceiveHeader("img-src http://example.com", *secure_origin,
ContentSecurityPolicyType::kReport,
Expand All @@ -149,9 +149,6 @@ TEST_F(ContentSecurityPolicyTest, CopyStateFrom) {
example_url, ResourceRequest::RedirectStatus::kNoRedirect,
ReportingDisposition::kSuppressReporting,
ContentSecurityPolicy::CheckHeaderType::kCheckReportOnly));
EXPECT_TRUE(csp2->AllowPluginType("application/x-type-1",
"application/x-type-1", example_url,
ReportingDisposition::kSuppressReporting));
EXPECT_TRUE(csp2->AllowImageFromSource(
example_url, example_url, ResourceRequest::RedirectStatus::kNoRedirect,
ReportingDisposition::kSuppressReporting,
Expand All @@ -161,41 +158,6 @@ TEST_F(ContentSecurityPolicyTest, CopyStateFrom) {
ResourceRequest::RedirectStatus::kNoRedirect,
ReportingDisposition::kSuppressReporting,
ContentSecurityPolicy::CheckHeaderType::kCheckReportOnly));
EXPECT_FALSE(csp2->AllowPluginType("application/x-type-2",
"application/x-type-2", example_url,
ReportingDisposition::kSuppressReporting));
}

TEST_F(ContentSecurityPolicyTest, CopyPluginTypesFrom) {
csp->DidReceiveHeader("script-src 'none'; plugin-types application/x-type-1",
*secure_origin, ContentSecurityPolicyType::kEnforce,
ContentSecurityPolicySource::kHTTP);
csp->DidReceiveHeader("img-src http://example.com", *secure_origin,
ContentSecurityPolicyType::kEnforce,
ContentSecurityPolicySource::kHTTP);

const KURL example_url("http://example.com");
const KURL not_example_url("http://not-example.com");

auto* csp2 = MakeGarbageCollected<ContentSecurityPolicy>();
csp2->CopyPluginTypesFrom(csp.Get());
EXPECT_TRUE(csp2->AllowScriptFromSource(
example_url, String(), IntegrityMetadataSet(), kParserInserted,
example_url, ResourceRequest::RedirectStatus::kNoRedirect,
ReportingDisposition::kSuppressReporting));
EXPECT_TRUE(csp2->AllowPluginType("application/x-type-1",
"application/x-type-1", example_url,
ReportingDisposition::kSuppressReporting));
EXPECT_TRUE(csp2->AllowImageFromSource(
example_url, example_url, ResourceRequest::RedirectStatus::kNoRedirect,
ReportingDisposition::kSuppressReporting));
EXPECT_TRUE(
csp2->AllowImageFromSource(not_example_url, not_example_url,
ResourceRequest::RedirectStatus::kNoRedirect,
ReportingDisposition::kSuppressReporting));
EXPECT_FALSE(csp2->AllowPluginType("application/x-type-2",
"application/x-type-2", example_url,
ReportingDisposition::kSuppressReporting));
}

TEST_F(ContentSecurityPolicyTest, IsActiveForConnectionsWithConnectSrc) {
Expand Down Expand Up @@ -605,7 +567,6 @@ TEST_F(ContentSecurityPolicyTest, DirectiveType) {
{CSPDirectiveName::MediaSrc, "media-src"},
{CSPDirectiveName::NavigateTo, "navigate-to"},
{CSPDirectiveName::ObjectSrc, "object-src"},
{CSPDirectiveName::PluginTypes, "plugin-types"},
{CSPDirectiveName::ReportURI, "report-uri"},
{CSPDirectiveName::Sandbox, "sandbox"},
{CSPDirectiveName::ScriptSrc, "script-src"},
Expand Down Expand Up @@ -1209,11 +1170,6 @@ TEST_F(ContentSecurityPolicyTest, EmptyCSPIsNoOp) {
EXPECT_TRUE(csp->AllowWasmEval(ReportingDisposition::kReport,
ContentSecurityPolicy::kWillNotThrowException,
g_empty_string));
EXPECT_TRUE(csp->AllowPluginType("application/x-type-1",
"application/x-type-1", example_url));
EXPECT_TRUE(csp->AllowPluginType("application/x-type-1",
"application/x-type-1", example_url,
ReportingDisposition::kSuppressReporting));

CSPDirectiveName types_to_test[] = {
CSPDirectiveName::BaseURI, CSPDirectiveName::ConnectSrc,
Expand Down
8 changes: 0 additions & 8 deletions blink/renderer/core/frame/csp/conversion_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,6 @@ WebContentSecurityPolicy ConvertToPublic(
raw_directives[i++] = {directive.key, std::move(directive.value)};
}

base::Optional<WebVector<WebString>> plugin_types = base::nullopt;
if (policy->plugin_types.has_value())
plugin_types = policy->plugin_types.value();

return {ConvertToPublic(std::move(policy->self_origin)),
std::move(raw_directives),
std::move(directives),
Expand All @@ -146,7 +142,6 @@ WebContentSecurityPolicy ConvertToPublic(
ConvertToPublic(std::move(policy->header)),
policy->use_reporting_api,
std::move(policy->report_endpoints),
std::move(plugin_types),
policy->require_trusted_types_for,
ConvertToPublic(std::move(policy->trusted_types)),
std::move(policy->parsing_errors)};
Expand Down Expand Up @@ -176,9 +171,6 @@ network::mojom::blink::ContentSecurityPolicyPtr ConvertToMojoBlink(
policy_in.header.header_value, policy_in.header.type,
policy_in.header.source),
policy_in.use_reporting_api, ConvertToWTF(policy_in.report_endpoints),
policy_in.plugin_types.has_value()
? base::make_optional(ConvertToWTF(policy_in.plugin_types.value()))
: base::nullopt,
policy_in.require_trusted_types_for,
policy_in.trusted_types ? network::mojom::blink::CSPTrustedTypes::New(
ConvertToWTF(policy_in.trusted_types->list),
Expand Down
9 changes: 2 additions & 7 deletions blink/renderer/core/frame/csp/conversion_util_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ TEST(ContentSecurityPolicyConversionUtilTest, BackAndForthConversion) {
ContentSecurityPolicyHeader::New(
"my-csp", network::mojom::blink::ContentSecurityPolicyType::kEnforce,
network::mojom::blink::ContentSecurityPolicySource::kHTTP),
false, Vector<String>(), base::nullopt,
false, Vector<String>(),
network::mojom::blink::CSPRequireTrustedTypesFor::None, nullptr,
Vector<String>());

Expand Down Expand Up @@ -60,11 +60,6 @@ TEST(ContentSecurityPolicyConversionUtilTest, BackAndForthConversion) {
[](ContentSecurityPolicy& csp) {
csp.report_endpoints = {"endpoint1", "endpoint2"};
},
[](ContentSecurityPolicy& csp) { csp.plugin_types = Vector<String>(); },
[](ContentSecurityPolicy& csp) {
csp.plugin_types = Vector<String>(
{"application/pdf", "application/x-shockwave-flash"});
},
[](ContentSecurityPolicy& csp) {
csp.require_trusted_types_for =
network::mojom::blink::CSPRequireTrustedTypesFor::Script;
Expand Down Expand Up @@ -111,7 +106,7 @@ TEST(ContentSecurityPolicyConversionUtilTest,
network::mojom::blink::ContentSecurityPolicyHeader::New(
"my-csp", network::mojom::blink::ContentSecurityPolicyType::kEnforce,
network::mojom::blink::ContentSecurityPolicySource::kHTTP),
false, Vector<String>(), base::nullopt,
false, Vector<String>(),
network::mojom::blink::CSPRequireTrustedTypesFor::None, nullptr,
Vector<String>());

Expand Down
Loading

0 comments on commit 5cb87e7

Please sign in to comment.