Skip to content

Commit

Permalink
Switch remaining functions from SchemeIsSecure() to
Browse files Browse the repository at this point in the history
SchemeIsCryptographic().

We recently introduced SchemeIsCryptographic() and IsOriginSecure(),
which are meant to replace SchemeIsSecure().

IsOriginSecure() roughly means "do we trust this content not to be
tampered with before it reaches the user?" [1] This is a higher-level
definition that corresponds to the new "privileged contexts" spec. [2]

SchemeIsCryptographic() [3] is close to the old definition of
SchemeIsSecure(), and literally just checks if the scheme is a
cryptographic scheme (HTTPS or WSS as of right now). The difference is
that SchemeIsCryptographic() will not consider filesystem URLs secure.

IsOriginSecure() should be correct for most Fizz code.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/content/public/common/origin_util.h&sq=package:chromium&type=cs&l=19&rcl=143099866
[2] https://www.chromium.org/Home/chromium-security/prefer-secure-origins-for-powerful-new-features and https://w3c.github.io/webappsec/specs/powerfulfeatures/
[3] https://code.google.com/p/chromium/codesearch#chromium/src/url/gurl.h&sq=package:chromium&type=cs&l=250&rcl=1430998666

BUG=362214

Review URL: https://codereview.chromium.org/1136643004

Cr-Commit-Position: refs/heads/master@{#329310}
  • Loading branch information
lgarron authored and Commit bot committed May 12, 2015
1 parent 6a9b5b1 commit 9272555
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 22 deletions.
5 changes: 3 additions & 2 deletions content/browser/ssl/ssl_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,9 @@ void SSLPolicy::InitializeEntryIfNeeded(NavigationEntryImpl* entry) {
if (entry->GetSSL().security_style != SECURITY_STYLE_UNKNOWN)
return;

entry->GetSSL().security_style = entry->GetURL().SchemeIsCryptographic() ?
SECURITY_STYLE_AUTHENTICATED : SECURITY_STYLE_UNAUTHENTICATED;
entry->GetSSL().security_style = entry->GetURL().SchemeIsCryptographic()
? SECURITY_STYLE_AUTHENTICATED
: SECURITY_STYLE_UNAUTHENTICATED;
}

void SSLPolicy::OriginRanInsecureContent(const std::string& origin, int pid) {
Expand Down
8 changes: 4 additions & 4 deletions extensions/browser/updater/extension_downloader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ void ExtensionDownloader::StartBlacklistUpdate(
// by a public key signature like .crx files are.
scoped_ptr<ManifestFetchData> blacklist_fetch(CreateManifestFetchData(
extension_urls::GetWebstoreUpdateUrl(), request_id));
DCHECK(blacklist_fetch->base_url().SchemeIsSecure());
DCHECK(blacklist_fetch->base_url().SchemeIsCryptographic());
blacklist_fetch->AddExtension(kBlacklistAppID,
version,
&ping_data,
Expand Down Expand Up @@ -313,7 +313,7 @@ bool ExtensionDownloader::AddExtensionData(

// Make sure we use SSL for store-hosted extensions.
if (extension_urls::IsWebstoreUpdateUrl(update_url) &&
!update_url.SchemeIsSecure())
!update_url.SchemeIsCryptographic())
update_url = extension_urls::GetWebstoreUpdateUrl();

// Skip extensions with empty IDs.
Expand Down Expand Up @@ -589,7 +589,7 @@ void ExtensionDownloader::HandleManifestResults(
DCHECK(extension_urls::IsBlacklistUpdateUrl(crx_url)) << crx_url;

// Force https (crbug.com/129587).
if (!crx_url.SchemeIsSecure()) {
if (!crx_url.SchemeIsCryptographic()) {
url::Replacements<char> replacements;
std::string scheme("https");
replacements.SetScheme(scheme.c_str(),
Expand Down Expand Up @@ -766,7 +766,7 @@ void ExtensionDownloader::CreateExtensionFetcher() {
extension_fetcher_->SetAutomaticallyRetryOnNetworkChanges(3);

int load_flags = net::LOAD_DISABLE_CACHE;
bool is_secure = fetch->url.SchemeIsSecure();
bool is_secure = fetch->url.SchemeIsCryptographic();
if (fetch->credentials != ExtensionFetch::CREDENTIALS_COOKIES || !is_secure) {
load_flags |= net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES;
}
Expand Down
2 changes: 1 addition & 1 deletion google_apis/gaia/gaia_auth_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ std::string ExtractDomainName(const std::string& email_address) {
}

bool IsGaiaSignonRealm(const GURL& url) {
if (!url.SchemeIsSecure())
if (!url.SchemeIsCryptographic())
return false;

return url == GaiaUrls::GetInstance()->gaia_url();
Expand Down
19 changes: 9 additions & 10 deletions ios/web/net/request_tracker_impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ - (void)buildSSLStatus {
status_.content_status = web::SSLStatus::NORMAL_CONTENT;
}

if (!url_.SchemeIsSecure()) {
if (!url_.SchemeIsCryptographic()) {
// Should not happen as the sslInfo is valid.
NOTREACHED();
status_.security_style = web::SECURITY_STYLE_UNAUTHENTICATED;
Expand Down Expand Up @@ -495,7 +495,7 @@ - (NSString*)description {
GURLByRemovingRefFromGURL(url), request);
counts_.push_back(counts);
counts_by_request_[request] = counts;
if (page_url_.SchemeIsSecure() && !url.SchemeIsSecure())
if (page_url_.SchemeIsCryptographic() && !url.SchemeIsCryptographic())
has_mixed_content_ = true;
Notify();
}
Expand Down Expand Up @@ -811,7 +811,7 @@ - (NSString*)description {
if (!counts_.size())
return; // Nothing yet to notify.

if (!page_url_.SchemeIsSecure())
if (!page_url_.SchemeIsCryptographic())
return;

const GURL page_origin = page_url_.GetOrigin();
Expand Down Expand Up @@ -1103,12 +1103,12 @@ - (NSString*)description {
const TrackerCounts* split_position) {
DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::IO);
// Check if the mixed content before trimming was correct.
if (page_url_.SchemeIsSecure() && has_mixed_content_) {
if (page_url_.SchemeIsCryptographic() && has_mixed_content_) {
bool old_url_has_mixed_content = false;
const GURL origin = page_url_.GetOrigin();
ScopedVector<TrackerCounts>::iterator it = counts_.begin();
while (it != counts_.end() && *it != split_position) {
if (!(*it)->url.SchemeIsSecure() &&
if (!(*it)->url.SchemeIsCryptographic() &&
origin == (*it)->first_party_for_cookies_origin) {
old_url_has_mixed_content = true;
break;
Expand Down Expand Up @@ -1168,10 +1168,10 @@ - (NSString*)description {

// Locate the request with this url, if present.
bool new_url_has_mixed_content = false;
bool url_scheme_is_secure = url.SchemeIsSecure();
bool url_scheme_is_secure = url.SchemeIsCryptographic();
ScopedVector<TrackerCounts>::const_reverse_iterator rit = counts_.rbegin();
while (rit != counts_.rend() && (*rit)->url != url) {
if (url_scheme_is_secure && !(*rit)->url.SchemeIsSecure() &&
if (url_scheme_is_secure && !(*rit)->url.SchemeIsCryptographic() &&
(*rit)->first_party_for_cookies_origin == url.GetOrigin()) {
new_url_has_mixed_content = true;
}
Expand All @@ -1196,9 +1196,8 @@ - (NSString*)description {
if (url_scheme_is_secure && counts_.size()) {
TrackerCounts* back = counts_.back();
const GURL& back_url = back->url;
if (back_url.SchemeIsSecure() &&
back_url.GetOrigin() == url.GetOrigin() &&
!back->is_subrequest) {
if (back_url.SchemeIsCryptographic() &&
back_url.GetOrigin() == url.GetOrigin() && !back->is_subrequest) {
split_position = back;
}
}
Expand Down
2 changes: 1 addition & 1 deletion ios/web/net/request_tracker_impl_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ void EndPage(NSString* tab_id, const GURL& url) {
EXPECT_TRUE(requests_[i]->ssl_info().is_valid());
}
}
EXPECT_TRUE(!secure == !requests_[i]->url().SchemeIsSecure());
EXPECT_TRUE(!secure == !requests_[i]->url().SchemeIsCryptographic());
return requests_[i];
}

Expand Down
2 changes: 1 addition & 1 deletion remoting/host/third_party_auth_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ bool ParseUrlPolicy(const std::string& str, GURL* out) {
}
// We validate https-vs-http only on Release builds to help with manual testing.
#if defined(NDEBUG)
if (!gurl.SchemeIsSecure()) {
if (!gurl.SchemeIsCryptographic()) {
LOG(ERROR) << "Not a secure URL: " << str;
return false;
}
Expand Down
3 changes: 1 addition & 2 deletions sync/internal_api/sync_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,7 @@ void SyncManagerImpl::Init(InitArgs* args) {
connection_manager_.reset(new SyncAPIServerConnectionManager(
args->service_url.host() + args->service_url.path(),
args->service_url.EffectiveIntPort(),
args->service_url.SchemeIsSecure(),
args->post_factory.release(),
args->service_url.SchemeIsCryptographic(), args->post_factory.release(),
args->cancelation_signal));
connection_manager_->set_client_id(directory()->cache_guid());
connection_manager_->AddListener(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ ChromeMetadataSource::Request::Request(const std::string& key,
void ChromeMetadataSource::Download(const std::string& key,
const Callback& downloaded) {
GURL resource(validation_data_url_ + key);
if (!resource.SchemeIsSecure()) {
if (!resource.SchemeIsCryptographic()) {
downloaded(false, key, NULL);
return;
}
Expand Down

0 comments on commit 9272555

Please sign in to comment.