Skip to content

Commit

Permalink
Cherry-picking hangouts.google.com whitelisting change into M52.
Browse files Browse the repository at this point in the history
Urgent: whitelist hangouts.google.com further (should've been done in issue 1552383002 from Dec 2015, but...) to allow video effects plugin access; tag on meet.google.com for a similar purpose; tax: refactor whitelist checks into separate thing.

[email protected],[email protected]
BUG=614062
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/1974413003
Cr-Commit-Position: refs/heads/master@{#395172}
(cherry picked from commit 81dcbd6)

Review-Url: https://codereview.chromium.org/2011623002
Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#42}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
q-alex-zhao authored and Commit bot committed May 24, 2016
1 parent 1bc8147 commit 0901e14
Show file tree
Hide file tree
Showing 8 changed files with 262 additions and 123 deletions.
2 changes: 2 additions & 0 deletions chrome/chrome_renderer.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
{
'variables': {
'chrome_renderer_sources': [
'renderer/app_categorizer.h',
'renderer/app_categorizer.cc',
'renderer/banners/app_banner_client.cc',
'renderer/banners/app_banner_client.h',
'renderer/benchmarking_extension.cc',
Expand Down
1 change: 1 addition & 0 deletions chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@
'common/secure_origin_whitelist_unittest.cc',
'common/switch_utils_unittest.cc',
'common/variations/variations_util_unittest.cc',
'renderer/app_categorizer_unittest.cc',
'renderer/chrome_content_renderer_client_unittest.cc',
'renderer/content_settings_observer_unittest.cc',
'renderer/instant_restricted_id_cache_unittest.cc',
Expand Down
5 changes: 5 additions & 0 deletions chrome/renderer/OWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
per-file page_load_histograms*=file://components/page_load_metrics/OWNERS
# Instant/Search files.
per-file instant_restricted_id_cache*=file://chrome/browser/search/OWNERS
# Security checks and whitelists
per-file app_categorizer*=set noparent
per-file app_categorizer*[email protected]
per-file app_categorizer*[email protected]
per-file app_categorizer*[email protected]
80 changes: 80 additions & 0 deletions chrome/renderer/app_categorizer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/renderer/app_categorizer.h"

#include "base/macros.h"
#include "base/strings/string_util.h"
#include "url/gurl.h"

namespace {
// Note: all domain names here must be in lowercase (see GURL::DomainIs, which
// properly handles sub-domains).

const char* const kPredefinedHangoutsDomains[] = {
"hangouts.google.com",
"meet.google.com",
"talkgadget.google.com",
"plus.google.com",
"plus.sandbox.google.com"
};

const char* const kPredefinedPlusDomains[] = {
"plus.google.com",
"plus.sandbox.google.com"
};

bool IsInWhitelistedDomain(
const GURL& url, const char* const domains[], size_t number_of_domains) {
for (size_t i = 0; i < number_of_domains; ++i) {
if (url.DomainIs(domains[i])) {
return true;
}
}

return false;
}

} // namespace

bool AppCategorizer::IsHangoutsUrl(const GURL& url) {
// Whitelisted apps must be served over https.
return url.SchemeIsCryptographic() &&
base::StartsWith(url.path(), "/hangouts/",
base::CompareCase::INSENSITIVE_ASCII) &&
IsInWhitelistedDomain(
url,
kPredefinedHangoutsDomains,
arraysize(kPredefinedHangoutsDomains));
}

bool AppCategorizer::IsWhitelistedApp(
const GURL& manifest_url, const GURL& app_url) {
// Whitelisted apps must be served over https.
if (!app_url.SchemeIsCryptographic())
return false;

std::string manifest_url_path = manifest_url.path();
bool is_photo_app =
manifest_url.SchemeIsCryptographic() &&
manifest_url.DomainIs("ssl.gstatic.com") &&
(base::StartsWith(manifest_url_path, "/s2/oz/nacl/",
base::CompareCase::SENSITIVE) ||
base::StartsWith(manifest_url_path, "/photos/nacl/",
base::CompareCase::SENSITIVE)) &&
IsInWhitelistedDomain(
app_url,
kPredefinedPlusDomains,
arraysize(kPredefinedPlusDomains));

bool is_hangouts_app =
manifest_url.SchemeIsFileSystem() &&
manifest_url.inner_url() != NULL &&
manifest_url.inner_url()->SchemeIsCryptographic() &&
// The manifest must be loaded from the host's FileSystem.
(manifest_url.inner_url()->host() == app_url.host()) &&
IsHangoutsUrl(app_url);

return is_photo_app || is_hangouts_app;
}
16 changes: 16 additions & 0 deletions chrome/renderer/app_categorizer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_RENDERER_APP_CATEGORIZER_H_
#define CHROME_RENDERER_APP_CATEGORIZER_H_

class GURL;

class AppCategorizer {
public:
static bool IsHangoutsUrl(const GURL& url);
static bool IsWhitelistedApp(const GURL& manifest_url, const GURL& app_url);
};

#endif // CHROME_RENDERER_APP_CATEGORIZER_H_
125 changes: 125 additions & 0 deletions chrome/renderer/app_categorizer_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/renderer/app_categorizer.h"

#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

namespace {

const char* kChatAppURLs[] = {
"https://hangouts.google.com/hangouts/foo",
"https://hAnGoUtS.gOoGlE.com/HaNgOuTs/foo",
"https://meet.google.com/hangouts/foo",
"https://talkgadget.google.com/hangouts/foo",
"https://staging.talkgadget.google.com/hangouts/foo",
"https://plus.google.com/hangouts/foo",
"https://plus.sandbox.google.com/hangouts/foo"
};

const char* kChatManifestFSs[] = {
"filesystem:https://hangouts.google.com/foo",
"filesystem:https://hAnGoUtS.gOoGlE.com/foo",
"filesystem:https://meet.google.com/foo",
"filesystem:https://talkgadget.google.com/foo",
"filesystem:https://staging.talkgadget.google.com/foo",
"filesystem:https://plus.google.com/foo",
"filesystem:https://plus.sandbox.google.com/foo"
};

const char* kBadChatAppURLs[] = {
"http://talkgadget.google.com/hangouts/foo", // not https
"https://talkgadget.evil.com/hangouts/foo" // domain not whitelisted
};

const char* kPhotosAppURLs[] = {
"https://foo.plus.google.com",
"https://foo.plus.sandbox.google.com"
};

const char* kPhotosManifestURLs[] = {
"https://ssl.gstatic.com/photos/nacl/foo",
"https://ssl.gstatic.com/s2/oz/nacl/foo"
};

const char* kBadPhotosAppURLs[] = {
"https://plus.google.com/foo",
"https://plus.google.com/foo",
"https://plus.google.com/foo",
"http://plus.google.com/foo", // http scheme
"https://plus.evil.com/foo", // domain not whitelisted
};

const char* kBadPhotosManifestURLs[] = {
"http://ssl.gstatic.com/photos/nacl/foo", // http scheme
"https://lss.gstatic.com/photos/nacl/foo", // bad hostname
"https://ssl.gstatic.com/wrong/photos/nacl/foo", // bad path
"https://ssl.gstatic.com/photos/nacl/foo",
"https://ssl.gstatic.com/photos/nacl/foo",
};

} // namespace

TEST(AppCategorizerTest, IsHangoutsUrl) {
for (size_t i = 0; i < arraysize(kChatAppURLs); ++i) {
EXPECT_TRUE(AppCategorizer::IsHangoutsUrl(GURL(kChatAppURLs[i])));
}

for (size_t i = 0; i < arraysize(kBadChatAppURLs); ++i) {
EXPECT_FALSE(AppCategorizer::IsHangoutsUrl(GURL(kBadChatAppURLs[i])));
}
}

TEST(AppCategorizerTest, IsWhitelistedApp) {
// Hangouts app
{
EXPECT_EQ(arraysize(kChatAppURLs), arraysize(kChatManifestFSs));
for (size_t i = 0; i < arraysize(kChatAppURLs); ++i) {
EXPECT_TRUE(AppCategorizer::IsWhitelistedApp(
GURL(kChatManifestFSs[i]), GURL(kChatAppURLs[i])));
}
for (size_t i = 0; i < arraysize(kBadChatAppURLs); ++i) {
EXPECT_FALSE(AppCategorizer::IsWhitelistedApp(
GURL("filesystem:https://irrelevant.com/"),
GURL(kBadChatAppURLs[i])));
}

// Manifest URL not filesystem
EXPECT_FALSE(AppCategorizer::IsWhitelistedApp(
GURL("https://hangouts.google.com/foo"),
GURL("https://hangouts.google.com/hangouts/foo")));

// Manifest URL not https
EXPECT_FALSE(AppCategorizer::IsWhitelistedApp(
GURL("filesystem:http://hangouts.google.com/foo"),
GURL("https://hangouts.google.com/hangouts/foo")));

// Manifest URL hostname does not match that of the app URL
EXPECT_FALSE(AppCategorizer::IsWhitelistedApp(
GURL("filesystem:https://meet.google.com/foo"),
GURL("https://hangouts.google.com/hangouts/foo")));
}

// Photos app
{
EXPECT_EQ(arraysize(kPhotosAppURLs), arraysize(kPhotosManifestURLs));
for (size_t i = 0; i < arraysize(kPhotosAppURLs); ++i) {
EXPECT_TRUE(AppCategorizer::IsWhitelistedApp(
GURL(kPhotosManifestURLs[i]), GURL(kPhotosAppURLs[i])));
}
// The app/manifest two sides do not have any coorelation for the Photos app
for (size_t i = 0; i < arraysize(kPhotosAppURLs); ++i) {
EXPECT_TRUE(AppCategorizer::IsWhitelistedApp(
GURL(kPhotosManifestURLs[(i + 1) % arraysize(kPhotosAppURLs)]),
GURL(kPhotosAppURLs[i])));
}

EXPECT_EQ(arraysize(kBadPhotosAppURLs), arraysize(kBadPhotosManifestURLs));
for (size_t i = 0; i < arraysize(kBadPhotosAppURLs); ++i) {
EXPECT_FALSE(AppCategorizer::IsWhitelistedApp(
GURL(kBadPhotosManifestURLs[i]), GURL(kBadPhotosAppURLs[i])));
}
}
}
63 changes: 10 additions & 53 deletions chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "chrome/grit/generated_resources.h"
#include "chrome/grit/locale_settings.h"
#include "chrome/grit/renderer_resources.h"
#include "chrome/renderer/app_categorizer.h"
#include "chrome/renderer/banners/app_banner_client.h"
#include "chrome/renderer/benchmarking_extension.h"
#include "chrome/renderer/chrome_render_frame_observer.h"
Expand Down Expand Up @@ -903,44 +904,9 @@ bool ChromeContentRendererClient::IsNaClAllowed(
const Extension* extension,
WebPluginParams* params) {
// Temporarily allow these whitelisted apps and WebUIs to use NaCl.
std::string app_url_host = app_url.host();
std::string manifest_url_path = manifest_url.path();

bool is_whitelisted_web_ui =
app_url.spec() == chrome::kChromeUIAppListStartPageURL;

bool is_photo_app =
// Whitelisted apps must be served over https.
app_url.SchemeIsCryptographic() && manifest_url.SchemeIsCryptographic() &&
(base::EndsWith(app_url_host, "plus.google.com",
base::CompareCase::INSENSITIVE_ASCII) ||
base::EndsWith(app_url_host, "plus.sandbox.google.com",
base::CompareCase::INSENSITIVE_ASCII)) &&
manifest_url.DomainIs("ssl.gstatic.com") &&
(manifest_url_path.find("s2/oz/nacl/") == 1 ||
manifest_url_path.find("photos/nacl/") == 1);

std::string manifest_fs_host;
if (manifest_url.SchemeIsFileSystem() && manifest_url.inner_url()) {
manifest_fs_host = manifest_url.inner_url()->host();
}
bool is_hangouts_app =
// Whitelisted apps must be served over secure scheme.
app_url.SchemeIsCryptographic() && manifest_url.SchemeIsFileSystem() &&
manifest_url.inner_url()->SchemeIsCryptographic() &&
(base::EndsWith(app_url_host, "talkgadget.google.com",
base::CompareCase::INSENSITIVE_ASCII) ||
base::EndsWith(app_url_host, "plus.google.com",
base::CompareCase::INSENSITIVE_ASCII) ||
base::EndsWith(app_url_host, "plus.sandbox.google.com",
base::CompareCase::INSENSITIVE_ASCII) ||
base::EndsWith(app_url_host, "hangouts.google.com",
base::CompareCase::INSENSITIVE_ASCII)) &&
// The manifest must be loaded from the host's FileSystem.
(manifest_fs_host == app_url_host);

bool is_whitelisted_app = is_photo_app || is_hangouts_app;

bool is_invoked_by_webstore_installed_extension = false;
bool is_extension_unrestricted = false;
bool is_extension_force_installed = false;
Expand Down Expand Up @@ -973,7 +939,7 @@ bool ChromeContentRendererClient::IsNaClAllowed(
// 5) --enable-nacl is set.
bool is_nacl_allowed_by_location =
is_whitelisted_web_ui ||
is_whitelisted_app ||
AppCategorizer::IsWhitelistedApp(manifest_url, app_url) ||
is_extension_unrestricted ||
is_extension_force_installed ||
is_invoked_by_webstore_installed_extension;
Expand Down Expand Up @@ -1229,29 +1195,20 @@ ChromeContentRendererClient::OverrideSpeechSynthesizer(

bool ChromeContentRendererClient::AllowPepperMediaStreamAPI(
const GURL& url) {
#if !defined(OS_ANDROID)
// Allow only the Hangouts app to use the MediaStream APIs. It's OK to check
// the whitelist in the renderer, since we're only preventing access until
// these APIs are public and stable.
std::string url_host = url.host();
if (url.SchemeIs("https") &&
(base::EndsWith(url_host, "talkgadget.google.com",
base::CompareCase::INSENSITIVE_ASCII) ||
base::EndsWith(url_host, "plus.google.com",
base::CompareCase::INSENSITIVE_ASCII) ||
base::EndsWith(url_host, "plus.sandbox.google.com",
base::CompareCase::INSENSITIVE_ASCII)) &&
base::StartsWith(url.path(), "/hangouts/",
base::CompareCase::INSENSITIVE_ASCII)) {
return true;
}
#if defined(OS_ANDROID)
return false;
#else
// Allow access for tests.
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnablePepperTesting)) {
return true;
}

// Allow only the Hangouts app to use the MediaStream APIs. It's OK to check
// the whitelist in the renderer, since we're only preventing access until
// these APIs are public and stable.
return (AppCategorizer::IsHangoutsUrl(url));
#endif // !defined(OS_ANDROID)
return false;
}

void ChromeContentRendererClient::AddSupportedKeySystems(
Expand Down
Loading

0 comments on commit 0901e14

Please sign in to comment.