Fix CSP source list intersection for CSPEE in blink

As explained in https://github.com/w3c/webappsec-cspee/pull/18,
Content-Security-Policy: Embedded Enforcement source list intersection
algorithm sometimes computes a wrong intersection of two lists of
source expressions.

Additionally, blink CSPEE source intersection algorithm was computing
a wrong intersection for http://*.com and http://*.example.com.

We fix those problems and add a unit test and WP tests.

Change-Id: Ie7b85d8c7e978af6b5e87141d257c66e5556be95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2385458Reviewed-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@{#803799}

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

@@ -246,7 +246,13 @@ CSPSource* CSPSource::Intersect(CSPSource* other) const {
         stricter->host_wildcard_, stricter->port_wildcard_);
   }
 
-  String host = host_wildcard_ == kNoWildcard ? host_ : other->host_;
+  // Pick the host without wildcard, or if both have a wildcard, pick the
+  // longer one.
+  String host = (host_wildcard_ == kNoWildcard ||
+                 (other->host_wildcard_ == kHasWildcard &&
+                  host_.length() > other->host_.length()))
+                    ? host_
+                    : other->host_;
   // Since sources are similar and paths match, pick the longer one.
   String path = path_.length() > other->path_.length() ? path_ : other->path_;
   // Choose this port if the other port is empty, has wildcard or is a port for

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

@@ -898,28 +898,14 @@ HeapVector<Member<CSPSource>> SourceListDirective::GetIntersectCSPSources(
     if (schemes_map.Contains(source_a->GetScheme()))
       continue;
 
-    CSPSource* match(nullptr);
     for (const auto& source_b : other) {
       // No need to add a host source expression if it is subsumed by the
       // matching scheme source expression.
       if (schemes_map.Contains(source_b->GetScheme()))
         continue;
-      // If sourceA is scheme only but there was no intersection for it in the
-      // `other` list, we add all the sourceB with that scheme.
-      if (source_a->IsSchemeOnly()) {
-        if (CSPSource* local_match = source_b->Intersect(source_a))
-          normalized.push_back(local_match);
-        continue;
-      }
-      if (source_b->Subsumes(source_a)) {
-        match = source_a;
-        break;
-      }
-      if (CSPSource* local_match = source_b->Intersect(source_a))
-        match = local_match;
+      if (CSPSource* match = source_b->Intersect(source_a))
+        normalized.push_back(match);
     }
-    if (match)
-      normalized.push_back(match);
   }
   return normalized;
 }

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

@@ -294,7 +294,6 @@ TEST_F(SourceListDirectiveTest, GetIntersectCSPSourcesSchemes) {
   struct TestCase {
     String sources;
     String expected;
-    String expected_reversed;
   } cases[] = {{"http:", "http:"},
                {"https:", "https:"},
                {"ws:", "wss: ws://another.test/bar/"},
@@ -309,11 +308,13 @@ TEST_F(SourceListDirectiveTest, GetIntersectCSPSourcesSchemes) {
                 "https://example1.com/foo/ http://example1.com/bar/page.html"},
                {"http://example1.com/foo/ https://example1.com/foo/",
                 "http://example1.com/foo/ https://example1.com/foo/ "
-                "http://example1.com/foo/ https://example1.com/foo/"},
+                "http://example1.com/foo/ https://example1.com/foo/ "
+                "https://example1.com/foo/ https://example1.com/foo/"},
                {"https://example1.com/foo/ http://example1.com/foo/",
                 "https://example1.com/foo/ http://example1.com/foo/ "
-                "http://example1.com/foo/ https://example1.com/foo/"},
-               // If exaclty the same policy is specified, it is optimized.
+                "https://example1.com/foo/ http://example1.com/foo/ "
+                "https://example1.com/foo/ https://example1.com/foo/"},
+               // If exactly the same policy is specified, it is optimized.
                {"http: http://example1.com/foo/ https://example1.com/foo/ "
                 "http://example1.com/bar/page.html wss: ws://another.test/bar/",
                 "http: wss: ws://another.test/bar/"}};
@@ -408,6 +409,7 @@ TEST_F(SourceListDirectiveTest, Subsumes) {
       {{"http://*.non-example3.com:*/bar/"}, false},
       {{"http://example3.com/foo/"}, false},
       {{"http://not-example1.com", "http://not-example1.com"}, false},
+      {{"http://*", "http://*.com http://*.example3.com:*/bar/"}, false},
   };
 
   for (const auto& test : cases) {

third_party/blink/web_tests/external/wpt/content-security-policy/embedded-enforcement/subsumption_algorithm-general.html

@@ -45,11 +45,28 @@
         "required_csp": "frame-src http://b.com:80", 
         "returned_csp": "child-src https://b.com:443", 
         "expected": IframeLoad.EXPECT_LOAD },
+      { "name": "Iframe should block if intersection allows sources which are not in required_csp.",
+        "required_csp": "style-src http://*.example.com:*",
+        "returned_csp": "style-src http://*.com:*",
+        "returned_csp_2": "style-src http://*.com http://*.example.com:*",
+        "expected": IframeLoad.EXPECT_BLOCK },
+      { "name": "Iframe should block if intersection allows sources which are not in required_csp (other ordering).",
+        "required_csp": "style-src http://*.example.com:*",
+        "returned_csp": "style-src http://*.com:*",
+        "returned_csp_2": "style-src http://*.example.com:* http://*.com",
+        "expected": IframeLoad.EXPECT_BLOCK },
+      { "name": "Iframe should load if intersection allows only sources which are in required_csp.",
+        "required_csp": "style-src http://*.example.com",
+        "returned_csp": "style-src http://*.example.com:*",
+        "returned_csp_2": "style-src http://*.com",
+        "expected": IframeLoad.EXPECT_LOAD },
     ];
 
     tests.forEach(test => {
       async_test(t =>  {
         var url = generateUrlWithPolicies(Host.CROSS_ORIGIN, test.returned_csp);
+        if (test.returned_csp_2)
+          url.searchParams.append("policy2", test.returned_csp_2);
         assert_iframe_with_csp(t, url, test.required_csp, test.expected, test.name, null);
       }, test.name);
     });

third_party/blink/web_tests/virtual/out-of-blink-cspee/external/wpt/content-security-policy/embedded-enforcement/subsumption_algorithm-general-expected.txt

@@ -4,8 +4,11 @@ FAIL Iframe with empty returned CSP should be blocked. assert_equals: expected (
 PASS Iframe with matching CSP should load.
 PASS Iframe with more restricting CSP should load.
 FAIL Iframe with less restricting CSP should be blocked. assert_equals: expected (undefined) undefined but got (boolean) true
-FAIL Iframe with a different CSP should be blocked. assert_unreached: No message should be sent from the frame. Reached unreachable code
+FAIL Iframe with a different CSP should be blocked. assert_equals: expected (undefined) undefined but got (boolean) true
 PASS Iframe with a matching and more restrictive ports should load.
 PASS Iframe should load even if the ports are different but are default for the protocols.
+FAIL Iframe should block if intersection allows sources which are not in required_csp. assert_equals: expected (undefined) undefined but got (boolean) true
+FAIL Iframe should block if intersection allows sources which are not in required_csp (other ordering). assert_unreached: No message should be sent from the frame. Reached unreachable code
+PASS Iframe should load if intersection allows only sources which are in required_csp.
 Harness: the test ran to completion.