[Trusted Types] Add warnings to require-trusted-types-for directive

Add warnings for empty/invalid require-trusted-types-for keywords in
CSP parser.

Bug: 1149293
Change-Id: I97d2daddba8ebad7eb34ffcbe8467e1e7d7bb9c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575305
Commit-Queue: Yifan Luo <lyf@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Yifan Luo <lyf@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836083}

services/network/public/cpp/content_security_policy/content_security_policy.cc

@@ -732,17 +732,34 @@ std::vector<std::string> ParsePluginTypes(
 
 // Parse the 'required-trusted-types-for' directive.
 // https://w3c.github.io/webappsec-trusted-types/dist/spec/#require-trusted-types-for-csp-directive
-//
-// TODO(https://crbug.com/1149293): Add a warning when parsing invalid values.
 network::mojom::CSPRequireTrustedTypesFor ParseRequireTrustedTypesFor(
-    base::StringPiece value) {
+    base::StringPiece value,
+    std::vector<std::string>& parsing_errors) {
+  network::mojom::CSPRequireTrustedTypesFor out =
+      network::mojom::CSPRequireTrustedTypesFor::None;
   for (const auto expression : base::SplitStringPiece(
            value, base::kWhitespaceASCII, base::TRIM_WHITESPACE,
            base::SPLIT_WANT_NONEMPTY)) {
-    if (expression == "'script'")
-      return network::mojom::CSPRequireTrustedTypesFor::Script;
+    if (expression == "'script'") {
+      out = network::mojom::CSPRequireTrustedTypesFor::Script;
+    } else {
+      const char* hint = nullptr;
+      if (expression == "script" || expression == "scripts" ||
+          expression == "'scripts'") {
+        hint = " Did you mean 'script'?";
+      }
+
+      parsing_errors.emplace_back(base::StringPrintf(
+          "Invalid expression in 'require-trusted-types-for' "
+          "Content Security Policy directive: %s.%s\n",
+          expression.as_string().c_str(), hint));
+    }
   }
-  return network::mojom::CSPRequireTrustedTypesFor::None;
+  if (out == network::mojom::CSPRequireTrustedTypesFor::None)
+    parsing_errors.emplace_back(base::StringPrintf(
+        "'require-trusted-types-for' Content Security Policy "
+        "directive is empty; The directive has no effect.\n"));
+  return out;
 }
 
 // This implements tt-policy-name from
@@ -968,7 +985,7 @@ void AddContentSecurityPolicyFromHeader(base::StringPiece header,
 
       case CSPDirectiveName::RequireTrustedTypesFor:
         out->require_trusted_types_for =
-            ParseRequireTrustedTypesFor(directive.second);
+            ParseRequireTrustedTypesFor(directive.second, out->parsing_errors);
         break;
 
       case CSPDirectiveName::TrustedTypes:

services/network/public/cpp/content_security_policy/content_security_policy_unittest.cc

@@ -697,26 +697,32 @@ TEST(ContentSecurityPolicy, ParsePluginTypes) {
 TEST(ContentSecurityPolicy, ParseRequireTrustedTypesFor) {
   struct {
     const char* input;
+    const unsigned long errors;
     network::mojom::CSPRequireTrustedTypesFor expected;
   } cases[]{
       {
           "",
+          1u,
           network::mojom::CSPRequireTrustedTypesFor::None,
       },
       {
           "'script'",
+          0u,
           network::mojom::CSPRequireTrustedTypesFor::Script,
       },
       {
           "'wasm' 'script'",
+          1u,
           network::mojom::CSPRequireTrustedTypesFor::Script,
       },
       {
           "'script' 'wasm' 'script'",
+          1u,
           network::mojom::CSPRequireTrustedTypesFor::Script,
       },
       {
           "'wasm'",
+          2u,
           network::mojom::CSPRequireTrustedTypesFor::None,
       },
   };
@@ -729,7 +735,7 @@ TEST(ContentSecurityPolicy, ParseRequireTrustedTypesFor) {
             ->raw_directives[mojom::CSPDirectiveName::RequireTrustedTypesFor],
         testCase.input);
     EXPECT_EQ(policies[0]->directives.size(), 0u);
-    EXPECT_EQ(policies[0]->parsing_errors.size(), 0u);
+    EXPECT_EQ(policies[0]->parsing_errors.size(), testCase.errors);
     EXPECT_EQ(policies[0]->require_trusted_types_for, testCase.expected);
   }
 }

third_party/blink/renderer/core/frame/csp/content_security_policy.cc

@@ -1312,6 +1312,29 @@ void ContentSecurityPolicy::ReportInvalidPluginTypes(
   LogToConsole(message);
 }
 
+void ContentSecurityPolicy::ReportInvalidRequireTrustedTypesFor(
+    const String& require_trusted_types_for) {
+  String message;
+  if (require_trusted_types_for.IsNull()) {
+    message =
+        "'require-trusted-types-for' Content Security Policy directive is "
+        "empty; The directive has no effect.\n";
+  } else {
+    const char* hint = "";
+    if (require_trusted_types_for == "script" ||
+        require_trusted_types_for == "scripts" ||
+        require_trusted_types_for == "'scripts'") {
+      hint = " Did you mean 'script'?";
+    }
+
+    message =
+        "Invalid expression in 'require-trusted-types-for' "
+        "Content Security Policy directive: " +
+        require_trusted_types_for + "." + hint + "\n";
+  }
+  LogToConsole(message, mojom::ConsoleMessageLevel::kWarning);
+}
+
 void ContentSecurityPolicy::ReportInvalidSandboxFlags(
     const String& invalid_flags) {
   LogToConsole(

third_party/blink/renderer/core/frame/csp/content_security_policy.h

@@ -375,6 +375,7 @@ class CORE_EXPORT ContentSecurityPolicy final
                                   const String& value,
                                   const char);
   void ReportInvalidPluginTypes(const String&);
+  void ReportInvalidRequireTrustedTypesFor(const String&);
   void ReportInvalidSandboxFlags(const String&);
   void ReportInvalidSourceExpression(const String& directive_name,
                                      const String& source);

third_party/blink/renderer/core/frame/csp/require_trusted_types_for_directive.cc

@@ -4,6 +4,8 @@
 
 #include "require_trusted_types_for_directive.h"
 
+#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
+
 namespace blink {
 
 RequireTrustedTypesForDirective::RequireTrustedTypesForDirective(
@@ -20,9 +22,13 @@ RequireTrustedTypesForDirective::RequireTrustedTypesForDirective(
     // https://w3c.github.io/webappsec-trusted-types/dist/spec/#trusted-types-sink-group
     if (v == "'script'") {
       require_trusted_types_for_script_ = true;
-      break;
+    } else {
+      policy->ReportInvalidRequireTrustedTypesFor(v);
     }
   }
+  if (!require_trusted_types_for_script_) {
+    policy->ReportInvalidRequireTrustedTypesFor(String());
+  }
 }
 
 bool RequireTrustedTypesForDirective::require() const {

third_party/blink/renderer/core/frame/csp/require_trusted_types_for_directive_test.cc

@@ -21,8 +21,9 @@ TEST(RequireTrustedTypesForDirectiveTest, TestSinks) {
                     {"'script' 'script'", true}};
 
   for (const auto& test_case : test_cases) {
-    RequireTrustedTypesForDirective directive("require-trusted-types-for",
-                                              test_case.directive, nullptr);
+    RequireTrustedTypesForDirective directive(
+        "require-trusted-types-for", test_case.directive,
+        MakeGarbageCollected<ContentSecurityPolicy>());
     SCOPED_TRACE(testing::Message() << " require-trusted-types-for "
                                     << test_case.directive << ";");
     EXPECT_EQ(directive.require(), test_case.result);