Skip to content

Commit

Permalink
Origin isolation: allow isolating non-HTTPS
Browse files Browse the repository at this point in the history
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 <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822168}
  • Loading branch information
domenic authored and Commit Bot committed Oct 29, 2020
1 parent d6bba3d commit 4d1145d
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 20 deletions.
16 changes: 4 additions & 12 deletions content/browser/child_process_security_policy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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());
Expand All @@ -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_);

Expand Down
82 changes: 82 additions & 0 deletions content/browser/isolated_origin_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<SiteInstanceImpl*>(
shell()->web_contents()->GetMainFrame()->GetSiteInstance());

return ChildProcessSecurityPolicyImpl::GetInstance()
->ShouldOriginGetOptInIsolation(site_instance->GetIsolationContext(),
origin,
false /* origin_requests_isolation */);
}

private:
std::unique_ptr<net::test_server::HttpResponse> HandleResponse(
const net::test_server::HttpRequest& request) {
auto response = std::make_unique<net::test_server::BasicHttpResponse>();
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
Expand Down Expand Up @@ -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
Expand Down
37 changes: 30 additions & 7 deletions content/browser/isolated_origin_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "[*.]";
Expand Down Expand Up @@ -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;

Expand All @@ -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
Expand Down
13 changes: 12 additions & 1 deletion content/browser/isolated_origin_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4d1145d

Please sign in to comment.