0

Remove content::IsPotentiallyTrustworthyOrigin

content::IsPotentiallyTrustworthyOrigin is just directly calling
network::IsOriginPotentiallyTrustworthy. This CL removes
content::IsPotentiallyTrustworthyOrigin, move tests for
network::IsOriginPotentiallyTrustworthy to
is_potentially_trustworthy_unittest.cc and updates callers
accordingly. There is no behavior change.

Bug: 1153336
Change-Id: Ifc3a45d0d444225e763bb87935d503a882f8a09e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2563759
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833733}
This commit is contained in:
Frédéric Wang
2020-12-04 16:27:53 +00:00
committed by Chromium LUCI CQ
parent f02220368f
commit 86b11bed1f
9 changed files with 85 additions and 107 deletions

@ -94,12 +94,12 @@
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/origin_util.h"
#include "extensions/buildflags/buildflags.h"
#include "google_apis/gaia/gaia_urls.h"
#include "net/base/url_util.h"
#include "net/cert/cert_status_flags.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "third_party/re2/src/re2/re2.h"
#include "url/url_constants.h"
@ -751,7 +751,7 @@ ChromePasswordManagerClient::GetAutofillDownloadManager() {
}
bool ChromePasswordManagerClient::IsCommittedMainFrameSecure() const {
return content::IsPotentiallyTrustworthyOrigin(
return network::IsOriginPotentiallyTrustworthy(
web_contents()->GetMainFrame()->GetLastCommittedOrigin());
}

@ -16,6 +16,7 @@
#include "content/public/test/test_utils.h"
#include "extensions/common/constants.h"
#include "ppapi/buildflags/buildflags.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/loader/network_utils.h"
#include "url/gurl.h"
@ -110,7 +111,7 @@ TEST(ChromeContentClientTest, AdditionalSchemes) {
EXPECT_TRUE(blink::network_utils::IsOriginSecure(chrome_url));
EXPECT_FALSE(content::OriginCanAccessServiceWorkers(chrome_url));
EXPECT_TRUE(
content::IsPotentiallyTrustworthyOrigin(url::Origin::Create(chrome_url)));
network::IsOriginPotentiallyTrustworthy(url::Origin::Create(chrome_url)));
}
class OriginTrialInitializationTestThread

@ -20,8 +20,8 @@
#include "content/public/browser/content_browser_client.h"
#include "content/public/common/content_client.h"
#include "content/public/common/navigation_policy.h"
#include "content/public/common/origin_util.h"
#include "net/base/url_util.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "third_party/blink/public/common/loader/network_utils.h"
#include "third_party/blink/public/common/security_context/insecure_request_policy.h"
#include "third_party/blink/public/common/web_preferences/web_preferences.h"
@ -56,7 +56,7 @@ bool IsUrlPotentiallySecure(const GURL& url) {
return url.SchemeIs(url::kBlobScheme) ||
url.SchemeIs(url::kFileSystemScheme) ||
blink::network_utils::IsOriginSecure(url) ||
IsPotentiallyTrustworthyOrigin(url::Origin::Create(url));
network::IsOriginPotentiallyTrustworthy(url::Origin::Create(url));
}
// This method should return the same results as

@ -90,7 +90,6 @@
#include "content/public/common/content_switches.h"
#include "content/public/common/navigation_policy.h"
#include "content/public/common/network_service_util.h"
#include "content/public/common/origin_util.h"
#include "content/public/common/url_constants.h"
#include "content/public/common/url_utils.h"
#include "mojo/public/cpp/system/data_pipe.h"
@ -235,7 +234,8 @@ void UpdateLoadFlagsWithCacheFlags(int* load_flags,
// TODO(clamy): This should be function in FrameTreeNode.
bool IsSecureFrame(RenderFrameHostImpl* frame) {
while (frame) {
if (!IsPotentiallyTrustworthyOrigin(frame->GetLastCommittedOrigin()))
if (!network::IsOriginPotentiallyTrustworthy(
frame->GetLastCommittedOrigin()))
return false;
frame = frame->GetParent();
}

@ -27,8 +27,4 @@ bool OriginCanAccessServiceWorkers(const GURL& url) {
return false;
}
bool IsPotentiallyTrustworthyOrigin(const url::Origin& origin) {
return network::IsOriginPotentiallyTrustworthy(origin);
}
} // namespace content

@ -1,26 +0,0 @@
// Copyright 2015 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 "content/public/common/origin_util.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace content {
TEST(OriginUtilTest, IsPotentiallyTrustworthyOrigin) {
EXPECT_FALSE(
IsPotentiallyTrustworthyOrigin(url::Origin::Create(GURL("about:blank"))));
EXPECT_FALSE(IsPotentiallyTrustworthyOrigin(
url::Origin::Create(GURL("about:blank#ref"))));
EXPECT_FALSE(IsPotentiallyTrustworthyOrigin(
url::Origin::Create(GURL("about:srcdoc"))));
EXPECT_FALSE(IsPotentiallyTrustworthyOrigin(
url::Origin::Create(GURL("javascript:alert('blah')"))));
EXPECT_FALSE(IsPotentiallyTrustworthyOrigin(
url::Origin::Create(GURL("data:test/plain;blah"))));
}
} // namespace content

@ -16,13 +16,6 @@ namespace content {
// http (localhost only), https, or a custom-set secure scheme.
bool CONTENT_EXPORT OriginCanAccessServiceWorkers(const GURL& url);
// This is based on SecurityOrigin::isPotentiallyTrustworthy and tries to mimic
// its behavior.
//
// TODO(https://crbug.com/1153336): Remove this function and use
// network::IsOriginPotentiallyTrustworthy instead.
bool CONTENT_EXPORT IsPotentiallyTrustworthyOrigin(const url::Origin& origin);
} // namespace content
#endif // CONTENT_PUBLIC_COMMON_ORIGIN_UTIL_H_

@ -2065,7 +2065,6 @@ test("content_unittests") {
"../common/input/touch_event_stream_validator_unittest.cc",
"../common/inter_process_time_ticks_converter_unittest.cc",
"../common/net/ip_address_space_util_unittest.cc",
"../common/origin_util_unittest.cc",
"../common/service_manager/service_manager_connection_impl_unittest.cc",
"../common/service_worker/service_worker_utils_unittest.cc",
"../common/state_transitions_unittest.cc",

@ -21,7 +21,11 @@ bool IsOriginAllowlisted(const char* str) {
return IsOriginAllowlisted(url::Origin::Create(GURL(str)));
}
bool IsPotentiallyTrustworthy(const char* str) {
bool IsOriginPotentiallyTrustworthy(const char* str) {
return IsOriginPotentiallyTrustworthy(url::Origin::Create(GURL(str)));
}
bool IsUrlPotentiallyTrustworthy(const char* str) {
return IsUrlPotentiallyTrustworthy(GURL(str));
}
@ -32,7 +36,7 @@ std::vector<std::string> CanonicalizeAllowlist(
allowlist, rejected_patterns);
}
TEST(IsPotentiallyTrustworthy, MainTest) {
TEST(IsPotentiallyTrustworthy, Origin) {
const url::Origin unique_origin;
EXPECT_FALSE(IsOriginPotentiallyTrustworthy(unique_origin));
const url::Origin opaque_origin =
@ -40,85 +44,95 @@ TEST(IsPotentiallyTrustworthy, MainTest) {
.DeriveNewOpaqueOrigin();
EXPECT_FALSE(IsOriginPotentiallyTrustworthy(opaque_origin));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:blank"));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:blank?x=2"));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:blank#ref"));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:blank?x=2#ref"));
EXPECT_FALSE(IsOriginPotentiallyTrustworthy("about:blank"));
EXPECT_FALSE(IsOriginPotentiallyTrustworthy("about:blank#ref"));
EXPECT_FALSE(IsOriginPotentiallyTrustworthy("about:srcdoc"));
EXPECT_FALSE(IsOriginPotentiallyTrustworthy("javascript:alert('blah')"));
EXPECT_FALSE(IsOriginPotentiallyTrustworthy("data:test/plain;blah"));
}
EXPECT_TRUE(IsPotentiallyTrustworthy("about:srcdoc"));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:srcdoc?x=2"));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:srcdoc#ref"));
EXPECT_TRUE(IsPotentiallyTrustworthy("about:srcdoc?x=2#ref"));
TEST(IsPotentiallyTrustworthy, Url) {
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank?x=2"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank#ref"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank?x=2#ref"));
EXPECT_FALSE(IsPotentiallyTrustworthy("about:about"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc?x=2"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc#ref"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc?x=2#ref"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("about:about"));
// TODO(https://crbug.com/1119740): Should return true for data: URLs.
EXPECT_FALSE(IsPotentiallyTrustworthy("data:test/plain;blah"));
EXPECT_FALSE(IsPotentiallyTrustworthy("javascript:alert('blah')"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("data:test/plain;blah"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("javascript:alert('blah')"));
EXPECT_TRUE(IsPotentiallyTrustworthy("file:///test/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("file:///test/"));
EXPECT_TRUE(IsPotentiallyTrustworthy("file://localhost/test/"));
EXPECT_TRUE(IsPotentiallyTrustworthy("file://otherhost/test/"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file:///test/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file:///test/"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file://localhost/test/"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file://otherhost/test/"));
EXPECT_TRUE(IsPotentiallyTrustworthy("https://example.com/fun.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://example.com/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("https://example.com/fun.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://example.com/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("wss://example.com/fun.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("ws://example.com/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("wss://example.com/fun.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("ws://example.com/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://localhost/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://localhost./fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://pumpkin.localhost/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://localhost/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://localhost./fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://pumpkin.localhost/fun.html"));
EXPECT_TRUE(
IsPotentiallyTrustworthy("http://crumpet.pumpkin.localhost/fun.html"));
IsUrlPotentiallyTrustworthy("http://crumpet.pumpkin.localhost/fun.html"));
EXPECT_TRUE(
IsPotentiallyTrustworthy("http://pumpkin.localhost:8080/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy(
IsUrlPotentiallyTrustworthy("http://pumpkin.localhost:8080/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy(
"http://crumpet.pumpkin.localhost:3000/fun.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://localhost.com/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("https://localhost.com/fun.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://localhost.com/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("https://localhost.com/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://127.0.0.1/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("ftp://127.0.0.1/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://127.3.0.1/fun.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://127.example.com/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("https://127.example.com/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://127.0.0.1/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("ftp://127.0.0.1/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://127.3.0.1/fun.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://127.example.com/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("https://127.example.com/fun.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://[::1]/fun.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://[::2]/fun.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://[::1].example.com/fun.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://[::1]/fun.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::2]/fun.html"));
EXPECT_FALSE(
IsUrlPotentiallyTrustworthy("http://[::1].example.com/fun.html"));
// IPv4 mapped IPv6 literals for loopback.
EXPECT_FALSE(IsPotentiallyTrustworthy("http://[::ffff:127.0.0.1]/"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://[::ffff:7f00:1]"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::ffff:127.0.0.1]/"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::ffff:7f00:1]"));
// IPv4 compatible IPv6 literal for loopback.
EXPECT_FALSE(IsPotentiallyTrustworthy("http://[::127.0.0.1]"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::127.0.0.1]"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://loopback"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://loopback"));
// TODO(https://crbug.com/1153337): Return false?
EXPECT_TRUE(IsPotentiallyTrustworthy("http://localhost6"));
EXPECT_TRUE(IsPotentiallyTrustworthy("ftp://localhost6.localdomain6"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://localhost.localdomain"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://localhost6"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("ftp://localhost6.localdomain6"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://localhost.localdomain"));
EXPECT_FALSE(
IsPotentiallyTrustworthy("filesystem:http://www.example.com/temporary/"));
EXPECT_FALSE(
IsPotentiallyTrustworthy("filesystem:ftp://www.example.com/temporary/"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
"filesystem:http://www.example.com/temporary/"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
"filesystem:ftp://www.example.com/temporary/"));
EXPECT_TRUE(
IsPotentiallyTrustworthy("filesystem:ftp://127.0.0.1/temporary/"));
EXPECT_TRUE(IsPotentiallyTrustworthy(
IsUrlPotentiallyTrustworthy("filesystem:ftp://127.0.0.1/temporary/"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy(
"filesystem:https://www.example.com/temporary/"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
"blob:http://www.example.com/guid-goes-here"));
EXPECT_FALSE(
IsPotentiallyTrustworthy("blob:http://www.example.com/guid-goes-here"));
EXPECT_FALSE(
IsPotentiallyTrustworthy("blob:ftp://www.example.com/guid-goes-here"));
EXPECT_TRUE(IsPotentiallyTrustworthy("blob:ftp://127.0.0.1/guid-goes-here"));
IsUrlPotentiallyTrustworthy("blob:ftp://www.example.com/guid-goes-here"));
EXPECT_TRUE(
IsPotentiallyTrustworthy("blob:https://www.example.com/guid-goes-here"));
IsUrlPotentiallyTrustworthy("blob:ftp://127.0.0.1/guid-goes-here"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy(
"blob:https://www.example.com/guid-goes-here"));
}
class SecureOriginAllowlistTest : public testing::Test {
@ -131,8 +145,8 @@ class SecureOriginAllowlistTest : public testing::Test {
TEST_F(SecureOriginAllowlistTest, UnsafelyTreatInsecureOriginAsSecure) {
EXPECT_FALSE(IsOriginAllowlisted("http://example.com/a.html"));
EXPECT_FALSE(IsOriginAllowlisted("http://127.example.com/a.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://example.com/a.html"));
EXPECT_FALSE(IsPotentiallyTrustworthy("http://127.example.com/a.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://example.com/a.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://127.example.com/a.html"));
// Add http://example.com and http://127.example.com to allowlist by
// command-line and see if they are now considered secure origins.
@ -146,13 +160,13 @@ TEST_F(SecureOriginAllowlistTest, UnsafelyTreatInsecureOriginAsSecure) {
// They should be now allow-listed.
EXPECT_TRUE(IsOriginAllowlisted("http://example.com/a.html"));
EXPECT_TRUE(IsOriginAllowlisted("http://127.example.com/a.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://example.com/a.html"));
EXPECT_TRUE(IsPotentiallyTrustworthy("http://127.example.com/a.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://example.com/a.html"));
EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://127.example.com/a.html"));
// Check that similarly named sites are not considered secure.
EXPECT_FALSE(IsPotentiallyTrustworthy("http://128.example.com/a.html"));
EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://128.example.com/a.html"));
EXPECT_FALSE(
IsPotentiallyTrustworthy("http://foobar.127.example.com/a.html"));
IsUrlPotentiallyTrustworthy("http://foobar.127.example.com/a.html"));
// When port is not specified, default port is assumed.
EXPECT_TRUE(IsOriginAllowlisted("http://example.com:80/a.html"));
@ -209,7 +223,8 @@ TEST_F(SecureOriginAllowlistTest, HostnamePatterns) {
GURL input_url(test.test_input);
url::Origin input_origin = url::Origin::Create(input_url);
EXPECT_EQ(test.expected_secure, IsOriginAllowlisted(input_origin));
EXPECT_EQ(test.expected_secure, IsPotentiallyTrustworthy(test.test_input));
EXPECT_EQ(test.expected_secure,
IsUrlPotentiallyTrustworthy(test.test_input));
}
}