Refactor AllowTrustedTypePolicy to provide violation information

Previously, AllowTrustedTypePolicy only returned whether the violation
should be considered as a type error. Now, an enum is added to report
if which type of violation occurred. This enum also gets set in case
of a report-only violation (in which case the method returns true).

Bug: chromium:1142804
Change-Id: I7f5c53ce30ad9c81ef3e3f5a1ae62234eab5ac53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517061Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Alfonso Castaño <alcastano@google.com>
Cr-Commit-Position: refs/heads/master@{#827227}

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

@@ -855,15 +855,31 @@ bool ContentSecurityPolicy::AllowWorkerContextFromSource(
                          url, RedirectStatus::kNoRedirect);
 }
 
-bool ContentSecurityPolicy::AllowTrustedTypePolicy(const String& policy_name,
-                                                   bool is_duplicate) const {
+// The return value indicates whether the policy is allowed or not.
+// If the return value is false, the out-parameter violation_details indicates
+// the type of the violation, and if the return value is true,
+// it indicates if a report-only violation occurred.
+bool ContentSecurityPolicy::AllowTrustedTypePolicy(
+    const String& policy_name,
+    bool is_duplicate,
+    AllowTrustedTypePolicyDetails& violation_details) const {
   bool is_allowed = true;
+  violation_details = AllowTrustedTypePolicyDetails::kAllowed;
   for (const auto& policy : policies_) {
     if (!CheckHeaderTypeMatches(CheckHeaderType::kCheckAll,
                                 policy->HeaderType())) {
       continue;
     }
-    is_allowed &= policy->AllowTrustedTypePolicy(policy_name, is_duplicate);
+    auto new_violation_details = AllowTrustedTypePolicyDetails::kAllowed;
+    bool new_allowed = policy->AllowTrustedTypePolicy(policy_name, is_duplicate,
+                                                      new_violation_details);
+    // Report the first violation that is enforced.
+    // If there is none, report the first violation that is report-only.
+    if ((is_allowed && !new_allowed) ||
+        violation_details == AllowTrustedTypePolicyDetails::kAllowed) {
+      violation_details = new_violation_details;
+    }
+    is_allowed &= new_allowed;
   }
 
   return is_allowed;

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

@@ -213,6 +213,13 @@ class CORE_EXPORT ContentSecurityPolicy final
     kCheckReportOnly
   };
 
+  // Helper type for the method AllowTrustedTypePolicy.
+  enum AllowTrustedTypePolicyDetails {
+    kAllowed,
+    kDisallowedName,
+    kDisallowedDuplicateName
+  };
+
   static const size_t kMaxSampleLength = 40;
 
   ContentSecurityPolicy();
@@ -289,8 +296,10 @@ class CORE_EXPORT ContentSecurityPolicy final
       CheckHeaderType = CheckHeaderType::kCheckAll) const;
   bool AllowWorkerContextFromSource(const KURL&) const;
 
-  bool AllowTrustedTypePolicy(const String& policy_name,
-                              bool is_duplicate) const;
+  bool AllowTrustedTypePolicy(
+      const String& policy_name,
+      bool is_duplicate,
+      AllowTrustedTypePolicyDetails& violation_details) const;
 
   // Passing 'String()' into the |nonce| arguments in the following methods
   // represents an unnonced resource load.

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

@@ -758,10 +758,15 @@ bool CSPDirectiveList::AllowFromSource(
   return result;
 }
 
-bool CSPDirectiveList::AllowTrustedTypePolicy(const String& policy_name,
-                                              bool is_duplicate) const {
-  if (!trusted_types_ || trusted_types_->Allows(policy_name, is_duplicate))
+bool CSPDirectiveList::AllowTrustedTypePolicy(
+    const String& policy_name,
+    bool is_duplicate,
+    ContentSecurityPolicy::AllowTrustedTypePolicyDetails& violation_details)
+    const {
+  if (!trusted_types_ ||
+      trusted_types_->Allows(policy_name, is_duplicate, violation_details)) {
     return true;
+  }
 
   ReportViolation(
       "trusted-types", ContentSecurityPolicy::DirectiveType::kTrustedTypes,

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

@@ -86,8 +86,11 @@ class CORE_EXPORT CSPDirectiveList final
                        const IntegrityMetadataSet& = IntegrityMetadataSet(),
                        ParserDisposition = kParserInserted) const;
 
-  bool AllowTrustedTypePolicy(const String& policy_name,
-                              bool is_duplicate) const;
+  bool AllowTrustedTypePolicy(
+      const String& policy_name,
+      bool is_duplicate,
+      ContentSecurityPolicy::AllowTrustedTypePolicyDetails& violation_details)
+      const;
 
   bool AllowDynamic(ContentSecurityPolicy::DirectiveType) const;
   bool AllowDynamicWorker() const;

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

@@ -67,14 +67,28 @@ bool StringListDirective::AllowOrProcessValue(const String& src) {
   return IsPolicyName(src);
 }
 
-bool StringListDirective::Allows(const String& value, bool is_duplicate) {
-  if (is_duplicate && !allow_duplicates_)
-    return false;
-  if (is_duplicate && value == "default")
-    return false;
-  if (!IsPolicyName(value))
-    return false;
-  return allow_any_ || list_.Contains(value);
+bool StringListDirective::Allows(
+    const String& value,
+    bool is_duplicate,
+    ContentSecurityPolicy::AllowTrustedTypePolicyDetails& violation_details) {
+  if (is_duplicate && !allow_duplicates_) {
+    violation_details = ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+        kDisallowedDuplicateName;
+  } else if (is_duplicate && value == "default") {
+    violation_details = ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+        kDisallowedDuplicateName;
+  } else if (!IsPolicyName(value)) {
+    violation_details =
+        ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kDisallowedName;
+  } else if (!(allow_any_ || list_.Contains(value))) {
+    violation_details =
+        ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kDisallowedName;
+  } else {
+    violation_details =
+        ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kAllowed;
+  }
+  return violation_details ==
+         ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kAllowed;
 }
 
 void StringListDirective::Trace(Visitor* visitor) const {

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

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_CSP_STRING_LIST_DIRECTIVE_H_
 
 #include "base/macros.h"
+#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/csp/csp_directive.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
@@ -19,7 +20,10 @@ class CORE_EXPORT StringListDirective final : public CSPDirective {
                       const String& value,
                       ContentSecurityPolicy*);
   void Trace(Visitor*) const override;
-  bool Allows(const String& string_piece, bool is_duplicate);
+  bool Allows(
+      const String& string_piece,
+      bool is_duplicate,
+      ContentSecurityPolicy::AllowTrustedTypePolicyDetails& violation_details);
   bool IsAllowDuplicates() const { return allow_duplicates_; }
 
  private:

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

@@ -6,6 +6,7 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
+#include "third_party/blink/renderer/core/frame/csp/csp_directive_list.h"
 
 namespace blink {
 
@@ -40,6 +41,7 @@ TEST_F(StringListDirectiveTest, TestAllowLists) {
       {"* 'none'", "default none abc", "", false},
       {"'allow-duplicates' 'none'", "", "default none abc", true},
   };
+  ContentSecurityPolicy::AllowTrustedTypePolicyDetails violation_details;
 
   for (const auto& test_case : test_cases) {
     StringListDirective directive("trusted-types", test_case.directive,
@@ -51,8 +53,20 @@ TEST_F(StringListDirectiveTest, TestAllowLists) {
       SCOPED_TRACE(testing::Message()
                    << " trusted-types " << test_case.directive
                    << "; allow: " << value);
-      EXPECT_TRUE(directive.Allows(value, false));
-      EXPECT_EQ(directive.Allows(value, true), test_case.allow_dupes);
+      EXPECT_TRUE(directive.Allows(value, false, violation_details));
+      EXPECT_EQ(violation_details,
+                ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kAllowed);
+      EXPECT_EQ(directive.Allows(value, true, violation_details),
+                test_case.allow_dupes);
+      if (test_case.allow_dupes) {
+        EXPECT_EQ(
+            violation_details,
+            ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kAllowed);
+      } else {
+        EXPECT_EQ(violation_details,
+                  ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+                      kDisallowedDuplicateName);
+      }
     }
 
     Vector<String> not_allowed;
@@ -61,8 +75,20 @@ TEST_F(StringListDirectiveTest, TestAllowLists) {
       SCOPED_TRACE(testing::Message()
                    << " trusted-types " << test_case.directive
                    << "; do not allow: " << value);
-      EXPECT_FALSE(directive.Allows(value, false));
-      EXPECT_FALSE(directive.Allows(value, true));
+      EXPECT_FALSE(directive.Allows(value, false, violation_details));
+      EXPECT_EQ(violation_details,
+                ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+                    kDisallowedName);
+      EXPECT_FALSE(directive.Allows(value, true, violation_details));
+      if (!test_case.allow_dupes || value == "default") {
+        EXPECT_EQ(violation_details,
+                  ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+                      kDisallowedDuplicateName);
+      } else {
+        EXPECT_EQ(violation_details,
+                  ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+                      kDisallowedName);
+      }
     }
   }
 }

third_party/blink/renderer/core/trustedtypes/trusted_type_policy_factory.cc

@@ -46,27 +46,30 @@ TrustedTypePolicy* TrustedTypePolicyFactory::createPolicy(
   }
   UseCounter::Count(GetExecutionContext(),
                     WebFeature::kTrustedTypesCreatePolicy);
+
   if (RuntimeEnabledFeatures::TrustedDOMTypesEnabled(GetExecutionContext()) &&
-      GetExecutionContext()->GetContentSecurityPolicy() &&
-      !GetExecutionContext()
-           ->GetContentSecurityPolicy()
-           ->AllowTrustedTypePolicy(policy_name,
-                                    policy_map_.Contains(policy_name))) {
-    // For a better error message, we'd like to disambiguate between
-    // "disallowed" and "disallowed because of a duplicate name". Instead of
-    // piping the reason through all the layers, we'll just check whether it
-    // had also been disallowed as a non-duplicate name.
-    bool disallowed_because_of_duplicate_name =
-        policy_map_.Contains(policy_name) &&
-        GetExecutionContext()
-            ->GetContentSecurityPolicy()
-            ->AllowTrustedTypePolicy(policy_name, false);
-    const String message =
-        disallowed_because_of_duplicate_name
-            ? "Policy with name \"" + policy_name + "\" already exists."
-            : "Policy \"" + policy_name + "\" disallowed.";
-    exception_state.ThrowTypeError(message);
-    return nullptr;
+      GetExecutionContext()->GetContentSecurityPolicy()) {
+    ContentSecurityPolicy::AllowTrustedTypePolicyDetails violation_details =
+        ContentSecurityPolicy::AllowTrustedTypePolicyDetails::kAllowed;
+    bool disallowed = !GetExecutionContext()
+                           ->GetContentSecurityPolicy()
+                           ->AllowTrustedTypePolicy(
+                               policy_name, policy_map_.Contains(policy_name),
+                               violation_details);
+    if (disallowed) {
+      // For a better error message, we'd like to disambiguate between
+      // "disallowed" and "disallowed because of a duplicate name".
+      bool disallowed_because_of_duplicate_name =
+          violation_details ==
+          ContentSecurityPolicy::AllowTrustedTypePolicyDetails::
+              kDisallowedDuplicateName;
+      const String message =
+          disallowed_because_of_duplicate_name
+              ? "Policy with name \"" + policy_name + "\" already exists."
+              : "Policy \"" + policy_name + "\" disallowed.";
+      exception_state.ThrowTypeError(message);
+      return nullptr;
+    }
   }
   UseCounter::Count(GetExecutionContext(),
                     WebFeature::kTrustedTypesPolicyCreated);