Allow the installability check to respect --unsafely-treat-insecure-origin-as-secure.
InstallableManager uses SecurityStateTabHelper to check the security
status of a tab, and strictly verifies the status of the SSL
certificate. However, this means that any origins whitelisted using the
--unsafely-treat-insecure-origin-as-secure flag are still regarded as
insecure for the purposes of the check.
This CL allows InstallableManager to respect the flag by exposing
content::IsWhitelistedAsSecureOrigin in content/public. This allows the
strictness of the check to be maintained whilst allowing users of the
flag to have their sites considered as installable (e.g. testing sites
on local network IP addresses).
BUG=864391
Change-Id: Id801cc74524e760a1a502dd2a7de7b026cb86c33
Reviewed-on: https://chromium-review.googlesource.com/c/1401830
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622182}diff --git a/chrome/browser/installable/installable_manager.cc b/chrome/browser/installable/installable_manager.cc
index 298bbaf..f4f34d3 100644
--- a/chrome/browser/installable/installable_manager.cc
+++ b/chrome/browser/installable/installable_manager.cc
@@ -17,10 +17,12 @@
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/storage_partition.h"
+#include "content/public/common/origin_util.h"
#include "content/public/common/url_constants.h"
#include "net/base/url_util.h"
#include "third_party/blink/public/common/manifest/manifest_icon_selector.h"
#include "third_party/blink/public/common/manifest/web_display_mode.h"
+#include "url/origin.h"
#if defined(OS_ANDROID)
#include "chrome/browser/android/shortcut_helper.h"
@@ -72,13 +74,17 @@
return false;
// chrome:// URLs are considered secure.
- if (web_contents->GetVisibleURL().scheme() == content::kChromeUIScheme)
+ const GURL& url = web_contents->GetVisibleURL();
+ if (url.scheme() == content::kChromeUIScheme)
return true;
- // Whitelist localhost. Check the VisibleURL to match what the
+ // SecurityStateTabHelper ignores origins that are manually listed as secure.
+ // Check those explicitly, using the VisibleURL to match what
// SecurityStateTabHelper looks at.
- if (net::IsLocalhost(web_contents->GetVisibleURL()))
+ if (net::IsLocalhost(url) ||
+ content::IsWhitelistedAsSecureOrigin(url::Origin::Create(url))) {
return true;
+ }
security_state::SecurityInfo security_info;
SecurityStateTabHelper::FromWebContents(web_contents)
@@ -218,6 +224,10 @@
metrics_->RecordAddToHomescreenInstallabilityTimeout();
}
+bool InstallableManager::IsContentSecureForTesting() {
+ return IsContentSecure(web_contents());
+}
+
bool InstallableManager::IsIconFetched(const IconPurpose purpose) const {
const auto it = icons_.find(purpose);
return it != icons_.end() && it->second.fetched;
diff --git a/chrome/browser/installable/installable_manager.h b/chrome/browser/installable/installable_manager.h
index 35604ebe..6054019 100644
--- a/chrome/browser/installable/installable_manager.h
+++ b/chrome/browser/installable/installable_manager.h
@@ -66,6 +66,8 @@
void RecordAddToHomescreenManifestAndIconTimeout();
void RecordAddToHomescreenInstallabilityTimeout();
+ bool IsContentSecureForTesting();
+
protected:
// For mocking in tests.
virtual void OnWaitingForServiceWorker() {}
diff --git a/chrome/browser/installable/installable_manager_browsertest.cc b/chrome/browser/installable/installable_manager_browsertest.cc
index 34391cc..90e82ad 100644
--- a/chrome/browser/installable/installable_manager_browsertest.cc
+++ b/chrome/browser/installable/installable_manager_browsertest.cc
@@ -22,6 +22,11 @@
namespace {
+const char kInsecureOrigin[] = "http://www.google.com";
+const char kOtherInsecureOrigin[] = "http://maps.google.com";
+const char kUnsafeSecureOriginFlag[] =
+ "unsafely-treat-insecure-origin-as-secure";
+
InstallableParams GetManifestParams() {
InstallableParams params;
params.check_eligibility = true;
@@ -253,6 +258,13 @@
}
};
+class InstallableManagerWhitelistOriginBrowserTest
+ : public InstallableManagerBrowserTest {
+ void SetUpCommandLine(base::CommandLine* command_line) override {
+ command_line->AppendSwitchASCII(kUnsafeSecureOriginFlag, kInsecureOrigin);
+ }
+};
+
IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
ManagerBeginsInEmptyState) {
// Ensure that the InstallableManager starts off with everything null.
@@ -1513,3 +1525,14 @@
EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
}
}
+
+IN_PROC_BROWSER_TEST_F(InstallableManagerWhitelistOriginBrowserTest,
+ SecureOriginCheckRespectsUnsafeFlag) {
+ // The whitelisted origin should be regarded as secure.
+ ui_test_utils::NavigateToURL(browser(), GURL(kInsecureOrigin));
+ EXPECT_TRUE(GetManager(browser())->IsContentSecureForTesting());
+
+ // While a non-whitelisted origin should not.
+ ui_test_utils::NavigateToURL(browser(), GURL(kOtherInsecureOrigin));
+ EXPECT_FALSE(GetManager(browser())->IsContentSecureForTesting());
+}
diff --git a/content/common/origin_util.cc b/content/common/origin_util.cc
index 2b15f617..878cb24a 100644
--- a/content/common/origin_util.cc
+++ b/content/common/origin_util.cc
@@ -24,17 +24,6 @@
base::ContainsValue(url::GetNoAccessSchemes(), origin.scheme());
}
-bool IsWhitelistedSecureOrigin(const url::Origin& origin) {
- if (base::ContainsValue(content::GetSecureOriginsAndPatterns(),
- origin.Serialize()))
- return true;
- for (const auto& origin_or_pattern : content::GetSecureOriginsAndPatterns()) {
- if (base::MatchPattern(origin.host(), origin_or_pattern))
- return true;
- }
- return false;
-}
-
} // namespace
namespace content {
@@ -54,7 +43,18 @@
if (base::ContainsValue(url::GetSecureSchemes(), url.scheme()))
return true;
- return IsWhitelistedSecureOrigin(url::Origin::Create(url));
+ return IsWhitelistedAsSecureOrigin(url::Origin::Create(url));
+}
+
+bool IsWhitelistedAsSecureOrigin(const url::Origin& origin) {
+ if (base::ContainsValue(content::GetSecureOriginsAndPatterns(),
+ origin.Serialize()))
+ return true;
+ for (const auto& origin_or_pattern : content::GetSecureOriginsAndPatterns()) {
+ if (base::MatchPattern(origin.host(), origin_or_pattern))
+ return true;
+ }
+ return false;
}
bool OriginCanAccessServiceWorkers(const GURL& url) {
@@ -83,7 +83,7 @@
return true;
}
- return IsWhitelistedSecureOrigin(origin);
+ return IsWhitelistedAsSecureOrigin(origin);
}
} // namespace content
diff --git a/content/public/common/origin_util.h b/content/public/common/origin_util.h
index b352f37c..4e4bbc3a 100644
--- a/content/public/common/origin_util.h
+++ b/content/public/common/origin_util.h
@@ -12,13 +12,21 @@
namespace content {
-// Returns true if the origin is trustworthy: that is, if its contents can be
-// said to have been transferred to the browser in a way that a network attacker
-// cannot tamper with or observe.
+// Returns true if |url|'s origin is trustworthy. There are two cases:
+// a) it can be said that |url|'s contents were transferred to the browser in a
+// way that a network attacker cannot tamper with or observe. (see
+// https://www.w3.org/TR/powerful-features/#is-origin-trustworthy).
+// b) IsWhitelistedAsSecureOrigin(url::Origin::Create(url)) returns true.
//
-// See https://www.w3.org/TR/powerful-features/#is-origin-trustworthy.
+// Note that this is not equivalent to checking if an entire site is secure
+// (i.e. no degraded security state UI is displayed to the user), since there
+// may be insecure iframes present even if this method returns true.
bool CONTENT_EXPORT IsOriginSecure(const GURL& url);
+// Returns true if |origin| is whitelisted as secure, e.g. it was specified via
+// the --unsafely-treat-insecure-origin-as-secure flag, and false otherwise.
+bool CONTENT_EXPORT IsWhitelistedAsSecureOrigin(const url::Origin& origin);
+
// Returns true if the origin can register a service worker. Scheme must be
// http (localhost only), https, or a custom-set secure scheme.
bool CONTENT_EXPORT OriginCanAccessServiceWorkers(const GURL& url);
