[Sheriff] Revert "Rewrite SecurityOrigin::IsSecure() using GURL and url::Origin"

This reverts commit af98eac8.

Reason for revert: SecurityOriginTest.CustomScheme fails on debug builds after this cl. https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/22213

Original change's description:
> Rewrite SecurityOrigin::IsSecure() using GURL and url::Origin
>
> SecurityOrigin::IsSecure() is very similar to the concept of
> "potentially trustworthy URL" defined in [1] and implemented by
> network::IsUrlPotentiallyTrustworthy(), but it has historically been
> hard to compare them. After previous refactoring they are quite close.
>
> This CL is yet another iteration. SecurityOrigin::IsSecure() is
> rewritten to use a similar implementation based on GURL and url::Origin.
> Spec comments are also added to make the difference more obvious.
>
> IsSecure() currently returns true if the URL scheme is in the secure
> list [2] or possibly one wrapped in a blob: or filesystem:. The
> constructor of url::Origin automatically unwraps this kind of URLs
> but is also more restrictive about accepted schemes [3]. In particular,
> about: and data: URLs or URLs with schemes registered as "secure"
> but not "standard" will now generate invalid origins.
>
> Special cases from network::IsUrlPotentiallyTrustworthy() are added to
> preserve behavior with about: and data: schemes, although some weird
> edge cases like "about:about" or "blob:about" are no longer treated as
> secure. Behavior for all standard secure schemes (including https, wss,
> quic-transport) are preserved. All these changes makes IsSecure()
> closer to  the spec definition of "potentially trustworthy URL".
>
> The rest of the IsSecure() handles opaque and allowedlist origins, which
> is again just a subset of the work done by
> network::IsUrlPotentiallyTrustworthy() and can be rewritten accordingly.
>
> [1] https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
> [2] https://source.chromium.org/chromium/chromium/src/+/master:url/url_util.cc;l=63;drc=65a062c4528790ff3651920ba9a3934df184e1bc
> [3] https://source.chromium.org/chromium/chromium/src/+/master:url/origin.cc;l=47;drc=721926d0cf09b42968ec8481792fcd6fa4edab7b
>
> Bug: 1153336
> Change-Id: I8f4a42f89a9ae13016a20a1e242c1efd48874162
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617709
> Reviewed-by: Mike West <mkwst@chromium.org>
> Commit-Queue: Frédéric Wang <fwang@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#844034}

TBR=fwang@igalia.com,mkwst@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I720d669fd57861565991b5ad32b654f96f1281c4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1153336
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2633048Reviewed-by: Anatoliy Potapchuk <apotapchuk@chromium.org>
Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844061}

third_party/blink/renderer/platform/weborigin/security_origin.cc

@@ -306,56 +306,19 @@ String SecurityOrigin::RegistrableDomain() const {
 }
 
 bool SecurityOrigin::IsSecure(const KURL& url) {
-  // This method is a stricter alternative to "potentially trustworthy url":
-  // https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
-  // TODO(https://crbug.com/1153336): Align the behavior of this function with
-  // network::IsUrlPotentiallyTrustworthy()?
-
-  GURL gurl = GURL(url);
-
-  // 1. If url is "about:blank" or "about:srcdoc", return "Potentially
-  //    Trustworthy".
-  if (gurl.IsAboutBlank() || gurl.IsAboutSrcdoc())
+  if (base::Contains(url::GetSecureSchemes(), url.Protocol().Ascii()))
     return true;
 
-  // 2. If url’s scheme is "data", return "Potentially Trustworthy".
-  if (gurl.SchemeIs(url::kDataScheme))
+  // URLs that wrap inner URLs are secure if those inner URLs are secure.
+  if (ShouldUseInnerURL(url) &&
+      base::Contains(url::GetSecureSchemes(),
+                     ExtractInnerURL(url).Protocol().Ascii()))
     return true;
 
-  // 3. Return the result of executing §3.2 Is origin potentially trustworthy?
-  //    on url’s origin.
-  //    Note: The origin of blob: and filesystem: URLs is the origin of the
-  //    context in which they were created. Therefore, blobs created in a
-  //    trustworthy origin will themselves be potentially trustworthy.
-  // https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-origin
-  scoped_refptr<SecurityOrigin> security_origin = SecurityOrigin::Create(url);
-  url::Origin origin = security_origin->ToUrlOrigin();
-
-  // 1. If origin is an opaque origin, return "Not Trustworthy".
-  if (origin.opaque())
-    return false;
-
-  // 2. Assert: origin is a tuple origin.
-  DCHECK(!origin.opaque());
-
-  // 3. If origin’s scheme is either "https" or "wss", return "Potentially
-  //    Trustworthy".
-  // This is handled by the url::GetSecureSchemes() call below.
-
-  // 7. If origin’s scheme component is one which the user agent considers to be
-  //    authenticated, return "Potentially Trustworthy".
-  //    Note: See §7.1 Packaged Applications for detail here.
-  if (base::Contains(url::GetSecureSchemes(), origin.scheme()))
-    return true;
-
-  // 8. If origin has been configured as a trustworthy origin, return
-  //    "Potentially Trustworthy".
-  //    Note: See §7.2 Development Environments for detail here.
-  if (network::SecureOriginAllowlist::GetInstance().IsOriginAllowlisted(origin))
-    return true;
-
-  // 9. Return "Not Trustworthy".
-  return false;
+  scoped_refptr<SecurityOrigin> origin = SecurityOrigin::Create(url);
+  return !origin->IsOpaque() &&
+         network::SecureOriginAllowlist::GetInstance().IsOriginAllowlisted(
+             origin->ToUrlOrigin());
 }
 
 base::Optional<base::UnguessableToken>

third_party/blink/renderer/platform/weborigin/security_origin_test.cc

@@ -231,6 +231,9 @@ TEST_F(SecurityOriginTest, IsSecure) {
     bool is_secure;
     const char* url;
   } inputs[] = {
+      // TODO(crbug.com/1153336): Should SecurityOrigin::IsSecure be aligned
+      // with network::IsURLPotentiallyTrustworthy?
+      // https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy
       {false, "blob:ftp://evil:99/578223a1-8c13-17b3-84d5-eca045ae384a"},
       {false, "blob:http://example.com/578223a1-8c13-17b3-84d5-eca045ae384a"},
       {false, "file:///etc/passwd"},
@@ -242,14 +245,14 @@ TEST_F(SecurityOriginTest, IsSecure) {
       {true, "wss://example.com/"},
       {true, "about:blank"},
       {true, "about:srcdoc"},
-      {false, "about:about"},
+      {true, "about:about"},
       {true, "data:text/html,Hello"},
       {false,
        "filesystem:http://example.com/578223a1-8c13-17b3-84d5-eca045ae384a"},
       {true,
        "filesystem:https://example.com/578223a1-8c13-17b3-84d5-eca045ae384a"},
-      {false, "blob:data:text/html,Hello"},
-      {false, "blob:about:blank"},
+      {true, "blob:data:text/html,Hello"},
+      {true, "blob:about:blank"},
       {false, "filesystem:data:text/html,Hello"},
       {false, "filesystem:about:blank"},
       {false,
@@ -286,7 +289,9 @@ TEST_F(SecurityOriginTest, CustomScheme) {
   {
     url::ScopedSchemeRegistryForTests scoped_registry;
     url::AddSecureScheme(custom_scheme);
-    EXPECT_FALSE(SecurityOrigin::IsSecure(url));
+    // TODO(crbug.com/1153336): Should SecurityOrigin::IsSecure always consider
+    // non-standard schemes as insecure?
+    EXPECT_TRUE(SecurityOrigin::IsSecure(url));
     scoped_refptr<const SecurityOrigin> origin =
         SecurityOrigin::CreateFromString(custom_scheme_example);
     EXPECT_FALSE(origin->IsPotentiallyTrustworthy());