diff options
12 files changed, 10 insertions, 352 deletions
diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.cc b/chrome/browser/safe_browsing/browser_feature_extractor.cc index b0b5549..7663551 100644 --- a/chrome/browser/safe_browsing/browser_feature_extractor.cc +++ b/chrome/browser/safe_browsing/browser_feature_extractor.cc @@ -12,7 +12,6 @@ #include "base/format_macros.h" #include "base/stl_util.h" #include "base/stringprintf.h" -#include "base/string_util.h" #include "base/task.h" #include "base/time.h" #include "chrome/common/safe_browsing/csd.pb.h" @@ -21,18 +20,14 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/safe_browsing/browser_features.h" #include "chrome/browser/safe_browsing/client_side_detection_service.h" -#include "chrome/browser/safe_browsing/safe_browsing_util.h" #include "content/browser/browser_thread.h" #include "content/browser/cancelable_request.h" #include "content/browser/tab_contents/tab_contents.h" #include "content/public/common/page_transition_types.h" -#include "crypto/sha2.h" #include "googleurl/src/gurl.h" namespace safe_browsing { -const int BrowserFeatureExtractor::kHashPrefixLength = 5; - BrowseInfo::BrowseInfo() {} BrowseInfo::~BrowseInfo() {} @@ -201,7 +196,6 @@ void BrowserFeatureExtractor::ExtractFeatures(const BrowseInfo* info, } ExtractBrowseInfoFeatures(*info, request); - ComputeURLHash(request); pending_extractions_.insert(std::make_pair(request, callback)); MessageLoop::current()->PostTask( FROM_HERE, @@ -455,36 +449,4 @@ bool BrowserFeatureExtractor::GetHistoryService(HistoryService** history) { return false; } -void BrowserFeatureExtractor::ComputeURLHash( - ClientPhishingRequest* request) { - // Put the url into SafeBrowsing host suffix / path prefix format, with - // query parameters stripped. - std::string host, path, query; - safe_browsing_util::CanonicalizeUrl(GURL(request->url()), - &host, &path, &query); - DCHECK(!host.empty()) << request->url(); - DCHECK(!path.empty()) << request->url(); - - // Lowercase the URL. Note: canonicalization converts the URL to ASCII. - // Percent encoded characters will not be lowercased but this is consistent - // with what we're doing on the server side. - StringToLowerASCII(&host); - StringToLowerASCII(&path); - - // Remove leading 'www.' from the host. - if (host.size() > 4 && host.substr(0, 4) == "www.") { - host.erase(0, 4); - } - // Remove everything after the last '/' to broaden the pattern. - if (path.size() > 1 && *(path.rbegin()) != '/') { - // The pattern never ends in foo.com/test? because we stripped CGI params. - // Remove everything that comes after the last '/'. - size_t last_path = path.rfind("/"); - path.erase(last_path + 1); - } - - request->set_hash_prefix(crypto::SHA256HashString(host + path).substr( - 0, kHashPrefixLength)); -} - -}; // namespace safe_browsing +} // namespace safe_browsing diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.h b/chrome/browser/safe_browsing/browser_feature_extractor.h index 7f6264f..d227407 100644 --- a/chrome/browser/safe_browsing/browser_feature_extractor.h +++ b/chrome/browser/safe_browsing/browser_feature_extractor.h @@ -82,10 +82,6 @@ class BrowserFeatureExtractor { ClientPhishingRequest* request, DoneCallback* callback); - // The size of hash prefix to use for ClientPhishingRequest.hash_prefix. - // Public for testing. - static const int kHashPrefixLength; - private: friend class DeleteTask<BrowserFeatureExtractor>; typedef std::pair<ClientPhishingRequest*, DoneCallback*> ExtractionData; @@ -147,10 +143,6 @@ class BrowserFeatureExtractor { // is set it will return true and false otherwise. bool GetHistoryService(HistoryService** history); - // Computes the SHA-256 hash prefix for the URL and adds it to the - // ClientPhishingRequest. - void ComputeURLHash(ClientPhishingRequest* request); - TabContents* tab_; ClientSideDetectionService* service_; CancelableRequestConsumer request_consumer_; diff --git a/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc b/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc index 385c4bc..cd87e65 100644 --- a/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc +++ b/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc @@ -25,7 +25,6 @@ #include "content/browser/tab_contents/test_tab_contents.h" #include "content/common/view_messages.h" #include "content/public/common/page_transition_types.h" -#include "crypto/sha2.h" #include "googleurl/src/gurl.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -498,52 +497,4 @@ TEST_F(BrowserFeatureExtractorTest, SafeBrowsingFeatures) { EXPECT_DOUBLE_EQ(1.0, features[features::kSafeBrowsingIsSubresource]); EXPECT_DOUBLE_EQ(2.0, features[features::kSafeBrowsingThreatType]); } - -TEST_F(BrowserFeatureExtractorTest, URLHashes) { - ClientPhishingRequest request; - request.set_url("http://host.com/"); - request.set_client_score(0.8f); - - history_service()->AddPage(GURL("http://host.com/"), - history::SOURCE_BROWSED); - SimpleNavigateAndCommit(GURL("http://host.com/")); - - EXPECT_TRUE(ExtractFeatures(&request)); - EXPECT_EQ(crypto::SHA256HashString("host.com/").substr( - 0, BrowserFeatureExtractor::kHashPrefixLength), - request.hash_prefix()); - - request.set_url("http://www.host.com/path/"); - history_service()->AddPage(GURL("http://www.host.com/path/"), - history::SOURCE_BROWSED); - SimpleNavigateAndCommit(GURL("http://www.host.com/path/")); - - EXPECT_TRUE(ExtractFeatures(&request)); - EXPECT_EQ(crypto::SHA256HashString("host.com/path/").substr( - 0, BrowserFeatureExtractor::kHashPrefixLength), - request.hash_prefix()); - - request.set_url("http://user@www.host.com:1111/path/123?args"); - history_service()->AddPage( - GURL("http://user@www.host.com:1111/path/123?args"), - history::SOURCE_BROWSED); - SimpleNavigateAndCommit( - GURL("http://user@www.host.com:1111/path/123?args")); - - EXPECT_TRUE(ExtractFeatures(&request)); - EXPECT_EQ(crypto::SHA256HashString("host.com/path/").substr( - 0, BrowserFeatureExtractor::kHashPrefixLength), - request.hash_prefix()); - - // Check that escaping matches the SafeBrowsing specification. - request.set_url("http://www.host.com/A%21//B"); - history_service()->AddPage(GURL("http://www.host.com/A%21//B"), - history::SOURCE_BROWSED); - SimpleNavigateAndCommit(GURL("http://www.host.com/A%21//B")); - - EXPECT_TRUE(ExtractFeatures(&request)); - EXPECT_EQ(crypto::SHA256HashString("host.com/a!/").substr( - 0, BrowserFeatureExtractor::kHashPrefixLength), - request.hash_prefix()); -} } // namespace safe_browsing diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc index 6c1219a..27014e1 100644 --- a/chrome/browser/safe_browsing/client_side_detection_host.cc +++ b/chrome/browser/safe_browsing/client_side_detection_host.cc @@ -83,27 +83,6 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest return; } - // For UMA users we don't run the phishing classifier if the - // connection was proxied because we won't have the correct remote - // IP address (we don't want UMA users to classify URLs from a private - // IP). For non-UMA users the verdict request will be sanitized - // which means it's OK to classify URLs behind proxies. - // TODO(noelutz): classify these URLs for UMA users but sanitize - // the verdict request. - if (params_.was_fetched_via_proxy && - (!sb_service_ || sb_service_->CanReportStats())) { - VLOG(1) << "Skipping phishing classification for URL: " << params_.url - << " because it was fetched via a proxy."; - UMA_HISTOGRAM_ENUMERATION("SBClientPhishing.PreClassificationCheckFail", - NO_CLASSIFY_PROXY_FETCH, - NO_CLASSIFY_MAX); - return; - } - - // We could classify URLs hosted on a private IP for non-UMA users - // since we're sanitizing the request but the probability that - // something is phishing on a private network is low enough that - // we don't bother. if (csd_service_->IsPrivateIPAddress(params_.socket_address.host())) { VLOG(1) << "Skipping phishing classification for URL: " << params_.url << " because of hosting on private IP: " @@ -154,7 +133,7 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest // Enum used to keep stats about why the pre-classification check failed. enum PreClassificationCheckFailures { - NO_CLASSIFY_PROXY_FETCH, + OBSOLETE_NO_CLASSIFY_PROXY_FETCH, NO_CLASSIFY_PRIVATE_IP, NO_CLASSIFY_OFF_THE_RECORD, NO_CLASSIFY_MATCH_CSD_WHITELIST, diff --git a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc index 76a578a..60da4ea 100644 --- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc +++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc @@ -106,10 +106,6 @@ class MockSafeBrowsingService : public SafeBrowsingService { client->OnBlockingPageComplete(false); } - bool CanReportStats() const { - return true; // tests for UMA users. - } - private: DISALLOW_COPY_AND_ASSIGN(MockSafeBrowsingService); }; @@ -577,8 +573,8 @@ TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) { // Test that canceling pending should classify requests works as expected. GURL first_url("http://first.phishy.url.com"); - // The proxy checks is done synchronously so check that it has been done - // for the first URL. + // The first few checks are done synchronously so check that they have been + // done for the first URL. ExpectPreClassificationChecks(first_url, &kFalse, &kFalse, &kFalse, NULL, NULL, NULL); NavigateAndCommit(first_url); @@ -674,19 +670,6 @@ TEST_F(ClientSideDetectionHostTest, ShouldClassifyUrl) { SafeBrowsingMsg_StartPhishingDetection::ID); ASSERT_FALSE(msg); - // If the connection is proxied, no IPC should be triggered. - // Note: for this test to work correctly, the new URL must be on the - // same domain as the previous URL, otherwise it will create a new - // RenderViewHost that won't have simulate_fetch_via_proxy set. - url = GURL("http://host3.com/abc"); - rvh()->set_simulate_fetch_via_proxy(true); - ExpectPreClassificationChecks(url, NULL, NULL, NULL, NULL, NULL, NULL); - NavigateAndCommit(url); - WaitAndCheckPreClassificationChecks(); - msg = process()->sink().GetFirstMessageMatching( - SafeBrowsingMsg_StartPhishingDetection::ID); - ASSERT_FALSE(msg); - // If the tab is incognito there should be no IPC. Also, we shouldn't // even check the csd-whitelist. url = GURL("http://host4.com/"); diff --git a/chrome/browser/safe_browsing/client_side_detection_service.cc b/chrome/browser/safe_browsing/client_side_detection_service.cc index 3903131..3942499 100644 --- a/chrome/browser/safe_browsing/client_side_detection_service.cc +++ b/chrome/browser/safe_browsing/client_side_detection_service.cc @@ -15,13 +15,11 @@ #include "base/task.h" #include "base/time.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/safe_browsing/browser_features.h" -#include "chrome/browser/safe_browsing/safe_browsing_service.h" +#include "chrome/browser/safe_browsing/safe_browsing_util.h" #include "chrome/common/net/http_return.h" #include "chrome/common/safe_browsing/client_model.pb.h" #include "chrome/common/safe_browsing/csd.pb.h" #include "chrome/common/safe_browsing/safebrowsing_messages.h" -#include "chrome/renderer/safe_browsing/features.h" #include "content/browser/browser_thread.h" #include "content/browser/renderer_host/render_process_host.h" #include "content/common/notification_service.h" @@ -65,10 +63,8 @@ ClientSideDetectionService::CacheState::CacheState(bool phish, base::Time time) ClientSideDetectionService::ClientSideDetectionService( net::URLRequestContextGetter* request_context_getter) : enabled_(false), - sb_service_(g_browser_process->safe_browsing_service()), ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)), request_context_getter_(request_context_getter) { - InitializeAllowedFeatures(); registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED, NotificationService::AllSources()); } @@ -269,39 +265,6 @@ void ClientSideDetectionService::EndFetchModel(ClientModelStatus status) { ScheduleFetchModel(delay_ms); } -void ClientSideDetectionService::SanitizeRequestForPingback( - const ClientPhishingRequest& full_request, - ClientPhishingRequest* sanitized_request) { - DCHECK(full_request.IsInitialized()); - sanitized_request->Clear(); - if (full_request.has_hash_prefix()) { - sanitized_request->set_hash_prefix(full_request.hash_prefix()); - } - sanitized_request->set_client_score(full_request.client_score()); - if (full_request.has_is_phishing()) { - sanitized_request->set_is_phishing(full_request.is_phishing()); - } - - for (int i = 0; i < full_request.feature_map_size(); ++i) { - const ClientPhishingRequest_Feature& feature = full_request.feature_map(i); - if (allowed_features_.find(feature.name()) != allowed_features_.end()) { - sanitized_request->add_feature_map()->CopyFrom(feature); - } - } - - if (full_request.has_model_version()) { - sanitized_request->set_model_version(full_request.model_version()); - } - - for (int i = 0; i < full_request.non_model_feature_map_size(); ++i) { - const ClientPhishingRequest_Feature& feature = - full_request.non_model_feature_map(i); - if (allowed_features_.find(feature.name()) != allowed_features_.end()) { - sanitized_request->add_non_model_feature_map()->CopyFrom(feature); - } - } -} - void ClientSideDetectionService::StartClientReportPhishingRequest( ClientPhishingRequest* verdict, ClientReportPhishingRequestCallback* callback) { @@ -315,16 +278,8 @@ void ClientSideDetectionService::StartClientReportPhishingRequest( return; } - // Create the version of the request proto that we'll send over the network. - ClientPhishingRequest request_to_send; - if (sb_service_ && sb_service_->CanReportStats()) { - request_to_send.CopyFrom(*request); - } else { - SanitizeRequestForPingback(*request, &request_to_send); - } - std::string request_data; - if (!request_to_send.SerializeToString(&request_data)) { + if (!request->SerializeToString(&request_data)) { UMA_HISTOGRAM_COUNTS("SBClientPhishing.RequestNotSerialized", 1); VLOG(1) << "Unable to serialize the CSD request. Proto file changed?"; if (cb.get()) { @@ -515,44 +470,6 @@ bool ClientSideDetectionService::InitializePrivateNetworks() { return true; } -void ClientSideDetectionService::InitializeAllowedFeatures() { - static const char* const kAllowedFeatures[] = { - // Renderer (model) features. - features::kUrlHostIsIpAddress, - features::kUrlNumOtherHostTokensGTOne, - features::kUrlNumOtherHostTokensGTThree, - features::kPageHasForms, - features::kPageActionOtherDomainFreq, - features::kPageHasTextInputs, - features::kPageHasPswdInputs, - features::kPageHasRadioInputs, - features::kPageHasCheckInputs, - features::kPageExternalLinksFreq, - features::kPageSecureLinksFreq, - features::kPageNumScriptTagsGTOne, - features::kPageNumScriptTagsGTSix, - features::kPageImgOtherDomainFreq, - // Browser (non-model) features. - features::kUrlHistoryVisitCount, - features::kUrlHistoryTypedCount, - features::kUrlHistoryLinkCount, - features::kUrlHistoryVisitCountMoreThan24hAgo, - features::kHttpHostVisitCount, - features::kHttpsHostVisitCount, - features::kFirstHttpHostVisitMoreThan24hAgo, - features::kFirstHttpsHostVisitMoreThan24hAgo, - features::kHasSSLReferrer, - features::kPageTransitionType, - features::kIsFirstNavigation, - features::kSafeBrowsingIsSubresource, - features::kSafeBrowsingThreatType, - }; - - for (size_t i = 0; i < arraysize(kAllowedFeatures); ++i) { - allowed_features_.insert(kAllowedFeatures[i]); - } -} - // static void ClientSideDetectionService::SetBadSubnets(const ClientSideModel& model, BadSubnetMap* bad_subnets) { diff --git a/chrome/browser/safe_browsing/client_side_detection_service.h b/chrome/browser/safe_browsing/client_side_detection_service.h index 0a25f8b..7e583a1 100644 --- a/chrome/browser/safe_browsing/client_side_detection_service.h +++ b/chrome/browser/safe_browsing/client_side_detection_service.h @@ -24,7 +24,6 @@ #include "base/basictypes.h" #include "base/callback_old.h" #include "base/gtest_prod_util.h" -#include "base/hash_tables.h" #include "base/memory/linked_ptr.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" @@ -171,8 +170,6 @@ class ClientSideDetectionService : public URLFetcher::Delegate, IsFalsePositiveResponse); FRIEND_TEST_ALL_PREFIXES(ClientSideDetectionServiceTest, ModelHasValidHashIds); - FRIEND_TEST_ALL_PREFIXES(ClientSideDetectionServiceTest, - SanitizeRequestForPingback); // CacheState holds all information necessary to respond to a caller without // actually making a HTTP request. @@ -203,14 +200,6 @@ class ClientSideDetectionService : public URLFetcher::Delegate, static const base::TimeDelta kNegativeCacheInterval; static const base::TimeDelta kPositiveCacheInterval; - // Given a ClientSidePhishingRequest populated by the renderer and browser - // feature extractors, sanitizes it so that no data specifically identifying - // the URL or page content is included. This is used when sending a pingback - // if the user is not opted in to UMA. - void SanitizeRequestForPingback( - const ClientPhishingRequest& original_request, - ClientPhishingRequest* sanitized_request); - // Starts sending the request to the client-side detection frontends. // This method takes ownership of both pointers. void StartClientReportPhishingRequest( @@ -245,10 +234,6 @@ class ClientSideDetectionService : public URLFetcher::Delegate, // that we consider non-public IP addresses. Returns true on success. bool InitializePrivateNetworks(); - // Initializes the |allowed_features_| hash_set with the features that - // can be sent in sanitized pingbacks. - void InitializeAllowedFeatures(); - // Send the model to the given renderer. void SendModelToProcess(RenderProcessHost* process); @@ -281,9 +266,6 @@ class ClientSideDetectionService : public URLFetcher::Delegate, scoped_ptr<base::TimeDelta> model_max_age_; scoped_ptr<URLFetcher> model_fetcher_; - // This pointer may be NULL if SafeBrowsing is disabled. - scoped_refptr<SafeBrowsingService> sb_service_; - // Map of client report phishing request to the corresponding callback that // has to be invoked when the request is done. struct ClientReportInfo; @@ -312,9 +294,6 @@ class ClientSideDetectionService : public URLFetcher::Delegate, // The network blocks that we consider private IP address ranges. std::vector<AddressRange> private_networks_; - // Features which are allowed to be sent in sanitized pingbacks. - base::hash_set<std::string> allowed_features_; - // Map of bad subnets which are copied from the client model and put into // this map to speed up lookups. BadSubnetMap bad_subnets_; diff --git a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc index fa09a39..4df9564 100644 --- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc +++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc @@ -10,14 +10,11 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" -#include "base/stringprintf.h" #include "base/task.h" #include "base/time.h" -#include "chrome/browser/safe_browsing/browser_features.h" #include "chrome/browser/safe_browsing/client_side_detection_service.h" #include "chrome/common/safe_browsing/client_model.pb.h" #include "chrome/common/safe_browsing/csd.pb.h" -#include "chrome/renderer/safe_browsing/features.h" #include "content/browser/browser_thread.h" #include "content/common/net/url_fetcher.h" #include "content/test/test_url_fetcher_factory.h" @@ -663,92 +660,6 @@ TEST_F(ClientSideDetectionServiceTest, SetEnabled) { Mock::VerifyAndClearExpectations(service); } -TEST_F(ClientSideDetectionServiceTest, SanitizeRequestForPingback) { - ClientPhishingRequest request; - request.set_url("http://www.us.host.com/blah"); - request.set_hash_prefix("hash"); - request.set_client_score(0.8f); - request.set_is_phishing(true); - AddFeature(std::string(features::kUrlTldToken) + "com", 1.0, &request); - AddFeature(std::string(features::kUrlDomainToken) + "host", 1.0, &request); - AddFeature(std::string(features::kUrlOtherHostToken) + "us", 1.0, &request); - AddFeature(std::string(features::kUrlOtherHostToken) + "www", 1.0, &request); - AddFeature(features::kUrlNumOtherHostTokensGTOne, 1.0, &request); - AddFeature(std::string(features::kUrlPathToken) + "blah", 1.0, &request); - AddFeature(features::kPageHasForms, 1.0, &request); - AddFeature(std::string(features::kPageTerm) + "term", 1.0, &request); - AddFeature(features::kPageImgOtherDomainFreq, 0.5, &request); - request.set_model_version(3); - AddNonModelFeature(features::kUrlHistoryVisitCount, 5.0, &request); - AddNonModelFeature(StringPrintf("%s=http://referrer.com/", - features::kReferrer), - 1.0, &request); - AddNonModelFeature(StringPrintf("%s[0]=%s", - features::kRedirect, - "http://www.redirect.com/"), - 1.0, &request); - AddNonModelFeature(StringPrintf("%s%s=http://hostreferrer.com/", - features::kHostPrefix, features::kReferrer), - 1.0, &request); - AddNonModelFeature(StringPrintf("%s%s[1]=%s", - features::kHostPrefix, - features::kRedirect, - "http://www.other_redirect.com/"), - 1.0, &request); - AddNonModelFeature(std::string(features::kBadIpFetch) + "1.2.3.4", - 1.0, &request); - AddNonModelFeature(std::string(features::kSafeBrowsingMaliciousUrl) + - "http://malicious.com/", 1.0, &request); - AddNonModelFeature(std::string(features::kSafeBrowsingOriginalUrl) + - "http://original.com/", 1.0, &request); - - csd_service_.reset(ClientSideDetectionService::Create(NULL)); - - ClientPhishingRequest sanitized_request; - csd_service_->SanitizeRequestForPingback(request, &sanitized_request); - - // For easier debugging, we'll check the output protobuf fields individually. - ClientPhishingRequest expected; - expected.set_hash_prefix(request.hash_prefix()); - expected.set_client_score(request.client_score()); - expected.set_is_phishing(request.is_phishing()); - AddFeature(features::kUrlNumOtherHostTokensGTOne, 1.0, &expected); - AddFeature(features::kPageHasForms, 1.0, &expected); - AddFeature(features::kPageImgOtherDomainFreq, 0.5, &expected); - expected.set_model_version(3); - AddNonModelFeature(features::kUrlHistoryVisitCount, 5.0, &expected); - - EXPECT_FALSE(sanitized_request.has_url()); - EXPECT_EQ(expected.hash_prefix(), sanitized_request.hash_prefix()); - EXPECT_FLOAT_EQ(expected.client_score(), sanitized_request.client_score()); - EXPECT_EQ(expected.is_phishing(), sanitized_request.is_phishing()); - - ASSERT_EQ(expected.feature_map_size(), sanitized_request.feature_map_size()); - for (int i = 0; i < expected.feature_map_size(); ++i) { - EXPECT_EQ(expected.feature_map(i).name(), - sanitized_request.feature_map(i).name()) << "Feature " << i; - EXPECT_DOUBLE_EQ(expected.feature_map(i).value(), - sanitized_request.feature_map(i).value()) - << "Feature " << i; - } - EXPECT_EQ(expected.model_version(), sanitized_request.model_version()); - ASSERT_EQ(expected.non_model_feature_map_size(), - sanitized_request.non_model_feature_map_size()); - for (int i = 0; i < expected.non_model_feature_map_size(); ++i) { - EXPECT_EQ(expected.non_model_feature_map(i).name(), - sanitized_request.non_model_feature_map(i).name()) - << "Non-model feature " << i; - EXPECT_DOUBLE_EQ(expected.non_model_feature_map(i).value(), - sanitized_request.non_model_feature_map(i).value()) - << "Non-model feature " << i; - } - - // Also check the serialized forms in case there's a field that we forget - // to add above. - EXPECT_EQ(expected.SerializeAsString(), - sanitized_request.SerializeAsString()); -} - TEST_F(ClientSideDetectionServiceTest, IsFalsePositiveResponse) { GURL url("http://www.google.com/"); ClientPhishingResponse response; diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc index fae8588..745a856 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.cc +++ b/chrome/browser/safe_browsing/safe_browsing_service.cc @@ -170,10 +170,7 @@ SafeBrowsingService::SafeBrowsingService() download_hashcheck_timeout_ms_(kDownloadHashCheckTimeoutMs) { #if !defined(OS_CHROMEOS) if (!CommandLine::ForCurrentProcess()->HasSwitch( - switches::kDisableClientSidePhishingDetection) && - (!CommandLine::ForCurrentProcess()->HasSwitch( - switches::kDisableSanitizedClientSidePhishingDetection) || - CanReportStats())) { + switches::kDisableClientSidePhishingDetection)) { csd_service_.reset( safe_browsing::ClientSideDetectionService::Create( g_browser_process->system_request_context())); @@ -917,20 +914,14 @@ void SafeBrowsingService::Start() { !cmdline->HasSwitch(switches::kSbDisableDownloadProtection); // We only download the csd-whitelist if client-side phishing detection is - // enabled and if the user has opted in with stats collection. Note: we - // cannot check whether the metrics_service() object is created because it - // may be initialized after this method is called. + // enabled. #ifdef OS_CHROMEOS // Client-side detection is disabled on ChromeOS for now, so don't bother // downloading the whitelist. enable_csd_whitelist_ = false; #else enable_csd_whitelist_ = - (!cmdline->HasSwitch(switches::kDisableClientSidePhishingDetection) && - (!cmdline->HasSwitch( - switches::kDisableSanitizedClientSidePhishingDetection) || - (local_state && - local_state->GetBoolean(prefs::kMetricsReportingEnabled)))); + !cmdline->HasSwitch(switches::kDisableClientSidePhishingDetection); #endif enable_download_whitelist_ = cmdline->HasSwitch( diff --git a/chrome/browser/safe_browsing/safe_browsing_test.cc b/chrome/browser/safe_browsing/safe_browsing_test.cc index e0d12d37..9ecd542 100644 --- a/chrome/browser/safe_browsing/safe_browsing_test.cc +++ b/chrome/browser/safe_browsing/safe_browsing_test.cc @@ -307,7 +307,7 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest { // TODO(gcasto): Generate new testing data that includes the // client-side phishing whitelist. command_line->AppendSwitch( - switches::kDisableSanitizedClientSidePhishingDetection); + switches::kDisableClientSidePhishingDetection); // In this test, we fetch SafeBrowsing data and Mac key from the same // server. Although in real production, they are served from different diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 0b672a2..bd3fe443 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -296,12 +296,6 @@ const char kDisableRemoteFonts[] = "disable-remote-fonts"; const char kDisableRestoreBackgroundContents[] = "disable-restore-background-contents"; -// Disable the sanitized version of client-side phishing detection, for use by -// non-UMA users. Any features containing portions of the URL or page content -// are not sent as part of the pingback in this mode. -const char kDisableSanitizedClientSidePhishingDetection[] = - "disable-sanitized-client-side-phishing-detection"; - // Disable site-specific tailoring to compatibility issues in WebKit. const char kDisableSiteSpecificQuirks[] = "disable-site-specific-quirks"; diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index f94bc9a..abce30c 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -89,7 +89,6 @@ extern const char kDisablePreconnect[]; extern const char kDisablePromptOnRepost[]; extern const char kDisableRemoteFonts[]; extern const char kDisableRestoreBackgroundContents[]; -extern const char kDisableSanitizedClientSidePhishingDetection[]; extern const char kDisableSiteSpecificQuirks[]; extern const char kDisableSSL3[]; extern const char kDisableSync[]; |