From 4d1145dec9702c9a85931e4e935dcc0c0ca12bae Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Thu, 29 Oct 2020 15:05:33 +0000 Subject: [PATCH] Origin isolation: allow isolating non-HTTPS URLs which are secure contexts can use the origin isolation feature, even if they're not HTTPS. The most important example of this is http://localhost. Fixed: 1142894 Change-Id: I0c8d1c73465c2c289ae2996695f0c42f78cf100c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503849 Reviewed-by: Alex Moshchuk Reviewed-by: Charlie Reis Reviewed-by: James MacLean Commit-Queue: Domenic Denicola Cr-Commit-Position: refs/heads/master@{#822168} --- .../child_process_security_policy_impl.cc | 16 +--- .../browser/isolated_origin_browsertest.cc | 82 +++++++++++++++++++ content/browser/isolated_origin_util.cc | 37 +++++++-- content/browser/isolated_origin_util.h | 13 ++- 4 files changed, 128 insertions(+), 20 deletions(-) diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc index 1bb9dc8bd86678..c3f22af01106f5 100644 --- a/content/browser/child_process_security_policy_impl.cc +++ b/content/browser/child_process_security_policy_impl.cc @@ -2104,8 +2104,7 @@ bool ChildProcessSecurityPolicyImpl::ShouldOriginGetOptInIsolation( // Note: we cannot check the feature flags and early-out here, because the // origin trial might be active (in which case no feature flags are active). - // We only isolate HTTPS, so early-out if we see other schemes. - if (!origin.GetURL().SchemeIs(url::kHttpsScheme)) + if (!IsolatedOriginUtil::IsValidOriginForOptInIsolation(origin)) return false; base::AutoLock origins_isolation_opt_in_lock(origins_isolation_opt_in_lock_); @@ -2153,9 +2152,7 @@ void ChildProcessSecurityPolicyImpl::AddNonIsolatedOriginIfNeeded( const url::Origin& origin, bool is_global_walk_or_frame_removal) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - // Origin Policy only exists for HTTPS, and header-based opt-in requests are - // also HTTPS-only, so nothing we isolate will be HTTP. - if (!origin.GetURL().SchemeIs(url::kHttpsScheme)) + if (!IsolatedOriginUtil::IsValidOriginForOptInIsolation(origin)) return; BrowsingInstanceId browsing_instance_id( @@ -2223,9 +2220,7 @@ void ChildProcessSecurityPolicyImpl:: void ChildProcessSecurityPolicyImpl::AddOptInIsolatedOriginForBrowsingInstance( const IsolationContext& isolation_context, const url::Origin& origin) { - // Origin Policy only exists for HTTPS, so nothing we isolate will be HTTP. - if (!origin.GetURL().SchemeIs(url::kHttpsScheme)) - return; + DCHECK(IsolatedOriginUtil::IsValidOriginForOptInIsolation(origin)); BrowsingInstanceId browsing_instance_id( isolation_context.browsing_instance_id()); @@ -2252,11 +2247,8 @@ void ChildProcessSecurityPolicyImpl::AddOptInIsolatedOriginForBrowsingInstance( bool ChildProcessSecurityPolicyImpl::UpdateOriginIsolationOptInListIfNecessary( const url::Origin& origin) { - // Avoid dealing with non-HTTPS and other non-valid-for-isolation origins. - if (!origin.GetURL().SchemeIs(url::kHttpsScheme) || - !IsolatedOriginUtil::IsValidIsolatedOrigin(origin)) { + if (!IsolatedOriginUtil::IsValidOriginForOptInIsolation(origin)) return false; - } base::AutoLock origins_isolation_opt_in_lock(origins_isolation_opt_in_lock_); diff --git a/content/browser/isolated_origin_browsertest.cc b/content/browser/isolated_origin_browsertest.cc index e60795947e0257..5fa9d0bb755c09 100644 --- a/content/browser/isolated_origin_browsertest.cc +++ b/content/browser/isolated_origin_browsertest.cc @@ -297,6 +297,57 @@ class OriginIsolationOptInHeaderTest : public OriginIsolationOptInServerTest { DISALLOW_COPY_AND_ASSIGN(OriginIsolationOptInHeaderTest); }; +// Used for a few tests that check non-HTTPS secure context behavior. +class OriginIsolationOptInHttpServerHeaderTest : public IsolatedOriginTestBase { + public: + OriginIsolationOptInHttpServerHeaderTest() = default; + + void SetUpCommandLine(base::CommandLine* command_line) override { + IsolatedOriginTestBase::SetUpCommandLine(command_line); + ASSERT_TRUE(embedded_test_server()->InitializeAndListen()); + + // This is needed for this test to run properly on platforms where + // --site-per-process isn't the default, such as Android. + IsolateAllSitesForTesting(command_line); + + feature_list_.InitAndEnableFeature(features::kOriginIsolationHeader); + + embedded_test_server()->RegisterRequestHandler(base::BindRepeating( + &OriginIsolationOptInHttpServerHeaderTest::HandleResponse, + base::Unretained(this))); + } + + void SetUpOnMainThread() override { + host_resolver()->AddRule("*", "127.0.0.1"); + embedded_test_server()->StartAcceptingConnections(); + } + + bool ShouldOriginGetOptInIsolation(const url::Origin& origin) { + auto* site_instance = static_cast( + shell()->web_contents()->GetMainFrame()->GetSiteInstance()); + + return ChildProcessSecurityPolicyImpl::GetInstance() + ->ShouldOriginGetOptInIsolation(site_instance->GetIsolationContext(), + origin, + false /* origin_requests_isolation */); + } + + private: + std::unique_ptr HandleResponse( + const net::test_server::HttpRequest& request) { + auto response = std::make_unique(); + response->set_code(net::HTTP_OK); + response->set_content_type("text/html"); + response->AddCustomHeader("Origin-Isolation", "?1"); + + response->set_content("isolate me!"); + return std::move(response); + } + + base::test::ScopedFeatureList feature_list_; + DISALLOW_COPY_AND_ASSIGN(OriginIsolationOptInHttpServerHeaderTest); +}; + // This class allows testing the interaction of OptIn isolation and command-line // isolation for origins. Tests using this class will isolate foo.com and // bar.com by default using command-line isolation, but any opt-in isolation @@ -414,6 +465,37 @@ IN_PROC_BROWSER_TEST_F(OriginIsolationOptInHeaderTest, Basic) { EXPECT_TRUE(ShouldOriginGetOptInIsolation(origin)); } +// These tests ensure that non-HTTPS secure contexts (see +// https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy) are +// able to use origin isolation. +IN_PROC_BROWSER_TEST_F(OriginIsolationOptInHttpServerHeaderTest, Localhost) { + GURL url(embedded_test_server()->GetURL("localhost", "/")); + url::Origin origin(url::Origin::Create(url)); + + EXPECT_FALSE(ShouldOriginGetOptInIsolation(origin)); + EXPECT_TRUE(NavigateToURL(shell(), url)); + EXPECT_TRUE(ShouldOriginGetOptInIsolation(origin)); +} + +IN_PROC_BROWSER_TEST_F(OriginIsolationOptInHttpServerHeaderTest, DotLocalhost) { + GURL url(embedded_test_server()->GetURL("test.localhost", "/")); + url::Origin origin(url::Origin::Create(url)); + + EXPECT_FALSE(ShouldOriginGetOptInIsolation(origin)); + EXPECT_TRUE(NavigateToURL(shell(), url)); + EXPECT_TRUE(ShouldOriginGetOptInIsolation(origin)); +} + +IN_PROC_BROWSER_TEST_F(OriginIsolationOptInHttpServerHeaderTest, + OneTwentySeven) { + GURL url(embedded_test_server()->GetURL("127.0.0.1", "/")); + url::Origin origin(url::Origin::Create(url)); + + EXPECT_FALSE(ShouldOriginGetOptInIsolation(origin)); + EXPECT_TRUE(NavigateToURL(shell(), url)); + EXPECT_TRUE(ShouldOriginGetOptInIsolation(origin)); +} + // Further tests deep-dive into various scenarios for the isolation opt-ins. // They use the origin policy mechanism, under the assumption that it will be // the same for the header mechanism since they both trigger the same behavior diff --git a/content/browser/isolated_origin_util.cc b/content/browser/isolated_origin_util.cc index 570491adfbc47c..82d72f1c4e41a5 100644 --- a/content/browser/isolated_origin_util.cc +++ b/content/browser/isolated_origin_util.cc @@ -9,6 +9,7 @@ #include "base/logging.h" #include "base/strings/string_util.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" +#include "services/network/public/cpp/is_potentially_trustworthy.h" #include "url/gurl.h" const char* kAllSubdomainsWildcard = "[*.]"; @@ -107,6 +108,22 @@ bool IsolatedOriginUtil::DoesOriginMatchIsolatedOrigin( // static bool IsolatedOriginUtil::IsValidIsolatedOrigin(const url::Origin& origin) { + return IsValidIsolatedOriginImpl(origin, true); +} + +// static +bool IsolatedOriginUtil::IsValidOriginForOptInIsolation( + const url::Origin& origin) { + // Per https://html.spec.whatwg.org/C/#initialise-the-document-object, + // non-secure contexts cannot be isolated via opt-in origin isolation. + return IsValidIsolatedOriginImpl(origin, false) && + network::IsOriginPotentiallyTrustworthy(origin); +} + +// static +bool IsolatedOriginUtil::IsValidIsolatedOriginImpl( + const url::Origin& origin, + bool check_has_registry_domain) { if (origin.opaque()) return false; @@ -123,13 +140,19 @@ bool IsolatedOriginUtil::IsValidIsolatedOrigin(const url::Origin& origin) { // Disallow hosts such as http://co.uk/, which don't have a valid // registry-controlled domain. This prevents subdomain matching from // grouping unrelated sites on a registry into the same origin. - const bool has_registry_domain = - net::registry_controlled_domains::HostHasRegistryControlledDomain( - origin.host(), - net::registry_controlled_domains::INCLUDE_UNKNOWN_REGISTRIES, - net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); - if (!has_registry_domain) - return false; + // + // This is not relevant for opt-in origin isolation, which doesn't need to + // match subdomains. (And it'd be bad to check this in that case, as it + // prohibits http://localhost/; see https://crbug.com/1142894.) + if (check_has_registry_domain) { + const bool has_registry_domain = + net::registry_controlled_domains::HostHasRegistryControlledDomain( + origin.host(), + net::registry_controlled_domains::INCLUDE_UNKNOWN_REGISTRIES, + net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); + if (!has_registry_domain) + return false; + } // For now, disallow hosts with a trailing dot. // TODO(alexmos): Enabling this would require carefully thinking about diff --git a/content/browser/isolated_origin_util.h b/content/browser/isolated_origin_util.h index 9486db24f78d46..4c6f4be5e75288 100644 --- a/content/browser/isolated_origin_util.h +++ b/content/browser/isolated_origin_util.h @@ -91,10 +91,21 @@ class CONTENT_EXPORT IsolatedOriginUtil { const url::Origin& isolated_origin); // Check if |origin| is a valid isolated origin. Invalid isolated origins - // include unique origins, origins that don't have an HTTP or HTTPS scheme, + // include opaque origins, origins that don't have an HTTP or HTTPS scheme, // and origins without a valid registry-controlled domain. IP addresses are // allowed. static bool IsValidIsolatedOrigin(const url::Origin& origin); + + // Check if |origin| is a valid origin for opt-in origin isolation. Invalid + // origins for this purpose include opaque origins, origins that don't have a + // HTTP or HTTPS scheme, and origins that are not secure contexts. + static bool IsValidOriginForOptInIsolation(const url::Origin& origin); + + private: + // Used to implement both IsValidIsolatedOrigin and + // IsValidOriginForOptInIsolation. + static bool IsValidIsolatedOriginImpl(const url::Origin& origin, + bool check_has_registry_domain); }; } // namespace content