Skip to content

Commit

Permalink
Remove suborigin implementation
Browse files Browse the repository at this point in the history
Bug: 807932
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I134ad824f7a67bf8ffb6a27a6012043fae00cad8
Reviewed-on: https://chromium-review.googlesource.com/897218
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535671}
  • Loading branch information
toyoshim authored and Commit Bot committed Feb 9, 2018
1 parent 52669e3 commit 5641d75
Show file tree
Hide file tree
Showing 186 changed files with 187 additions and 4,960 deletions.
37 changes: 3 additions & 34 deletions chrome/browser/browsing_data/browsing_data_local_storage_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ using content::DOMStorageContext;

namespace {

// Only websafe state and suborigins are considered browsing data.
// Only websafe state is considered browsing data.
bool HasStorageScheme(const GURL& origin_url) {
return BrowsingDataHelper::HasWebScheme(origin_url) ||
origin_url.SchemeIsSuborigin();
return BrowsingDataHelper::HasWebScheme(origin_url);
}

void GetUsageInfoCallback(
Expand Down Expand Up @@ -77,8 +76,7 @@ void BrowsingDataLocalStorageHelper::StartFetching(
void BrowsingDataLocalStorageHelper::DeleteOrigin(const GURL& origin_url,
base::OnceClosure callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
dom_storage_context_->DeleteLocalStorageForPhysicalOrigin(
origin_url, std::move(callback));
dom_storage_context_->DeleteLocalStorage(origin_url, std::move(callback));
}

//---------------------------------------------------------
Expand All @@ -94,15 +92,10 @@ void CannedBrowsingDataLocalStorageHelper::AddLocalStorage(
return;
pending_local_storage_info_.insert(origin_url);
url::Origin origin = url::Origin::Create(origin_url);
if (!origin.suborigin().empty()) {
pending_origins_to_pending_suborigins_.insert(
std::make_pair(origin.GetPhysicalOrigin().GetURL(), origin_url));
}
}

void CannedBrowsingDataLocalStorageHelper::Reset() {
pending_local_storage_info_.clear();
pending_origins_to_pending_suborigins_.clear();
}

bool CannedBrowsingDataLocalStorageHelper::empty() const {
Expand Down Expand Up @@ -135,31 +128,7 @@ void CannedBrowsingDataLocalStorageHelper::DeleteOrigin(
const GURL& origin_url,
base::OnceClosure callback) {
pending_local_storage_info_.erase(origin_url);
// All suborigins associated with |origin_url| must be removed.
// BrowsingDataLocalStorageHelper::DeleteOrigin takes care of doing that on
// the backend so it's not necessary to call it for each suborigin, but it is
// necessary to clear up the pending storage here.
BrowsingDataLocalStorageHelper::DeleteOrigin(origin_url, std::move(callback));
if (pending_origins_to_pending_suborigins_.count(origin_url) > 0) {
auto it = pending_origins_to_pending_suborigins_.find(origin_url);
while (it != pending_origins_to_pending_suborigins_.end()) {
pending_local_storage_info_.erase(it->second);
pending_origins_to_pending_suborigins_.erase(it);
it = pending_origins_to_pending_suborigins_.find(origin_url);
}
}

// Similarly, if |origin_url| has a suborigin, the physical origin associated
// with that suborigin must also be deleted. This is also taken care of on the
// backend, so it's only necessary to clean up the pending storage.
url::Origin origin = url::Origin::Create(origin_url);
if (!origin.suborigin().empty()) {
GURL physical_origin(origin.GetPhysicalOrigin().GetURL());
pending_local_storage_info_.erase(physical_origin);
pending_origins_to_pending_suborigins_.erase(physical_origin);
}

pending_origins_to_pending_suborigins_.erase(origin_url);
}

CannedBrowsingDataLocalStorageHelper::~CannedBrowsingDataLocalStorageHelper() {}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ class CannedBrowsingDataLocalStorageHelper
~CannedBrowsingDataLocalStorageHelper() override;

std::set<GURL> pending_local_storage_info_;
std::multimap<GURL, GURL> pending_origins_to_pending_suborigins_;

DISALLOW_COPY_AND_ASSIGN(CannedBrowsingDataLocalStorageHelper);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,6 @@ TEST_F(CannedBrowsingDataLocalStorageTest, Delete) {
EXPECT_EQ(2u, helper->GetLocalStorageCount());
helper->DeleteOrigin(origin1, base::BindOnce(&base::DoNothing));
EXPECT_EQ(1u, helper->GetLocalStorageCount());

// Local storage for a suborigin
// (https://www.chromestatus.com/feature/5569465034997760) should be deleted
// when the corresponding physical origin is deleted.
const GURL suborigin_on_origin_3("http-so://suborigin.foo.example.com");
helper->AddLocalStorage(suborigin_on_origin_3);
EXPECT_EQ(2u, helper->GetLocalStorageCount());
helper->DeleteOrigin(origin3, base::BindOnce(&base::DoNothing));
EXPECT_EQ(0u, helper->GetLocalStorageCount());
helper->AddLocalStorage(suborigin_on_origin_3);
EXPECT_EQ(1u, helper->GetLocalStorageCount());
helper->DeleteOrigin(origin3, base::BindOnce(&base::DoNothing));
EXPECT_EQ(0u, helper->GetLocalStorageCount());

// Similarly, the suborigin should be deleted when the corresponding
// physical origin is deleted.
helper->AddLocalStorage(origin3);
helper->AddLocalStorage(suborigin_on_origin_3);
EXPECT_EQ(2u, helper->GetLocalStorageCount());
helper->DeleteOrigin(suborigin_on_origin_3, base::BindOnce(&base::DoNothing));
EXPECT_EQ(0u, helper->GetLocalStorageCount());
}

TEST_F(CannedBrowsingDataLocalStorageTest, IgnoreExtensionsAndDevTools) {
Expand Down
16 changes: 2 additions & 14 deletions chrome/browser/browsing_data/cookies_tree_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,23 +78,11 @@ std::string CanonicalizeHost(const GURL& url) {
// would become google.com.mail.1, and then a standard string comparison works
// to order hosts by registry controlled domain first. Leading dots are
// ignored, ".google.com" is the same as "google.com".
//
// Suborigins, an experimental web platform feature defined in
// https://w3c.github.io/webappsec-suborigins/, are treated as part of the
// physical origin they are associated with. From a users perspective, they
// are part of and should be visualized as part of that host. For example,
// given a a suborigin 'foobar' at 'https://example.com', this is serialized
// into the URL as 'https-so://foobar.example.com'. Thus, the host for this
// URL is canonicalized as 'example.com' to treat it as being part of that
// host, and thus the suborigin is striped from the URL.
if (url.SchemeIsFile()) {
return std::string(url::kFileScheme) +
url::kStandardSchemeSeparator;
return std::string(url::kFileScheme) + url::kStandardSchemeSeparator;
}

// Pass through url::Origin to get the real host, which has the effect of
// stripping the suborigin from the URL.
std::string host = url::Origin::Create(url).host();
std::string host = url.host();
std::string retval =
net::registry_controlled_domains::GetDomainAndRegistry(
host,
Expand Down
40 changes: 0 additions & 40 deletions chrome/browser/browsing_data/cookies_tree_model_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2005,44 +2005,4 @@ TEST_F(CookiesTreeModelTest, CookiesFilterWithoutSource) {
EXPECT_EQ("A", GetDisplayedCookies(&cookies_model));
}

TEST_F(CookiesTreeModelTest, Suborigins) {
LocalDataContainer* container = new LocalDataContainer(
mock_browsing_data_cookie_helper_, mock_browsing_data_database_helper_,
mock_browsing_data_local_storage_helper_,
mock_browsing_data_session_storage_helper_,
mock_browsing_data_appcache_helper_,
mock_browsing_data_indexed_db_helper_,
mock_browsing_data_file_system_helper_, mock_browsing_data_quota_helper_,
mock_browsing_data_channel_id_helper_,
mock_browsing_data_service_worker_helper_,
mock_browsing_data_shared_worker_helper_,
mock_browsing_data_cache_storage_helper_,
mock_browsing_data_flash_lso_helper_,
mock_browsing_data_media_license_helper_);
CookiesTreeModel cookies_model(container, special_storage_policy());

mock_browsing_data_local_storage_helper_
->AddLocalStorageSamplesWithSuborigins();
mock_browsing_data_local_storage_helper_->Notify();
{
SCOPED_TRACE(
"Suborigins get their own storage partitions and are folded into their "
"respective hosts");
// root
// host4 -> local storage -> http-so://foobar.host4:4
// -> http://host4:4
// host3 -> local storage -> http-so://foobar.host3:3
// host2 -> local storage -> http://host2:2
// host1 -> local storage -> http://host1:1
EXPECT_EQ(14, cookies_model.GetRoot()->GetTotalNodeCount());
EXPECT_EQ(
"http://host1:1/,http://host2:2/,http-so://foobar.host3:3/,"
"http-so://foobar.host4:4/,http://host4:4/",
GetDisplayedLocalStorages(&cookies_model));
// Delete host4, which should delete foobar_host4:4 in addition to host4:4
DeleteStoredObjects(cookies_model.GetRoot()->GetChild(3));
EXPECT_EQ(10, cookies_model.GetRoot()->GetTotalNodeCount());
}
}

} // namespace
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,6 @@ void MockBrowsingDataLocalStorageHelper::AddLocalStorageSamples() {
origins_[kOrigin2] = true;
}

void MockBrowsingDataLocalStorageHelper::
AddLocalStorageSamplesWithSuborigins() {
AddLocalStorageSamples();
const GURL kSuborigin3("http-so://foobar.host3:3");
const GURL kOrigin4("http://host4:4");
const GURL kSuborigin4("http-so://foobar.host4:4");
response_.push_back(BrowsingDataLocalStorageHelper::LocalStorageInfo(
kSuborigin3, 1, base::Time()));
origins_[kSuborigin3] = true;
response_.push_back(BrowsingDataLocalStorageHelper::LocalStorageInfo(
kOrigin4, 1, base::Time()));
origins_[kOrigin4] = true;
response_.push_back(BrowsingDataLocalStorageHelper::LocalStorageInfo(
kSuborigin4, 1, base::Time()));
origins_[kSuborigin4] = true;
}

void MockBrowsingDataLocalStorageHelper::Notify() {
callback_.Run(response_);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class MockBrowsingDataLocalStorageHelper

// Adds some LocalStorageInfo samples.
void AddLocalStorageSamples();
void AddLocalStorageSamplesWithSuborigins();

// Notifies the callback.
void Notify();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static boolean isOriginSecure(String url) {
}

/**
* Returns true for a valid URL with a cryptographic scheme, e.g., HTTPS, HTTPS-SO, WSS.
* Returns true for a valid URL with a cryptographic scheme, e.g., HTTPS, WSS.
*
* @param url The URL to check.
* @return Whether the scheme of the URL is cryptographic.
Expand Down
2 changes: 1 addition & 1 deletion components/payments/content/origin_security_checker.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class OriginSecurityChecker {
static bool IsOriginSecure(const GURL& url);

// Returns true for a valid |url| with a cryptographic scheme, e.g., HTTPS,
// HTTPS-SO, WSS.
// WSS.
static bool IsSchemeCryptographic(const GURL& url);

// Returns true for a valid |url| with localhost or file:// scheme origin.
Expand Down
3 changes: 1 addition & 2 deletions components/security_state/core/security_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ void GetSecurityInfo(
// |kHttpFormWarningFeature| feature.
bool IsHttpWarningInFormEnabled();

// Returns true for a valid |url| with a cryptographic scheme, e.g., HTTPS,
// HTTPS-SO, WSS.
// Returns true for a valid |url| with a cryptographic scheme, e.g., HTTPS, WSS.
bool IsSchemeCryptographic(const GURL& url);

// Returns true for a valid |url| with localhost or file:// scheme origin.
Expand Down
3 changes: 0 additions & 3 deletions components/security_state/core/security_state_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ namespace {

const char kHttpsUrl[] = "https://foo.test/";
const char kHttpUrl[] = "http://foo.test/";
const char kHttpsSoUrl[] = "https-so://foo.test/";
const char kLocalhostUrl[] = "http://localhost";
const char kFileOrigin[] = "file://example_file";
const char kWssUrl[] = "wss://foo.test/";
Expand Down Expand Up @@ -610,8 +609,6 @@ TEST(SecurityStateTest, FieldEdit) {
TEST(SecurityStateTest, CryptographicSchemeUrl) {
// HTTPS is a cryptographic scheme.
EXPECT_TRUE(IsSchemeCryptographic(GURL(kHttpsUrl)));
// HTTPS-SO is a cryptographic scheme.
EXPECT_TRUE(IsSchemeCryptographic(GURL(kHttpsSoUrl)));
// WSS is a cryptographic scheme.
EXPECT_TRUE(IsSchemeCryptographic(GURL(kWssUrl)));
// HTTP is not a cryptographic scheme.
Expand Down
15 changes: 3 additions & 12 deletions content/browser/child_process_security_policy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,6 @@ ChildProcessSecurityPolicyImpl::ChildProcessSecurityPolicyImpl() {
RegisterPseudoScheme(url::kAboutScheme);
RegisterPseudoScheme(url::kJavaScriptScheme);
RegisterPseudoScheme(kViewSourceScheme);
RegisterPseudoScheme(url::kHttpSuboriginScheme);
RegisterPseudoScheme(url::kHttpsSuboriginScheme);
}

ChildProcessSecurityPolicyImpl::~ChildProcessSecurityPolicyImpl() {
Expand Down Expand Up @@ -750,15 +748,6 @@ bool ChildProcessSecurityPolicyImpl::CanSetAsOriginHeader(int child_id,
if (!url.is_valid())
return false; // Can't set invalid URLs as origin headers.

const std::string& scheme = url.scheme();

// Suborigin URLs are a special case and are allowed to be an origin header.
if (scheme == url::kHttpSuboriginScheme ||
scheme == url::kHttpsSuboriginScheme) {
DCHECK(IsPseudoScheme(scheme));
return true;
}

// about:srcdoc cannot be used as an origin
if (url == kAboutSrcDocURL)
return false;
Expand All @@ -773,8 +762,10 @@ bool ChildProcessSecurityPolicyImpl::CanSetAsOriginHeader(int child_id,
// document origin.
{
base::AutoLock lock(lock_);
if (base::ContainsKey(schemes_okay_to_appear_as_origin_headers_, scheme))
if (base::ContainsKey(schemes_okay_to_appear_as_origin_headers_,
url.scheme())) {
return true;
}
}
return false;
}
Expand Down
19 changes: 9 additions & 10 deletions content/browser/child_process_security_policy_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
void RevokeReadRawCookies(int child_id);

// Whether the given origin is valid for an origin header. Valid origin
// headers are commitable URLs plus suborigin URLs.
// headers are commitable URLs.
bool CanSetAsOriginHeader(int child_id, const GURL& url);

// Explicit permissions checks for FileSystemURL specified files.
Expand Down Expand Up @@ -227,15 +227,14 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
// of the site for https://isolated.foo.com.
//
// Note that origins from |origins| must not be unique - URLs that render with
// unique origins, such as data: URLs, are not supported. Suborigins (see
// https://w3c.github.io/webappsec-suborigins/ -- not to be confused with
// subdomains) and non-standard schemes are also not supported. Sandboxed
// frames (e.g., <iframe sandbox>) *are* supported, since process placement
// decisions will be based on the URLs such frames navigate to, and not the
// origin of committed documents (which might be unique). If an isolated
// origin opens an about:blank popup, it will stay in the isolated origin's
// process. Nested URLs (filesystem: and blob:) retain process isolation
// behavior of their inner origin.
// unique origins, such as data: URLs, are not supported. Non-standard
// schemes are also not supported. Sandboxed frames (e.g., <iframe sandbox>)
// *are* supported, since process placement decisions will be based on the
// URLs such frames navigate to, and not the origin of committed documents
// (which might be unique). If an isolated origin opens an about:blank
// popup, it will stay in the isolated origin's process. Nested URLs
// (filesystem: and blob:) retain process isolation behavior of their inner
// origin.
//
// Note that it is okay if |origins| contains duplicates - the set of origins
// will be deduplicated inside the method.
Expand Down
34 changes: 0 additions & 34 deletions content/browser/child_process_security_policy_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ TEST_F(ChildProcessSecurityPolicyTest, IsPseudoSchemeTest) {
EXPECT_TRUE(p->IsPseudoScheme(url::kAboutScheme));
EXPECT_TRUE(p->IsPseudoScheme(url::kJavaScriptScheme));
EXPECT_TRUE(p->IsPseudoScheme(kViewSourceScheme));
EXPECT_TRUE(p->IsPseudoScheme(url::kHttpSuboriginScheme));
EXPECT_TRUE(p->IsPseudoScheme(url::kHttpsSuboriginScheme));

EXPECT_FALSE(p->IsPseudoScheme("registered-pseudo-scheme"));
p->RegisterPseudoScheme("registered-pseudo-scheme");
Expand Down Expand Up @@ -382,38 +380,6 @@ TEST_F(ChildProcessSecurityPolicyTest, JavaScriptTest) {
p->Remove(kRendererID);
}

TEST_F(ChildProcessSecurityPolicyTest, SuboriginTest) {
ChildProcessSecurityPolicyImpl* p =
ChildProcessSecurityPolicyImpl::GetInstance();

p->Add(kRendererID);

// Suborigin URLs are not requestable or committable.
EXPECT_FALSE(
p->CanRequestURL(kRendererID, GURL("http-so://foobar.example.com")));
EXPECT_FALSE(
p->CanRequestURL(kRendererID, GURL("https-so://foobar.example.com")));
EXPECT_FALSE(p->CanRedirectToURL(GURL("http-so://foobar.example.com")));
EXPECT_FALSE(p->CanRedirectToURL(GURL("https-so://foobar.example.com")));
EXPECT_FALSE(
p->CanCommitURL(kRendererID, GURL("http-so://foobar.example.com")));
EXPECT_FALSE(
p->CanCommitURL(kRendererID, GURL("https-so://foobar.example.com")));

// It's not possible to grant suborigins requestable status.
p->GrantRequestURL(kRendererID, GURL("https-so://foobar.example.com"));
EXPECT_FALSE(
p->CanCommitURL(kRendererID, GURL("https-so://foobar.example.com")));

// Suborigin URLs are valid origin headers.
EXPECT_TRUE(p->CanSetAsOriginHeader(kRendererID,
GURL("http-so://foobar.example.com")));
EXPECT_TRUE(p->CanSetAsOriginHeader(kRendererID,
GURL("https-so://foobar.example.com")));

p->Remove(kRendererID);
}

TEST_F(ChildProcessSecurityPolicyTest, RegisterWebSafeSchemeTest) {
ChildProcessSecurityPolicyImpl* p =
ChildProcessSecurityPolicyImpl::GetInstance();
Expand Down
2 changes: 1 addition & 1 deletion content/browser/dom_storage/dom_storage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DOMStorageBrowserTest : public ContentBrowserTest {
shell()->web_contents()->GetBrowserContext())
->GetDOMStorageContext();
base::RunLoop loop;
context->DeleteLocalStorageForPhysicalOrigin(origin, loop.QuitClosure());
context->DeleteLocalStorage(origin, loop.QuitClosure());
loop.Run();
}
};
Expand Down
Loading

0 comments on commit 5641d75

Please sign in to comment.