Remove SecurityPolicy APIs for handling a trustworthy safelist

This CL removes the following methods:

WebSecurityPolicy::AddOriginToTrustworthySafelist
SecurityPolicy::AddOriginToTrustworthySafelist
SecurityPolicy::IsOriginTrustworthySafelisted

Outside tests, these methods are only used to populate SecurityPolicy's
safelist from SecureOriginAllowlist. The last one is only called from
SecurityOrigin::IsSecure and SecurityOrigin::IsPotentiallyTrustworthy,
but they can rely on SecureOriginAllowlist::IsOriginAllowlisted instead.

Some tests also rely on SecurityPolicy::AddOriginToTrustworthySafelist,
but they can directly set the command line parameters to configure
the allow list accordingly.

There is no observable behavior change but this makes easier to compare
SecurityOrigin::IsSecure and SecurityOrigin::IsPotentiallyTrustworthy
with network::IsURLPotentiallyTrustworthy, to facilitate unification of
all these APIs.

Bug: 1153336
Change-Id: I2df13abbdf697ec78136a16373b0b9fc6372bf88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612947Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Cr-Commit-Position: refs/heads/master@{#844000}

chrome/renderer/chrome_content_renderer_client.cc

@@ -429,12 +429,6 @@ void ChromeContentRendererClient::RenderThreadStarted() {
   pdf::PepperPDFHost::SetPrintClient(pdf_print_client_.get());
 #endif
 
-  for (auto& origin_or_hostname_pattern :
-       network::SecureOriginAllowlist::GetInstance().GetCurrentAllowlist()) {
-    WebSecurityPolicy::AddOriginToTrustworthySafelist(
-        WebString::FromUTF8(origin_or_hostname_pattern));
-  }
-
   for (auto& scheme :
        secure_origin_allowlist::GetSchemesBypassingSecureContextCheck()) {
     WebSecurityPolicy::AddSchemeToSecureContextSafelist(

chromecast/renderer/cast_content_renderer_client.cc

@@ -156,12 +156,6 @@ void CastContentRendererClient::RenderThreadStarted() {
       std::make_unique<extensions::ExtensionsGuestViewContainerDispatcher>();
   thread->AddObserver(guest_view_container_dispatcher_.get());
 #endif
-
-  for (auto& origin_or_hostname_pattern :
-       network::SecureOriginAllowlist::GetInstance().GetCurrentAllowlist()) {
-    blink::WebSecurityPolicy::AddOriginToTrustworthySafelist(
-        blink::WebString::FromUTF8(origin_or_hostname_pattern));
-  }
 }
 
 void CastContentRendererClient::RenderViewCreated(

third_party/blink/public/web/web_security_policy.h

@@ -101,11 +101,6 @@ class WebSecurityPolicy {
       const WebURL& source_origin);
   BLINK_EXPORT static void ClearOriginAccessList();
 
-  // Adds an origin or hostname pattern that is always considered trustworthy.
-  // This method does not perform canonicalization; the caller is responsible
-  // for canonicalizing the input.
-  BLINK_EXPORT static void AddOriginToTrustworthySafelist(const WebString&);
-
   // Add a scheme that is always considered a secure context. The caller is
   // responsible for canonicalizing the input.
   BLINK_EXPORT static void AddSchemeToSecureContextSafelist(const WebString&);

third_party/blink/renderer/core/core_initializer.cc

@@ -142,8 +142,6 @@ void CoreInitializer::Initialize() {
 
   style_change_extra_data::Init();
 
-  SecurityPolicy::Init();
-
   RegisterEventFactory();
 
   StringImpl::FreezeStaticStrings();

third_party/blink/renderer/core/exported/web_security_policy.cc

@@ -112,11 +112,6 @@ void WebSecurityPolicy::ClearOriginAccessList() {
   SecurityPolicy::ClearOriginAccessList();
 }
 
-void WebSecurityPolicy::AddOriginToTrustworthySafelist(
-    const WebString& origin) {
-  SecurityPolicy::AddOriginToTrustworthySafelist(origin);
-}
-
 void WebSecurityPolicy::AddSchemeToSecureContextSafelist(
     const WebString& scheme) {
   SchemeRegistry::RegisterURLSchemeBypassingSecureContextCheck(scheme);

third_party/blink/renderer/modules/payments/DEPS

 include_rules = [
+    "+base/test/scoped_command_line.h",
     "+components/payments/mojom",
+    "+services/network/public/cpp/is_potentially_trustworthy.h",
+    "+services/network/public/cpp/network_switches.h",
 ]

third_party/blink/renderer/modules/payments/payments_validators_test.cc

@@ -6,6 +6,9 @@
 
 #include <ostream>  // NOLINT
 
+#include "base/test/scoped_command_line.h"
+#include "services/network/public/cpp/is_potentially_trustworthy.h"
+#include "services/network/public/cpp/network_switches.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_address_errors.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_payer_errors.h"
@@ -363,7 +366,12 @@ TEST(PaymentMethodValidatorTest, IsValidPaymentMethodSafelisted) {
   EXPECT_FALSE(PaymentsValidators::IsValidMethodFormat("http://alicepay.com"))
       << "http://alicepay.com is not a valid method format by default";
 
-  SecurityPolicy::AddOriginToTrustworthySafelist("http://alicepay.com");
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitchASCII(
+      network::switches::kUnsafelyTreatInsecureOriginAsSecure,
+      "http://alicepay.com");
+  network::SecureOriginAllowlist::GetInstance().ResetForTesting();
 
   EXPECT_TRUE(PaymentsValidators::IsValidMethodFormat("http://alicepay.com"))
       << "http://alicepay.com should be valid if safelisted";

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

@@ -315,8 +315,10 @@ bool SecurityOrigin::IsSecure(const KURL& url) {
                      ExtractInnerURL(url).Protocol().Ascii()))
     return true;
 
-  return SecurityPolicy::IsOriginTrustworthySafelisted(
-      *SecurityOrigin::Create(url).get());
+  scoped_refptr<SecurityOrigin> origin = SecurityOrigin::Create(url);
+  return !origin->IsOpaque() &&
+         network::SecureOriginAllowlist::GetInstance().IsOriginAllowlisted(
+             origin->ToUrlOrigin());
 }
 
 base::Optional<base::UnguessableToken>
@@ -472,7 +474,8 @@ bool SecurityOrigin::IsPotentiallyTrustworthy() const {
   // 8. If origin has been configured as a trustworthy origin, return
   //    "Potentially Trustworthy".
   //    Note: See §7.2 Development Environments for detail here.
-  if (SecurityPolicy::IsOriginTrustworthySafelisted(*this))
+  if (network::SecureOriginAllowlist::GetInstance().IsOriginAllowlisted(
+          ToUrlOrigin()))
     return true;
 
   // 9. Return "Not Trustworthy".

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

@@ -33,7 +33,10 @@
 #include <stdint.h>
 
 #include "base/stl_util.h"
+#include "base/test/scoped_command_line.h"
 #include "net/base/url_util.h"
+#include "services/network/public/cpp/is_potentially_trustworthy.h"
+#include "services/network/public/cpp/network_switches.h"
 #include "services/network/public/mojom/cors.mojom-blink.h"
 #include "services/network/public/mojom/cors_origin_pattern.mojom-blink.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -323,16 +326,26 @@ TEST_F(SecurityOriginTest, IsSecureViaTrustworthy) {
   for (const char* test : urls) {
     KURL url(test);
     EXPECT_FALSE(SecurityOrigin::IsSecure(url));
-    SecurityPolicy::AddOriginToTrustworthySafelist(
-        SecurityOrigin::CreateFromString(url)->ToRawString());
-    EXPECT_TRUE(SecurityOrigin::IsSecure(url));
+    {
+      base::test::ScopedCommandLine scoped_command_line;
+      base::CommandLine* command_line =
+          scoped_command_line.GetProcessCommandLine();
+      command_line->AppendSwitchASCII(
+          network::switches::kUnsafelyTreatInsecureOriginAsSecure, test);
+      network::SecureOriginAllowlist::GetInstance().ResetForTesting();
+      EXPECT_TRUE(SecurityOrigin::IsSecure(url));
+    }
   }
 }
 
 TEST_F(SecurityOriginTest, IsSecureViaTrustworthyHostnamePattern) {
   KURL url("http://bar.foo.com");
   EXPECT_FALSE(SecurityOrigin::IsSecure(url));
-  SecurityPolicy::AddOriginToTrustworthySafelist("*.foo.com");
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitchASCII(
+      network::switches::kUnsafelyTreatInsecureOriginAsSecure, "*.foo.com");
+  network::SecureOriginAllowlist::GetInstance().ResetForTesting();
   EXPECT_TRUE(SecurityOrigin::IsSecure(url));
 }
 
@@ -340,7 +353,11 @@ TEST_F(SecurityOriginTest, IsSecureViaTrustworthyHostnamePattern) {
 TEST_F(SecurityOriginTest, IsSecureViaTrustworthyHostnamePatternEmptyHostname) {
   KURL url("file://foo");
   EXPECT_FALSE(SecurityOrigin::IsSecure(url));
-  SecurityPolicy::AddOriginToTrustworthySafelist("*.foo.com");
+  base::test::ScopedCommandLine scoped_command_line;
+  base::CommandLine* command_line = scoped_command_line.GetProcessCommandLine();
+  command_line->AppendSwitchASCII(
+      network::switches::kUnsafelyTreatInsecureOriginAsSecure, "*.foo.com");
+  network::SecureOriginAllowlist::GetInstance().ResetForTesting();
   EXPECT_FALSE(SecurityOrigin::IsSecure(url));
 }
 

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

@@ -62,17 +62,6 @@ static network::cors::OriginAccessList& GetOriginAccessList() {
   return origin_access_list;
 }
 
-using OriginSet = HashSet<String>;
-
-static OriginSet& TrustworthyOriginSafelist() {
-  DEFINE_STATIC_LOCAL(OriginSet, safelist, ());
-  return safelist;
-}
-
-void SecurityPolicy::Init() {
-  TrustworthyOriginSafelist();
-}
-
 bool SecurityPolicy::ShouldHideReferrer(const KURL& url, const KURL& referrer) {
   bool referrer_is_secure_url = referrer.ProtocolIs("https");
   bool scheme_is_allowed =
@@ -166,42 +155,6 @@ Referrer SecurityPolicy::GenerateReferrer(
                   referrer_policy_no_default);
 }
 
-void SecurityPolicy::AddOriginToTrustworthySafelist(
-    const String& origin_or_pattern) {
-#if DCHECK_IS_ON()
-  // Must be called before we start other threads.
-  DCHECK(WTF::IsBeforeThreadCreated());
-#endif
-  // Origins and hostname patterns must be canonicalized (including
-  // canonicalization to 8-bit strings) before being inserted into
-  // TrustworthyOriginSafelist().
-  CHECK(origin_or_pattern.Is8Bit());
-  TrustworthyOriginSafelist().insert(origin_or_pattern);
-}
-
-bool SecurityPolicy::IsOriginTrustworthySafelisted(
-    const SecurityOrigin& origin) {
-  // Early return if |origin| cannot possibly be matched.
-  if (origin.IsOpaque() || TrustworthyOriginSafelist().IsEmpty())
-    return false;
-
-  if (TrustworthyOriginSafelist().Contains(origin.ToRawString()))
-    return true;
-
-  // KURL and SecurityOrigin hosts should be canonicalized to 8-bit strings.
-  CHECK(origin.Host().Is8Bit());
-  StringUTF8Adaptor host_adaptor(origin.Host());
-  for (const auto& origin_or_pattern : TrustworthyOriginSafelist()) {
-    StringUTF8Adaptor origin_or_pattern_adaptor(origin_or_pattern);
-    if (base::MatchPattern(host_adaptor.AsStringPiece(),
-                           origin_or_pattern_adaptor.AsStringPiece())) {
-      return true;
-    }
-  }
-
-  return false;
-}
-
 bool SecurityPolicy::IsOriginAccessAllowed(
     const SecurityOrigin* active_origin,
     const SecurityOrigin* target_origin) {

third_party/blink/renderer/platform/weborigin/security_policy.h

@@ -50,10 +50,6 @@ class PLATFORM_EXPORT SecurityPolicy {
   STATIC_ONLY(SecurityPolicy);
 
  public:
-  // This must be called during initialization (before we create
-  // other threads).
-  static void Init();
-
   // True if the referrer should be omitted according to the
   // ReferrerPolicyNoReferrerWhenDowngrade. If you intend to send a
   // referrer header, you should use generateReferrer instead.
@@ -91,9 +87,6 @@ class PLATFORM_EXPORT SecurityPolicy {
   static bool IsOriginAccessToURLAllowed(const SecurityOrigin* active_origin,
                                          const KURL&);
 
-  static void AddOriginToTrustworthySafelist(const String&);
-  static bool IsOriginTrustworthySafelisted(const SecurityOrigin&);
-
   static bool ReferrerPolicyFromString(const String& policy,
                                        ReferrerPolicyLegacyKeywordsSupport,
                                        network::mojom::ReferrerPolicy* result);

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

@@ -30,7 +30,10 @@
 
 #include "third_party/blink/renderer/platform/weborigin/security_policy.h"
 
+#include "base/test/scoped_command_line.h"
 #include "base/test/scoped_feature_list.h"
+#include "services/network/public/cpp/is_potentially_trustworthy.h"
+#include "services/network/public/cpp/network_switches.h"
 #include "services/network/public/mojom/cors.mojom-blink.h"
 #include "services/network/public/mojom/cors_origin_pattern.mojom-blink.h"
 #include "services/network/public/mojom/referrer_policy.mojom-shared.h"
@@ -358,8 +361,17 @@ TEST(SecurityPolicyTest, TrustworthySafelist) {
     scoped_refptr<const SecurityOrigin> origin =
         SecurityOrigin::CreateFromString(url);
     EXPECT_FALSE(origin->IsPotentiallyTrustworthy());
-    SecurityPolicy::AddOriginToTrustworthySafelist(origin->ToString());
-    EXPECT_TRUE(origin->IsPotentiallyTrustworthy());
+
+    {
+      base::test::ScopedCommandLine scoped_command_line;
+      base::CommandLine* command_line =
+          scoped_command_line.GetProcessCommandLine();
+      command_line->AppendSwitchASCII(
+          network::switches::kUnsafelyTreatInsecureOriginAsSecure,
+          origin->ToString().Latin1());
+      network::SecureOriginAllowlist::GetInstance().ResetForTesting();
+      EXPECT_TRUE(origin->IsPotentiallyTrustworthy());
+    }
   }
 
   // Tests that adding URLs that have inner-urls to the safelist
@@ -385,9 +397,17 @@ TEST(SecurityPolicyTest, TrustworthySafelist) {
 
     EXPECT_FALSE(origin1->IsPotentiallyTrustworthy());
     EXPECT_FALSE(origin2->IsPotentiallyTrustworthy());
-    SecurityPolicy::AddOriginToTrustworthySafelist(origin1->ToString());
-    EXPECT_TRUE(origin1->IsPotentiallyTrustworthy());
-    EXPECT_TRUE(origin2->IsPotentiallyTrustworthy());
+    {
+      base::test::ScopedCommandLine scoped_command_line;
+      base::CommandLine* command_line =
+          scoped_command_line.GetProcessCommandLine();
+      command_line->AppendSwitchASCII(
+          network::switches::kUnsafelyTreatInsecureOriginAsSecure,
+          origin1->ToString().Latin1());
+      network::SecureOriginAllowlist::GetInstance().ResetForTesting();
+      EXPECT_TRUE(origin1->IsPotentiallyTrustworthy());
+      EXPECT_TRUE(origin2->IsPotentiallyTrustworthy());
+    }
   }
 }